December 2015 - Wine-Devel - list.winehq.org

Re: [PATCH 2/2] wined3d: Print opcode names instead of enum values.

by Józef Kucia

On Thu, Dec 3, 2015 at 4:48 PM, Henri Verbeet <hverbeet(a)gmail.com> wrote: > On 3 December 2015 at 11:04, Józef Kucia <jkucia(a)codeweavers.com> wrote: >> +const char *debug_d3dshaderinstructionhandler(enum WINED3D_SHADER_INSTRUCTION_HANDLER handler_idx) >> +{ >> + if (handler_idx >= sizeof(shader_opcode_names) / sizeof(*shader_opcode_names)) >> + { >> + FIXME("Unrecognized shader instruction handler %#x.\n", handler_idx); >> + return "UNRECOGNIZED"; > You can do something like "wine_dbg_sprintf("UNRECOGNIZED(%#x)", > handler_idx);" and have it print the handler_idx. (See also e.g. > debug_d3dstate().) Note that we could also move this to utils.c, not > sure if that's worth it. > I agree that wine_dbg_sprintf is probably better. However this code should never be executed unless something is very wrong.

Dec. 4, 2015

Re: [PATCH 1/2] wined3d: Correctly parse SM4 dcl_immediateConstantBuffer opcode.

by Józef Kucia

This patch meant to be a minimal patch to prevent the infinitive loop. It's mostly for bug 39153 which is "implement enough D3D11 to run Tomb Raider 2013 without disabling d3d11 again" ;-) It allows the game to crash properly instead of looping indefinitely. It crashes because of some unsupported texture format with this patch. In order to make the patch small I haven't added WINED3DSIH_ entry for this opcode yet. Otherwise, I would have to reindent opcode tables also in shader.c, arb_program_shader.c and glsl_shader.c. The subject should actually be "wined3d: Correctly calculate length for SM4 dcl_immediateConstantBuffer opcode." On Thu, Dec 3, 2015 at 4:48 PM, Henri Verbeet <hverbeet(a)gmail.com> wrote: >> opcode_token = *(*ptr)++; >> opcode = opcode_token & WINED3D_SM4_OPCODE_MASK; >> - len = ((opcode_token & WINED3D_SM4_INSTRUCTION_LENGTH_MASK) >> WINED3D_SM4_INSTRUCTION_LENGTH_SHIFT) - 1; >> + >> + if (opcode == WINED3D_SM4_OP_DCL_IMMEDIATE_CONSTANT_BUFFER) >> + len = *(*ptr) - 1; >> + else >> + len = ((opcode_token & WINED3D_SM4_INSTRUCTION_LENGTH_MASK) >> WINED3D_SM4_INSTRUCTION_LENGTH_SHIFT) - 1; >> > I don't think this is the right place to handle this. You should be > able to handle this the same way as the other DCL_ opcodes. It should > probably also have an entry in the opcode_info table. (Compare e.g. > 6d948e1a8cdd8a9a5ae2b6124468275cbf13f2b8.) > I think I still need some additional code to handle this case before len is added to *ptr. WINED3D_SM4_INSTRUCTION_LENGTH is 0 for DCL_IMMEDIATE_CONSTANT_BUFFER opcodes. In the result we are adding 4294967295 to the pointer. I could use other type than DWORD so it would effectively decrement pointer pointer also on 64 bit but I doubt we want that. I could also check if len is 0 before decrementing it but it is not right for other opcodes than DCL_IMMEDIATE_CONSTANT_BUFFER.

Dec. 4, 2015

Re: [PATCH 3/3] ddraw: Do not mark surfaces as lost unconditionally when setting the cooperative level.

by Marvin

Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at https://testbot.winehq.org/JobDetails.pl?Key=18780 Your paranoid android. === w1064 (32 bit ddraw2) === ddraw2.c:2337: Test failed: Expected {0, 0, 512, 384}, got {0, 0, 1024, 768}. === w1064 (32 bit ddraw1) === ddraw1.c:2186: Test failed: Expected {0, 0, 512, 384}, got {0, 0, 1024, 768}. === w1064 (32 bit ddraw7) === ddraw7.c:2253: Test failed: Expected {0, 0, 512, 384}, got {0, 0, 1024, 768}. === w1064 (32 bit ddraw4) === ddraw4.c:2522: Test failed: Expected {0, 0, 512, 384}, got {0, 0, 1024, 768}.

Dec. 4, 2015

Re: [PATCH 2/3] ddraw: Mark surfaces as lost when the device window is deactivated.

by Marvin

Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at https://testbot.winehq.org/JobDetails.pl?Key=18779 Your paranoid android. === w1064 (32 bit ddraw2) === ddraw2.c:2337: Test failed: Expected {0, 0, 512, 384}, got {0, 0, 1024, 768}. === w1064 (32 bit ddraw1) === ddraw1.c:2186: Test failed: Expected {0, 0, 512, 384}, got {0, 0, 1024, 768}. === w1064 (32 bit ddraw7) === ddraw7.c:2253: Test failed: Expected {0, 0, 512, 384}, got {0, 0, 1024, 768}. === w1064 (32 bit ddraw4) === ddraw4.c:2522: Test failed: Expected {0, 0, 512, 384}, got {0, 0, 1024, 768}.

Dec. 4, 2015

Re: fusion: add a stub for CopyPDBs

by Austin English

On Fri, Dec 4, 2015 at 1:39 AM, Sebastian Lackner <sebastian(a)fds-team.de> wrote: > On 02.12.2015 07:19, Austin English wrote: >> +void WINAPI CopyPDBs(const WCHAR *module) > > Did you find the prototype somewhere? The output doesn't really look a WCHAR string: The only example I've found was here: https://github.com/dotnet/coreclr/blob/master/src/vm/ceeload.cpp#L3896 > fixme:fusion:CopyPDBs (L"\4141\636a\304d\7257\5355\666c\4262\3552\7445\5063\3353\5338\3533\5656\6f63\7674\427a\7457\6249\4d41\7858\7362\5135\6164\6b36\4c71\5277\3351\642f\796c\5755\7452\3735\6a50\3658\5a59\546d\5763\4276\7a45\4b43\5362\7655\5132\4b39\4339\4d37\5131\4f79\5645\452b\5364\4272\4a51\3832\4458\5435\6b77\3454\3135\2f52"...) stub! > > It could be a char *, but on the other hand also some internal data, so its probably better to use void *. Ok. > For the result, several hints on the internet seem to suggest that its HRESULT. Yeah, you're right. I misread the source (FusionCopyPDBs vs CopyPDBs). Later in FusionCopyPDBs there's: HRESULT hr; hr = CopyPDBs(pAssembly->GetFusionAssembly()); Thanks for reviewing. -- -Austin

Dec. 4, 2015

Re: fusion: add a stub for CopyPDBs

by Sebastian Lackner

On 02.12.2015 07:19, Austin English wrote: > +void WINAPI CopyPDBs(const WCHAR *module) Did you find the prototype somewhere? The output doesn't really look a WCHAR string: fixme:fusion:CopyPDBs (L"\4141\636a\304d\7257\5355\666c\4262\3552\7445\5063\3353\5338\3533\5656\6f63\7674\427a\7457\6249\4d41\7858\7362\5135\6164\6b36\4c71\5277\3351\642f\796c\5755\7452\3735\6a50\3658\5a59\546d\5763\4276\7a45\4b43\5362\7655\5132\4b39\4339\4d37\5131\4f79\5645\452b\5364\4272\4a51\3832\4458\5435\6b77\3454\3135\2f52"...) stub! It could be a char *, but on the other hand also some internal data, so its probably better to use void *. For the result, several hints on the internet seem to suggest that its HRESULT. Regards, Sebastian

Dec. 4, 2015

Re: [PATCH 2/2] wined3d: Print opcode names instead of enum values.

by Henri Verbeet

On 3 December 2015 at 11:04, Józef Kucia <jkucia(a)codeweavers.com> wrote: > +const char *debug_d3dshaderinstructionhandler(enum WINED3D_SHADER_INSTRUCTION_HANDLER handler_idx) > +{ > + if (handler_idx >= sizeof(shader_opcode_names) / sizeof(*shader_opcode_names)) > + { > + FIXME("Unrecognized shader instruction handler %#x.\n", handler_idx); > + return "UNRECOGNIZED"; You can do something like "wine_dbg_sprintf("UNRECOGNIZED(%#x)", handler_idx);" and have it print the handler_idx. (See also e.g. debug_d3dstate().) Note that we could also move this to utils.c, not sure if that's worth it.

Dec. 3, 2015

Re: [PATCH 1/2] wined3d: Correctly parse SM4 dcl_immediateConstantBuffer opcode.

by Henri Verbeet

On 3 December 2015 at 11:04, Józef Kucia <jkucia(a)codeweavers.com> wrote: > Signed-off-by: Józef Kucia <jkucia(a)codeweavers.com> > --- > This fixes infinite loop when dcl_immediateConstantBuffer is parsed. > > Unfortunately, I had to reindent whole wined3d_sm4_opcode enum in order to make > space for WINED3D_SM4_OP_DCL_IMMEDIATE_CONSTANT_BUFFER. I was also considering > to use a shorter name: WINED3D_SM4_OP_DCL_IMMEDIATE_BUFFER. > --- It's fine, if perhaps unfortunate. You could do something like WINED3D_SM4_OP_DCL_IMM_CB if we really want something shorter, but it's probably best to just follow the opcode name. > + WINED3D_SM4_OP_MAX = 0x34, > + WINED3D_SM4_OP_DCL_IMMEDIATE_CONSTANT_BUFFER = 0x35, > + WINED3D_SM4_OP_MOV = 0x36, This is odd. I'd expect a DCL_ opcode to be in the > 0x58 range. Still, there's a hole there in the normal range, and "min" and "max" are also reversed compared to what you'd expect, so I suppose it's at least plausible. > opcode_token = *(*ptr)++; > opcode = opcode_token & WINED3D_SM4_OPCODE_MASK; > - len = ((opcode_token & WINED3D_SM4_INSTRUCTION_LENGTH_MASK) >> WINED3D_SM4_INSTRUCTION_LENGTH_SHIFT) - 1; > + > + if (opcode == WINED3D_SM4_OP_DCL_IMMEDIATE_CONSTANT_BUFFER) > + len = *(*ptr) - 1; > + else > + len = ((opcode_token & WINED3D_SM4_INSTRUCTION_LENGTH_MASK) >> WINED3D_SM4_INSTRUCTION_LENGTH_SHIFT) - 1; > I don't think this is the right place to handle this. You should be able to handle this the same way as the other DCL_ opcodes. It should probably also have an entry in the opcode_info table. (Compare e.g. 6d948e1a8cdd8a9a5ae2b6124468275cbf13f2b8.)

Dec. 3, 2015

Re: mshtml: Add IHTMLXMLHttpRequest::responseXML() method implementation. (try 3)

by Jacek Caban

Hi Zhenbo, On 12/03/15 02:34, Zhenbo Li wrote: > Thanks for the reply. The patch looks good to me now, thanks. However, we're in the code freeze, so the patch can't be committed now. Please resend the patch after 1.8 is released. Jacek

Dec. 3, 2015

Re: programs/xcopy: Fix casing of usage message

by Austin English

On Wed, Dec 2, 2015 at 10:45 PM, Alexandre Julliard <julliard(a)winehq.org> wrote: > Austin English <austinenglish(a)gmail.com> writes: > >> On Wed, Dec 2, 2015 at 10:33 PM, Alexandre Julliard <julliard(a)winehq.org> wrote: >>> Austin English <austinenglish(a)gmail.com> writes: >>> >>>> I'm not sure about that. I don't know any other languages than >>>> English, and some are in different scripts that I can't recognize >>>> which letter I'm supposed to change. >>>> >>>> If that's why AJ wants I can do it, but I think that updating PO files >>>> should be the job of the translators, not developers. >>>> >>>> Alexandre, care to comment? >>> >>> I'm not convinced that we want to change this. It's localized, so no >>> reasonable app is going to depend on it. >> >> Sure, but we all know not all apps are reasonable. > > Of course. If there's a real app that depends on it, then it should be > fixed. But if it's just someone's personal script, I don't think it > qualifies. Okay, I'll close it WONTFIX then, thanks. -- -Austin

Dec. 3, 2015