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

Re: [5/5] WineD3D: Make the MAC ARBvp implementation happy about ARL
by Ivan Gyurdiev 24 Jul '08

24 Jul '08

> Unfortunately we can't make it a different function in the shader instruction table because the difference between MOV and ARL is decided with the dest register. Sure you can, it seems that this MOV/MOVA specific stuff all belongs in vshader_hw_mov (both sides of the MOV if statement can be moved). There should also be a comment explaining what this ARL block is doing, and why (as part of "wined3d: Relative addressing offsets are limited to [-64; 63] in arb."). Ivan

2 1

Re: general question..
by James Hawkins 24 Jul '08

24 Jul '08

On Mon, Jul 21, 2008 at 9:24 PM, Chris Ahrendt <celticht32(a)aol.com> wrote: > > Well here is my list so far : > > device.c - filled with goto's if you need the routines I can supply them... > they can be moved to a routine and called... > wine$ find . -name "device.c" ./dlls/ddraw/device.c ./dlls/dinput/tests/device.c ./dlls/dinput/device.c ./dlls/wined3d/device.c ./dlls/d3d8/tests/device.c ./dlls/d3d8/device.c ./dlls/mountmgr.sys/device.c ./dlls/d3d9/tests/device.c ./dlls/d3d9/device.c ./programs/winedevice/device.c ./server/device.c > context.c - same except in this case its just a return with noting else.. > why do a goto why not just do the return? > This is probably ok to change. I can only imagine that the original author thought there might be cleanup needed at some later point. > directx.c - same... this can be moved to a routine and simplified.. > this is in routine WineD3D_CreateFakeGLContext which I was tracing down some > stuff... > cleanup usually does not warrant a new function. This is exactly what we use goto for. > pixelshader.c - this has a recompile goto.. again why not make this into a > routine? > > provider.c - same > This function is ugly, but not because it uses goto. The function is 144 lines long, which is too long. I'm not familiar with this code, but I'm pretty sure some factorization is in order. The level of indentation could be reduced by breaking on failure instead of proceeding on success. Assuming this previous change is made, the use of the goto would be that much more appealing. > > These are just the ones I have run accross so far.. > Why do you say that... or is the idea.. hack and get it done... one of the > problems with goto's is it makes finding and putting in patches a pain in > the ass.. and thats putting it bluntly.. > Complete sentences go a long way towards effective communication :-) The use of goto's cannot possibly be considered as a hack. I also don't see how you find goto's a problem when patching the code. > Of course.. Do you have a suggestion on how I should word the RFC? > No, just send in a patch to wine-devel stating why you think the change is correct. P.S. Make sure you reply-all to cc wine-devel. We're an open project and we communicate openly so everyone can be a part of the discussion. -- James Hawkins

6 15

Re: [1/10] [try 5] appwiz.cpl: Add skeleton Add/Remove Programs control panel
by Alexandre Julliard 24 Jul '08

24 Jul '08

Owen Rudge <owen(a)owenrudge.net> writes: > +#include "En.rc" > + > +LANGUAGE LANG_NEUTRAL, SUBLANG_NEUTRAL > + > +ICO_MAIN ICON "../../programs/uninstaller/uninstaller.ico" > +ICO_SETUP ICON "../../programs/msiexec/msiexec.ico" > +ICO_BIN ICON "../shell32/trash_file.ico" You can't depend on files belonging to other dlls. You should load the icons from the corresponding dlls at run-time, or if absolutely necessary duplicate the files. -- Alexandre Julliard julliard(a)winehq.org

1 0

Re: [Gdiplus try4 1/6] Implement GdipCreateRegion and mark the tests todo_wine instead of skipping over them all.
by Alexandre Julliard 24 Jul '08

24 Jul '08

