1282536 - memset macIOSurface to 0xFF if the surface has no alpha on OS X

Status: NEW

(Core :: Graphics, defect, P3)

Description

[Mason Chang [Inactive] [:mchang]]

Because Skia expects the alpha to be 0xFF on RGBX Surfaces since it doesn't support it, clear macIOSurfaces that have no alpha to 0xFF.

Comment 1

[Mason Chang [Inactive] [:mchang]]

Attachment: memset.patch

It looks like when we create a MacIOSurface, we already know if we have alpha or not and keep it that way. If we don't have alpha, memset the IOSurface buffer to 0xFF.

Comment 2

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 8765574 [details] [diff] [review]
memset.patch

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

I'm going to defer to Matt, since I'm not comfortable enough with this code to r+ it myself.

One comment is that you might be able to rid of the "true" argument at https://hg.mozilla.org/integration/mozilla-inbound/rev/7a6d37ee074f#l6.123 with this change.

Comment 3

[Matt Woodrow (:mattwoodrow)]

Comment on attachment 8765574 [details] [diff] [review]
memset.patch

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

::: gfx/2d/MacIOSurface.cpp
@@ +341,5 @@
>  
> +  if (!aHasAlpha) {
> +    // Skia expects RGBX surfaces to have 0xFF as the alpha value, so
> +    // memset that here.
> +    void* rawData = ioSurface->GetBaseAddress();

Don't we need the surface to be locked for writing in order to call this? The memory might be on the GPU.

I also don't like baking skia specific assumptions into this code, can we instead make the caller (or preferably DrawTargetSkia) do this (ideally after we've locked the surface)