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

Re: [2/3] winex11.drv: Don't modify current state if glXMakeCurrent fails.

by Roderick Colenbrander

The first part of the patch was fine, the only concern is the error code. Since there is no WGL standard, the error codes thrown by different drivers can be inconsistent :( I'm not sure if it is better to set an error code at all or not. I'm not sure what I would prefer... Roderick On Tue, Mar 29, 2011 at 5:55 PM, Matteo Bruni <matteo.mystral(a)gmail.com> wrote: > 2011/3/29 Roderick Colenbrander <thunderbird2k(a)gmail.com>: >> I'm not sure if the only value error wglMakeCurrent should return is >> INVALID_HANDLE. What if glXMakeCurrent fails because of an invalid >> context? The return value may be similar, but don't remember for sure. >> >> Roderick > > Practically speaking it seems to be very dependent on the specific GL > driver used. For example, current Nvidia Windows drivers just accept a > wglMakeCurrent call with a destroyed context (no errors at all), while > I think AMD drivers crash in the same case. I'm not sure I > specifically tested that on glX too, but in general, failing on a > destroyed drawable was the only consistent "edge case" behavior I > found. > >> >> On Tue, Mar 29, 2011 at 7:42 AM, Matteo Bruni <matteo.mystral(a)gmail.com> wrote: >>> >>> >>> >>> >>> >> >

March 29, 2011

Re: [3/3] winex11.drv: Make wglMakeCurrent return the correct error when the drawable is invalid.

by Roderick Colenbrander

On Tue, Mar 29, 2011 at 5:58 PM, Matteo Bruni <matteo.mystral(a)gmail.com> wrote: > 2011/3/29 Roderick Colenbrander <thunderbird2k(a)gmail.com>: >> The fact that a pixel format is not set on the physDev doesn't >> necessarily mean it is invalid, it just means SetPixelFormat wasn't >> called on it. Though slightly earlier in wglMakeCurrent we already >> catch that situation when compare the pixel format of the context and >> the hdc. > > True, but as you say the check I'm adding doesn't really change the > behavior outside of a different SetLastError() call. Maybe the WARN > message I'm adding could be improved. > >> >> Is there not something better for which we could check? Is the >> drawable of the physDev perhaps something to check for? (I have no >> idea in what state the physDev is after a destroy window; should we be >> able to enter x11drv at all? perhaps this should be handled inside the >> gdi32 code, just a thought) >> > > Checking for the drawable or any other field in the physDev doesn't > seem to be really better. I'm not sure about the feasibility of > handling that on the gdi32 side, I think I gave it a try at some point > but then I resorted to this other approach. But, of course, I may be > failing to see a better solution ;) > >> Roderick >> >> On Tue, Mar 29, 2011 at 7:42 AM, Matteo Bruni <matteo.mystral(a)gmail.com> wrote: >>> > I would vote for a more correct way of checking whether we are dealing with an invalid drawable. Perhaps Alexandre has a good suggestion. Roderick

March 29, 2011

Re: [3/3] winex11.drv: Make wglMakeCurrent return the correct error when the drawable is invalid.

by Matteo Bruni

2011/3/29 Roderick Colenbrander <thunderbird2k(a)gmail.com>: > The fact that a pixel format is not set on the physDev doesn't > necessarily mean it is invalid, it just means SetPixelFormat wasn't > called on it. Though slightly earlier in wglMakeCurrent we already > catch that situation when compare the pixel format of the context and > the hdc. True, but as you say the check I'm adding doesn't really change the behavior outside of a different SetLastError() call. Maybe the WARN message I'm adding could be improved. > > Is there not something better for which we could check? Is the > drawable of the physDev perhaps something to check for? (I have no > idea in what state the physDev is after a destroy window; should we be > able to enter x11drv at all? perhaps this should be handled inside the > gdi32 code, just a thought) > Checking for the drawable or any other field in the physDev doesn't seem to be really better. I'm not sure about the feasibility of handling that on the gdi32 side, I think I gave it a try at some point but then I resorted to this other approach. But, of course, I may be failing to see a better solution ;) > Roderick > > On Tue, Mar 29, 2011 at 7:42 AM, Matteo Bruni <matteo.mystral(a)gmail.com> wrote: >>

March 29, 2011

Re: [2/3] winex11.drv: Don't modify current state if glXMakeCurrent fails.

by Matteo Bruni

2011/3/29 Roderick Colenbrander <thunderbird2k(a)gmail.com>: > I'm not sure if the only value error wglMakeCurrent should return is > INVALID_HANDLE. What if glXMakeCurrent fails because of an invalid > context? The return value may be similar, but don't remember for sure. > > Roderick Practically speaking it seems to be very dependent on the specific GL driver used. For example, current Nvidia Windows drivers just accept a wglMakeCurrent call with a destroyed context (no errors at all), while I think AMD drivers crash in the same case. I'm not sure I specifically tested that on glX too, but in general, failing on a destroyed drawable was the only consistent "edge case" behavior I found. > > On Tue, Mar 29, 2011 at 7:42 AM, Matteo Bruni <matteo.mystral(a)gmail.com> wrote: >> >> >> >> >> >

March 29, 2011

Re: [3/3] winex11.drv: Make wglMakeCurrent return the correct error when the drawable is invalid.

by Roderick Colenbrander

The fact that a pixel format is not set on the physDev doesn't necessarily mean it is invalid, it just means SetPixelFormat wasn't called on it. Though slightly earlier in wglMakeCurrent we already catch that situation when compare the pixel format of the context and the hdc. Is there not something better for which we could check? Is the drawable of the physDev perhaps something to check for? (I have no idea in what state the physDev is after a destroy window; should we be able to enter x11drv at all? perhaps this should be handled inside the gdi32 code, just a thought) Roderick On Tue, Mar 29, 2011 at 7:42 AM, Matteo Bruni <matteo.mystral(a)gmail.com> wrote: > > > > >

March 29, 2011

Re: [2/3] winex11.drv: Don't modify current state if glXMakeCurrent fails.

by Roderick Colenbrander

I'm not sure if the only value error wglMakeCurrent should return is INVALID_HANDLE. What if glXMakeCurrent fails because of an invalid context? The return value may be similar, but don't remember for sure. Roderick On Tue, Mar 29, 2011 at 7:42 AM, Matteo Bruni <matteo.mystral(a)gmail.com> wrote: > > > > >

March 29, 2011

Re: wldap32: Accept a NULL value in ldap_set_option.

by Marvin

Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at http://testbot.winehq.org/JobDetails.pl?Key=10187 Your paranoid android. === WNT4WSSP6 (32 bit parse) === parse.c:120: Test failed: ldap_set_optionW failed 0x00000059 === W2KPROSP4 (32 bit parse) === parse.c:120: Test failed: ldap_set_optionW failed 0x00000059

March 29, 2011

Re: mshtml: Fixed node leak in HTMLEventObj

by Alexandre Julliard

Jacek Caban <jacek(a)codeweavers.com> writes: > --- > dlls/mshtml/htmlevent.c | 2 ++ > 1 files changed, 2 insertions(+), 0 deletions(-) This causes the mshtml:events test to hang for me. -- Alexandre Julliard julliard(a)winehq.org

March 29, 2011

Re: [PATCH] kernel32: Populate HKLM\HARDWARE\DEVICEMAP\Scsi on Mac OS.

by Alexandre Julliard

Charles Davis <cdavis(a)mymail.mines.edu> writes: > From: Charles Davis <cdavis(a)mines.edu> > > --- > dlls/kernel32/Makefile.in | 2 +- > dlls/kernel32/oldconfig.c | 157 +++++++++++++++++++++++++++++++++++++++++++- > 2 files changed, 154 insertions(+), 5 deletions(-) This code needs to be moved to the mountmgr first. -- Alexandre Julliard julliard(a)winehq.org

March 29, 2011

Re: [PATCH 3/4] comdlg32: Add implementation of DllRegisterServer/DllUnregisterServer. (resend)

by Alexandre Julliard

David Hedberg <david.hedberg(a)gmail.com> writes: > @@ -274,3 +277,82 @@ HRESULT WINAPI DllGetClassObject(REFCLSID rclsid, REFIID riid, void **ppv) > > return CLASS_E_CLASSNOTAVAILABLE; > } > + > +/*********************************************************************** > + * Register/Unregister DLL, based on shdocvw/factory.c > + */ You should use the new IRegistrar mechanism instead. -- Alexandre Julliard julliard(a)winehq.org

March 29, 2011