575032 - create global shared GL context

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

It simplifies things quite a bit if all of our GL contexts shared the same object namespace; that is, textures etc. from one can be used directly in another.  More importantly, it gives us a global context to use for cleanup operations, even off-thread.  It also enables us to use FBOs for things like WebGL that want their own GL context, but need to render quickly into another context.

Comment 1

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Attachment: 1 - clean up/unify GLContextProviders

This patch gives each GLContextProvider its own class, and makes the creation methods static.  This means that things like GLContextProviderNull and OSMesa are always available, and that GLContextProviderWGL etc. can be directly referenced if needed.  There will soon be another "always-available" (on win32, at least) context provider via ANGLE, so this prepares the way there.

It also gives each method a ContextFormat parameter, even though it's ignored in most current cases; we should fix that, but it's not critical.

Comment 2

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Attachment: 1 - clean up/unify GLContextProviders, v2

Just some misc fixes compared to original version.

Comment 3

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Attachment: 2 - implement context sharing

This is the bulk of the work.  Note that as part of this, I also got rid of pbuffers -- not having them is actually simpler then having them.  The two methods that should be used going forward are:

- CreateForWindow: as today; creates a gl context for a nsIWidget, takes boolean flag that says whether to share resources with the global context (default true).

- CreateOffscreen: creates a context backed by an opaque thing (or maybe even nothing, if the system supports it); the consumer is supposed to set up an FBO with whatever characteristics that they need and render into that.  Also takes a boolean flag, though not very useful without context sharing (you can glReadPixels to get data out, I suppose).

I've left CreateForNativePixmap in here, but I think we should be able to get rid of it.

The big challenge with context sharing is that we have to be extremely careful with GL resources -- we have to Delete everything when we're done with it, because destroying the context will still leave all those live.  To help with that, this also implements a simple DEBUG-only tracker of GL resources; whenever any context is destroyed, it'll print the state of all known resources, the context in which they were created, and that context's status (dead or alive).

I've been thinking about extending this to non-DEBUG versions, and to provide a Cleanup method that will delete all resources that were allocated in the given context.  That, coupled with some kind of Export method to remove certain things from the cleanup list, could help a lot.  (Or perhaps we make it so that everything by default will get deleted when the context is destroyed, unless explicitly Exported, e.g. ExportTexture(k)).

This patch has been tested on WGL, and builds on the other platforms; I tried getting GLX working, but VirtualBox has bugs in its GLX implementation (only implements GLX 1.2 fully, and even then there are some quirks), and I couldn't get it to create a useful context for me.  I'll try EGL and CGL when I get around to it, but I could use some help on the GLX portion.

Comment 4

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Attachment: 3 - implement cleanup for LayerManagerOGL

Implement resource cleanup for LayerManagerOGL.  Mainly the shaders were being leaked; one thing we could do here is just create the shaders once in the global context and keep using them instead of creating/destroying them each time a LayerManagerOGL is created.  (Also would mean we'd only have one set around, instead of one for each manager.)  But that can happen in a followup bug.

Comment 5

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Why would we ever want to create contexts that don't share?

Comment 6

[Bas Schouten (:bas.schouten)]

Comment on attachment 454566 [details] [diff] [review]
1 - clean up/unify GLContextProviders, v2

Can't say I'm a big fan of all this macro magic, but it looks fine to me :).

Comment 7

[Bas Schouten (:bas.schouten)]

Comment on attachment 454567 [details] [diff] [review]
2 - implement context sharing

># HG changeset patch
># Parent edba9a4bb504a78e212ad79994eddc4885a8fddf
>
>diff --git a/content/canvas/src/Makefile.in b/content/canvas/src/Makefile.in
>--- a/content/canvas/src/Makefile.in
>+++ b/content/canvas/src/Makefile.in
>@@ -45,7 +45,7 @@ include $(DEPTH)/config/autoconf.mk
> MODULE		= content
> LIBRARY_NAME	= gkconcvs_s
> LIBXUL_LIBRARY  = 1
>-FAIL_ON_WARNINGS = 1
>+#FAIL_ON_WARNINGS = 1

Fail on warnings is good.. why do we do this?

