929299 - fix DrawTargetSkia to not copy and render in place

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Andreas Gal :gal]

DrawTargetSkia copies the pixel data it is given when using InitWithData and then draws into the copy, ultimately losing it when the DT is deallocated. Instead, attach the bitmap to the pixel data and draw into that. The pixel data must stick around or this fails. I think InitWithData is meant this way.

Dito for SourceSourceSkia. It currently allocates a separate bitmap and copies the data in there. Thats very inefficient and I think not designed this way in Moz2D.

Comment 1

[Andreas Gal :gal]

Attachment: Don't allocate pixel data for DrawTargetSkia and SourceSurfaceSkia when using InitWithData.

Comment 2

[George Wright (:gw280) (needinfo me!)]

Comment on attachment 820107 [details] [diff] [review]
Don't allocate pixel data for DrawTargetSkia and SourceSurfaceSkia when using InitWithData.

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

::: gfx/2d/DrawTargetSkia.cpp
@@ +755,1 @@
>    mSize = aSize;

bitmap never gets given the aData pointer here?

::: gfx/2d/SourceSurfaceSkia.cpp
@@ +77,5 @@
>      mBitmap.unlockPixels();
>      mBitmap.notifyPixelsChanged();
>      mBitmap.setIsOpaque(true);
> +  } else {
> +    mBitmap = temp;

We can't do this because temp's pixels are currently set to aData, and CreateSourceSurfaceFromData (which in turns calls InitFromData) explicitly doesn't take ownership of aData.

Comment 3

[Andreas Gal :gal]

Attachment: Make DrawTargetSkia render into the memory buffer provided. The caller is responsible to make sure the memory isn't freed until Flush() is called.

Comment 4

[Andreas Gal :gal]

On a related note, skia silently fails to render RGB in various exciting ways, only BGR is handled properly in most paths. Is that intentional?

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/4d642c4a393c

Comment 6

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/4d642c4a393c