Closed
Bug 894087
Opened 11 years ago
Closed 11 years ago
getScreenDepth() doesn't handle all 24-bit pixel formats
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
Firefox 25
People
(Reporter: bnicholson, Assigned: bnicholson)
References
Details
Attachments
(1 file)
1.34 KB,
patch
|
cwiiis
:
review+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•11 years ago
|
||
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.
Comment 2•11 years ago
|
||
Would probably be useful to know what the getPixelFormat call is returning on that device: http://hg.mozilla.org/mozilla-central/file/e5d74eebd0e2/mobile/android/base/GeckoAppShell.java#l1375
Assignee | ||
Comment 3•11 years ago
|
||
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
Assignee | ||
Comment 4•11 years ago
|
||
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)
Assignee | ||
Updated•11 years ago
|
Summary: getScreenDepth() can be called before GeckoInterface is set, preventing 24-bit color → getScreenDepth() doesn't handle all 24-bit pixel formats
Comment 5•11 years ago
|
||
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 6•11 years ago
|
||
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+
Assignee | ||
Comment 7•11 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/ecc75387637a
Comment 8•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/ecc75387637a
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 25
Updated•3 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•