Closed Bug 719233 Opened 8 years ago Closed 8 years ago

Whitelist only known working devices for direct texture support

Categories

(Firefox for Android :: General, defect, P1)

ARM
Android
defect

Tracking

()

RESOLVED FIXED
Firefox 12
Tracking Status
firefox11 --- fixed

People

(Reporter: snorp, Assigned: snorp)

References

Details

Attachments

(1 file, 1 obsolete file)

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.
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.
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+
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
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 12
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.
Depends on: 720361
Snorp, please add this and whatever it depends on in the aurora landing queue
Can we push this to 11 as well?
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+
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.