Closed Bug 942499 Opened 6 years ago Closed 6 years ago

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

Categories

(Core :: Graphics, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla28

People

(Reporter: bjacob, Assigned: bjacob)

References

Details

(Whiteboard: [qa-])

Attachments

(3 files)

No description provided.
Doing this one next.
Assignee: nobody → bjacob
Attachment #8340782 - Flags: review?(bgirard) → review+
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.
Attachment #8340783 - Flags: review?(bgirard)
Also make it use nsCocoaFeatures::OnLionOrLater instead of custom calls to deprecated Gestalt function.
Attachment #8340784 - Flags: review?(jgilbert)
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.
Attachment #8340783 - Flags: review?(bgirard) → review+
Attachment #8340784 - Flags: review?(jgilbert) → review+
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.
Attachment #8340783 - Flags: feedback?(bgirard)
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.
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
When do we not want WorkAroundDriverBugs() to be true?
(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 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.
Attachment #8340783 - Flags: feedback?(bgirard) → feedback+
(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.
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
Whiteboard: [qa-]
You need to log in before you can comment on or make changes to this bug.