Last Comment Bug 741730 - Remove USE_GLES2
: Remove USE_GLES2
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Graphics (show other bugs)
: unspecified
: ARM Linux
: -- normal (vote)
: mozilla14
Assigned To: Benoit Jacob [:bjacob] (mostly away)
:
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-04-03 03:16 PDT by Oleg Romashin (:romaxa)
Modified: 2012-04-11 09:02 PDT (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Define use_gles2 for EGL providers globally (2.07 KB, patch)
2012-04-03 03:18 PDT, Oleg Romashin (:romaxa)
no flags Details | Diff | Splinter Review
kill USE_GLES2 (18.30 KB, patch)
2012-04-07 08:01 PDT, Benoit Jacob [:bjacob] (mostly away)
romaxa: review+
Details | Diff | Splinter Review

Description Oleg Romashin (:romaxa) 2012-04-03 03:16:45 PDT
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
Comment 1 Oleg Romashin (:romaxa) 2012-04-03 03:18:08 PDT
Created attachment 611762 [details] [diff] [review]
Define use_gles2 for EGL providers globally
Comment 2 Mike Hommey [:glandium] 2012-04-03 03:40:41 PDT
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).
Comment 3 Benoit Jacob [:bjacob] (mostly away) 2012-04-07 07:20:24 PDT
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.
Comment 4 Benoit Jacob [:bjacob] (mostly away) 2012-04-07 07:23:22 PDT
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.
Comment 5 Benoit Jacob [:bjacob] (mostly away) 2012-04-07 08:01:55 PDT
Created attachment 613109 [details] [diff] [review]
kill USE_GLES2

Here's my counter-offer :-)
Comment 6 Oleg Romashin (:romaxa) 2012-04-07 13:30:00 PDT
Comment on attachment 613109 [details] [diff] [review]
kill USE_GLES2

Yeah, I like this idea more too., tested quickly and it works also good
Comment 7 Benoit Jacob [:bjacob] (mostly away) 2012-04-10 08:53:10 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/fb6f8a081a47
Comment 8 Matt Brubeck (:mbrubeck) 2012-04-11 09:02:15 PDT
https://hg.mozilla.org/mozilla-central/rev/fb6f8a081a47

Note You need to log in before you can comment on or make changes to this bug.