Closed Bug 741730 Opened 12 years ago Closed 12 years ago

Remove USE_GLES2

Categories

(Core :: Graphics, defect)

ARM
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla14

People

(Reporter: romaxa, Assigned: bjacob)

Details

Attachments

(2 files)

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
Assignee: nobody → romaxa
Attachment #611762 - Flags: review?(mh+mozilla)
Attachment #611762 - Flags: review?(bjacob)
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)
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.
Attachment #611762 - Flags: review?(bjacob)
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.
Attached patch kill USE_GLES2Splinter Review
Here's my counter-offer :-)
Attachment #613109 - Flags: review?(romaxa)
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+
Summary: Define USE_GLES2 in global defines → Remove USE_GLES2
https://hg.mozilla.org/integration/mozilla-inbound/rev/fb6f8a081a47
Assignee: romaxa → bjacob
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla14
https://hg.mozilla.org/mozilla-central/rev/fb6f8a081a47
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: