942505 - Move everything SharedHandle-related out of GLContext

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Comment 1

[Benoit Jacob [:bjacob] (mostly away)]

I'm going to take this one.

Comment 2

[Benoit Jacob [:bjacob] (mostly away)]

Attachment: patch

https://tbpl.mozilla.org/?tree=Try&rev=5ff94a726ea5

There are several changes in this patch that aren't just moving code around:

1. Some dead code was removed, especially UpdateSharedHandle, and the mTemporary* thing that was there in GLContextProviderEGL.cpp and was only used by it.

2. The mShareWithEGLImage thing in GLContextProviderEGL.cpp becomes a static function in GLSharedHandleHelpers.cpp

3. EGLTextureWrapper::CreateEGLImage used to take a GLuint, now takes a uintptr_t. Reason: we're reinterpreting it as a EGL type that's effectively a void*, and we now -Werror on this.

... plus a couple of small things that I don't remember.

Comment 3

[Benoit Jacob [:bjacob] (mostly away)]

Note that this takes 338 lines of code out of GLContextProviderEGL.cpp, and 65 out of GLContext.h.

Comment 4

[Benoit Jacob [:bjacob] (mostly away)]

Another try on android. https://tbpl.mozilla.org/?tree=Try&rev=24cdf4fa0b38

The reason why this was needed is this code path:

http://hg.mozilla.org/mozilla-central/file/e9337081c744/gfx/gl/GLContextProviderEGL.cpp#l940

Can this be hit on the GLContext underlying a WebGL context? If yes, this is a conformance bug!

Comment 5

[Benoit Jacob [:bjacob] (mostly away)]

I now remember another thing that departs slightly from strict copy-and-paste. The cross-platform-ness was a bit trivial, as this only has a nontrivial implementation on EGL, and I don't suppose that this is going to change, so this is now using a single implementation with e.g.:

+    // unimplemented outside of EGL
+    if (gl->GetContextType() != ContextTypeEGL)
+        return 0;

Comment 6

[Benoit Jacob [:bjacob] (mostly away)]

https://tbpl.mozilla.org/?tree=Try&rev=b4db4504feb0

Comment 7

[Kelsey Gilbert [:jgilbert]]

(In reply to Benoit Jacob [:bjacob] from comment #4)
> Another try on android. https://tbpl.mozilla.org/?tree=Try&rev=24cdf4fa0b38
> 
> The reason why this was needed is this code path:
> 
> http://hg.mozilla.org/mozilla-central/file/e9337081c744/gfx/gl/
> GLContextProviderEGL.cpp#l940
> 
> Can this be hit on the GLContext underlying a WebGL context? If yes, this is
> a conformance bug!

WebGL contexts shouldn't be using this path anymore. IIRC, this is only used by plugins now.

Comment 8

[Benoit Jacob [:bjacob] (mostly away)]

https://tbpl.mozilla.org/?tree=Try&rev=eb3be360fb34

Comment 9

[Benoit Jacob [:bjacob] (mostly away)]

https://tbpl.mozilla.org/?tree=Try&rev=1b48093b8244

Comment 10

[Benoit Jacob [:bjacob] (mostly away)]

https://tbpl.mozilla.org/?tree=Try&rev=01672e3e55a0

http://hg.mozilla.org/integration/mozilla-inbound/rev/3856aa97c77c

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/3856aa97c77c