March 2016 - Wine-Devel - list.winehq.org

Re: [PATCH 2/3] user32: Added more dialog owner tests.
by Dmitry Timoshkov March 2, 2016

March 2, 2016

Hi Jacek, Jacek Caban <jacek(a)codeweavers.com> wrote: > + parent = CreateWindowExA(0, "TestParentClass", "Test parent", > + WS_OVERLAPPEDWINDOW | WS_VISIBLE, > + 100, 100, 200, 200, 0, 0, 0, NULL); > + ok (parent != 0, "Failed to create parent window\n"); > + > + child = CreateWindowExA(0, "TestWindowClass", "Test child", > + WS_OVERLAPPEDWINDOW | WS_VISIBLE, > + 100, 100, 200, 200, 0, 0, 0, NULL); > + ok (child != 0, "Failed to create child window\n"); > + > + child2 = CreateWindowExA(0, "SimpleWindowClass", "Test child2", > + WS_OVERLAPPEDWINDOW | WS_VISIBLE | WS_CHILD, > + 100, 100, 200, 200, child, 0, 0, NULL); > + ok (child2 != 0, "Failed to create child window\n"); > + > + SetParent(child, parent); > + SetFocus(child); Is that really required to use SetParent() instead of specifying correct parent in the CreateWindowEx call? > + flush_sequence(); > + DialogBoxA( 0, "TEST_DIALOG", child2, TestModalDlgProc2 ); > + ok_sequence(WmModalDialogSeq_2, "ModalDialog2", TRUE); Is this test supposed to show that actual dialog owner is parent window? If yes, then this theory is not tested at all. > + SetForegroundWindow(hdlg); > + > + { > + const struct message WmEndDialogSeq[] = { > + { WM_ENABLE, sent }, > + { WM_WINDOWPOSCHANGING, sent|wparam, SWP_HIDEWINDOW|SWP_NOACTIVATE|SWP_NOSIZE|SWP_NOMOVE }, > + { HCBT_ACTIVATE, hook|wparam, (WPARAM)hchild }, > + { WM_NCACTIVATE, sent|wparam|lparam, WA_INACTIVE, (LPARAM)hchild }, > + { WM_ACTIVATE, sent|wparam|lparam, WA_INACTIVE, (LPARAM)hchild }, > + /* FIXME: Following two are optional because Wine sends WM_QUERYNEWPALETTE instead of WM_WINDOWPOSCHANGING */ > + { WM_WINDOWPOSCHANGING, sent|wparam|optional, SWP_NOSIZE|SWP_NOMOVE }, > + { WM_QUERYNEWPALETTE, sent|optional }, > + { WM_NCACTIVATE, sent|wparam|lparam, WA_ACTIVE, (LPARAM)hdlg }, > + { WM_GETTEXT, sent|optional|defwinproc }, > + { WM_ACTIVATE, sent|wparam|lparam, WA_ACTIVE, (LPARAM)hdlg }, > + { HCBT_SETFOCUS, hook|wparam, (WPARAM)hchild }, > + { WM_KILLFOCUS, sent|wparam, (WPARAM)hchild }, > + { WM_SETFOCUS, sent|defwinproc|wparam, (WPARAM)hdlg }, > + { 0 } > + }; > + > + flush_sequence(); > + EndDialog(hdlg, 0); > + ok_sequence(WmEndDialogSeq, "EndDialog", FALSE); > + } Please don't create fake embedded blocks. Also it would be really nice to send the tests first so it would be more clear when a todo_wine is removed with a subsequent patch in order to see whether the patch is actually related to the previously sent tests. -- Dmitry.

3 17

Re: [PATCH 1/3] winemac.drv: Fix our handling of bitmaps on the clipboard.
by Ken Thomases March 2, 2016

March 2, 2016

On Feb 26, 2016, at 3:55 AM, Charles Davis <cdavis5x(a)gmail.com> wrote: > > dlls/winemac.drv/clipboard.c | 87 ++++++++++++++++++++++++++++++++++++++------ > 1 file changed, 76 insertions(+), 11 deletions(-) Mostly looks good. One request, though: please move create_bitmap_from_dib() to above import_clipboard_data(), just to keep all of the import_* functions together. Also, do you happen to have or know of a convenient program to test this with? Thanks, Ken

1 1

The Wine Project has been accepted as a GSoC 2016 mentor organization!
by André Hentschel March 1, 2016

March 1, 2016

Hi, I'm a little late, but yes, we're accepted again!

2 2

Re: [PATCH 4/4] d3d10core/tests: Port test_multiple_render_targets() from d3d11.
by Marvin March 1, 2016

March 1, 2016

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=21044 Your paranoid android. === build (build) === Make failed

1 0

Problems in winedbg.1, msiexec.1, winemaker.1
by esr＠thyrsus.com March 1, 2016

March 1, 2016

This is automatically generated email about markup problems in a man page for which you appear to be responsible. If you are not the right person or list, please tell me so I can correct my database. See http://catb.org/~esr/doclifter/bugs.html for details on how and why these patches were generated. Feel free to email me with any questions. Note: These patches do not change the modification date of any manual page. You may wish to do that by hand. I apologize if this message seems spammy or impersonal. The volume of markup bugs I am tracking is over five hundred - there is no real alternative to generating bugmail from a database and template. -- Eric S. Raymond Problems with winedbg.1: Ambiguous or invalid backslash. This doesn't cause groff a problem. but it confuses doclifter and may confuse older troff implementations. --- winedbg.1-unpatched 2014-06-02 09:40:08.604693388 -0400 +++ winedbg.1 2014-06-02 09:41:09.350110336 -0400 @@ -163,7 +163,7 @@ .IP \fBbreak\ \fIN\fR Adds a breakpoint at line \fIN\fR of current source file. .IP \fBbreak\fR -Adds a breakpoint at current \f$PC\fR address. +Adds a breakpoint at current \fB$PC\fR address. .IP \fBwatch\ *\ \fIN\fR Adds a watch command (on write) at address \fIN\fR (on 4 bytes). .IP \fBwatch\ \fIid\fR Problems with msiexec.1: Ambiguous or invalid backslash. This doesn't cause groff a problem. but it confuses doclifter and may confuse older troff implementations. --- msiexec.1-unpatched 2014-06-02 17:15:58.428528019 -0400 +++ msiexec.1 2014-06-02 17:18:59.360645276 -0400 @@ -24,25 +24,25 @@ \fR[\fBp\fR|\fBo\fR|\fBe\fR|\fBd\fR|\fBc\fR|\fBa\fR|\fBu\fR|\fBm\fR|\fBs\fR|\fBv\fR]\ \ \fR{\fIpackage\fR|\fIproductcode\fR} Repair an installation. Default options are \'omus\' -.IP "\flp\fR" +.IP "\fBlp\fR" Reinstall the file if it is missing. -.IP "\flo\fR" +.IP "\fBlo\fR" Reinstall the file if it is missing or if any older version is installed. -.IP "\fle\fR" +.IP "\fBle\fR" Reinstall the file if it is missing, or if the installed version is equal or older. -.IP "\fld\fR" +.IP "\fBld\fR" Reinstall the file if it is missing or a different version is installed. -.IP "\flc\fR" +.IP "\fBlc\fR" Reinstall the file if it is missing or the checksum does not match. -.IP "\fla\fR" +.IP "\fBla\fR" Reinstall all files. -.IP "\flu\fR" +.IP "\fBlu\fR" Rewrite all required user registry entries. -.IP "\flm\fR" +.IP "\fBlm\fR" Rewrite all required machine registry entries. -.IP "\fls\fR" +.IP "\fBls\fR" Overwrite any conflicting shortcuts. -.IP "\flv\fR" +.IP "\fBlv\fR" Recache the local installation package from the source installation package. .SH PATCHING .IP \fB/p\ \fR{\fIpatch\fR}\ \fR[\fIproperty\fR=\fIfoobar\fR] @@ -65,35 +65,35 @@ [\fBi\fR|\fBw\fR|\fBe\fR|\fBa\fR|\fBr\fR|\fBu\fR|\fBc\fR|\fBm\fR|\fBo\fR|\fBp\fR|\fBv\fR]\ [\fB+\fR|\fB!\fR]\ {\fIlogfile\fR} Enable logging to \fIlogfile\fR. Defaults are \'iwearmo\'. -.IP "\fl*\fR" +.IP "\fBl*\fR" Enable all logging options except \'v\' and \'x\'. -.IP "\fli\fR" +.IP "\fBli\fR" Log status messages. -.IP "\flw\fR" +.IP "\fBlw\fR" Log nonfatal warnings. -.IP "\fle\fR" +.IP "\fBle\fR" Log all error messages. -.IP "\fla\fR" +.IP "\fBla\fR" Log start of actions. -.IP "\flr\fR" +.IP "\fBlr\fR" Log action specifc records. -.IP "\flu\fR" +.IP "\fBlu\fR" Log user requests. -.IP "\flc\fR" +.IP "\fBlc\fR" Log initial UI parameters. -.IP "\flm\fR" +.IP "\fBlm\fR" Log out of memory errors. -.IP "\flo\fR" +.IP "\fBlo\fR" Log out of diskspace messages. -.IP "\flp \fR" +.IP "\fBlp \fR" Log terminal properties. -.IP "\flv \fR" +.IP "\fBlv \fR" Verbose logging. -.IP "\flx \fR" +.IP "\fBlx \fR" Log extra debugging messages. -.IP "\fl+ \fR" +.IP "\fBl+ \fR" Append logging to existing file. -.IP "\fl! \fR" +.IP "\fBl! \fR" Flush each line to log. .SH OTHER OPTIONS .IP \fB/h Problems with winemaker.1: Unbalanced group in command synopis. You probably forgot to open or close a [ ] or { } group properly. --- winemaker.1-unpatched 2014-06-02 09:52:02.517334795 -0400 +++ winemaker.1 2014-06-02 09:54:21.272566508 -0400 @@ -29,7 +29,6 @@ .br [ .BR "--generated-files " "] [ " "--nogenerated-files " "] -] .br [ .BR "--wine32 " "]

