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

Metatrader 5 uses all CPU time
by Vyacheslav Napadovsky March 8, 2016

March 8, 2016

Hello. I'm trying to launch Metatrader 5 in ubuntu with winehq-devel=1.9.5~ubuntu14.04.1 When I log in to my demo account the wineserver and terminal.exe (Metetrader 5 itself) start consume all available CPU. And if I'm trying to "Show all symbols" in "Market overview" then Metatrader UI just hangs. Here's last 10000 lines of 1.9GB log I dumped with WINEDEBUG=fixme-all,warn+all,+relay wine terminal.exe > terminal.log 2>&1 http://sendfile.su/1211848 (can't post it with pastebin.com, because it's over 500kB). Note, I killed process at the end with `killall terminal.exe` Moreover, when terminal.exe is running (without showing all symbols), `vmstat 1` shows that user space / system space consumes 50 / 50 % of CPU. And one more notice, if I run terminal.exe in a Virtual Box and assign this VM only one CPU core, than terminal.exe works fine consuming about 7% cpu, but with 2 cores it consumes 50%/50% with wineserver. On native Windows OS terminal.exe consumes less than 4% CPU with showing all symbols (I have 4 CPU cores on my notebook). Any clues where to dig next? -- Vyacheslav Napadovsky

1 0

wine dll loader
by Vyacheslav Napadovsky March 8, 2016

March 8, 2016

Hello. I'm developing .dll plugin for thirdparty software. In Windows 8.0 all works fine (my plugin loads and unloads correctly), but in wine, I got DLL_PROCESS_ATTACH, then 7 DLL_THREAD_ATTACH at plugin loading stage and only 1 DLL_THREAD_DETACH at plugin unloading, i.e my plugin remains active (because no DLL_PROCESS_DETACH came in). Maybe that's because I spawn new thread (CreateThread) at DLL_PROCESS_ATTACH. Please, suggest ways to debug this issue. Regards, Vyacheslav Napadovsky

3 6

Re: [PATCH] opencl/tests: Add initial tests
by Henri Verbeet March 8, 2016

March 8, 2016

On 8 March 2016 at 13:14, Detlef Riekenberg <wine.dev(a)web.de> wrote: > + * Copyright 2013 Detlef Riekenberg Is the year correct? > +INT WINAPI clGetPlatformIDs(UINT, PVOID*, UINT*); That doesn't look right. I also don't think anything in this test is particularly Windows specific.

1 0

Re: [PATCH] opencl/tests: Add initial tests
by Marvin March 8, 2016

March 8, 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=21276 Your paranoid android. === build (build) === Recreation of tests/Makefile failed

1 0

Re: user32: Do not block sending/posting WM_NOTIFY messages.
by Alexandre Julliard March 8, 2016

March 8, 2016

