Made ENTER_GL() - LEAVE_GL() free X11 lock on exception inside opengl, code

2008-06-28 Thread Louis. Lenders
>And as was shown, Wine correctly returns NULL when its called without a 
>context, so there is >something else making it crash. It may be a buggy 
>driver. It may be memory corruption Wine is >causing.. who knows.

i don't think it's buggy driver,  this  happens as far  as i know for various 
people, so i'd guess various cards/drivers.

Is there a way to track down what AutoCAD returns in this call to glGetString 
on Windows?

Btw, the installer even finishes fine with native opengl32.dll ( no, this is 
not a joke!), so somehow i have the feeling that an exception handler is the 
right way anyway, but i lack knowledge of opengl, so i'll shut up for now ;) )



  __
Not happy with your email address?.
Get the one you really want - millions of new email addresses available now at 
Yahoo! http://uk.docs.yahoo.com/ymail/new.html


Re: Made ENTER_GL() - LEAVE_GL() free X11 lock on exception inside opengl, code

2008-06-28 Thread Chris Robinson
On Saturday 28 June 2008 01:50:09 pm Massimo Del Fedele wrote:
> Chris Robinson ha scritto:
> > I think the issue is that it shouldn't be crashing. Especially if we
> > don't know why it's crashing,...
>
> well, there are not so many places... it crashes on a glGetString(),
> which accepts a scalar value and returns a static string pointer.
> No dynamic data, no bad pointers outside it.
> So it must be a crash INSIDE opengl code.
> It's called (wrongly) without an opengl context, but that's how autocad
> installer does, so it should NOT crash because of that.

And as was shown, Wine correctly returns NULL when its called without a 
context, so there is something else making it crash. It may be a buggy 
driver. It may be memory corruption Wine is causing.. who knows.

> but I'm
> still convinced that as it doesn't crash in windows it should not do it
> in wine.

Of course, which is why we should try to find the reason it crashes and fix 
it, not hide the crash behind a costly try block.




Re: Made ENTER_GL() - LEAVE_GL() free X11 lock on exception inside opengl, code

2008-06-28 Thread Massimo Del Fedele
Chris Robinson ha scritto:
> On Saturday 28 June 2008 07:51:53 am Massimo Del Fedele wrote:
>> Roderick Colenbrander ha scritto:
>>> Hi,
>>>
>>> I don't think we want to go this way. First of all we want to emulate
>>> win32 opengl. If windows does this we MIGHT have to do something like
>>> this.
>> Well, I've never seen windows hang for an opengl call exception... But I
>>can be wrong. IMHO the exception should be catched and/or propagated,
>> not allowed to block the whole app.
>> Here the problem is that an exception thrown by gl code skips the
>> wine_tsx11_unlock() call, blocking all following X11 code.
> 
> I think the issue is that it shouldn't be crashing. Especially if we don't 
> know why it's crashing,...

well, there are not so many places... it crashes on a glGetString(), 
which accepts a scalar value and returns a static string pointer.
No dynamic data, no bad pointers outside it.
So it must be a crash INSIDE opengl code.
It's called (wrongly) without an opengl context, but that's how autocad 
installer does, so it should NOT crash because of that.
In windows it just returns NULL pointer, in wine, with a simple test app 
it also returns NULL, so we can't reproduce with a simple test case.
Maybe some previous GL call damages some internal opengl data... but I'm 
still convinced that as it doesn't crash in windows it should not do it 
in wine. Or, at least it should free the lock when crashing.

> this patch is just hiding the error more than 
> anything. 

Not exactly. Latest patch (the try...finally) doesn't hide anything, it 
just frees the lock upon crash in gl code, which imho is correct 
behaviour. Autocad installer ignores the crash and follows, with the 
patch. Without it, it hangs on next x11 call.

For all we know it can be a buffer overrun somewhere, and you don't
> want to just work around things like that.
> 
>>> Second all opengl calls hit ENTER_GL / LEAVE_GL and these few extra calls
>>> affect performance. 
>> which extra calls ?
> 
> The __TRY stuff. It's rather expensive to set up an exception block, even in 
> C++ where it's part of the language (let alone how Wine does it in C). But we 
> do know D3D doesn't catch exceptions in most cases (it can crash in plenty of 
> ways), so I don't think this is correct anyway.
> 
The first patch proposed (and another, more complete), IS a workaround.
We checks if a context exists and, if none, we just return NULL instead 
locking-calling-unlocking. Windows behaviour is that one, so it should 
do no harm.

The problem here is not catching/not catching the exception; I agree 
that it should be uncaught and let to user code. The problem is the 
stale lock with has nothing to do with user code.
If some app has a code like that :

