824642 - PlanarYCbCrImage::GetAsSurface does not create surface optimized for platform

Status: RESOLVED FIXED

(Core :: Graphics: Layers, defect)

Description

[Oleg Romashin (:romaxa)]

While testing OMTC embedding on Arm device platform I noticed that we do not use arm neon yuv conversion.
we do create surface in PlanarYCbCrImage::GetAsSurface, and mOffscreenFormat always gfxASurface::ImageFormatUnknown.
I guess it would make sense use platform::OffscreenFormat in case if mOffscreenFormat is not set.

Comment 1

[Oleg Romashin (:romaxa)]

Attachment: Use platform offscreen format when default not set

Comment 2

[Joe Drew (not getting mail)]

Comment on attachment 695664 [details] [diff] [review]
Use platform offscreen format when default not set

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

I would prefer if GetOffscreenFormat() had that logic in it, actually.

While I'm supportive of this in general, I'm not sure if it's a bug that SetOffscreenFormat() hasn't been called, so I'll leave it up to nical for the final review.

Comment 3

[Nicolas Silva [:nical]]

Comment on attachment 695664 [details] [diff] [review]
Use platform offscreen format when default not set

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

I agree with Joe, GetOffscreenFormat should have this logic. Actually I wonder why we store mOffscreenFormat as member of PlanarYCbCrImage rather than making GetOffscreenFormat virtual and returning the platform's offscreen format by default, no one seems to set the offscreen format except sub-classes. I'd be happy with either of these two solutions.

Comment 4

[Oleg Romashin (:romaxa)]

Attachment: Use platform offscreen format when default not set

Comment 5

[Nicolas Silva [:nical]]

Comment on attachment 703782 [details] [diff] [review]
Use platform offscreen format when default not set

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

Thanks

Comment 6

[Oleg Romashin (:romaxa)]

https://tbpl.mozilla.org/?tree=Try&rev=da28d6bafd71 - use this patch https://hg.mozilla.org/try/raw-rev/dc583e035959

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/cff1c1aa5261

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/cff1c1aa5261