Whitelist only known working devices for direct texture support

RESOLVED FIXED in Firefox 11

Status

()

Firefox for Android
General
P1
normal
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: snorp, Assigned: snorp)

Tracking

unspecified
Firefox 12
ARM
Android
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox11 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

The direct texture (gralloc) support is somewhat fragile, and has problems on a few devices. Initially, nsWindow::HasDirectTexture() was designed to filter out the problem devices, but it doesn't appear to be enough. Whitelisting known-good devices will allow us to use the feature without sacrificing stability.
Created attachment 589646 [details] [diff] [review]
Only use direct texturing on whitelisted devices
Attachment #589646 - Flags: review?(blassey.bugs)
The attached patch whitelists the Droid Pro and Galaxy Nexus, which are the devices I have in my possession that work without issue.

There are also prefs to force the device to be either whitelisted or blacklisted. Those would be direct-texture.force.enabled and direct-texture.force.disabled, respectively.
Comment on attachment 589646 [details] [diff] [review]
Only use direct texturing on whitelisted devices

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

::: widget/android/AndroidGraphicBuffer.cpp
@@ +116,5 @@
>      int32_t left;
>      int32_t top;
>      int32_t right;
>      int32_t bottom;
> +} ARect;

any reason to change this other than "just because"?
Attachment #589646 - Flags: review?(blassey.bugs) → review+
(In reply to Brad Lassey [:blassey] from comment #3)
> Comment on attachment 589646 [details] [diff] [review]
> Only use direct texturing on whitelisted devices
> 
> Review of attachment 589646 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: widget/android/AndroidGraphicBuffer.cpp
> @@ +116,5 @@
> >      int32_t left;
> >      int32_t top;
> >      int32_t right;
> >      int32_t bottom;
> > +} ARect;
> 
> any reason to change this other than "just because"?

There is a conflicting definition in AndroidJavaWrappers.h

This bug has one problem that I'm trying to work out. Rendering is messed up (wrong tile size?) until you rotate or otherwise resize the screen.
Created attachment 589721 [details] [diff] [review]
Only use direct texturing on whitelisted devices

Fixed the rendering issue, needed to resend SIZE_CHANGED after changing tile system
Attachment #589721 - Flags: review?(blassey.bugs)
Attachment #589646 - Attachment is obsolete: true
Attachment #589721 - Flags: review?(blassey.bugs) → review+
http://hg.mozilla.org/integration/mozilla-inbound/rev/aba1658ce66c
Comment on attachment 589721 [details] [diff] [review]
Only use direct texturing on whitelisted devices

[Triage Comment]
Attachment #589721 - Flags: approval-mozilla-aurora+
https://hg.mozilla.org/mozilla-central/rev/aba1658ce66c
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 12
Blocks: 714289
Blocks: 713569

Comment 9

6 years ago
Wouldn't it be easier to maintain with a downloaded black(white)list such as on desktop (see https://wiki.mozilla.org/Firefox/Projects/VideoDriverBlacklisting)?
It has come up often in discussion of this bug. As I understand from the outcome of one of the meetings, Snorp is checking with the GFX team to see how feasible implementing that blacklist and when make a good time frame for inclusion.

Updated

6 years ago
Depends on: 720361
Snorp, please add this and whatever it depends on in the aurora landing queue
status-firefox11: --- → affected
Can we push this to 11 as well?

Updated

6 years ago
Depends on: 721383
Comment on attachment 589721 [details] [diff] [review]
Only use direct texturing on whitelisted devices

[Triage Comment]
also needed on beta now
Attachment #589721 - Flags: approval-mozilla-beta+
https://hg.mozilla.org/releases/mozilla-beta/rev/5f2ccd447bba
status-firefox11: affected → fixed
I just confirmed that this tinderbox build:
http://ftp.mozilla.org/pub/mozilla.org/mobile/tinderbox-builds/mozilla-beta-android/1328588649/fennec-11.0.en-US.android-arm.apk
now suffers from bug 721383.

Can this be backed out of Firefox 11, please?
(In reply to Martijn Wargers [:mw22] (QA - IRC nick: mw22) from comment #15)
> I just confirmed that this tinderbox build:
> http://ftp.mozilla.org/pub/mozilla.org/mobile/tinderbox-builds/mozilla-beta-
> android/1328588649/fennec-11.0.en-US.android-arm.apk
> now suffers from bug 721383.
> 
> Can this be backed out of Firefox 11, please?

what devices are you seeing the hang? Do we also hang on those devices on Aurora and Nightly?
See also comments in bug 721383, I did bisecting there. I can see this hang/crash on the LG Optimus Black, I haven't see it on other devices. Yes, I'm also seeing it on Aurora and Nightly.
The actual whitelist change is easy enough to back out, but the initialization changes are going to be relatively painful. I would prefer to just figure out what is going on there, instead.

Is the Black the only phone that has this issue?
You need to log in before you can comment on or make changes to this bug.