Closed Bug 1055265 Opened 7 years ago Closed 4 years ago

Flash fails because SkBitmapDevice doesn't want to work with kRGBA_8888_SkColorType


(Firefox for Android Graveyard :: Plugins, defect)

Not set


(Not tracked)



(Reporter: snorp, Unassigned)



(1 file, 3 obsolete files)

I am not really sure what happened here, but it looks like we are not able to create a SkBitmap-backed canvas with RGBA_8888 anymore. Flash uses this as part of the ANP canvas API.
This patch fixes it but I don't really understand why. kN32_SkColorType appears to have different values in SkANP.cpp vs SkBitmapDevice.cpp so this change makes it pass the assert in SkBitmapDevice. I really expected to the red and green channels swapped on the screen, but that doesn't appear to be happening?
Attachment #8474787 - Flags: review?(gwright)
Comment on attachment 8474787 [details] [diff] [review]
Fix pixel format issue with Flash

Review of attachment 8474787 [details] [diff] [review]:

Well this is odd. Do we know what's going on here?
OK, so it looks like basically Skia sets kN32_ColorType to either BGRA or RGBA depending on what endianness it was built with. I think the most correct thing we can do here is to set colorType to kN32_ColorType and let Skia just choose the right one for us.
Attached patch colours.patch (obsolete) — Splinter Review
Let skia choose the correct byte order for us based on what endianness it was built with. This ensures that we always pass the assertion no matter what architecture we're on.

As for the colours, I suspect that the ANP enum type is just "this is a 32-bit pixel format in whatever order makes sense on the current architecture", and that's why we're not seeing R and B swapped?
Attachment #8480208 - Flags: review?(snorp)
Attached patch colours.patch (obsolete) — Splinter Review
Ensure the skia byteorder is consistent amongst different areas of the codebase as well
Attachment #8480212 - Flags: review?(snorp)
Attachment #8474787 - Attachment is obsolete: true
Attachment #8474787 - Flags: review?(gwright)
Attachment #8480208 - Flags: review?(snorp) → review+
Comment on attachment 8480212 [details] [diff] [review]

Review of attachment 8480212 [details] [diff] [review]:

Ah, that explains why we had the discrepancy. Thanks.
Attachment #8480212 - Flags: review?(snorp) → review+
Final version of the patch. It stops the crash, but nothing's rendering?
Attachment #8480208 - Attachment is obsolete: true
Attachment #8480212 - Attachment is obsolete: true
Attachment #8480718 - Flags: review?(snorp)
Comment on attachment 8480718 [details] [diff] [review]

Review of attachment 8480718 [details] [diff] [review]:

works here, so I'm happy
Attachment #8480718 - Flags: review?(snorp) → review+
Flash is going away (bug 1381916), so there's no point in pursuing this further.
Closed: 4 years ago
Resolution: --- → WONTFIX
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.