Closed Bug 942505 Opened 6 years ago Closed 6 years ago

Move everything SharedHandle-related out of GLContext

Categories

(Core :: Graphics, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla28

People

(Reporter: bjacob, Assigned: bjacob)

References

Details

(Whiteboard: [qa-])

Attachments

(1 file)

No description provided.
I'm going to take this one.
Assignee: nobody → bjacob
Summary: Move everything ShareHandle-related out of GLContext → Move everything SharedHandle-related out of GLContext
Attached patch patchSplinter Review
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.
Attachment #8340196 - Flags: review?(jgilbert)
Note that this takes 338 lines of code out of GLContextProviderEGL.cpp, and 65 out of GLContext.h.
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!
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;
(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.
Attachment #8340196 - Flags: review?(jgilbert) → review+
https://hg.mozilla.org/mozilla-central/rev/3856aa97c77c
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Depends on: 946579
Whiteboard: [qa-]
You need to log in before you can comment on or make changes to this bug.