try {
   do_some_gl_call();
} catch (...)
   error_handling()
}
some_x11_call();  <== it will hang here

On ANY opengl exception it'll not have any chance to correct the error 
and continue.

Max





Re: Made ENTER_GL() - LEAVE_GL() free X11 lock on exception inside opengl, code

2008-06-28 Thread Chris Robinson
On Saturday 28 June 2008 07:51:53 am Massimo Del Fedele wrote:
> Roderick Colenbrander ha scritto:
> > Hi,
> >
> > I don't think we want to go this way. First of all we want to emulate
> > win32 opengl. If windows does this we MIGHT have to do something like
> > this.
>
> Well, I've never seen windows hang for an opengl call exception... But I
>can be wrong. IMHO the exception should be catched and/or propagated,
> not allowed to block the whole app.
> Here the problem is that an exception thrown by gl code skips the
> wine_tsx11_unlock() call, blocking all following X11 code.

I think the issue is that it shouldn't be crashing. Especially if we don't 
know why it's crashing, this patch is just hiding the error more than 
anything. For all we know it can be a buffer overrun somewhere, and you don't 
want to just work around things like that.

> > Second all opengl calls hit ENTER_GL / LEAVE_GL and these few extra calls
> > affect performance. 
>
> which extra calls ?

The __TRY stuff. It's rather expensive to set up an exception block, even in 
C++ where it's part of the language (let alone how Wine does it in C). But we 
do know D3D doesn't catch exceptions in most cases (it can crash in plenty of 
ways), so I don't think this is correct anyway.




Re: Made ENTER_GL() - LEAVE_GL() free X11 lock on exception inside opengl, code

2008-06-28 Thread Roderick Colenbrander
Hi,

I don't think we want to go this way. First of all we want to emulate win32 
opengl. If windows does this we MIGHT have to do something like this. Second 
all opengl calls hit ENTER_GL / LEAVE_GL and these few extra calls affect 
performance.

Personally I think that when you are hitting such an X11 lock exception that 
there is just a bug somewhere in wine. E.g. in case of a Direct3D app a piece 
of wined3d not doing proper locking. The same can be the case for plain opengl 
apps where a call might not have called wine_tsx11_unlock.

Regards,
Roderick Colenbrander

>  From 31327546fc30520c80433fc964d7bd3ba4f80fa9 Mon Sep 17 00:00:00 2001
> From: Massimo Del Fedele <[EMAIL PROTECTED]>
> Date: Sat, 28 Jun 2008 12:00:17 +0200
> Subject: Made ENTER_GL() - LEAVE_GL() free X11 lock on exception inside 
> opengl
>code
> 
> ---
>   dlls/opengl32/opengl_ext.h |8 ++--
>   dlls/opengl32/wgl.c|6 ++
>   2 files changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/dlls/opengl32/opengl_ext.h b/dlls/opengl32/opengl_ext.h
> index 8ae7c2f..9892351 100644
> --- a/dlls/opengl32/opengl_ext.h
> +++ b/dlls/opengl32/opengl_ext.h
> @@ -40,6 +40,9 @@
>   #define WINAPI  __stdcall
>   #define APIENTRYWINAPI
> 
> +#include "wine/library.h"
> +#include "wine/exception.h"
> +
>   /* X11 locking */
> 
>   extern void (*wine_tsx11_lock_ptr)(void);
> @@ -47,8 +50,9 @@ extern void (*wine_tsx11_unlock_ptr)(void);
> 
>   /* As GLX relies on X, this is needed */
>   void enter_gl(void);
> -#define ENTER_GL() enter_gl()
> -#define LEAVE_GL() wine_tsx11_unlock_ptr()
> +void CALLBACK leave_gl(BOOL);
> +#define ENTER_GL() enter_gl(); __TRY {
> +#define LEAVE_GL() } __FINALLY(leave_gl)
> 
> 
>   typedef struct {
> diff --git a/dlls/opengl32/wgl.c b/dlls/opengl32/wgl.c
> index 6fbdeb4..7e31e20 100644
> --- a/dlls/opengl32/wgl.c
> +++ b/dlls/opengl32/wgl.c
> @@ -114,6 +114,12 @@ void enter_gl(void)
>   return;
>   }
> 
> +void CALLBACK leave_gl(BOOL dummy)
> +{
> +wine_tsx11_unlock_ptr();
> +return;
> +}
> +
>   const GLubyte * WINAPI wine_glGetString( GLenum name );
> 
>   /***
> -- 
> 1.5.4.3
> 
> 
> 

-- 
Der GMX SmartSurfer hilft bis zu 70% Ihrer Onlinekosten zu sparen! 
Ideal für Modem und ISDN: http://www.gmx.net/de/go/smartsurfer