Bug 55072 - Make OpenGL implementation of the GraphicsContext3D class shareable
Summary: Make OpenGL implementation of the GraphicsContext3D class shareable
Status: RESOLVED CONFIGURATION CHANGED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebGL (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-02-23 12:32 PST by Zan Dobersek
Modified: 2021-03-08 02:33 PST (History)
6 users (show)

See Also:


Attachments
Modifying the Extensions3DOpenGL class (20.17 KB, patch)
2011-02-25 00:54 PST, Zan Dobersek
no flags Details | Formatted Diff | Diff
Proposed changes (64.65 KB, patch)
2011-03-16 08:56 PDT, Zan Dobersek
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Zan Dobersek 2011-02-23 12:32:11 PST
The goal of this bug is to modify the current OpenGL implementation of GraphicsContext3D so that it becomes usable by both Mac and Gtk port and perhaps any other port that would decide to use it in future. 

Discussion has already been started on the mailing list[1].

[1] https://lists.webkit.org/pipermail/webkit-dev/2011-February/016027.html
Comment 1 Zan Dobersek 2011-02-23 12:33:51 PST
Add Zhenyao Mo and Kenneth Russell to the CC list, as requested.
Comment 2 Zan Dobersek 2011-02-25 00:54:56 PST
Created attachment 83783 [details]
Modifying the Extensions3DOpenGL class

As discussed with Kenneth on IRC, Gtk port cannot foresee what extensions functions the drivers will be able to offer. By using phrase 'extensions functions' I mean _every_ function that can be extensible (for instance glIsProgramARB, glUniform1fARB, glBlitFramebufferEXT etc). Therefor Gtk port is planning to go through a list of these functions and try loading every function with possible extension suffix through glXGetProcAddress.

Because of that it is necessary to move calls to OpenGL extensions functions out of the GraphicsContext3D class in order to make it usable to the Gtk port as well.

This patch recommends adding further extensions functions to the Extensions3DOpenGL class that would then be implemented separately in the Gtk and Mac port. The patch offers a bare file with basic implementation for the latter, but I'm not sure whether this would be a .mm or a .cpp file (not familiar with the Mac build system). Changes to the GraphicsContext3D class are not included, but it would basically come down to replacing calls to any extensions function with m_extensions->function().

Seeing this just now, the recently added vertex array extensions functions should probably also be branched out into separate files as implementing them on the Gtk port would probably need a different approach.

This patch is of illustrative nature and would welcome opinions on this approach, so no ChangeLog entries were generated and a license header is also missing from the new file.
Comment 3 Zhenyao Mo 2011-02-25 05:54:06 PST
I don't think I understand.  Why do you add all the core GL functions to Extension3DOpenGL?  They are already in GraphicsContext3D.  Extension3D is for extensions only.
Comment 4 Kenneth Russell 2011-02-25 12:11:46 PST
(In reply to comment #3)
> I don't think I understand.  Why do you add all the core GL functions to Extension3DOpenGL?  They are already in GraphicsContext3D.  Extension3D is for extensions only.

These aren't all of the core OpenGL functions, but those which need to be looked up as extensions on some platforms.

The general approach seems OK to me. You'd want to comment the large block of functions indicating the reason they're there. You'll also need to make sure it builds and works on both Mac and Gtk.
Comment 5 Zan Dobersek 2011-03-16 08:56:11 PDT
Created attachment 85933 [details]
Proposed changes

This patch includes the following:

- 91 extensible functions are added to the Extensions3DOpenGL class. These functions are meant to be implemented for each port that uses OpenGL implementation of GraphicsContext3D separately. A comment explaining the situation is added at the top of these functions.
- An implementation is included for the Mac port, located in platform/graphics/mac/Extensions3DOpenGLMac.cpp. The implementation is basically using the same OpenGL functions as they were used in GraphicsContext3DOpenGL.cpp. I'd like some input on whether this should be a .mm file or a .cpp file (as it is currently - I'm not familiar with the Mac build). I'd also be grateful if someone with access to a Mac build could add this file (Extensions3DOpenGLMac.cpp) to the build.
- Call to every OpenGL function in GraphicsContext3DOpenGL.cpp that could be extensible is replaced with an appropriate call to a Extensions3DOpenGL class function. This is the part of the patch that really introduces shareable GC3D. The same is done in GraphicsContext3DMac.cpp.
- Extensions3DOpenGL::blitFramebuffer and Extensions3DOpenGL::renderbufferStorageMultisample are moved to Extensions3DOpenGLMac.cpp and are still executing the same OpenGL calls.

With these modifications the Gtk port (and any other port that would want to use OpenGL GC3D) simply needs to implement functions of Extensions3DOpenGL class in a way that calls those extensions functions that are actually available.
Comment 6 Kimmo Kinnunen 2021-03-08 02:33:21 PST
A lot has changed since this was filed. Taking the liberty to close this. Please file a bug with new description should some parts of this still apply.