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

NEW
Unassigned

Status

()

Core
Graphics
P3
normal
a year ago
2 months ago

People

(Reporter: mchang, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: gfx-noted)

Attachments

(1 attachment)

(Reporter)

Description

a year ago
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.
(Reporter)

Updated

a year ago
Whiteboard: gfx-noted
Blocks: 1261166
(Reporter)

Comment 1

a year ago
Created attachment 8765574 [details] [diff] [review]
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.
Attachment #8765574 - Flags: review?(bugmail.mozilla)
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.
Attachment #8765574 - Flags: review?(bugmail.mozilla) → review?(matt.woodrow)
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)
Attachment #8765574 - Flags: review?(matt.woodrow) → review-
Priority: -- → P3
(Reporter)

Updated

2 months ago
Assignee: mchang → nobody
You need to log in before you can comment on or make changes to this bug.