December 2008 - Wine-devel - list.winehq.org

Re: server: implement move/rename change notifications and support for multiple change notification in one reply
by Dan Kegel 17 Dec '08

17 Dec '08

On Tue, Dec 16, 2008 at 5:21 PM, Michael Pujos <pujos.michael(a)laposte.net> wrote: >> I don't see an attachment to >> http://www.winehq.org/pipermail/wine-patches/2008-December/066224.html >> or rather, there's only a zero-length one. > > I just attached (In thunderbird) the file outputted by "git format-patch > --keep-subject origin" > Curiously the link above point to a zero length file but I received it > correctly in my e-mail client from the mailing-list... That archive is a bit finicky. I see the attachment in another archive: http://marc.info/?l=wine-patches&m=122947592925831&w=2 Suggestion: next time use a shorter filename, maybe that's what screwed up the archive. It looks like you're changing the protocol. Will old clients be able to connect successfully? It looks like they won't. you need to support old clients. We're not allowed to break compatibility now that we've released wine-1.0. - Dan

3 4

Re: wined3d: Implement check for shader creation if hardware supports requested version.
by Paul Vriens 17 Dec '08

17 Dec '08

Pauli Nieminen wrote: > + > + if( version <= caps->VertexShaderVersion ) > + { > + ok(hret == D3D_OK && vshader_ptr != NULL, "Vertex shader (0x%x) creation failed but d3dcaps claim to support it. hret = 0x%x, vshader_ptr = %p", version, hret, vshader_ptr); > + IDirect3DVertexShader9_Release(vshader_ptr); > + } else { > + ok(hret == D3DERR_INVALIDCALL && vshader_ptr == NULL,"Vertex shader (0x%x) creation succesed but d3dcaps claim not to support it. hret = 0x%x, vshader_ptr = %p", version, hret, vshader_ptr); > + } Hi, Just some style issues I've seen. You should be using a: if () { } else { } block to stick with the style. > > +/** > + * checks if shader version supported that is required by new > + * shader. > + */ > +static inline BOOL is_pshader_version_supported(IWineD3DDeviceImpl *deviceImpl, const DWORD *byte_code) > +{ > + struct shader_caps shader_caps; > + const DWORD shader_version = *byte_code; /* First instruction should be shader version */ > + const WineD3D_GL_Info *gl_info = &deviceImpl->adapter->gl_info; > + > + deviceImpl->shader_backend->shader_get_caps(deviceImpl->devType ,gl_info, &shader_caps); > + // Check which shader this is Please use C-style comments, not C++. > + if(shader_is_pshader_version(shader_version)) > + { > + // Check if high enough shader version is supported And there are a few others of those. -- Cheers, Paul.

3 4

Re: wined3d: Reduce and improve error message spam when shader creation fails.
by Henri Verbeet 17 Dec '08

17 Dec '08

These should probably all be WARNs as well.

1 0

Re: wined3d: Implement check for shader creation if hardware supports requested version.
by Henri Verbeet 17 Dec '08

17 Dec '08