Sebastian Lackner <sebastian(a)fds-team.de> writes: > @@ -11188,6 +11188,35 @@ static void test_quit_message(void) > ok_sequence(WmStopQuitSeq, "WmStopQuitSeq", FALSE); > } > > +static void test_notify_message(void) > +{ > + HWND hwnd; > + BOOL ret; > + > + hwnd = CreateWindowExA(0, "TestWindowClass", NULL, WS_OVERLAPPEDWINDOW, > + CW_USEDEFAULT, CW_USEDEFAULT, 300, 300, 0, NULL, NULL, 0); > + ok(hwnd != 0, "Failed to create window\n"); > + > + ret = SendNotifyMessageA(hwnd, WM_NOTIFY, 0x1234, 0xdeadbeef); > + ok(ret == TRUE, "SendNotifyMessageA failed with error %u\n", GetLastError()); > + ret = SendNotifyMessageW(hwnd, WM_NOTIFY, 0x1234, 0xdeadbeef); > + ok(ret == TRUE, "SendNotifyMessageW failed with error %u\n", GetLastError()); > + ret = SendMessageCallbackA(hwnd, WM_NOTIFY, 0x1234, 0xdeadbeef, NULL, 0); > + ok(ret == TRUE, "SendMessageCallbackA failed with error %u\n", GetLastError()); > + ret = SendMessageCallbackW(hwnd, WM_NOTIFY, 0x1234, 0xdeadbeef, NULL, 0); > + ok(ret == TRUE, "SendMessageCallbackW failed with error %u\n", GetLastError()); > + ret = PostMessageA(hwnd, WM_NOTIFY, 0x1234, 0xdeadbeef); > + ok(ret == TRUE, "PostMessageA failed with error %u\n", GetLastError()); > + ret = PostMessageW(hwnd, WM_NOTIFY, 0x1234, 0xdeadbeef); > + ok(ret == TRUE, "PostMessageW failed with error %u\n", GetLastError()); > + ret = PostThreadMessageA(GetCurrentThreadId(), WM_NOTIFY, 0x1234, 0xdeadbeef); > + ok(ret == TRUE, "PostThreadMessageA failed with error %u\n", GetLastError()); > + ret = PostThreadMessageW(GetCurrentThreadId(), WM_NOTIFY, 0x1234, 0xdeadbeef); > + ok(ret == TRUE, "PostThreadMessageW failed with error %u\n", GetLastError()); It would be interesting to test the received message contents too, also for the case of valid data. -- Alexandre Julliard julliard(a)winehq.org

1 0

Re: [v3] d3dx9: support setting texture through SetValue in effect.
by Matteo Bruni March 7, 2016

March 7, 2016

2016-03-06 9:39 GMT+01:00 Paul Gofman <gofmanp(a)gmail.com>: > Signed-off-by: Paul Gofman <gofmanp(a)gmail.com> > --- a/dlls/d3dx9_36/tests/effect.c > +++ b/dlls/d3dx9_36/tests/effect.c > @@ -2389,6 +2389,43 @@ static void test_effect_parameter_value(IDirect3DDevice9 *device) > } > } > > +static void test_effect_setvalue_object(IDirect3DDevice9 *device) > +{ > + ID3DXEffect *effect; > + D3DXHANDLE parameter; > + IDirect3DTexture9 *texture; > + IDirect3DTexture9 *texture_set; > + HRESULT hr; > + ULONG count; > + > + hr = D3DXCreateEffect(device, test_effect_parameter_value_blob_object, > + sizeof(test_effect_parameter_value_blob_object), NULL, NULL, 0, NULL, &effect, NULL); > + ok(hr == D3D_OK, "Got result %#x, expected 0 (D3D_OK).\n", hr); > + > + parameter = effect->lpVtbl->GetParameterByName(effect, NULL, "tex"); > + ok(parameter != NULL, "GetParameterByName failed, got %p\n", parameter); > + > + texture = NULL; > + hr = D3DXCreateTexture(device, D3DX_DEFAULT, D3DX_DEFAULT, D3DX_DEFAULT, 0, 0, D3DPOOL_DEFAULT, &texture); > + ok(hr == D3D_OK, "Got result %#x, expected 0 (D3D_OK).\n", hr); > + hr = effect->lpVtbl->SetValue(effect, parameter, &texture, sizeof(IDirect3DTexture9 **)); That sizeof() happens to have the right value (because IDirect3DTexture9 * is also a pointer type) although it's technically incorrect. It's generally preferred to use the variable instead of the type in sizeof when possible (e.g. in this case sizeof(texture)), that would have avoided this mistake and it also helps in other cases. The other two calls to {GS}etValue below have the same issue.

1 0

[PATCH RFC] programs/winedevice.exe: allow group loading
by Aric Stewart March 7, 2016

March 7, 2016

Here is a patch Sebastian Lackner wanted to see. This is continued work to allow group loading and working off of moving to using IoCreateDriver and such in winedevice. Still using a mailslot which I know Sebastian does not like. But it works quite well. -aric --- programs/winedevice/device.c | 190 ++++++++++++++++++++++++++++++++++++++----- 1 file changed, 170 insertions(+), 20 deletions(-)

1 0

Re: [PATCH] webservices: Fixed memory leaks in one error condition (Coverity)
by Hans Leidekker March 7, 2016

March 7, 2016

Hi Marcus, > diff --git a/dlls/webservices/reader.c b/dlls/webservices/reader.c > index 72e75c3..688bc2b 100644 > --- a/dlls/webservices/reader.c > +++ b/dlls/webservices/reader.c > @@ -928,7 +928,7 @@ static HRESULT read_attribute( struct reader *reader, WS_XML_ATTRIBUTE **ret ) > WS_XML_UTF8_TEXT *text; > unsigned int len = 0, ch, skip, quote; > const unsigned char *start; > - WS_XML_STRING *prefix, *localname; > + WS_XML_STRING *prefix, *localname = NULL; > HRESULT hr = WS_E_INVALID_FORMAT; > > if (!(attr = heap_alloc_zero( sizeof(*attr) ))) return E_OUTOFMEMORY; > @@ -948,7 +948,11 @@ static HRESULT read_attribute( struct reader *reader, WS_XML_ATTRIBUTE **ret ) > if (WsXmlStringEquals( prefix, &xmlns, NULL ) == S_OK) > { > attr->isXmlNs = 1; > - if (!(attr->prefix = alloc_xml_string( localname->bytes, localname->length ))) goto error; > + heap_free (prefix); > + if (!(attr->prefix = alloc_xml_string( localname->bytes, localname->length ))) { > + heap_free (localname); > + goto error; > + } > attr->localName = localname; > } > else Please replace tabs with spaces and use the same formatting as surrounding code. Otherwise it looks good, thanks.

1 0

Re: ddraw/tests: Fix function name in multiple ok() messages. (resend)
by Marvin March 7, 2016

March 7, 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=21242 Your paranoid android. === w1064 (32 bit dsurface) === dsurface.c:144: Test failed: Destination data after blitting is not correct dsurface.c:178: Test failed: Destination data after blitting is not correct dsurface.c:357: Test failed: Surface data after srckey blit does not match dsurface.c:382: Test failed: Surface data after src override key blit does not match dsurface.c:436: Test failed: Surface data after dest key blit does not match dsurface.c:462: Test failed: Surface data after dest key override blit does not match dsurface.c:524: Test failed: Surface data with src key and dest key blit does not match === w864 (64 bit dsurface) === dsurface.c:144: Test failed: Destination data after blitting is not correct dsurface.c:178: Test failed: Destination data after blitting is not correct dsurface.c:357: Test failed: Surface data after srckey blit does not match dsurface.c:382: Test failed: Surface data after src override key blit does not match dsurface.c:436: Test failed: Surface data after dest key blit does not match dsurface.c:462: Test failed: Surface data after dest key override blit does not match dsurface.c:524: Test failed: Surface data with src key and dest key blit does not match === w1064 (64 bit dsurface) === dsurface.c:144: Test failed: Destination data after blitting is not correct dsurface.c:178: Test failed: Destination data after blitting is not correct dsurface.c:357: Test failed: Surface data after srckey blit does not match dsurface.c:382: Test failed: Surface data after src override key blit does not match dsurface.c:436: Test failed: Surface data after dest key blit does not match dsurface.c:462: Test failed: Surface data after dest key override blit does not match dsurface.c:524: Test failed: Surface data with src key and dest key blit does not match

1 0

Re: user32: Do not block sending/posting WM_NOTIFY messages.
by Marvin March 7, 2016

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

2 1

Re: make_xftmpl: Remove getopt prototype
by Alexandre Julliard March 7, 2016

March 7, 2016

André Hentschel <nerv(a)dawncrow.de> writes: > @@ -92,8 +92,6 @@ static const struct keyword reserved_words[] = { > {"WORD", TOKEN_WORD} > }; > > -extern int getopt(int argc, char *const *argv, const char *optstring); This was explicitly added for MSVC builds. -- Alexandre Julliard julliard(a)winehq.org

1 0

Problems about License Compatibility
by Jianqiu Zhang March 7, 2016

March 7, 2016

Hi, I am writing packet.dll which is a dependency of windows native wpcap.dll , Source can be get from winpcap.org , When I try to impelment it in wine, I found that I need to use the Packet32.h from packet.dll , Which has a different license than wine(LGPL), And I am not familar with solving license compability, Is there anyone that know how to solve this problem? Thanks in advance I Attach the License for Packet32.h below -- snip -- /* * Copyright (c) 1999 - 2005 NetGroup, Politecnico di Torino (Italy) * Copyright (c) 2005 - 2007 CACE Technologies, Davis (California) * All rights reserved. * * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions * are met: * * 1. Redistributions of source code must retain the above copyright * notice, this list of conditions and the following disclaimer. * 2. Redistributions in binary form must reproduce the above copyright * notice, this list of conditions and the following disclaimer in the * documentation and/or other materials provided with the distribution. * 3. Neither the name of the Politecnico di Torino, CACE Technologies * nor the names of its contributors may be used to endorse or promote * products derived from this software without specific prior written * permission. * * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. * */

3 2

Re: ntdll: add stub for RtlAddAccessDeniedObjectAce and forward to AddAccessDeniedObjectAce in advapi32
by Alexandre Julliard March 7, 2016

March 7, 2016

Louis Lenders <xerox.xerox2000x(a)gmail.com> writes: > Signed-off-by: Louis Lenders <xerox.xerox2000x at gmail.com> I'll apply this one, but next time could you please put a valid email address here? Thanks. -- Alexandre Julliard julliard(a)winehq.org

1 0

Re: [v2] d3dx9: support setting texture through SetValue in effect.
by Paul Gofman March 6, 2016

March 6, 2016

I will add a fall through message, thanks for the notice On 03/06/2016 11:29 AM, Paul Gofman wrote: > Hi Alistair, no, there is actual copy of data below there which is > required for this case either. Thanks, Paul. > > On Sun, 6 Mar 2016 at 06:16, Alistair Leslie-Hughes > <leslie_alistair(a)hotmail.com <mailto:leslie_alistair(a)hotmail.com>> wrote: > > Hi Paul, > I think you have missed a break in the switch or a message to > state fall through. > > > Alistair >

1 0

re: [v2] d3dx9: support setting texture through SetValue in effect.
by Alistair Leslie-Hughes March 6, 2016

March 6, 2016

Hi Paul, I think you have missed a break in the switch or a message to state fall through. Alistair

1 0

wine 1.9.5 shared wow64 compile error
by Olivier F. R. Dierick March 6, 2016

March 6, 2016

Hello, I've updated my git tree with 1.9.5 and now I have this error while compiling 32bit Wine with the --with-wine64 option (in a shared wow64 setup): ---begin make output--- ... rm -f fonts && ln -s ../1.9.5_wow_64/fonts fonts rm: impossible de supprimer « fonts »: est un dossier make: *** [fonts] Erreur 1 ... ---end make output--- It says fonts is a directory and can't be removed. I do out-of-tree builds. The 32bit directory doesn't exists when I run configure. After running configure, the 32bit build tree is there and contains a fonts directory with a single Makefile in it. I had no such error with Wine 1.9.4 and I don't have it when compiling stand-alone 32bit Wine or the 64bit shared wow part. I did a regression test and found this commit to be the culprit: ---begin git bisect output--- dcc16c7cac1cbf93096b072fe2f7b32ad08d1fa5 is the first bad commit commit dcc16c7cac1cbf93096b072fe2f7b32ad08d1fa5 Author: Alexandre Julliard <julliard(a)winehq.org> Date: Mon Feb 29 15:06:42 2016 +0900 makefiles: Don't generate global rules in disabled directories. Signed-off-by: Alexandre Julliard <julliard(a)winehq.org> ... ---end git bisect output--- I look forward to hearing from you soon -- Olivier F. R. Dierick o.dierick(a)piezo-forte.be

2 2

Wine-1.9.5 source code Cppcheck 1.69 scanning
by Игорь Чебаненко March 4, 2016

March 4, 2016

Source code scanned with Cppcheck 1.69. Yes,I know that in this scanning report I have messages called "noise messages",but I hope,that you will see real bugs,and will ingnore this type of messages.

1 0

Re: [PATCH 07/10] d3d10_1: Implement D3D10CreateDeviceAndSwapChain1().
by Henri Verbeet March 4, 2016

March 4, 2016

On 4 March 2016 at 11:43, Józef Kucia <jkucia(a)codeweavers.com> wrote: > It doesn't seem to be easy to declare D3D10CreateDeviceAndSwapChain1() > without using cpp_quote() because D3D10_DRIVER_TYPE is unknown to idl > (it's declared in d3d10misc.h). My knowledge about idl is limited > so maybe someone knows better. The easiest thing to do would be to use "enum D3D10_DRIVER_TYPE" instead of "D3D10_DRIVER_TYPE". I wouldn't be opposed to just using idl for d3d10misc.h either though. > +cpp_quote("HRESULT WINAPI D3D10CreateDeviceAndSwapChain1(IDXGIAdapter *,") > +cpp_quote(" D3D10_DRIVER_TYPE, HMODULE, UINT, D3D10_FEATURE_LEVEL1,") > +cpp_quote(" UINT, DXGI_SWAP_CHAIN_DESC *, IDXGISwapChain **, ID3D10Device1 **);") I'd prefer this with parameter names either way.

