Closed
Bug 741730
Opened 13 years ago
Closed 13 years ago
Remove USE_GLES2
Categories
(Core :: Graphics, defect)
Tracking
()
RESOLVED
FIXED
mozilla14
People
(Reporter: romaxa, Assigned: bjacob)
Details
Attachments
(2 files)
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
romaxa
:
review+
|
Details | Diff | Splinter Review |
Was facing again problem with undefined USE_GLES2 on custom platform build
and I think we can now define USE_GLES2 right in mozilla-defines.h i default provider is EGL instead of defining multiple platforms which kindof should have it enabled
Reporter | ||
Comment 1•13 years ago
|
||
Assignee: nobody → romaxa
Attachment #611762 -
Flags: review?(mh+mozilla)
Attachment #611762 -
Flags: review?(bjacob)
Comment 2•13 years ago
|
||
Comment on attachment 611762 [details] [diff] [review]
Define use_gles2 for EGL providers globally
Review of attachment 611762 [details] [diff] [review]:
-----------------------------------------------------------------
I'm tempted to say this should probably be more generic (that is, having a define for the default backend, for all values of the default backend)
I'm wondering, though, if it's expected that USE_GLES2 is undefined on windows, or does USE_GLES2 implies EGL is the default (which it is not on windows).
Attachment #611762 -
Flags: review?(mh+mozilla)
Assignee | ||
Comment 3•13 years ago
|
||
What's the point of USE_GLES2? MXR finds only 9 uses of it, and it seems to be for all the wrong reasons:
http://mxr.mozilla.org/mozilla-central/search?string=USE_GLES2
http://hg.mozilla.org/mozilla-central/file/fc1432924480/gfx/gl/GLContext.h#l534
here, USE_GLES2 is used to initialize GLContext::mIsGLES2. That's bogus. A given platform could have both ES and non-ES providers. The right way to know if a context is ES is: has it been created by EGL?
http://mxr.mozilla.org/mozilla-central/source/gfx/gl/GLContext.cpp#2301
this should use mIsGLES2
http://mxr.mozilla.org/mozilla-central/source/gfx/gl/GLContext.cpp#2373
http://mxr.mozilla.org/mozilla-central/source/gfx/gl/GLContext.cpp#2478
this should either use mIsGLES2 or, if one really doesn't want to compile some code in the other path, move code to a virtual function that's overridden in the EGL provider to provide the ES variant.
Let's kill USE_GLES2.
Assignee | ||
Updated•13 years ago
|
Attachment #611762 -
Flags: review?(bjacob)
Assignee | ||
Comment 4•13 years ago
|
||
To be clear, there was a real bug here: on Windows, USE_GLES2 was not defined but ANGLE provides GLES2, so we'd have run into trouble if we had tried to use ANGLE for layers like Chromium does, or if USE_GLES2 usage had crept into the parts of GLContext that WebGL uses.
Assignee | ||
Comment 5•13 years ago
|
||
Here's my counter-offer :-)
Attachment #613109 -
Flags: review?(romaxa)
Reporter | ||
Comment 6•13 years ago
|
||
Comment on attachment 613109 [details] [diff] [review]
kill USE_GLES2
Yeah, I like this idea more too., tested quickly and it works also good
Attachment #613109 -
Flags: review?(romaxa) → review+
Assignee | ||
Updated•13 years ago
|
Summary: Define USE_GLES2 in global defines → Remove USE_GLES2
Assignee | ||
Comment 7•13 years ago
|
||
Assignee: romaxa → bjacob
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla14
Comment 8•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
•