2008/12/17 Pauli Nieminen <suokkos(a)gmail.com>: > +static inline BOOL is_pshader_version_supported(IWineD3DDeviceImpl *deviceImpl, const DWORD *byte_code) > +{ > + struct shader_caps shader_caps; > + const DWORD shader_version = *byte_code; /* First instruction should be shader version */ Please don't do that, use the reg_maps.shader_version instead. > + ERR("First instruction in shader isn't correct shader version for pixel shader. (0x%x)\n", shader_version); ERR is for internal errors, this should be a WARN. You probably don't need to print anything at all here though, doing it from the caller makes more sense (and you've got a message there already anyway). > + ERR("No hardware support for pixel shader version (0x%x).\n",*pFunction); Like above, this should be a WARN. That also means you can get rid of the code to only print the message once.

1 0

Re: d3d9/tests: Add missing new lines to ok() and split long lines
by Paul Vriens 17 Dec '08

17 Dec '08

Pauli Nieminen wrote: > --- > dlls/d3d9/tests/shader.c | 16 ++++++++++++---- > 1 files changed, 12 insertions(+), 4 deletions(-) > > diff --git a/dlls/d3d9/tests/shader.c b/dlls/d3d9/tests/shader.c > index 55b414a..caae395 100644 > --- a/dlls/d3d9/tests/shader.c > +++ b/dlls/d3d9/tests/shader.c > @@ -85,10 +85,14 @@ static inline void test_create_vshader_version_check(IDirect3DDevice9 *device_pt > > if( version <= caps->VertexShaderVersion ) > { > - ok(hret == D3D_OK && vshader_ptr != NULL, "Vertex shader (0x%x) creation failed but d3dcaps claim to support it. hret = 0x%x, vshader_ptr = %p", version, hret, vshader_ptr); > + ok(hret == D3D_OK && vshader_ptr != NULL, > + "Vertex shader (0x%x) creation failed but d3dcaps claim to support it. hret = 0x%x, vshader_ptr = %p\n", > + version, hret, vshader_ptr); > IDirect3DVertexShader9_Release(vshader_ptr); > } else { > - ok(hret == D3DERR_INVALIDCALL && vshader_ptr == NULL,"Vertex shader (0x%x) creation succesed but d3dcaps claim not to support it. hret = 0x%x, vshader_ptr = %p", version, hret, vshader_ptr); > + ok(hret == D3DERR_INVALIDCALL && vshader_ptr == NULL, > + "Vertex shader (0x%x) creation succesed but d3dcaps claim not to support it. hret = 0x%x, vshader_ptr = %p\n", > + version, hret, vshader_ptr); > } > > } > @@ -104,10 +108,14 @@ static inline void test_create_pshader_version_check(IDirect3DDevice9 *device_pt > > if( version <= caps->PixelShaderVersion ) > { > - ok(hret == D3D_OK && pshader_ptr != NULL, "Pixel shader (0x%x) creation failed but d3dcaps claim to support it. hret = 0x%x, pshader_ptr = %p", version, hret, pshader_ptr); > + ok(hret == D3D_OK && pshader_ptr != NULL, > + "Pixel shader (0x%x) creation failed but d3dcaps claim to support it. hret = 0x%x, pshader_ptr = %p\n", > + version, hret, pshader_ptr); > IDirect3DPixelShader9_Release(pshader_ptr); > } else { > - ok(hret == D3DERR_INVALIDCALL && pshader_ptr == NULL,"Pixel shader (0x%x) creation succesed but d3dcaps claim not to support it. hret = 0x%x, pshader_ptr = %p", version, hret, pshader_ptr); > + ok(hret == D3DERR_INVALIDCALL && pshader_ptr == NULL, > + "Pixel shader (0x%x) creation succesed but d3dcaps claim not to support it. hret = 0x%x, pshader_ptr = %p\n", > + version, hret, pshader_ptr); > } > > } Hi, These fixes are on top of your previous patch which is not committed). If it's a series of patches you should mark them appropriate ([1/2] and [2/2] for example). In this case it's probably better to incorporate this into the previous patch and resend that one. -- Cheers, Paul.

1 0

[try2] ntoskrnl.exe: Implement Io{Allocate, Get}DriverObjectExtension.
by Alexander Morozov 17 Dec '08

17 Dec '08