1 0

Re: [PATCH 1/2] user32: Store pointer to timer callback in SetTimer function instead of treating it as window procedure
by Alexandre Julliard March 4, 2016

March 4, 2016

Piotr Caban <piotr(a)codeweavers.com> writes: > I decided to leave 16-bit version of SetTimer as is for now. I haven't > seen any application running out of window procedures in this > case. This patch will change behavior of following 16-bit application: > SetTimer(hWnd, TIMER_ID, 1, tfunc); > GetMessage32(&msg, 0, 0, 0, TRUE); //get's WM_TIMER message > DispatchMessage32(&msg, TRUE); > Now this code will no longer call tfunc callback. According to my test > new behavior is correct. The more likely case is a 16-bit app retrieving a 32-bit timer and calling it through DispatchMessage16. How is this going to work with your proposed changes? -- Alexandre Julliard julliard(a)winehq.org

2 4

Re: [PATCH 2/2 v2] d3drm: Implement AddDestroyCallback/DeleteDestroyCallback for a viewport
by Henri Verbeet March 4, 2016

March 4, 2016

On 3 March 2016 at 09:10, Nikolay Sivov <nsivov(a)codeweavers.com> wrote: > +HRESULT d3drm_object_delete_destroy_callback(struct d3drm_object *object, D3DRMOBJECTCALLBACK cb, void *ctx) > +{ > + struct destroy_callback *callback, *callback2; > + > + if (!cb) > + return D3DRMERR_BADVALUE; > + > + LIST_FOR_EACH_ENTRY_SAFE(callback, callback2, &object->destroy_callbacks, struct destroy_callback, entry) > + { > + if (callback->cb == cb && callback->ctx == ctx) > + { > + list_remove(&callback->entry); > + HeapFree(GetProcessHeap(), 0, callback); > + } > + } > + > + return D3DRM_OK; > +} I won't hold it against the patch (much), but is that correct? E.g. suppose you do something like the following: add cb1 add cb2 add cb3 add cb2 delete cb2 What would the sequence be that ends up getting called?

