Closed Bug 894087 Opened 6 years ago Closed 6 years ago

getScreenDepth() doesn't handle all 24-bit pixel formats

Categories

(Firefox for Android :: General, defect)

All
Android
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 25

People

(Reporter: bnicholson, Assigned: bnicholson)

References

Details

Attachments

(1 file)

adb shell cat /proc/meminfo
MemTotal:         860816 kB

Running on Android 4.1.2, if that matters. Verified that 16-bit color is being used by logging sScreenDepth in GeckoAppShell.
I found this after seeing banding on http://www.lagom.nl/lcd-test/gradient.php.

I opened the same page in Chrome and the gradient looks normal, so I assume the device does actually support 24-bit color.
It's not even hitting that code path. getGeckoInterface() == null when getScreenDepth() is first called, so looks like we have initialization race setting GeckoInterface, and the depth gets stuck at 16.
Summary: 24-bit color not enabled on Droid RAZR → getScreenDepth() can be called before GeckoInterface is set, preventing 24-bit color
Sorry...I was a bit too hasty before, and I made a dumb mistake doing the logging. Turns out GeckoInterface is initialized just fine, and the problem is that getPixelFormat() is returning 5, a number that's not even documented on http://developer.android.com/reference/android/graphics/PixelFormat.html. I found http://stackoverflow.com/questions/11949709/confused-about-pixelformat, and it looks like 5 is mapped to BGRA_8888. Rather than checking for each format, though, I think we can simply look at PixelFormat.bitsPerPixel.

I also removed the getGeckoInterface() != null check to prevent potential problems like comment 3. I think it's better to crash than fail silently so we can any races if they exist.
Assignee: nobody → bnicholson
Status: NEW → ASSIGNED
Attachment #776824 - Flags: review?(bugmail.mozilla)
Summary: getScreenDepth() can be called before GeckoInterface is set, preventing 24-bit color → getScreenDepth() doesn't handle all 24-bit pixel formats
Comment on attachment 776824 [details] [diff] [review]
Fix 24-bit color detection

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

Looks OK to me, bouncing to Chris to make sure there isn't some reason he didn't do it this way in the first place.
Attachment #776824 - Flags: review?(bugmail.mozilla) → review?(chrislord.net)
Comment on attachment 776824 [details] [diff] [review]
Fix 24-bit color detection

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

Ignorance is why I didn't do it like this in the first place. LGTM.
Attachment #776824 - Flags: review?(chrislord.net) → review+
https://hg.mozilla.org/mozilla-central/rev/ecc75387637a
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 25
You need to log in before you can comment on or make changes to this bug.