Closed
Bug 741689
Opened 13 years ago
Closed 13 years ago
GLContext init failure when ANGLE and !EXT fb extensions are present
Categories
(Core :: Graphics, defect, P2)
Tracking
()
RESOLVED
FIXED
mozilla14
People
(Reporter: m1, Assigned: jgilbert)
Details
Attachments
(1 file)
8.71 KB,
patch
|
bjacob
:
review+
|
Details | Diff | Splinter Review |
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_6_8) AppleWebKit/535.19 (KHTML, like Gecko) Chrome/18.0.1025.109 Safari/535.19
Steps to reproduce:
Tried to run B2G on a device with a particular OpenGL implementation
Actual results:
GLContext init fell down because this OpenGL implementation provides GL_ANGLE_framebuffer_multisample and
GL_ANGLE_framebuffer_blit, but NOT
GL_EXT_framebuffer_multisample and GL_EXT_framebuffer_blit.
There are a couple places in gecko GL initialization there the code
blocks that assume that ANGLE *and* EXT are either both present or
not, even though those blocks are wrapped by ANGLE *or* EXT
conditionals:
https://github.com/cgjones/mozilla-central/blob/master/gfx/gl/GLContext.cpp#L399
https://github.com/cgjones/mozilla-central/blob/master/gfx/gl/GLContext.cpp#L411
This causes GL initialization to fail as the glBlitFramebufferEXT and
glRenderbufferStorageMultisampleEXT symbols are not found.
Expected results:
GLContext init should not have failed.
Reporter | ||
Updated•13 years ago
|
Severity: normal → major
OS: Mac OS X → Gonk
Priority: -- → P2
Hardware: x86 → ARM
Reporter | ||
Updated•13 years ago
|
Component: General → Graphics
gfx folks, is there any reason we need to fail when the GL_EXT* variants aren't present?
Comment 2•13 years ago
|
||
This sounds like a bug, but over to Jeff who knows the multisample stuff best.
Assignee: nobody → jgilbert
Comment 3•13 years ago
|
||
To be clear, these extensions are only used for WebGL anti-aliasing, so you could make a quick and dirty patch ignoring them and not trying to do WebGL anti-aliasing.
Comment 4•13 years ago
|
||
Michael, this code:
if (IsExtensionSupported(GLContext::ANGLE_framebuffer_blit) ||
IsExtensionSupported(GLContext::EXT_framebuffer_blit)) {
SymLoadStruct auxSymbols[] = {
{ (PRFuncPtr*) &mSymbols.fBlitFramebuffer, { "BlitFramebuffer", "BlitFramebufferEXT", "BlitFramebufferANGLE", NULL } },
{ NULL, { NULL } },
};
if (!LoadSymbols(&auxSymbols[0], trygl, prefix)) {
NS_RUNTIMEABORT("GL supports framebuffer_blit without supplying glBlitFramebuffer");
mInitialized = false;
}
}
Is not meant to require both BlitFramebufferEXT and BlitFramebufferANGLE symbols. The symbol name list passed to LoadSymbols() is supposed to mean "try these symbols, use the first one that works". It's not supposed to mean "require all these symbols to exist".
This LoadSymbols function is implemented in GLLibraryLoaded.cpp, and looking at the code, it seems to be indeed doing that.
Can you give more details about why you think it's requiring both symbols to exist? Maybe running this code in GDB, stepping into LoadSymbols, will allow to understand what's happening.
Reporter | ||
Comment 5•13 years ago
|
||
Hi Benoit,
I didn't step through this in GDB like you requested but I think it's pretty clear what's happening by code inspection:
1) LoadSymbols() fails to resolve the *EXT symbols. failCount++.
2) The last line of LoadSymbols():
return failCount == 0 ? true : false;
3) Upon return we then hit the NS_RUNTIMEABORT()
Thanks,
Michael
Comment 6•13 years ago
|
||
It seems to me that failCount++; only happens when none of the symbols is found. Indeed it comes after the end of the loop,
for (int i = 0; i < MAX_SYMBOL_NAMES; i++)
and is only reached if (*ss->symPointer == 0) after then end of that loop, which means that none of the symbols is found.
Reporter | ||
Comment 7•13 years ago
|
||
/me shamed for not looking closer
Okay, I think I know what's *actually* going on now. While the glBlitFramebufferANGLE symbol exists in the adreno200 GLES2 library and it reports support for it, the Android libEGL wrapper library does not currently contain a gl2.cpp shim for glBlitFramebufferANGLE [1]. This is a little bit disturbing, in that it seems like we cannot fully trust what glGetString(GL_EXTENSIONS) tells us.
[1] https://www.codeaurora.org/gitweb/quic/la/?p=platform/frameworks/base.git;a=tree;f=opengl/libs/GLES2;hb=ics
Comment 8•13 years ago
|
||
No worries, I still owe you a much bigger one :-)
Assignee | ||
Comment 9•13 years ago
|
||
According to comment 7, if we cannot trust the extension string, we can, instead of ABORTing when we don't find the symbols, we can mark the extension as unsupported at runtime.
I can take this.
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Updated•13 years ago
|
Attachment #612010 -
Flags: review?(bjacob) → review+
Assignee | ||
Comment 11•13 years ago
|
||
Target Milestone: --- → mozilla14
Comment 12•13 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•