980547 - Add a Moz2D helper to copy the pixels from a DataSourceSurface's data to a packed array

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Jonathan Watt [:jwatt]]

I'd like to add a Moz2D helper to copy the pixel data from a DataSourceSurface's data to a packed array. SurfaceToPackedBGRA does something similar, but it allocates the array that will contain the packed data. I want to avoid this allocation (and a subsequent copy) when the array for the packed data is already available. Specifically I'm thinking about the lpBits variable in nsDragService::CreateDragImage (widget/windows/nsDragService.cpp) since that is allocated by CreateDIBSection and we want to copy to it even in the case that |imgSurface->Data()| is not packed.

Comment 1

[Jonathan Watt [:jwatt]]

Attachment: patch

Comment 2

[Jonathan Watt [:jwatt]]

Attachment: patch

Also convert to use Map instead of calling GetData/GetStride.

Comment 3

[Jonathan Watt [:jwatt]]

Err, than should be |delete []|, obviously. Fixed locally.

Comment 4

[Bas Schouten (:bas.schouten)]

Comment on attachment 8387124 [details] [diff] [review]
patch

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

::: gfx/2d/DataSurfaceHelpers.h
@@ +38,5 @@
> +                             int32_t aStride, int32_t aBytesPerPixel)
> +{
> +  int bytesPerRow = aSize.width * aBytesPerPixel;
> +
> +  if (aStride == bytesPerRow) {

Personally I'd just always do the row by row approach, the advantage of a single memcpy is going to be minimal. I don't feel strongly though.

@@ +78,3 @@
>  
> +  CopySurfaceDataToPackedArray(map.mData, imageBuffer, size,
> +                               map.mStride, 4 * sizeof(uint8_t));

nit: If you're going to do sizeof(uint8_t) as far as I'm concerned you might as well just do 4 :)

Comment 5

[Jonathan Watt [:jwatt]]

This was committed in https://hg.mozilla.org/integration/mozilla-inbound/rev/ee113a965f7a with the wrong commit message (the commit message for bug 980885).

Comment 6

[Jonathan Watt [:jwatt]]

And it stuck.