Please, use this patch instead of previous "ntoskrnl.exe: Implement Io{Allocate,Get}DriverObjectExtension." >From b5441d22fe51640849436b563ca3ab6c9a80c1a5 Mon Sep 17 00:00:00 2001 From: Alexander Morozov <amorozov(a)etersoft.ru> Date: Wed, 17 Dec 2008 12:44:46 +0300 Subject: [PATCH] ntoskrnl.exe: Implement Io{Allocate,Get}DriverObjectExtension. --- dlls/ntoskrnl.exe/ntoskrnl.c | 53 +++++++++++++++++++++++++++++++++++++++-- 1 files changed, 50 insertions(+), 3 deletions(-) diff --git a/dlls/ntoskrnl.exe/ntoskrnl.c b/dlls/ntoskrnl.exe/ntoskrnl.c index a669d3b..73054e3 100644 --- a/dlls/ntoskrnl.exe/ntoskrnl.c +++ b/dlls/ntoskrnl.exe/ntoskrnl.c @@ -63,6 +63,16 @@ struct IrpInstance IRP *irp; }; +static struct list DriverObjExtensions = LIST_INIT(DriverObjExtensions); + +struct DriverObjExtension +{ + struct list entry; + void *ptr; + DRIVER_OBJECT *driver; + void *id_addr; +}; + #ifdef __i386__ #define DEFINE_FASTCALL1_ENTRYPOINT( name ) \ __ASM_GLOBAL_FUNC( name, \ @@ -267,9 +277,28 @@ NTSTATUS WINAPI IoAllocateDriverObjectExtension( PDRIVER_OBJECT DriverObject, ULONG DriverObjectExtensionSize, PVOID *DriverObjectExtension ) { - FIXME( "%p, %p, %u, %p\n", DriverObject, ClientIdentificationAddress, + struct DriverObjExtension *ext; + + TRACE( "%p, %p, %u, %p\n", DriverObject, ClientIdentificationAddress, DriverObjectExtensionSize, DriverObjectExtension ); - return STATUS_NOT_IMPLEMENTED; + + *DriverObjectExtension = NULL; + if (IoGetDriverObjectExtension( DriverObject, ClientIdentificationAddress )) + return STATUS_OBJECT_NAME_COLLISION; + ext = ExAllocatePool( NonPagedPool, sizeof(*ext) ); + if (ext == NULL) + return STATUS_INSUFFICIENT_RESOURCES; + ext->ptr = ExAllocatePool( NonPagedPool, DriverObjectExtensionSize ); + if (ext->ptr == NULL) + { + ExFreePool( ext ); + return STATUS_INSUFFICIENT_RESOURCES; + } + ext->driver = DriverObject; + ext->id_addr = ClientIdentificationAddress; + list_add_tail( &DriverObjExtensions, &ext->entry ); + *DriverObjectExtension = ext->ptr; + return STATUS_SUCCESS; } @@ -279,7 +308,16 @@ NTSTATUS WINAPI IoAllocateDriverObjectExtension( PDRIVER_OBJECT DriverObject, PVOID WINAPI IoGetDriverObjectExtension( PDRIVER_OBJECT DriverObject, PVOID ClientIdentificationAddress ) { - FIXME( "%p, %p\n", DriverObject, ClientIdentificationAddress ); + struct DriverObjExtension *ext; + + TRACE( "%p, %p\n", DriverObject, ClientIdentificationAddress ); + + LIST_FOR_EACH_ENTRY( ext, &DriverObjExtensions, struct DriverObjExtension, entry ) + { + if (DriverObject == ext->driver && + ClientIdentificationAddress == ext->id_addr) + return ext->ptr; + } return NULL; } @@ -1138,6 +1176,7 @@ PVOID WINAPI MmGetSystemRoutineAddress(PUNICODE_STRING SystemRoutineName) BOOL WINAPI DllMain( HINSTANCE inst, DWORD reason, LPVOID reserved ) { LARGE_INTEGER count; + struct DriverObjExtension *ext, *ext2; switch(reason) { @@ -1146,6 +1185,14 @@ BOOL WINAPI DllMain( HINSTANCE inst, DWORD reason, LPVOID reserved ) RtlAddVectoredExceptionHandler( TRUE, vectored_handler ); KeQueryTickCount( &count ); /* initialize the global KeTickCount */ break; + case DLL_PROCESS_DETACH: + LIST_FOR_EACH_ENTRY_SAFE( ext, ext2, &DriverObjExtensions, + struct DriverObjExtension, entry ) + { + list_remove( &ext->entry ); + ExFreePool( ext->ptr ); + ExFreePool( ext ); + } } return TRUE; } -- 1.6.0.2.GIT

2 1

Re: [PATCH] wined3d: Implement check for shader creation if hardware supports requested version.
by Henri Verbeet 17 Dec '08

17 Dec '08