>+
>+    void GLAPIENTRY fDeleteRenderbuffers(GLsizei n, GLuint *names) {
>+        priv_fDeleteRenderbuffers(n, names);
>+        if (mSharedContext) {
>+            GLContext *tip = mSharedContext;
>+            while (tip->mSharedContext)
>+                tip = tip->mSharedContext;
>+            tip->DeletedRenderbuffers(this, n, names);
>+        }
>+    }

For all these wrapper functions possibly some Macro's could make this a little bit more readable? The functions are largely the same.
 
>     EGLint pbattrs[] = {
>-        LOCAL_EGL_WIDTH, aSize.width,
>-        LOCAL_EGL_HEIGHT, aSize.height,
>+        LOCAL_EGL_WIDTH, 16,
>+        LOCAL_EGL_HEIGHT, 16,
>         LOCAL_EGL_NONE
>     };

Is 16 some magical size here? If it is can we explain where it comes from and make it a constant?

>+static HWND
>+CreateDummyWindow(HDC *aWindowDC = nsnull)
>+{
>+    WNDCLASSW wc;
>+    if (!GetClassInfoW(GetModuleHandle(NULL), L"GLContextWGLClass", &wc)) {
>+        ZeroMemory(&wc, sizeof(WNDCLASSW));
>+        wc.style = CS_OWNDC;
>+        wc.hInstance = GetModuleHandle(NULL);
>+        wc.lpfnWndProc = DefWindowProc;
>+        wc.lpszClassName = L"GLContextWGLClass";
>+        if (!RegisterClassW(&wc)) {
>+            NS_WARNING("Failed to register GLContextWGLClass?!");
>+            // er. failed to register our class?
>+            return NULL;
>+        }
>+    }
>+
>+    HWND win = CreateWindowW(L"GLContextWGLClass", L"GLContextWGL", 0,
>+                             0, 0, 16, 16,
>+                             NULL, NULL, GetModuleHandle(NULL), NULL);
>+    NS_ENSURE_TRUE(win, NULL);
>+
>+    HDC dc = GetDC(win);
>+    NS_ENSURE_TRUE(dc, NULL);
>+
>+    if (gSharedWindowPixelFormat == 0) {
>+        PIXELFORMATDESCRIPTOR pfd;
>+        ZeroMemory(&pfd, sizeof(PIXELFORMATDESCRIPTOR));
>+        pfd.nSize = sizeof(PIXELFORMATDESCRIPTOR);
>+        pfd.nVersion = 1;
>+        pfd.dwFlags = PFD_DRAW_TO_WINDOW | PFD_SUPPORT_OPENGL;
>+        pfd.iPixelType = PFD_TYPE_RGBA;
>+        pfd.cColorBits = 24;
>+        pfd.cDepthBits = 0;
>+        pfd.iLayerType = PFD_MAIN_PLANE;
>+
>+        gSharedWindowPixelFormat = ChoosePixelFormat(dc, &pfd);
>+    }
>+
>+    if (!SetPixelFormat(dc, gSharedWindowPixelFormat, NULL)) {
>+        NS_WARNING("SetPixelFormat failed!");
>+        DestroyWindow(win);
>+        return NULL;
>+    }
>+
>+    if (aWindowDC) {
>+        *aWindowDC = dc;
>+    }
>+
>+    return win;
>+}

Do we really need a window here? Can't some other type of DDB suffice?

Comment 8

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Comment on attachment 454567 [details] [diff] [review]
2 - implement context sharing

Part 2 of this (and a merged-in part 3, actually) ended up being merged into the patch for bug 575469.  We couldn't enforce/depend on context sharing, so instead context sharing is something that we try to do for all window contexts, but we might not get it. (Android is actually the only place where it doesn't reliably work, unfortunately.)

For offscreen contexts, in the patch in that bug we try to create a /non/ shared texturable context first, and eventually fall back to a shared FBO.

Comment 9

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Comment on attachment 454568 [details] [diff] [review]
3 - implement cleanup for LayerManagerOGL

part 2 got integrated into part 2 in bug 575469

Comment 10

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Attachment: updated part 1 to trunk

Comment 11

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

http://hg.mozilla.org/mozilla-central/rev/a6078800db5a