983591 - Add a Moz2d helper to copy pixels from a B8G8R8X8 surface to a packed B8G8R8 array

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Jonathan Watt [:jwatt]]

For bug 950372 I need a Moz2d helper to copy pixels from a B8G8R8A8 surface to a packed B8G8R8 array.

Comment 1

[Jonathan Watt [:jwatt]]

Attachment: patch

Comment 2

[Bas Schouten (:bas.schouten)]

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

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

::: gfx/2d/DataSurfaceHelpers.cpp
@@ +117,5 @@
> +  }
> +
> +  IntSize size = aSurface->GetSize();
> +
> +  uint8_t* imageBuffer = new (std::nothrow) uint8_t[size.width * size.height * 3 * sizeof(uint8_t)];

Hrm, I'm personally not a big fan of just returning pointers with ownership to the outside world like this. Perhaps we should support a B8G8R8 packed DataSourceSurface type?

Comment 3

[Jonathan Watt [:jwatt]]

(In reply to Bas Schouten (:bas.schouten) from comment #2)
> Hrm, I'm personally not a big fan of just returning pointers with ownership
> to the outside world like this. Perhaps we should support a B8G8R8 packed
> DataSourceSurface type?

That wouldn't actually meet my usecases, where the calling code actually wants to have ownership of the buffer, not just a pointer into packed data that it does not own. It would be wasteful to create a B8G8R8 packed DataSourceSurface and pack the pixels into its buffer, and then have to allocate another packed buffer and copy the data.

Comment 4

[Jonathan Watt [:jwatt]]

And note SurfaceToPackedBGRA gives precedent to doing this.

Comment 5

[Jonathan Watt [:jwatt]]

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

Comment 6

[Ryan VanderMeulen [:RyanVM]]

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

Comment 7

[Jeff Muizelaar [:jrmuizel]]

for (int col = 0; col < aSrcSize.height; ++col) {

should be

for (int col = 0; col < aSrcSize.width; ++col) {

Comment 8

[Jonathan Watt [:jwatt]]

Oops! Do you want me to put up a patch to fix that, or have you fixed it elsewhere and you were you just noting that here for completeness?

Comment 9

[Jeff Muizelaar [:jrmuizel]]

I haven't fixed it yet.