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

RESOLVED FIXED in mozilla31

Status

()

defect
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: jwatt, Assigned: jwatt)

Tracking

31 Branch
mozilla31
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

5 years ago
For bug 950372 I need a Moz2d helper to copy pixels from a B8G8R8A8 surface to a packed B8G8R8 array.
(Assignee)

Comment 1

5 years ago
Posted patch patchSplinter Review
Attachment #8391135 - Flags: review?(bas)
(Assignee)

Updated

5 years ago
Summary: Add a Moz2d helper to copy pixels from a B8G8R8A8 surface to a packed B8G8R8 array → Add a Moz2d helper to copy pixels from a B8G8R8X8 surface to a packed B8G8R8 array
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?
(Assignee)

Comment 3

5 years ago
(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.
(Assignee)

Comment 4

5 years ago
And note SurfaceToPackedBGRA gives precedent to doing this.
Attachment #8391135 - Flags: review?(bas) → review+
https://hg.mozilla.org/mozilla-central/rev/ea0518320f9f
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
for (int col = 0; col < aSrcSize.height; ++col) {

should be

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

Comment 8

4 years ago
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?
I haven't fixed it yet.
(Assignee)

Updated

4 years ago
Depends on: 1172964
You need to log in before you can comment on or make changes to this bug.