2 1

Re: [v3 6/7] d3dx9: implement state save and restore in effect.
by Matteo Bruni March 3, 2016

March 3, 2016

2016-03-03 13:22 GMT+01:00 Paul Gofman <gofmanp(a)gmail.com>: > Signed-off-by: Paul Gofman <gofmanp(a)gmail.com> > --- For your next submissions, can you please write a short changelog of the differences with the previous version(s) of the same patch? Ideally that goes after the '---'. > @@ -3493,18 +3501,34 @@ static HRESULT WINAPI ID3DXEffectImpl_Begin(ID3DXEffect *iface, UINT *passes, DW > if (flags & ~(D3DXFX_DONOTSAVESTATE | D3DXFX_DONOTSAVESAMPLERSTATE | D3DXFX_DONOTSAVESHADERSTATE)) > WARN("Invalid flags (%#x) specified.\n", flags); > > - if (This->manager || flags & D3DXFX_DONOTSAVESTATE) > + if (effect->manager || flags & D3DXFX_DONOTSAVESTATE) > { > TRACE("State capturing disabled.\n"); > } > else > { > - FIXME("State capturing not supported, yet!\n"); > + HRESULT hr; > + unsigned int i; > + > + if (!technique->saved_state) > + { > + hr = IDirect3DDevice9_BeginStateBlock(effect->device); > + if (FAILED(hr)) > + ERR("BeginStateBlock failed, hr %#x.\n", hr); > + for (i = 0; i < technique->pass_count; i++) > + d3dx9_apply_pass_states(effect, &technique->passes[i]); This is likely correct but testing it with a technique with at least 2 passes would be nice.

1 0

Re: [v3 5/7] d3dx9: implement sampler and texture effect states application.
by Matteo Bruni March 3, 2016

March 3, 2016

2016-03-03 13:22 GMT+01:00 Paul Gofman <gofmanp(a)gmail.com>: > Signed-off-by: Paul Gofman <gofmanp(a)gmail.com> > --- > dlls/d3dx9_36/effect.c | 41 +++++++++++++++++++++++++++++++++++++++-- > dlls/d3dx9_36/tests/effect.c | 2 +- > 2 files changed, 40 insertions(+), 3 deletions(-) > > diff --git a/dlls/d3dx9_36/effect.c b/dlls/d3dx9_36/effect.c > index e1f01f2..6813c79 100644 > --- a/dlls/d3dx9_36/effect.c > +++ b/dlls/d3dx9_36/effect.c > @@ -2681,7 +2681,8 @@ HRESULT d3dx_set_shader_const_state(IDirect3DDevice9 *device, enum SHADER_CONSTA > return D3D_OK; > } > > -static HRESULT d3dx9_apply_state(struct ID3DXEffectImpl *effect, struct d3dx_pass *pass, struct d3dx_state *state) > +static HRESULT d3dx9_apply_state(struct ID3DXEffectImpl *effect, struct d3dx_pass *pass, > + struct d3dx_state *state, int parent_index) I would have used an unsigned int here, flagging "none" with ~0u.

1 0

Re: [v3 4/7] d3dx9: implement shader constants effect states application.
by Matteo Bruni March 3, 2016

March 3, 2016

2016-03-03 13:22 GMT+01:00 Paul Gofman <gofmanp(a)gmail.com>: > Signed-off-by: Paul Gofman <gofmanp(a)gmail.com> > --- > + UINT nelem; "unsigned int element_count;" would be better.

1 0

Re: [v3 3/7] d3dx9: implement light and material effect states application.
by Matteo Bruni March 3, 2016

March 3, 2016

2016-03-03 13:22 GMT+01:00 Paul Gofman <gofmanp(a)gmail.com>: > Signed-off-by: Paul Gofman <gofmanp(a)gmail.com> > --- > + light_tbl[] = > + { > + {FIELD_OFFSET(D3DLIGHT9, Type), "LC_TYPE"}, > + {FIELD_OFFSET(D3DLIGHT9, Diffuse), "LT_DIFFUSE"}, > + {FIELD_OFFSET(D3DLIGHT9, Specular), "LT_SPECULAR"}, > + {FIELD_OFFSET(D3DLIGHT9, Ambient), "LT_AMBIENT"}, > + {FIELD_OFFSET(D3DLIGHT9, Position), "LT_POSITION"}, > + {FIELD_OFFSET(D3DLIGHT9, Direction), "LT_DIRECTION"}, > + {FIELD_OFFSET(D3DLIGHT9, Range), "LT_RANGE"}, > + {FIELD_OFFSET(D3DLIGHT9, Falloff), "LT_FALLOFF"}, > + {FIELD_OFFSET(D3DLIGHT9, Attenuation0), "LT_ATTENUATION0"}, > + {FIELD_OFFSET(D3DLIGHT9, Attenuation1), "LT_ATTENUATION1"}, > + {FIELD_OFFSET(D3DLIGHT9, Attenuation2), "LT_ATTENUATION2"}, > + {FIELD_OFFSET(D3DLIGHT9, Theta), "LT_THETA"}, > + {FIELD_OFFSET(D3DLIGHT9, Phi), "LT_PHI"} > + }; This is misindented. > + switch (op) > + { > + case LT_TYPE: > + TRACE("LT_TYPE %u.\n", *(D3DLIGHTTYPE *)value); > + 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].offset) = c; It's probably a bit nicer to cast to (BYTE *) instead of (char *). > diff --git a/dlls/d3dx9_36/tests/effect.c b/dlls/d3dx9_36/tests/effect.c > index 81a7d12..3bcba72 100644 > --- a/dlls/d3dx9_36/tests/effect.c > +++ b/dlls/d3dx9_36/tests/effect.c > @@ -2910,11 +2910,11 @@ static void test_effect_states(IDirect3DDevice9 *device) > } > > hr = IDirect3DDevice9_GetLightEnable(device, 2, &bval); > - todo_wine ok(hr == D3D_OK, "Got result %x, expected 0 (D3D_OK).\n", hr); > + ok(hr == D3D_OK, "Got result %x, expected 0 (D3D_OK).\n", hr); > if (hr == D3D_OK) > ok(bval, "Got result %u, expected TRUE.\n", bval); > hr = IDirect3DDevice9_GetLight(device, 2, &light); > - todo_wine ok(hr == D3D_OK, "Got result %x, expected 0 (D3D_OK).\n", hr); > + ok(hr == D3D_OK, "Got result %x, expected 0 (D3D_OK).\n", hr); > if (hr == D3D_OK) > 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); > @@ -2950,9 +2950,9 @@ static void test_effect_states(IDirect3DDevice9 *device) > todo_wine ok(!memcmp(mat.m, test_mat.m, sizeof(mat)), "World matrix not restored.\n"); > > hr = IDirect3DDevice9_GetLightEnable(device, 2, &bval); > - todo_wine ok(hr == D3D_OK, "Got result %x, expected 0 (D3D_OK).\n", hr); > + ok(hr == D3D_OK, "Got result %x, expected 0 (D3D_OK).\n", hr); > if (hr == D3D_OK) > - ok(!bval, "Got result %u, expected 0.\n", bval); > + todo_wine ok(!bval, "Got result %u, expected 0.\n", bval); It would be nice if the test also checked the material.

1 0

Re: [v3 1/7] d3dx9: add test for basic effect state management.
by Matteo Bruni March 3, 2016

March 3, 2016

2016-03-03 13:22 GMT+01:00 Paul Gofman <gofmanp(a)gmail.com>: > + > + if (effect) > + effect->lpVtbl->Release(effect); > +} The if is unnecessary, the effect creation should never fail (and if it failed you would have already crashed long before that).

1 0