Adam Petaccia <adam(a)tpetaccia.com> writes: > diff --git a/dlls/gdiplus/tests/region.c b/dlls/gdiplus/tests/region.c > index 6b1c5f0..ef5ab28 100644 > --- a/dlls/gdiplus/tests/region.c > +++ b/dlls/gdiplus/tests/region.c > @@ -62,50 +62,60 @@ static void test_getregiondata(void) > GpRect rect; > GpPath *path; > > + ZeroMemory(buf, sizeof(buf)); > + > status = GdipCreateRegion(&region); > -todo_wine > ok(status == Ok, "status %08x\n", status); > > - if(status != Ok) return; > - > +todo_wine > +{ > status = GdipGetRegionDataSize(region, &needed); > ok(status == Ok, "status %08x\n", status); > - ok(needed == 20, "got %d\n", needed); > + expect(20, needed); > status = GdipGetRegionData(region, (BYTE*)buf, sizeof(buf), &needed); > ok(status == Ok, "status %08x\n", status); > - ok(needed == 20, "got %d\n", needed); > + expect(20, needed); > expect_dword(buf, 12); > trace("buf[1] = %08x\n", buf[1]); > expect_magic((DWORD*)(buf + 2)); > +} > expect_dword(buf + 3, 0); > +todo_wine > +{ If you want to test that the buffer contains zero you should initialize it with something else, this will avoid having the test succeed even though Wine doesn't yet put anything in the buffer. -- Alexandre Julliard julliard(a)winehq.org

1 0

Re: advapi32[2/2]: remove access checks from advapi32 (makes KEY_WOW64_* work)
by Paul Vriens 24 Jul '08

24 Jul '08

Mikołaj Zalewski wrote: > + hkey1 = NULL; + ret = RegCreateKeyExA(HKEY_LOCAL_MACHINE, "Software", 0, NULL, 0, KEY_READ|KEY_WOW64_32KEY, NULL, &hkey1, NULL); + ok(ret == ERROR_SUCCESS && hkey1 != NULL, "RegOpenKeyEx with KEY_WOW64_64KEY failed (err=%u)\n", ret); + RegCloseKey(hkey1); } The call to RegCreateKeyExA uses the KEY_WOW64_32KEY flag. -- Cheers, Paul.

1 0

Re: oleaut32: fix the SafeArrayGetVartype tests on Vista and use that behaviour. [try 4]
by Alexandre Julliard 24 Jul '08

24 Jul '08

"Reece Dunn" <msclrhd(a)googlemail.com> writes: > This includes Rob Shearman's suggestion of marking the old behaviour > as broken() as it is clearly a bug, and also updated the > SafeArrayGetVartype implementation to return the correct value in this > case. > > The tests pass on Vista and on Wine. Not for me: ../../../tools/runtest -q -P wine -M oleaut32.dll -T ../../.. -p oleaut32_test.exe.so safearray.c && touch safearray.ok safearray.c:628: Test failed: SAGVT of array with vt 9 returned 13 make[2]: *** [safearray.ok] Error 1 -- Alexandre Julliard julliard(a)winehq.org

1 0

Re: advapi32[2/2]: allow generic rights in registry functions
by Alexandre Julliard 24 Jul '08

24 Jul '08

Mikołaj Zalewski <mikolaj(a)zalewski.pl> writes: > The GENERIC_* flags were also missing I'd suggest to simply kill that access mask check, there's no reason to second-guess the server checks anyway. -- Alexandre Julliard julliard(a)winehq.org

1 0

Re: wined3d: universal surface convertor function for unsigned integer color formats(4th attempt)
by Alexandre Julliard 24 Jul '08

24 Jul '08

Victor Eremin <ErV2005(a)rambler.ru> writes: > +void mask_copy(DWORD srcMask, BYTE srcPixelSize, > + DWORD dstMask, BYTE dstPixelSize, > + BYTE *src, BYTE* dst, DWORD src_pitch, DWORD dst_pitch, > + unsigned int w, unsigned int h){ > + unsigned int x, y; > + BYTE i; > + BYTE *dstp, *srcp, *dstmaskp, *srcmaskp, *channelValuePtr; > + DWORD channelValue; > + BYTE upShift, downShift; > + dstmaskp = (BYTE*)&dstMask; > + srcmaskp = (BYTE*)&srcMask; > + channelValuePtr = (BYTE*)&channelValue; You can't access DWORDs as bytes, that will break on big-endian platforms. -- Alexandre Julliard julliard(a)winehq.org

1 0

Request For Testing: Can you check this crypt32/protectdata test fix on Win2000.
by Reece Dunn 24 Jul '08

24 Jul '08

Hi, I want to verify that the "if (!protected) ..." are correct. That is, on non-Win2000 platforms, we can't verify LastError == ERROR_SUCCESS, so I have removed those as per Juan's request. However, there are two places where Win2000 is returning a different error code. So the question is, is this the correct way to do this (and because I have changed the condition for the first test that I have changed I want to verify it stll works on Win2000). Thanks, - Reece

1 0

Re: d3dx8 [try 2]: Implement D3DXSphereBoundProbe
by H. Verbeet 24 Jul '08

24 Jul '08

2008/7/23 David Adam <david.adam.cnrs(a)gmail.com>: > +#include <stdio.h> > +#include <stdlib.h> > +#include <stdarg.h> > +#include <assert.h> > + > +#define NONAMELESSUNION ... > +#include <assert.h> I don't think you need those. (I should've noticed this in the first patch, sorry for that.)

1 0