[Bug 17193] New: [PATCH] cope with missing wglGetExtensionsStringARB / wglGetPixelFormatAttribivARB

http://bugs.winehq.org/show_bug.cgi?id=17193

Summary: [PATCH] cope with missing wglGetExtensionsStringARB /
                    wglGetPixelFormatAttribivARB
           Product: Wine
           Version: unspecified
          Platform: Other
        OS/Version: other
            Status: UNCONFIRMED
          Severity: enhancement
          Priority: P2
         Component: directx-d3d
        AssignedTo: [e-mail]
        ReportedBy: [e-mail]

Created an attachment (id=19093)
 --> (http://bugs.winehq.org/attachment.cgi?id=19093)
patch

Currently, WineD3D assumes wglGetExtensionsStringARB and
wglGetPixelFormatAttribivARB are present, and if they're not, it will segfault
when attempting to call them.  This patch adds some checks so that:

- The pointers in GLINFO_LOCATION are set to NULL when a WGL function is not
present (this was only done for GL ones).
- Code paths exist to fail gracefully when either wglGetExtensionsStringARB or
wglGetPixelFormatAttribivARB are unimplemented, disabling the functionality
associated with them.

(This situation was found when using WineD3D with Chromium OpenGL as
implemented in VirtualBox OSE 2.1)

[Bug 17193] [PATCH] cope with missing wglGetExtensionsStringARB / wglGetPixelFormatAttribivARB

http://bugs.winehq.org/show_bug.cgi?id=17193

Robert Millan  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |patch

[Bug 17193] [PATCH] cope with missing wglGetExtensionsStringARB / wglGetPixelFormatAttribivARB

http://bugs.winehq.org/show_bug.cgi?id=17193

--- Comment #1 from Roderick Colenbrander   2009-02-02 15:25:57 ---
(In reply to comment #0)
> Created an attachment (id=19093)

--> (http://bugs.winehq.org/attachment.cgi?id=19093) [details]
> patch
>
> Currently, WineD3D assumes wglGetExtensionsStringARB and
> wglGetPixelFormatAttribivARB are present, and if they're not, it will segfault
> when attempting to call them.  This patch adds some checks so that:
> - The pointers in GLINFO_LOCATION are set to NULL when a WGL function is not
>
> present (this was only done for GL ones).

When wglGetProcAddress fails it is guaranteed to return NULL. This is opposed
to glXGetProcAddress which isn't required to return NULL. Setting function
pointers to NULL is explicitly done for GL extensions because in case of wine
we have some evil code where 'pwglGetProcAddress' is directly forwarded to
glXGetProcAddress.
> - Code paths exist to fail gracefully when either wglGetExtensionsStringARB or
>
> wglGetPixelFormatAttribivARB are unimplemented, disabling the functionality
> associated with them.
>
> (This situation was found when using WineD3D with Chromium OpenGL as
> implemented in VirtualBox OSE 2.1)

At this point our pixel format code requires WGL_ARB_pixel_format. I could add
extra code for a legacy PFD backend but I think it is better to fix Chromium
because this extension is very common and basically any serious or modern
OpenGL program uses it.

[Bug 17193] [PATCH] cope with missing wglGetExtensionsStringARB / wglGetPixelFormatAttribivARB

http://bugs.winehq.org/show_bug.cgi?id=17193

--- Comment #2 from Robert Millan   2009-02-02 16:28:34 ---
(In reply to comment #1)
> (In reply to comment #0)
> > - The pointers in GLINFO_LOCATION are set to NULL when a WGL function is not
> >
> > present (this was only done for GL ones).
>
> When wglGetProcAddress fails it is guaranteed to return NULL. This is opposed
> to glXGetProcAddress which isn't required to return NULL. Setting function
> pointers to NULL is explicitly done for GL extensions because in case of wine
> we have some evil code where 'pwglGetProcAddress' is directly forwarded to
> glXGetProcAddress.

Ah, ok.
> > - Code paths exist to fail gracefully when either wglGetExtensionsStringARB or
> >
> > wglGetPixelFormatAttribivARB are unimplemented, disabling the functionality
> > associated with them.
> >
> > (This situation was found when using WineD3D with Chromium OpenGL as
> > implemented in VirtualBox OSE 2.1)
>
> At this point our pixel format code requires WGL_ARB_pixel_format. I could add
> extra code for a legacy PFD backend but I think it is better to fix Chromium
> because this extension is very common and basically any serious or modern
> OpenGL program uses it.

It seems the pixelformat stuff is there, just not being exported (this was
known to be the case for GetExtensionsString already, which VBox folks fixed in
http://www.virtualbox.org/changeset/16220).  But looking at the code in HEAD,
wglGetPixelFormatAttribivEXT is exported there:

src/VBox/Additions/WINNT/Graphics/crOpenGL/getprocaddress.py:           if
(!crStrcmp( name, "wglGetPixelFormatAttribivEXT" )) return (CR_PROC)
wglGetPixelFormatAttribivEXT;

So I guess newer builds won't have this problem (I don't have the environment
to build this myself).

There's also the question on whether you want to bother supporting previous
releases of VirtualBox.  If you'd rather not do that, then I guess this bug can
be closed.

[Bug 17193] [PATCH] cope with missing wglGetExtensionsStringARB / wglGetPixelFormatAttribivARB

http://bugs.winehq.org/show_bug.cgi?id=17193

--- Comment #3 from Roderick Colenbrander   2009-02-03 03:10:17 ---
I'm thinking about fixing the code in order to make it cleaner but I'm not sure
when I have time for that.

[Bug 17193] [PATCH] cope with missing wglGetExtensionsStringARB / wglGetPixelFormatAttribivARB

http://bugs.winehq.org/show_bug.cgi?id=17193

--- Comment #4 from Robert Millan   2009-02-10 15:34:13 ---
> From http://www.virtualbox.org/ticket/2940:

"missing wglGetPixelFormatAttribivARB can be partially replaced by
DescribePixelFormat. this gives better results(e.g. dxdiag's test works)."

http://www.virtualbox.org/attachment/ticket/2940/vbox_opengl_workarounds.2.d...

[Bug 17193] [PATCH] cope with missing wglGetExtensionsStringARB / wglGetPixelFormatAttribivARB

http://bugs.winehq.org/show_bug.cgi?id=17193

Robert Millan  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #19093|0                           |1
        is obsolete|                            |

--- Comment #5 from Robert Millan   2009-03-14 07:42:02 ---
Created an attachment (id=19933)
 --> (http://bugs.winehq.org/attachment.cgi?id=19933)
getpixelformat.diff

Here's a new patch.  Resynced with git and applies/works with 1.1.17 as well.

[Bug 17193] [PATCH] cope with missing wglGetExtensionsStringARB / wglGetPixelFormatAttribivARB

http://bugs.winehq.org/show_bug.cgi?id=17193

Roderick Colenbrander  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

--- Comment #6 from Roderick Colenbrander   2009-03-15 05:40:07 ---
Today I had some time to look at this bug but as I didn't catch up reading all
my wine email, I hadn't seen this update. As you might have seen I have
submitted two patches. The first one adds WGL_ARB_pixel_format detection. The
second adds a DescribePixelFormat based WGL pixel format detection backend.
This code basically does the same as yours with a key difference that I'm also
filtering out non-ICD pixel formats. I don't want to use the bad GDI opengl
renderer.

See:
http://www.winehq.org/pipermail/wine-patches/2009-March/070763.html
http://www.winehq.org/pipermail/wine-patches/2009-March/070765.html

Do they work for you?

[Bug 17193] [PATCH] cope with missing wglGetExtensionsStringARB / wglGetPixelFormatAttribivARB

http://bugs.winehq.org/show_bug.cgi?id=17193

--- Comment #7 from Robert Millan   2009-03-15 16:19:10 ---
(In reply to comment #6)
> Today I had some time to look at this bug but as I didn't catch up reading all
> my wine email, I hadn't seen this update. As you might have seen I have
> submitted two patches. The first one adds WGL_ARB_pixel_format detection. The
> second adds a DescribePixelFormat based WGL pixel format detection backend.
> This code basically does the same as yours with a key difference that I'm also
> filtering out non-ICD pixel formats. I don't want to use the bad GDI opengl
> renderer.
>
> See:
> http://www.winehq.org/pipermail/wine-patches/2009-March/070763.html
> http://www.winehq.org/pipermail/wine-patches/2009-March/070765.html
>
> Do they work for you?

Thanks for your effort.  Unfortunately, when replacing baskanov's patch with
these two, dxdiag just hangs during initialization (first it reports that one
of its child processes performed an illegal operation, and then the parent
hangs).

[Bug 17193] [PATCH] cope with missing wglGetExtensionsStringARB / wglGetPixelFormatAttribivARB

http://bugs.winehq.org/show_bug.cgi?id=17193

--- Comment #8 from Roderick Colenbrander   2009-03-16 02:38:53 ---
Hmm, strange. Figure out if wined3d is actually entering the
DescribePixelFormat based pixel format initialization. You could force this by
replacing 'if(GL_SUPPORT(WGL_ARB_PIXEL_FORMAT)) with 'if(0)'. If the code is
entered figure out if the following part isn't entered:

+                ERR("Disabling Direct3D because no hardware accelerated pixel
formats have been found!\n");
+
+                WineD3D_ReleaseFakeGLContext();
+                HeapFree(GetProcessHeap(), 0, adapter->cfgs);
+                goto nogl_adapter;

[Bug 17193] [PATCH] cope with missing wglGetExtensionsStringARB / wglGetPixelFormatAttribivARB

http://bugs.winehq.org/show_bug.cgi?id=17193

--- Comment #9 from Robert Millan   2009-03-16 13:13:18 ---
Created an attachment (id=19988)
 --> (http://bugs.winehq.org/attachment.cgi?id=19988)
new version to complement Roderick's changes

It seems your patched don't completely superceed baskanov's one.  I've splitted
the missing part, and verified that by combining this with your patches I get a
working setup.

[Bug 17193] [PATCH] cope with missing wglGetExtensionsStringARB / wglGetPixelFormatAttribivARB

http://bugs.winehq.org/show_bug.cgi?id=17193

Robert Millan  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #19933|0                           |1
        is obsolete|                            |

[Bug 17193] [PATCH] cope with missing wglGetExtensionsStringARB / wglGetPixelFormatAttribivARB

http://bugs.winehq.org/show_bug.cgi?id=17193

--- Comment #10 from Robert Millan   2009-03-28 07:45:00 ---
I notice Roderick's changes made it to 1.1.18.  These work fine, when adding
the missing bit from Baskanov (see patch above).  Nice work, Roderick.

What do we need to get Baskanov's hunk merged?  It's pretty small &
straightforwarded as you can see.  Should I send it to wine-patches?  Is it
preferred to include indentation changes in the patch?

[Bug 17193] [PATCH] cope with missing wglGetExtensionsStringARB / wglGetPixelFormatAttribivARB

http://bugs.winehq.org/show_bug.cgi?id=17193

--- Comment #11 from Roderick Colenbrander   2009-03-28 17:27:25 ---
I will add a proper fix for this issue and another one soon.

[Bug 17193] [PATCH] cope with missing wglGetExtensionsStringARB / wglGetPixelFormatAttribivARB

http://bugs.winehq.org/show_bug.cgi?id=17193

--- Comment #12 from Roderick Colenbrander   2009-03-29 15:43:53 ---
I have submitted a patch. Second when running on virtualbox I recommend to
compile using -DWINE_NO_DEBUG_MSGS to prevent checkGLcalls in wined3d. Those
calls call glGetError and they might be very expensive on virtualbox.

[Bug 17193] [PATCH] cope with missing wglGetExtensionsStringARB / wglGetPixelFormatAttribivARB

http://bugs.winehq.org/show_bug.cgi?id=17193

--- Comment #13 from Robert Millan   2009-03-29 16:25:15 ---
(In reply to comment #12)
> I have submitted a patch. Second when running on virtualbox I recommend to
> compile using -DWINE_NO_DEBUG_MSGS to prevent checkGLcalls in wined3d. Those
> calls call glGetError and they might be very expensive on virtualbox.

I know (already doing this).  I tested the patch and it works fine, just sent a
mail to wine-patches as well.

[Bug 17193] [PATCH] cope with missing wglGetExtensionsStringARB / wglGetPixelFormatAttribivARB

http://bugs.winehq.org/show_bug.cgi?id=17193

Roderick Colenbrander  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|                            |FIXED

--- Comment #14 from Roderick Colenbrander   2009-03-30 15:50:14 ---
All patches are either in .18 or in git, so I'm marking this bug as fixed.

[Bug 17193] [PATCH] cope with missing wglGetExtensionsStringARB / wglGetPixelFormatAttribivARB

http://bugs.winehq.org/show_bug.cgi?id=17193

Robert Millan  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |CLOSED

--- Comment #15 from Robert Millan   2009-03-30 17:41:12 ---
Thanks Roderick.  Finally pristine Wine can run on VirtualBox.

[Bug 17193] [PATCH] cope with missing wglGetExtensionsStringARB / wglGetPixelFormatAttribivARB

http://bugs.winehq.org/show_bug.cgi?id=17193

Alexandre Julliard  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|CLOSED                      |UNCONFIRMED
         Resolution|FIXED                       |

--- Comment #16 from Alexandre Julliard   2009-03-31 03:53:36 ---
Please don't close bugs.

[Bug 17193] [PATCH] cope with missing wglGetExtensionsStringARB / wglGetPixelFormatAttribivARB

http://bugs.winehq.org/show_bug.cgi?id=17193

Alexandre Julliard  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|                            |FIXED

--- Comment #17 from Alexandre Julliard   2009-03-31 03:53:56 ---
Fixed again.

[Bug 17193] [PATCH] cope with missing wglGetExtensionsStringARB / wglGetPixelFormatAttribivARB

http://bugs.winehq.org/show_bug.cgi?id=17193

Alexandre Julliard  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |CLOSED

--- Comment #18 from Alexandre Julliard   2009-04-10 11:22:01 ---
Closing bugs fixed in 1.1.19.