942499 - Remove from GLContext capability-check helper functions such as CanUploadSubTextures, CanReadSRGBFromFBOTexture, CanUploadNonPowerOfTwo, WantsSmallTiles, etc.

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Comment 1

[Benoit Jacob [:bjacob] (mostly away)]

Doing this one next.

Comment 2

[Benoit Jacob [:bjacob] (mostly away)]

Attachment: Move CanUploadSubTextures and WantsSmallTiles out of GLContext

Comment 3

[Benoit Jacob [:bjacob] (mostly away)]

Attachment: Move CanUploadNonPowerOfTwo out of GLContext; remove the gfx.textures.poweroftwo.force-enabled preference

What do you think? The preference looked like something that had been useful at a precise point in the past, but doesn't really justify its complexity anymore (nontrivial interplay with gfxplatform and global variables to set up the bool var cache on the main thread on startup).

If we need to test, we can still do so by changing 1 line of code and recompile; and there has been no other instance of a GPU not supporting NPOT upload since those old versions of the Adreno driver that we had run into 1.5 years ago.

Comment 4

[Benoit Jacob [:bjacob] (mostly away)]

Attachment: Move CanReadSRGBFromFBOTexture out of GLContext

Also make it use nsCocoaFeatures::OnLionOrLater instead of custom calls to deprecated Gestalt function.

Comment 5

[Benoit Jacob [:bjacob] (mostly away)]

https://tbpl.mozilla.org/?tree=Try&rev=81c4c0638a07

Comment 6

[Andreas Gal :gal]

Comment on attachment 8340783 [details] [diff] [review]
Move CanUploadNonPowerOfTwo out of GLContext; remove the gfx.textures.poweroftwo.force-enabled preference

Thanks for the weekend cleanup duty.

Comment 7

[Andreas Gal :gal]

Comment on attachment 8340783 [details] [diff] [review]
Move CanUploadNonPowerOfTwo out of GLContext; remove the gfx.textures.poweroftwo.force-enabled preference

Setting needinfo? for BenWa for the pref. If he minds we can always put it back in. I doubt he will.

Comment 8

[Benoit Jacob [:bjacob] (mostly away)]

Thanks. I have a try push running at the moment, I was hoping to land a bunch of things together tomorrow.

https://tbpl.mozilla.org/?tree=Try&rev=e1f5ae3e4519

hm, mochitest orange on Mac caused by my srgb patch here.

Comment 9

[Benoit Jacob [:bjacob] (mostly away)]

hah, accidentally removed a logical negation around WorkAroundDriverBugs:

+static bool
+CanReadSRGBFromFBOTexture(GLContext* gl)
+{
+    if (gl->WorkAroundDriverBugs())
+        return true;

https://tbpl.mozilla.org/?tree=Try&rev=5f4c19777851

Comment 10

[Andreas Gal :gal]

When do we not want WorkAroundDriverBugs() to be true?

Comment 11

[Benoit Girard (:BenWa)]

(In reply to Andreas Gal :gal from comment #10)
> When do we not want WorkAroundDriverBugs() to be true?

WorkAroundDriverBugs AFAIK is primarily for working around drivers in Android 2.x (maybe 3.x too) where GL drivers were made available mostly just for testing with very poor quality. I suspect for android 4.x and b2g we have good quality driver that don't have the problems we work around. We should plan to clean this up in the future. Some work-around may be causing slight performance regressions.

Not a huge priority but it would be nice to do away with these.

Comment 12

[Benoit Girard (:BenWa)]

Comment on attachment 8340783 [details] [diff] [review]
Move CanUploadNonPowerOfTwo out of GLContext; remove the gfx.textures.poweroftwo.force-enabled preference

Review of attachment 8340783 [details] [diff] [review]:
-----------------------------------------------------------------

Remove pref is fine by made. We're past the point where it's useful.

Comment 13

[Benoit Jacob [:bjacob] (mostly away)]

(In reply to Andreas Gal :gal from comment #10)
> When do we not want WorkAroundDriverBugs() to be true?

WorkAroundDriverBugs() just reflects the gfx.work-around-driver-bugs pref.

It's always true in default config, but for debugging purposes and to test whether a workaround is still needed, it's useful to be able to turn it off. Having work-arounds enclosed in if (WorkAroundDriverBugs()) { ... } also gives us nice code self-documentation.

Comment 14

[Benoit Jacob [:bjacob] (mostly away)]

Another side benefit is that just looking for occurences of WorkAroundDriverBugs in the codebase you can find most of our driver workarounds.

http://dxr.mozilla.org/mozilla-central/search?q=WorkAroundDriverBugs

Comment 15

[Benoit Jacob [:bjacob] (mostly away)]

https://tbpl.mozilla.org/?tree=Try&rev=eb3be360fb34

Comment 16

[Benoit Jacob [:bjacob] (mostly away)]

https://tbpl.mozilla.org/?tree=Try&rev=1b48093b8244

Comment 17

[Benoit Jacob [:bjacob] (mostly away)]

https://tbpl.mozilla.org/?tree=Try&rev=01672e3e55a0

http://hg.mozilla.org/integration/mozilla-inbound/rev/7c7c405d9373
http://hg.mozilla.org/integration/mozilla-inbound/rev/22a1c5c13716
http://hg.mozilla.org/integration/mozilla-inbound/rev/c789f25ca86e

Comment 18

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/7c7c405d9373
https://hg.mozilla.org/mozilla-central/rev/22a1c5c13716
https://hg.mozilla.org/mozilla-central/rev/c789f25ca86e