3 3

Re: [PATCH] d3dx9: implement effect state management.
by Matteo Bruni March 1, 2016

March 1, 2016

2016-02-29 19:08 GMT+01:00 Paul Gofman <gofmanp(a)gmail.com>: > Preshaders (and FXLC-type constants requiring preshaders) are TODOs (on the way). > > Signed-off-by: Paul Gofman <gofmanp(a)gmail.com> > --- > dlls/d3dx9_36/effect.c | 375 +++++++++++++++++++++++++++++++++++++++++-- > dlls/d3dx9_36/tests/effect.c | 241 +++++++++++++++++++++++++++ > 2 files changed, 606 insertions(+), 10 deletions(-) You should split this patch into smaller pieces. One way to do that is to introduce the various state classes one at a time into d3dx9_apply_state (or, not everything at once, anyway). The state saving / restoring should also be a separate patch. WRT the tests, they should probably be split out too. You can put them before the implementation, inserting and removing "todo_wine" where necessary, or after. If it's not too complicated the first option is generally preferred. BTW it turns out I have a similar test somewhere in my queue. I'll have another look and I might send it if it still makes sense to. I'm leaving a few comments inline in the patch, below, mostly style issues I noticed at a quick look. Many of them apply multiple times through the patch. In general, make sure to be consistent with code style. Oh, and thank you for working on this! > diff --git a/dlls/d3dx9_36/effect.c b/dlls/d3dx9_36/effect.c > index f0bbd0b..33e063a 100644 > --- a/dlls/d3dx9_36/effect.c > +++ b/dlls/d3dx9_36/effect.c > @@ -162,6 +162,8 @@ struct d3dx_technique > > struct d3dx_parameter *annotations; > struct d3dx_pass *passes; > + > + struct IDirect3DStateBlock9 *saved_state; > }; > > struct param_table > @@ -642,6 +644,12 @@ static void free_technique(struct d3dx_technique *technique) > if (!technique) > return; > > + if (technique->saved_state) > + { > + IDirect3DStateBlock9_Release(technique->saved_state); > + technique->saved_state = NULL; > + } > + > if (technique->annotations) > { > for (i = 0; i < technique->annotation_count; ++i) > @@ -2490,6 +2498,332 @@ static HRESULT d3dx9_base_effect_set_array_range(struct d3dx9_base_effect *base, > return E_NOTIMPL; > } > > +static HRESULT d3dx9_get_param_value_ptr(struct ID3DXEffectImpl *This, struct d3dx_pass *pass, struct d3dx_state *state, void **param_value, struct d3dx_parameter **out_param) > +{ > + struct d3dx_parameter *param; > + param = state->parameter.referenced_param ? state->parameter.referenced_param : &state->parameter; > + > + switch(state->type) Space after the switch keyword. > + { > + case ST_CONSTANT: > + case ST_PARAMETER: > + *param_value = param->data; > + *out_param = param; > + return D3D_OK; > + case ST_ARRAY_SELECTOR: > + FIXME("array selector\n"); Uppercase at the start and period at the end of the message. > + break; > + case ST_FXLC: > + FIXME("FXLC not supported yet\n"); > + break; > + } > + *param_value = NULL; > + *out_param = NULL; > + return E_NOTIMPL; > +} > + > +void d3dx9_set_light_parameter(enum LIGHT_TYPE op, D3DLIGHT9 *light, void *value) > +{ > + static struct > + { > + int off; > + const char *name; > + } light_tbl[] = > + { > + {offsetof(D3DLIGHT9, Type), "LC_TYPE"}, You probably want to use FIELD_OFFSET instead. > + {offsetof(D3DLIGHT9, Diffuse), "LT_DIFFUSE"}, > + {offsetof(D3DLIGHT9, Specular), "LT_SPECULAR"}, > + {offsetof(D3DLIGHT9, Ambient), "LT_AMBIENT"}, > + {offsetof(D3DLIGHT9, Position), "LT_POSITION"}, > + {offsetof(D3DLIGHT9, Direction), "LT_DIRECTION"}, > + {offsetof(D3DLIGHT9, Range), "LT_RANGE"}, > + {offsetof(D3DLIGHT9, Falloff), "LT_FALLOFF"}, > + {offsetof(D3DLIGHT9, Attenuation0), "LT_ATTENUATION0"}, > + {offsetof(D3DLIGHT9, Attenuation1), "LT_ATTENUATION1"}, > + {offsetof(D3DLIGHT9, Attenuation2), "LT_ATTENUATION2"}, > + {offsetof(D3DLIGHT9, Theta), "LT_THETA"}, > + {offsetof(D3DLIGHT9, Phi), "LT_PHI"} > + }; > + switch(op) > + { > + case LT_TYPE: > + TRACE("LT_TYPE %d\n", *(D3DLIGHTTYPE*)value); Please put a space between the type and the '*' in the cast. > + light->Type = *(D3DLIGHTTYPE*)value; > + break; > + case LT_DIFFUSE: > + case LT_SPECULAR: > + case LT_AMBIENT: > + { > + D3DCOLORVALUE c = *(D3DCOLORVALUE*)value; > + > + TRACE("%s (%f %f %f %f)\n", light_tbl[op].name, c.r, c.g, c.b, c.a); > + *(D3DCOLORVALUE*)((char*)light + light_tbl[op].off) = c; > + break; > + } > + case LT_POSITION: > + case LT_DIRECTION: > + { > + D3DVECTOR v = *(D3DVECTOR*)value; > + > + TRACE("%s (%f %f %f)\n", light_tbl[op].name, v.x, v.y, v.z); > + *(D3DVECTOR*)((char*)light + light_tbl[op].off) = v; > + break; > + } > + case LT_RANGE: > + case LT_FALLOFF: > + case LT_ATTENUATION0: > + case LT_ATTENUATION1: > + case LT_ATTENUATION2: > + case LT_THETA: > + case LT_PHI: > + { > + float v = *(float*)value; > + TRACE("%s %f\n", light_tbl[op].name, v); > + *(float*)((char*)light + light_tbl[op].off) = v; > + break; > + } > + default: > + FIXME("unknown light parameter %d\n",op); > + break; > + } > +} > + > +void d3dx9_set_material_parameter(enum MATERIAL_TYPE op, D3DMATERIAL9 *material, void *value) > +{ > + static struct Please make it const too, if that works (I guess it might not). > + { > + int off; > + const char *name; > + } material_tbl[] = > + { > + {offsetof(D3DMATERIAL9, Diffuse), "MT_DIFFUSE"}, > + {offsetof(D3DMATERIAL9, Ambient), "MT_AMBIENT"}, > + {offsetof(D3DMATERIAL9, Specular), "MT_SPECULAR"}, > + {offsetof(D3DMATERIAL9, Emissive), "MT_EMISSIVE"}, > + {offsetof(D3DMATERIAL9, Power), "MT_POWER"} > + }; > + > + if (op < 0 || op > MT_POWER) > + { > + FIXME("unknown material parameter %d\n",op); > + return ; Stray whitespace. > + } > + > + switch(op) > + { > + case MT_POWER: > + { > + float v = *(float*)value; > + > + TRACE("%s %f\n", material_tbl[op].name, v); > + material->Power = v; > + break; > + } > + default: > + { > + D3DCOLORVALUE c = *(D3DCOLORVALUE*)value; > + > + TRACE("%s (%f %f %f %f)\n", material_tbl[op].name, c.r, c.g, c.b, c.a); > + *(D3DCOLORVALUE*)((char*)material + material_tbl[op].off) = c; > + break; > + } > + } > +} > + > +HRESULT d3dx_set_shader_const_state(IDirect3DDevice9 *device, enum SHADER_CONSTANT_TYPE op, int index, struct d3dx_parameter *param, void *value_ptr) Probably index should be unsigned int. > +{ > + static struct { '{' should be on its own line. > + D3DXPARAMETER_TYPE type; > + int elem_size; > + const char *name; > + } const_tbl[] = > + { > + {D3DXPT_FLOAT, sizeof(float)*4, "SCT_VSFLOAT"}, Space before and after the binary operator. > + {D3DXPT_BOOL, sizeof(BOOL), "SCT_VSBOOL"}, > + {D3DXPT_INT, sizeof(int)*4, "SCT_VSINT"}, > + {D3DXPT_FLOAT, sizeof(float)*4, "SCT_PSFLOAT"}, > + {D3DXPT_BOOL, sizeof(BOOL), "SCT_PSBOOL"}, > + {D3DXPT_INT, sizeof(int)*4, "SCT_PSINT"}, > + }; > + int nelem; > + > + if (op < 0 || op > SCT_PSINT) > + { > + FIXME("unknown op %d\n", op); > + return D3DERR_INVALIDCALL; > + } > + nelem = param->bytes / const_tbl[op].elem_size; > + TRACE("setting %s index %d, %d elements\n", const_tbl[op].name, index, nelem); > + if (param->type != const_tbl[op].type) > + { > + FIXME("unexpected param type: %d\n", param->type); > + return D3DERR_INVALIDCALL; > + } > + if (param->bytes % const_tbl[op].elem_size != 0) > + { > + FIXME("unexpected param size: %d (rows=%d, cols=%d)\n", param->bytes, param->rows, param->columns); %u, also we usually do without the '=' i.e. "rows %u, cols %u". > + return D3DERR_INVALIDCALL; > + } > + > + switch(op) > + { > + case SCT_VSFLOAT: > + return IDirect3DDevice9_SetVertexShaderConstantF(device, index, (const float*)value_ptr,nelem); > + case SCT_VSBOOL: > + return IDirect3DDevice9_SetVertexShaderConstantB(device, index, (const BOOL*)value_ptr,nelem); > + case SCT_VSINT: > + return IDirect3DDevice9_SetVertexShaderConstantI(device, index, (const int*)value_ptr,nelem); > + case SCT_PSFLOAT: > + return IDirect3DDevice9_SetPixelShaderConstantF(device, index, (const float*)value_ptr,nelem); > + case SCT_PSBOOL: > + return IDirect3DDevice9_SetPixelShaderConstantB(device, index, (const BOOL*)value_ptr,nelem); > + case SCT_PSINT: > + return IDirect3DDevice9_SetPixelShaderConstantI(device, index, (const int*)value_ptr,nelem); > + } > + return D3D_OK; > +} > + > +static HRESULT d3dx9_apply_state(struct ID3DXEffectImpl *This, struct d3dx_pass *pass, struct d3dx_state *state, int parent_index) > +{ > + IDirect3DDevice9 *device = This->device; > + struct d3dx_parameter *param; > + void *param_value; > + HRESULT hr; > + > + TRACE("operation=%d index=%d type=%u\n", state->operation, state->index, state->type); > + hr = d3dx9_get_param_value_ptr(This, pass, state, &param_value, &param); > + if (FAILED(hr)) > + return hr; > + > + switch(state_table[state->operation].class) > + { > + case SC_RENDERSTATE: > + TRACE("%s: operation %d value %u\n", state_table[state->operation].name, state_table[state->operation].op, *(DWORD*)param_value); > + return IDirect3DDevice9_SetRenderState(device, state_table[state->operation].op, *(DWORD*)param_value); > + case SC_FVF: > + TRACE("%s: value %#x\n", state_table[state->operation].name, *(DWORD*)param_value); > + return IDirect3DDevice9_SetFVF(device, *(DWORD*)param_value); > + case SC_TEXTURE: > + { > + int sampler; > + > + sampler = parent_index == -1 ? state->index : parent_index; > + TRACE("%s: sampler %d value %p\n", state_table[state->operation].name, sampler, *(IDirect3DBaseTexture9 **)param_value); > + return IDirect3DDevice9_SetTexture(device, sampler, *(IDirect3DBaseTexture9 **)param_value); > + } > + case SC_TEXTURESTAGE: > + TRACE("%s: stage %d value %u\n", state_table[state->operation].name, state->index, *(DWORD*)param_value); > + return IDirect3DDevice9_SetTextureStageState(device, state->index, state_table[state->operation].op, *(DWORD*)param_value); > + case SC_SETSAMPLER: > + { > + struct d3dx_sampler *sampler; > + HRESULT ret, hr; > + int i; > + > + sampler = (struct d3dx_sampler *)param_value; > + TRACE("%s: sampler %d applying %d states\n", state_table[state->operation].name, state->index, sampler->state_count); > + ret = D3D_OK; > + for (i = 0; i < sampler->state_count; i++) > + { > + hr = d3dx9_apply_state(This, pass, &sampler->states[i], state->index); > + if (FAILED(hr)) > + ret = hr; > + } > + return ret; > + } > + case SC_SAMPLERSTATE: > + { > + int sampler; > + > + sampler = parent_index == -1 ? state->index : parent_index; > + TRACE("%s sampler %d:%u\n", state_table[state->operation].name, sampler, *(DWORD*)param_value); > + return IDirect3DDevice9_SetSamplerState(device, sampler, state_table[state->operation].op, *(DWORD*)param_value); > + } > + case SC_VERTEXSHADER: > + TRACE("%s vertex shader %p\n", state_table[state->operation].name, *(IDirect3DVertexShader9 **)param_value); > + hr = IDirect3DDevice9_SetVertexShader(device, *(IDirect3DVertexShader9 **)param_value); > + if (FAILED(hr)) > + ERR("could not set vertex shader\n"); > + FIXME("not executing preshader and not setting constants\n"); > + return hr; > + case SC_PIXELSHADER: > + TRACE("%s pixel shader %p\n", state_table[state->operation].name, *(IDirect3DPixelShader9 **)param_value); > + hr = IDirect3DDevice9_SetPixelShader(device, *(IDirect3DPixelShader9 **)param_value); > + if (FAILED(hr)) > + ERR("could not set pixel shader\n"); > + FIXME("not executing preshader and not setting constants\n"); > + return hr; > + case SC_TRANSFORM: > + TRACE("%s %d\n", state_table[state->operation].name, state->index); > + return IDirect3DDevice9_SetTransform(device, state_table[state->operation].op + state->index, (D3DMATRIX*)param_value); > + case SC_LIGHTENABLE: > + TRACE("%s index %d %d\n", state_table[state->operation].name, state->index, *(BOOL*)param_value); > + return IDirect3DDevice9_LightEnable(device, state->index, *(BOOL*)param_value); > + case SC_LIGHT: > + { > + D3DLIGHT9 light; > + > + TRACE("%s op %d index %d\n", state_table[state->operation].name, state_table[state->operation].op, state->index); > + hr = IDirect3DDevice9_GetLight(device, state->index, &light); > + if (FAILED(hr)) > + { > + WARN("could not get light: %#x\n", hr); > + memset(&light, 0, sizeof light); > + } > + d3dx9_set_light_parameter(state_table[state->operation].op, &light, param_value); > + return IDirect3DDevice9_SetLight(device, state->index, &light); > + } > + case SC_MATERIAL: > + { > + D3DMATERIAL9 material; > + > + TRACE("%s op %d index %d\n", state_table[state->operation].name, state_table[state->operation].op, state->index); > + hr = IDirect3DDevice9_GetMaterial(device, &material); > + if (FAILED(hr)) > + { > + WARN("could not get material: %#x\n", hr); > + memset(&material, 0, sizeof material); > + } > + d3dx9_set_material_parameter(state_table[state->operation].op, &material, param_value); > + return IDirect3DDevice9_SetMaterial(device, &material); > + } > + case SC_NPATCHMODE: > + TRACE("%s %f\n", state_table[state->operation].name, *(float*)param_value); > + return IDirect3DDevice9_SetNPatchMode(device, *(float*)param_value); > + break; Break is unnecessary after the return (also it's wrongly indented). > + case SC_SHADERCONST: > + TRACE("%s op %d index %d\n", state_table[state->operation].name, state_table[state->operation].op, state->index); > + return d3dx_set_shader_const_state(device, state_table[state->operation].op, state->index, param, param_value); > + break; > + default: > + FIXME("%s not handled\n", state_table[state->operation].name); > + break; > + } > + return D3D_OK; > +} > + > +static HRESULT d3dx9_apply_pass_states(struct ID3DXEffectImpl *This, struct d3dx_pass *pass) > +{ > + int istate; Unsigned int, also you could just call it "i" (or "state_idx"). > + HRESULT ret; > + > + TRACE("This %p, pass %p, state_count=%d\n", This, pass, pass->state_count); > + > + ret = D3D_OK; > + for (istate = 0; istate < pass->state_count; istate++) > + { > + HRESULT hr; > + > + hr = d3dx9_apply_state(This, pass, &pass->states[istate], -1); > + if (FAILED(hr)) > + { > + WARN("error applying state: %#x\n", hr); > + ret = hr; > + } > + } > + return ret; > +} > + > static inline struct ID3DXEffectImpl *impl_from_ID3DXEffect(ID3DXEffect *iface) > { > return CONTAINING_RECORD(iface, struct ID3DXEffectImpl, ID3DXEffect_iface); > @@ -3158,7 +3492,23 @@ static HRESULT WINAPI ID3DXEffectImpl_Begin(ID3DXEffect *iface, UINT *passes, DW > } > else > { > - FIXME("State capturing not supported, yet!\n"); > + HRESULT hr; > + int i; > + > + if (!technique->saved_state) > + { > + hr = IDirect3DDevice9_BeginStateBlock(This->device); > + if (FAILED(hr)) > + ERR("BeginStateBlock failed: %#x\n",hr); > + for (i = 0; i < technique->pass_count; i++) > + d3dx9_apply_pass_states(This, &technique->passes[i]); > + hr = IDirect3DDevice9_EndStateBlock(This->device, &technique->saved_state); > + if (FAILED(hr)) > + ERR("EndStateBlock failed: %#x\n",hr); > + } > + hr = IDirect3DStateBlock9_Capture(technique->saved_state); > + if (FAILED(hr)) > + ERR("StateBlock Capture failed: %#x\n",hr); > } > > *passes = technique->pass_count; > @@ -3183,10 +3533,7 @@ static HRESULT WINAPI ID3DXEffectImpl_BeginPass(ID3DXEffect *iface, UINT pass) > if (technique && pass < technique->pass_count && !This->active_pass) > { > This->active_pass = &technique->passes[pass]; > - > - FIXME("No states applied, yet!\n"); > - > - return D3D_OK; > + return d3dx9_apply_pass_states(This, This->active_pass); > } > > WARN("Invalid argument supplied.\n"); > @@ -3198,15 +3545,13 @@ static HRESULT WINAPI ID3DXEffectImpl_CommitChanges(ID3DXEffect* iface) > { > struct ID3DXEffectImpl *This = impl_from_ID3DXEffect(iface); > > - FIXME("(%p)->(): stub\n", This); > - > if (!This->active_pass) > { > WARN("Called without an active pass.\n"); > return D3D_OK; > } > - > - return E_NOTIMPL; > + /* TODO: apply only changed states */ > + return d3dx9_apply_pass_states(This, This->active_pass); > } > > static HRESULT WINAPI ID3DXEffectImpl_EndPass(ID3DXEffect *iface) > @@ -3229,6 +3574,7 @@ static HRESULT WINAPI ID3DXEffectImpl_EndPass(ID3DXEffect *iface) > static HRESULT WINAPI ID3DXEffectImpl_End(ID3DXEffect *iface) > { > struct ID3DXEffectImpl *This = impl_from_ID3DXEffect(iface); > + struct d3dx_technique *technique = This->active_technique; > > TRACE("iface %p.\n", iface); > > @@ -3241,7 +3587,16 @@ static HRESULT WINAPI ID3DXEffectImpl_End(ID3DXEffect *iface) > } > else > { > - FIXME("State restoring not supported, yet!\n"); > + HRESULT hr; > + > + if (technique && technique->saved_state) > + { > + hr = IDirect3DStateBlock9_Apply(technique->saved_state); > + if (FAILED(hr)) > + ERR("stateblock apply failed: %#x\n",hr); > + } > + else > + ERR("no saved state"); > } > > This->started = FALSE; > diff --git a/dlls/d3dx9_36/tests/effect.c b/dlls/d3dx9_36/tests/effect.c > index 482f064..a1b35b7 100644 > --- a/dlls/d3dx9_36/tests/effect.c > +++ b/dlls/d3dx9_36/tests/effect.c > @@ -2692,6 +2692,246 @@ static void test_effect_compilation_errors(IDirect3DDevice9 *device) > effect->lpVtbl->Release(effect); > } > > +/* fxc.exe /T fx_2_0 > +vertexshader vs_arr1[2] = > +{ > + asm > + { > + vs_1_1 > + def c0, 1, 1, 1, 1 > + mov oPos, c0 > + }, > + asm > + { > + vs_2_0 > + def c0, 2, 2, 2, 2 > + mov oPos, c0 > + } > +}; Good idea with the shader array. > + > +sampler sampler1 = > +sampler_state > +{ > + MipFilter = LINEAR; > +}; > + > +float4x4 camera : VIEW = {4.0,0.0,0.0,0.0, 0.0,0.0,0.0,0.0, 0.0,0.0,0.0,0.0, 0.0,0.0,0.0,6.0}; > +technique tech0 > +{ > + pass p0 > + { > + vertexshader = vs_arr1[1]; Idea for a future preshader test, see what happens if you put an expression using parameters as the array index. Not a priority by any means. > + VertexShaderConstant1[3] = {2,2,2,2}; > + BlendOp = 2; > + AlphaOp[3] = 4; > + ZEnable = true; > + WorldTransform[1]={2.0,0.0,0.0,0.0, 0.0,0.0,0.0,0.0, 0.0,0.0,0.0,0.0, 0.0,0.0,0.0,4.0}; > + ViewTransform=(camera); > + LightEnable[2] = TRUE; > + LightType[2] = POINT; > + LightPosition[2] = {4.0f, 5.0f, 6.0f}; > + Sampler[1] = sampler1; > + } > +} I know this is just for reference but it would be better if this was formatted properly. > +*/ > +static const DWORD test_effect_states_effect_blob[] = > +{ > +0xfeff0901, 0x000002e8, 0x00000000, 0x00000010, 0x00000004, 0x00000020, 0x00000000, 0x00000002, > +0x00000001, 0x00000002, 0x00000008, 0x615f7376, 0x00317272, 0x0000000a, 0x00000004, 0x00000074, > +0x00000000, 0x00000000, 0x00000002, 0x00000002, 0x00000002, 0x00000000, 0x00000000, 0x00000000, > +0x00000001, 0x00000001, 0x00000001, 0x000000ab, 0x00000100, 0x00000044, 0x00000040, 0x00000009, > +0x706d6173, 0x3172656c, 0x00000000, 0x00000003, 0x00000002, 0x000000e0, 0x000000ec, 0x00000000, > +0x00000004, 0x00000004, 0x40800000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, > +0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, > +0x00000000, 0x40c00000, 0x00000007, 0x656d6163, 0x00006172, 0x00000005, 0x57454956, 0x00000000, > +0x00000003, 0x00000010, 0x00000004, 0x00000000, 0x00000000, 0x00000000, 0x40000000, 0x40000000, > +0x40000000, 0x40000000, 0x00000003, 0x00000003, 0x00000000, 0x00000000, 0x00000000, 0x00000004, > +0x00000001, 0x00000002, 0x00000002, 0x00000002, 0x00000000, 0x00000000, 0x00000000, 0x00000001, > +0x00000001, 0x00000004, 0x00000002, 0x00000002, 0x00000000, 0x00000000, 0x00000000, 0x00000001, > +0x00000001, 0x00000001, 0x00000002, 0x00000002, 0x00000000, 0x00000000, 0x00000000, 0x00000001, > +0x00000001, 0x40000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, > +0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, > +0x40800000, 0x00000003, 0x00000002, 0x00000000, 0x00000000, 0x00000000, 0x00000010, 0x00000001, > +0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, > +0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, > +0x00000003, 0x00000002, 0x00000000, 0x00000000, 0x00000000, 0x00000004, 0x00000004, 0x00000001, > +0x00000002, 0x00000002, 0x00000000, 0x00000000, 0x00000000, 0x00000001, 0x00000001, 0x00000001, > +0x00000002, 0x00000002, 0x00000000, 0x00000000, 0x00000000, 0x00000001, 0x00000001, 0x40800000, > +0x40a00000, 0x40c00000, 0x00000003, 0x00000002, 0x00000000, 0x00000000, 0x00000000, 0x00000003, > +0x00000001, 0x00000000, 0x0000000a, 0x00000004, 0x00000000, 0x00000000, 0x00000000, 0x00000003, > +0x00003070, 0x00000006, 0x68636574, 0x00000030, 0x00000003, 0x00000001, 0x00000005, 0x00000004, > +0x00000004, 0x00000018, 0x00000000, 0x00000000, 0x0000002c, 0x00000060, 0x00000000, 0x00000000, > +0x00000084, 0x000000a0, 0x00000000, 0x00000000, 0x000002dc, 0x00000000, 0x00000001, 0x000002d4, > +0x00000000, 0x0000000b, 0x00000092, 0x00000000, 0x000000fc, 0x000000f8, 0x00000098, 0x00000003, > +0x00000120, 0x00000110, 0x0000004b, 0x00000000, 0x00000140, 0x0000013c, 0x0000006b, 0x00000003, > +0x00000160, 0x0000015c, 0x00000000, 0x00000000, 0x00000180, 0x0000017c, 0x0000007d, 0x00000001, > +0x000001dc, 0x0000019c, 0x0000007c, 0x00000000, 0x00000238, 0x000001f8, 0x00000091, 0x00000002, > +0x00000258, 0x00000254, 0x00000084, 0x00000002, 0x00000278, 0x00000274, 0x00000088, 0x00000002, > +0x000002a0, 0x00000294, 0x000000b2, 0x00000001, 0x000002c0, 0x000002bc, 0x00000002, 0x00000003, > +0x00000001, 0x0000002c, 0xfffe0101, 0x00000051, 0xa00f0000, 0x3f800000, 0x3f800000, 0x3f800000, > +0x3f800000, 0x00000001, 0xc00f0000, 0xa0e40000, 0x0000ffff, 0x00000002, 0x0000002c, 0xfffe0200, > +0x05000051, 0xa00f0000, 0x40000000, 0x40000000, 0x40000000, 0x40000000, 0x02000001, 0xc00f0000, > +0xa0e40000, 0x0000ffff, 0x00000000, 0x00000000, 0xffffffff, 0x0000000a, 0x00000001, 0x00000009, > +0x706d6173, 0x3172656c, 0x00000000, 0x00000000, 0x00000000, 0xffffffff, 0x00000006, 0x00000000, > +0x0000016c, 0x46580200, 0x0030fffe, 0x42415443, 0x0000001c, 0x0000008b, 0x46580200, 0x00000001, > +0x0000001c, 0x20000100, 0x00000088, 0x00000030, 0x00000002, 0x00000004, 0x00000038, 0x00000048, > +0x656d6163, 0xab006172, 0x00030003, 0x00040004, 0x00000001, 0x00000000, 0x40800000, 0x00000000, > +0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, > +0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x40c00000, 0x4d007874, 0x6f726369, > +0x74666f73, 0x29522820, 0x534c4820, 0x6853204c, 0x72656461, 0x6d6f4320, 0x656c6970, 0x2e392072, > +0x392e3932, 0x332e3235, 0x00313131, 0x0002fffe, 0x54494c43, 0x00000000, 0x0024fffe, 0x434c5846, > +0x00000004, 0x10000004, 0x00000001, 0x00000000, 0x00000002, 0x00000000, 0x00000000, 0x00000004, > +0x00000000, 0x10000004, 0x00000001, 0x00000000, 0x00000002, 0x00000004, 0x00000000, 0x00000004, > +0x00000004, 0x10000004, 0x00000001, 0x00000000, 0x00000002, 0x00000008, 0x00000000, 0x00000004, > +0x00000008, 0x10000004, 0x00000001, 0x00000000, 0x00000002, 0x0000000c, 0x00000000, 0x00000004, > +0x0000000c, 0xf0f0f0f0, 0x0f0f0f0f, 0x0000ffff, 0x00000000, 0x00000000, 0xffffffff, 0x00000000, > +0x00000001, 0x0000000b, 0x615f7376, 0x5b317272, 0x00005d31, > +}; > + > +static DWORD test_effect_states_vshader_buf[sizeof test_effect_states_effect_blob / sizeof (DWORD)]; Please use parentheses for the argument of sizeof consistently (always use them, no space). > + > +static void test_effect_states(IDirect3DDevice9 *device) > +{ > + D3DMATRIX test_mat = > + { > + { > + -1.0f, 0.0f, 0.0f, 0.0f, > + 0.0f, 0.0f, 0.0f, 0.0f, > + 0.0f, 0.0f, 0.0f, 0.0f, > + 0.0f, 0.0f, 0.0f, 0.0f > + } > + }; > + D3DMATRIX mat; > + HRESULT hr; > + ID3DXEffect *effect; > + UINT cPasses; No camelcase or similar, just lowercase with underscores. > + DWORD value; > + IDirect3DVertexShader9 *vshader; > + UINT byte_code_size; > + BOOL bval; > + D3DLIGHT9 light; > + float float_data[4]; > + > + hr = D3DXCreateEffect(device, test_effect_states_effect_blob, sizeof(test_effect_states_effect_blob), > + NULL, NULL, 0, NULL, &effect, NULL); > + ok(hr == D3D_OK, "Got result %x, expected 0 (D3D_OK)\n", hr); > + > + /* 1. State affected in passes saved/restored even if no pass > + was performed. States not present in passes are not saved & > + restored > + */ The "*/" goes on the previous line. > + hr = IDirect3DDevice9_SetRenderState(device, D3DRS_BLENDOP, 1); > + ok(hr == D3D_OK, "Got result %x, expected 0 (D3D_OK)\n", hr); > + hr = IDirect3DDevice9_SetRenderState(device, D3DRS_ALPHAFUNC, 1); > + ok(hr == D3D_OK, "Got result %x, expected 0 (D3D_OK)\n", hr); > + > + hr = effect->lpVtbl->Begin(effect, &cPasses, 0); > + ok(hr == D3D_OK, "Got result %x, expected 0 (D3D_OK)\n", hr); > + ok(cPasses == 1, "Expected 1 pass, got %d\n", cPasses); > + > + hr = IDirect3DDevice9_SetRenderState(device, D3DRS_BLENDOP, 3); > + ok(hr == D3D_OK, "Got result %x, expected 0 (D3D_OK)\n", hr); > + hr = IDirect3DDevice9_SetRenderState(device, D3DRS_ALPHAFUNC, 2); > + ok(hr == D3D_OK, "Got result %x, expected 0 (D3D_OK)\n", hr); > + > + hr = effect->lpVtbl->End(effect); > + ok(hr == D3D_OK, "Got result %x, expected 0 (D3D_OK)\n", hr); > + > + hr = IDirect3DDevice9_GetRenderState(device, D3DRS_BLENDOP, &value); > + ok(hr == D3D_OK, "Got result %x, expected 0 (D3D_OK)\n", hr); > + ok(value == 1, "Got result %d, expected %d\n", value, 1); > + hr = IDirect3DDevice9_GetRenderState(device, D3DRS_ALPHAFUNC, &value); > + ok(value == 2, "Got result %d, expected %d\n", value, 2); > + > + /* 2. Test states application in BeginPass. No states are restored > + on EndPass. > + */ > + hr = IDirect3DDevice9_SetSamplerState(device, 1, D3DSAMP_MIPFILTER, 0); > + ok(hr == D3D_OK, "Got result %x, expected 0 (D3D_OK)\n", hr); > + hr = IDirect3DDevice9_SetRenderState(device, D3DRS_ZENABLE, 0); > + ok(hr == D3D_OK, "Got result %x, expected 0 (D3D_OK)\n", hr); > + > + hr = IDirect3DDevice9_GetLightEnable(device, 2, &bval); > + ok(!bval,"Got result %d, expected 0", bval); > + > + hr = IDirect3DDevice9_SetTransform(device, D3DTS_WORLDMATRIX(1), &test_mat); > + hr = effect->lpVtbl->Begin(effect, &cPasses, 0); > + ok(hr == D3D_OK, "Got result %x, expected 0 (D3D_OK)\n", hr); > + > + hr = IDirect3DDevice9_GetTransform(device, D3DTS_WORLDMATRIX(1), &mat); > + ok(hr == D3D_OK, "Got result %x, expected 0 (D3D_OK)\n", hr); > + ok(mat.m[0][0] == test_mat.m[0][0], "Unexpected value: %f\n", mat.m[0][0]); I would check the entire matrix with memcmp(). > + > + hr = effect->lpVtbl->BeginPass(effect, 0); > + ok(hr == D3D_OK, "Got result %x, expected 0 (D3D_OK)\n", hr); > + > + hr = IDirect3DDevice9_GetTransform(device, D3DTS_WORLDMATRIX(1), &mat); > + ok(hr == D3D_OK, "Got result %x, expected 0 (D3D_OK)\n", hr); > + ok(mat.m[0][0] == 2.0f && mat.m[3][3] == 4.0f, "World matrix not applied (%f %f)\n", mat.m[0][0], mat.m[3][3]); > + > + hr = IDirect3DDevice9_GetTransform(device, D3DTS_VIEW, &mat); > + ok(hr == D3D_OK, "Got result %x, expected 0 (D3D_OK)\n", hr); > + todo_wine ok(mat.m[0][0] == 4.0f && mat.m[3][3] == 6.0f, "View matrix not applied (%f %f)\n", mat.m[0][0], mat.m[3][3]); > + > + hr = IDirect3DDevice9_GetRenderState(device, D3DRS_BLENDOP, &value); > + ok(hr == D3D_OK, "Got result %x, expected 0 (D3D_OK)\n", hr); > + ok(value == 2, "Got result %d, expected %d\n", value, 2); > + > + hr = IDirect3DDevice9_GetVertexShader(device, &vshader); > + ok(hr == D3D_OK, "Got result %x, expected 0 (D3D_OK)\n", hr); > + byte_code_size = sizeof test_effect_states_vshader_buf; > + hr = IDirect3DVertexShader9_GetFunction(vshader, test_effect_states_vshader_buf,&byte_code_size); > + ok(hr == D3D_OK, "Got result %x, expected 0 (D3D_OK)\n", hr); > + ok(byte_code_size > 1, "Got unexpected byte code size %d\n", byte_code_size); > + ok(test_effect_states_vshader_buf[0] == 0xfffe0200, "Incorrect shader selected: %#x\n", test_effect_states_vshader_buf[0]); > + IDirect3DVertexShader9_Release(vshader); > + > + hr = IDirect3DDevice9_GetLightEnable(device, 2, &bval); > + ok(hr == D3D_OK, "Got result %x, expected 0 (D3D_OK)\n", hr); > + ok(bval,"Got result %d, expected TRUE", bval); > + hr = IDirect3DDevice9_GetLight(device, 2, &light); > + ok(hr == D3D_OK, "Got result %x, expected 0 (D3D_OK)\n", hr); > + ok(light.Position.x == 4.0f && light.Position.y == 5.0f && light.Position.z == 6.0f, > + "Got unexpected light position (%f, %f, %f)\n", light.Position.x, light.Position.y, light.Position.z); > + hr = IDirect3DDevice9_GetVertexShaderConstantF(device, 3, float_data, 1); > + ok(hr == D3D_OK, "Got result %x, expected 0 (D3D_OK)\n", hr); > + ok(float_data[0] == 2.0f && float_data[1] == 2.0f && float_data[2] == 2.0f && float_data[3] == 2.0f, > + "Got unexpected vertex shader floats: (%f %f %f %f)\n", > + float_data[0], float_data[1], float_data[2], float_data[3]); We use 8 spaces for line continuations in new code. > + > + hr = effect->lpVtbl->EndPass(effect); > + ok(hr == D3D_OK, "Got result %x, expected 0 (D3D_OK)\n", hr); > + hr = IDirect3DDevice9_GetRenderState(device, D3DRS_BLENDOP, &value); > + ok(hr == D3D_OK, "Got result %x, expected 0 (D3D_OK)\n", hr); > + ok(value == 2, "Got result %d, expected %d\n", value, 2); > + > + hr = IDirect3DDevice9_GetRenderState(device, D3DRS_ZENABLE, &value); > + ok(hr == D3D_OK, "Got result %x, expected 0 (D3D_OK)\n", hr); > + ok(value,"Got result %d, expected TRUE\n", value); > + > + hr = IDirect3DDevice9_GetSamplerState(device, 1, D3DSAMP_MIPFILTER, &value); > + ok(hr == D3D_OK, "Got result %x, expected 0 (D3D_OK)\n", hr); > + ok(value == D3DTEXF_LINEAR, "Unexpected sampler 1 mipfilter %d\n", value); > + > + hr = IDirect3DDevice9_GetTextureStageState(device, 3, D3DTSS_ALPHAOP, &value); > + ok(hr == D3D_OK, "Got result %x, expected 0 (D3D_OK)\n", hr); > + ok(value == 4, "Unexpected texture stage 3 AlphaOp %d\n", value); > + > + hr = effect->lpVtbl->End(effect); > + ok(hr == D3D_OK, "Got result %x, expected 0 (D3D_OK)\n", hr); > + > + hr = IDirect3DDevice9_GetTransform(device, D3DTS_WORLDMATRIX(1), &mat); > + ok(hr == D3D_OK, "Got result %x, expected 0 (D3D_OK)\n", hr); > + ok(mat.m[0][0] == test_mat.m[0][0] && mat.m[3][3] == test_mat.m[3][3], > + "World matrix not restored (%f %f)\n", mat.m[0][0], mat.m[3][3]); > + > + hr = IDirect3DDevice9_GetLightEnable(device, 2, &bval); > + ok(!bval,"Got result %d, expected 0", bval); > + > + if (effect) > + effect->lpVtbl->Release(effect); > +} > + > START_TEST(effect) > { > HWND wnd; > @@ -2730,6 +2970,7 @@ START_TEST(effect) > test_effect_parameter_value(device); > test_effect_variable_names(device); > test_effect_compilation_errors(device); > + test_effect_states(device); > > count = IDirect3DDevice9_Release(device); > ok(count == 0, "The device was not properly freed: refcount %u\n", count); > -- > 2.5.0

3 3

Re: [PATCH] winex11: Don't create a temporary context each time X11DRV_wglBindTexImageARB is called.
by Ken Thomases March 1, 2016

March 1, 2016

On Feb 26, 2016, at 5:29 PM, Miklós Máté <mtmkls(a)gmail.com> wrote: > @@ -2320,6 +2322,7 @@ static BOOL X11DRV_wglDestroyPbufferARB( struct wgl_pbuffer *object ) > TRACE("(%p)\n", object); > > pglXDestroyPbuffer(gdi_display, object->drawable); > + pglXDestroyContext(gdi_display, object->tmp_context); I think you need to check if object->tmp_context is non-zero before calling glXDestroyContext() to avoid a GLXBadContext error. > + if (!object->tmp_context) { > + object->tmp_context = pglXCreateNewContext(gdi_display, object->fmt->fbconfig, object->fmt->render_type, prev_context, True); > + object->prev_context = prev_context; > + } else if (object->prev_context != prev_context) { > + pglXDestroyContext(gdi_display, object->tmp_context); > + object->tmp_context = pglXCreateNewContext(gdi_display, object->fmt->fbconfig, object->fmt->render_type, prev_context, True); > + object->prev_context = prev_context; > + } Those two branches are mostly the same. This could be reduced to: if (!object->tmp_context || object->prev_context != prev_context) { if (object->tmp_context) pglXDestroyContext(gdi_display, object->tmp_context); object->tmp_context = pglXCreateNewContext(gdi_display, object->fmt->fbconfig, object->fmt->render_type, prev_context, True); object->prev_context = prev_context; } Is it necessary, when a context is destroyed, to go through all wgl_pbuffer structs looking for any whose prev_context matches the one being destroyed and clear it? Otherwise, you risk having a stale reference that could be reused for a different context and not realize you need to recreate tmp_context. Of course, enumerating all of them in a thread-safe manner is going to require a lot of added infrastructure and complexity. -Ken

3 3

Re: [PATCH 2/2] user32: Add SetTimer test with window procedure handle passed as timer callback
by Marvin March 1, 2016

March 1, 2016

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=21002 Your paranoid android. === w864 (32 bit msg) === msg.c:6360: Test failed: Update region shouldn't be empty msg.c:6594: Test failed: InvalidateErase: 0: the msg sequence is not complete: expected 0085 - actual 0000 msg.c:6596: Test failed: Paint: 0: the msg sequence is not complete: expected 000f - actual 0000 msg.c:6366: Test failed: Regions are different msg.c:6366: Test failed: Regions are different msg.c:6366: Test failed: Regions are different === w1064 (32 bit msg) === msg.c:6360: Test failed: Update region shouldn't be empty msg.c:6594: Test failed: InvalidateErase: 0: the msg sequence is not complete: expected 0085 - actual 0000 msg.c:6596: Test failed: Paint: 0: the msg sequence is not complete: expected 000f - actual 0000 msg.c:6366: Test failed: Regions are different msg.c:6366: Test failed: Regions are different msg.c:6366: Test failed: Regions are different === w1064 (64 bit msg) === msg.c:6360: Test failed: Update region shouldn't be empty msg.c:6594: Test failed: InvalidateErase: 0: the msg sequence is not complete: expected 0085 - actual 0000 msg.c:6596: Test failed: Paint: 0: the msg sequence is not complete: expected 000f - actual 0000 msg.c:6366: Test failed: Regions are different msg.c:6366: Test failed: Regions are different msg.c:6366: Test failed: Regions are different

1 0

Re: [PATCH 2/4] gdi32: Return NTM_DSIG when table is present
by Huw Davies March 1, 2016

March 1, 2016

On Tue, Mar 01, 2016 at 07:19:02AM +0300, Nikolay Sivov wrote: > Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> > --- > dlls/gdi32/freetype.c | 13 ++++++++++--- > dlls/gdi32/tests/font.c | 3 +++ > 2 files changed, 13 insertions(+), 3 deletions(-) > > diff --git a/dlls/gdi32/freetype.c b/dlls/gdi32/freetype.c > index 01cb913..57cf695 100644 > --- a/dlls/gdi32/freetype.c > +++ b/dlls/gdi32/freetype.c > @@ -1942,7 +1942,6 @@ static inline FT_Fixed get_font_version( FT_Face ft_face ) > static inline DWORD get_ntm_flags( FT_Face ft_face ) > { > DWORD flags = 0; > - FT_ULong table_size = 0; > FT_WinFNT_HeaderRec winfnt_header; > > if (ft_face->style_flags & FT_STYLE_FLAG_ITALIC) flags |= NTM_ITALIC; > @@ -1956,8 +1955,16 @@ static inline DWORD get_ntm_flags( FT_Face ft_face ) > > if (flags == 0) flags = NTM_REGULAR; > > - if (!pFT_Load_Sfnt_Table( ft_face, FT_MAKE_TAG( 'C','F','F',' ' ), 0, NULL, &table_size )) > - flags |= NTM_PS_OPENTYPE; > + if (FT_IS_SFNT( ft_face )) { > + FT_ULong table_size = 0; > + > + if (!pFT_Load_Sfnt_Table( ft_face, FT_MAKE_TAG( 'C','F','F',' ' ), 0, NULL, &table_size )) > + flags |= NTM_PS_OPENTYPE; > + > + table_size = 0; > + if (!pFT_Load_Sfnt_Table( ft_face, FT_MAKE_TAG( 'D','S','I','G' ), 0, NULL, &table_size )) > + flags |= NTM_DSIG; > + } > > return flags; > } > diff --git a/dlls/gdi32/tests/font.c b/dlls/gdi32/tests/font.c > index d39e701..1059d29 100644 > --- a/dlls/gdi32/tests/font.c > +++ b/dlls/gdi32/tests/font.c > @@ -78,6 +78,7 @@ static WORD system_lang_id; > #define MS_CMAP_TAG MS_MAKE_TAG('c','m','a','p') > #define MS_NAME_TAG MS_MAKE_TAG('n','a','m','e') > #define MS_CFF_TAG MS_MAKE_TAG('C','F','F',' ') > +#define MS_DSIG_TAG MS_MAKE_TAG('D','S','I','G') > > static void init(void) > { > @@ -3968,6 +3969,8 @@ static void test_text_metrics(const LOGFONTA *lf, const NEWTEXTMETRICA *ntm) > has_cff = GetFontData(hdc, MS_CFF_TAG, 0, NULL, 0) != GDI_ERROR; > ok(((ntm->ntmFlags & NTM_PS_OPENTYPE) == NTM_PS_OPENTYPE) == has_cff, "CFF test, got flags 0x%08x, CFF table %d, %s\n", > ntm->ntmFlags, has_cff, font_name); > + if (GetFontData(hdc, MS_DSIG_TAG, 0, NULL, 0) != GDI_ERROR) > + ok(ntm->ntmFlags & NTM_DSIG, "DSIG test, got flags 0x%08x, %s\n", ntm->ntmFlags, font_name); > > test_negative_width(hdc, lf); I get a test failure on OS/X. I suspect it might be something to do with the Helvetica -> Arial substitution. font.c:3974: Test failed: DSIG test, got flags 0x00040040, Helvetica Also, since you'll have to resend it, could you avoid adding more K&R style braces. Huw.

1 0

Re: [v5 PATCH] comctl32/tooltip: Protect TTM_ADDTOOLW from invalid text pointers
by Nikolay Sivov March 1, 2016

March 1, 2016

On 01.03.2016 9:46, Alistair Leslie-Hughes wrote: > diff --git a/dlls/comctl32/tooltips.c b/dlls/comctl32/tooltips.c > index 8bf6919..43c02ee 100644 > --- a/dlls/comctl32/tooltips.c > +++ b/dlls/comctl32/tooltips.c > @@ -1038,7 +1038,9 @@ TOOLTIPS_AddToolT (TOOLTIPS_INFO *infoPtr, const TTTOOLINFOW *ti, BOOL isW) > infoPtr->hwndSelf, ti->hwnd, ti->uId, > (ti->uFlags & TTF_IDISHWND) ? " TTF_IDISHWND" : ""); > > - if (ti->cbSize >= TTTOOLINFOW_V2_SIZE && !ti->lpszText && isW) > + if (ti->cbSize >= TTTOOLINFOW_V2_SIZE && isW && (!ti->lpszText || > + (ti->lpszText != LPSTR_TEXTCALLBACKW && > + IsBadStringPtrW(ti->lpszText, sizeof(WCHAR))))) > return FALSE; > > if (infoPtr->uNumTools == 0) { > IsBadStringPtrW() won't do what you expect, if you expect it to validate whole string up to NULL char. Also !ti->lpszText case is covered by IsBadStringPtrW(), you don't need to test for it separately.

1 0

Re: [PATCH] user32/text: Stop crash when Tab Length is zero
by Alexandre Julliard March 1, 2016

March 1, 2016

Alistair Leslie-Hughes <leslie_alistair(a)hotmail.com> writes: > @@ -948,7 +948,7 @@ INT WINAPI DrawTextExW( HDC hdc, LPWSTR str, INT i_count, > > if (flags & DT_EXPANDTABS) > { > - int tabstop = ((flags & DT_TABSTOP) && dtp) ? dtp->iTabLength : 8; > + int tabstop = ((flags & DT_TABSTOP) && dtp && dtp->iTabLength) ? dtp->iTabLength : 8; You should test the resulting text size to show that this is the correct behavior. -- Alexandre Julliard julliard(a)winehq.org

1 0

Re: [PATCH 2/4] gdi32: Return NTM_DSIG when table is present
by Marvin March 1, 2016

March 1, 2016

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=20990 Your paranoid android. === wvistau64 (32 bit font) === An error occurred while waiting for the test to complete: network read timed out (Connect:AgentVersion.h:0/9)

2 1

Re: [v5 PATCH] comctl32/tooltip: Protect TTM_ADDTOOLW from invalid text pointers
by Marvin March 1, 2016

March 1, 2016

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=20992 Your paranoid android. === wxppro (32 bit tooltips) === tooltips.c:471: Test failed: Adding the tool to the tooltip failed! === w2003std (32 bit tooltips) === tooltips.c:471: Test failed: Adding the tool to the tooltip failed! === wvistau64 (32 bit tooltips) === tooltips.c:471: Test failed: Adding the tool to the tooltip failed! === w2008s64 (32 bit tooltips) === tooltips.c:471: Test failed: Adding the tool to the tooltip failed! === w7u (32 bit tooltips) === tooltips.c:471: Test failed: Adding the tool to the tooltip failed! === w7pro64 (32 bit tooltips) === tooltips.c:471: Test failed: Adding the tool to the tooltip failed! === w8 (32 bit tooltips) === tooltips.c:471: Test failed: Adding the tool to the tooltip failed! === w864 (32 bit tooltips) === tooltips.c:471: Test failed: Adding the tool to the tooltip failed! === w1064 (32 bit tooltips) === tooltips.c:471: Test failed: Adding the tool to the tooltip failed! === wvistau64 (64 bit tooltips) === tooltips.c:471: Test failed: Adding the tool to the tooltip failed! === w2008s64 (64 bit tooltips) === tooltips.c:471: Test failed: Adding the tool to the tooltip failed! === w7pro64 (64 bit tooltips) === tooltips.c:471: Test failed: Adding the tool to the tooltip failed! === w864 (64 bit tooltips) === tooltips.c:471: Test failed: Adding the tool to the tooltip failed! === w1064 (64 bit tooltips) === tooltips.c:471: Test failed: Adding the tool to the tooltip failed!

1 0

Re: [PATCH 3/4] gdi32: Set NTM_TT_OPENTYPE flag for fonts with TrueType outlines
by Marvin March 1, 2016

March 1, 2016

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=20991 Your paranoid android. === wvistau64 (32 bit font) === An error occurred while waiting for the test to complete: network read timed out (Connect:AgentVersion.h:0/9)

1 0