2008/12/17 Pauli Nieminen <suokkos(a)gmail.com>: > +static inline void test_create_vshader_version_check(IDirect3DDevice9 *device_ptr, const D3DCAPS9 *caps, > + const DWORD version, const DWORD *shader_code) > +{ > + IDirect3DVertexShader9 *vshader_ptr = 0; > + HRESULT hret = 0; > + > + hret = IDirect3DDevice9_CreateVertexShader(device_ptr, shader_code, &vshader_ptr); > + > + > + if( version <= caps->VertexShaderVersion ) > + { > + ok(hret == D3D_OK && vshader_ptr != NULL, "Vertex shader (0x%x) creation failed but d3dcaps claim to support it. hret = 0x%x, vshader_ptr = %p", version, hret, vshader_ptr); > + } else { > + ok(hret == D3DERR_INVALIDCALL && vshader_ptr == NULL,"Vertex shader (0x%x) creation succesed but d3dcaps claim not to support it. hret = 0x%x, vshader_ptr = %p", version, hret, vshader_ptr); > + } > + > +} If creation succeeds, you need to Release the shader again. Same goes for the pixel shader version of this function. > - ok(hret == D3D_OK && shader_refcount == i && current_shader_ptr == shader_ptr, > + ok(hret == D3D_OK && shader_refcount == i && current_shader_ptr == shader_ptr, > I don't particularly like the trailing space either, but you shouldn't make unrelated whitespace changes in your patch. > +static inline BOOL is_shader_version_supported(IWineD3DDeviceImpl *deviceImpl, const DWORD *byte_code) > +{ > +// IWineD3DPixelShaderImpl *This = (IWineD3DPixelShaderImpl *)shader; > + struct shader_caps shader_caps; > + const DWORD shader_version = *byte_code; /* First instruction should be shader version */ > + const WineD3D_GL_Info *gl_info = &deviceImpl->adapter->gl_info; Please don't use tabs or C++ style comments. > /* Note that this does not count the loop register > * as an address register. */ > @@ -216,7 +244,27 @@ HRESULT shader_get_registers_used(IWineD3DBaseShader *iface, struct shader_reg_m > memset(reg_maps->bumpmat, 0, sizeof(reg_maps->bumpmat)); > memset(reg_maps->luminanceparams, 0, sizeof(reg_maps->luminanceparams)); > > - /* get_registers_used is called on every compile on some 1.x shaders, which can result > + /* If no hardware support this function should fail */ > + if (!is_shader_version_supported((IWineD3DDeviceImpl*)This->baseShader.device, pToken)) > + { > + static BOOL warned = FALSE; > + if (!warned) > + { > + warned = TRUE; > + ERR("No hardware support for shader version.\n"); > + } > + return WINED3DERR_INVALIDCALL; > + } > + > + > + if (!pToken) > + { > + WARN("Got a NULL pFunction, returning.\n"); > + This->baseShader.functionLength = 0; > + return WINED3D_OK; > + } > + > + /* get_registers_used is called on every compile on some 1.x shaders, which can result > I don't think shader_get_registers_used is the right place for this. The version check should be in IWineD3DVertexShaderImpl_SetFunction() and IWineD3DPixelShaderImpl_SetFunction(), after the call to shader_get_registers_used(). Putting it there means you can use the shader version in reg_maps.shader_version, and you can also check if the shader is actually a vertexshader or pixelshader. Ie, passing a pixel shader to CreateVertexShader() should most likely fail.

2 2

Re: MSI: fix some NULL checking in MSI_RecordGetStringA
by James Hawkins 16 Dec '08

16 Dec '08

On Tue, Dec 16, 2008 at 9:11 AM, Aric Stewart <aric(a)codeweavers.com> wrote: > --- > dlls/msi/record.c | 7 ++++--- > 1 files changed, 4 insertions(+), 3 deletions(-) > Can you add tests for this please? Thanks, James Hawkins

2 2

Re: jscript: api.js, added test routines for Math functions
by Jacek Caban 16 Dec '08

16 Dec '08

Hi Patrick, First of all, tests must pass on Wine and Windows. Your test won't pass on any of them! When you have a test that passes on Windows, please send one function implementation and tests in one patch. Patrick Rudolph wrote: > From c43e52c3446fb846a760002f0222f697075fad1b Mon Sep 17 00:00:00 2001 > From: Patrick Rudolph <patrick1804(a)web.de> > Date: Thu, 11 Dec 2008 17:48:38 +0100 > Subject: jscript: api.js added test routines for Math functions > > modified: dlls/jscript/tests/api.js > added tests for : > Math.PI > Math.E > Math.SQRT2 > Math.SQRT1_2 > Math.sin > Math.cos > Math.random > Math.tan > Math.atan > Math.asin > Math.acos > Math.log > math.atan2 > > --- > dlls/jscript/tests/api.js | 38 +++++++++++++++----------------------- > 1 files changed, 42 insertions(+), 0 deletions(-) > > diff --git a/dlls/jscript/tests/api.js b/dlls/jscript/tests/api.js > index 6430397..50a28d5 100644 > --- a/dlls/jscript/tests/api.js > +++ b/dlls/jscript/tests/api.js > > +i = (Math.PI()).toString(); > Math.PI is constant, not function so you can't call it. You'd know that if you'd verified that the test passes on Windows. > +ok(i === "3.14159265358979","Math.PI() = " + i); > It's bad idea to compare double values in tests, and comparing strings is strange here. Math.PI already has a test, look there how it's done there. > + > +i = (Math.E()).toString(); > +ok(i === "2.71828182845905","Math.E() = " + i); > Same here. > + > +i = (Math.SQRT2()).toString(); > +ok(i === "1.4142135623731","Math.SQRT2() = " + i); > And here. > +i = (Math.SQRT1_2()).toString(); > +ok(i === "0.707106781186547","Math.SQRT1_2() = " + i); > And here. > +i = Math.sin(0); > +ok(i === 0, "Math.sin(0) = " + i); > + > +i = Math.cos(0); > +ok(i === 1, "Math.cos(0) = " + i); > + > +i = Math.tan(0); > +ok(i === 0, "Math.tan(0) = " + i); > + > +i = Math.log(1); > +ok(i === 0 , "Math.log() = " + i); > + > +i = Math.random(); > +ok(i !== 0 , "Math.random() = " + i); > + > +i = Math.random(); > +ok(i !== 1 , "Math.random() = " + i); > + > +i = Math.atan2(0,1); > +ok(i === 0 , "Math.atan2(0,1) = " + i); > + > +i = Math.atan(0); > +ok(i === 0 , "Math.atan(0) = " + i); > + > +i = (Math.asin(1)).toString(); > +ok(i === "1.5707963267949" , "Math.asin(1) = " + i); > + > +i = (Math.acos(-1)).toString(); > +ok(i === "3.14159265358979" , "Math.acos(-1) = " + i); > There functions should have more tests like passing no argument, testing invalid asin/acos arguments, passing too much arguments (and should be sent in one patch, together with implementation). Also, in other patches, please add comments pointing to specifications of these function when you implement them. It would be good idea to get one patch accepted before resending the whole series. Jacek

1 0

Re: [1/5] jscript: Implement the String.anchor() method (resend).
by Jacek Caban 16 Dec '08

16 Dec '08

Hi Andrew, Andrew Nguyen wrote: > This revision incorporates Jacek Caban's suggestions. > --- > dlls/jscript/string.c | 55 +++++++++++++++++++++++++++++++++++++++++++- > dlls/jscript/tests/api.js | 17 ++++++++++++++ > 2 files changed, 70 insertions(+), 2 deletions(-) > + if(ret) { + sprintfW(ret, attrtagfmtW, tagname, attrname, (attrval ? attrval : undefinedW), string->str, tagname); + + V_VT(retv) = VT_BSTR; + V_BSTR(retv) = ret; + + SysFreeString(attrval); It looks for me much better now, but you leak attrval if retv is NULL or SysAllocStringLen fails. SysFreeString call should be moved just before the last return. Jacek

1 0