Note: There are a few cases of duplicates in user autocompletion which are being worked on.

GLContext init failure when ANGLE and !EXT fb extensions are present

RESOLVED FIXED in mozilla14

Status

()

Core
Graphics
P2
major
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: m1, Assigned: jgilbert)

Tracking

Trunk
mozilla14
ARM
Gonk (Firefox OS)
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

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

5 years ago
Severity: normal → major
OS: Mac OS X → Gonk
Priority: -- → P2
Hardware: x86 → ARM
(Reporter)

Updated

5 years ago
Component: General → Graphics
gfx folks, is there any reason we need to fail when the GL_EXT* variants aren't present?
This sounds like a bug, but over to Jeff who knows the multisample stuff best.
Assignee: nobody → jgilbert
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.
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

5 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
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

5 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
No worries, I still owe you a much bigger one :-)
(Assignee)

Comment 9

5 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
(Assignee)

Comment 10

5 years ago
Created attachment 612010 [details] [diff] [review]
Disable extensions when their functions are not properly exposed

Also some cleanup.
Attachment #612010 - Flags: review?(bjacob)
Attachment #612010 - Flags: review?(bjacob) → review+
(Assignee)

Comment 11

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/3a06c3ef2c1b
Target Milestone: --- → mozilla14
http://hg.mozilla.org/mozilla-central/rev/3a06c3ef2c1b
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.