Closed Bug 1279117 Opened 4 years ago Closed 4 years ago

Add SurfacePipe::WritePixelsToRow() to simplify row-at-a-time callers of SurfacePipe::WritePixels()

Categories

(Core :: ImageLib, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla50
Tracking Status
firefox50 --- fixed

People

(Reporter: seth, Assigned: seth)

References

Details

Attachments

(1 file)

Some image formats lend themselves to be into the output surface one row at a time. Although WritePixels() guarantees that we'll never write off the end of the output surface, callers still have to ensure that they don't write past the end of the row they're currently processing, or they can end up reading off the end of the buffer they're reading from. In all cases these checks boil down to ensuring that the number of pixels we've written is equal to the image's width.

We can simplify many WritePixels() callsites by simply adding a variant that returns after writing one row, which eliminates this redundant bounds checking on the caller side.
Depends on: 1279611, 1279617
Here's the patch. Both WritePixels() and WritePixelsToRow() call a helper method
to do the actual work, so the additional code required is quite small. (Though
of course there is a lot of test code, as usual.)
Attachment #8762194 - Flags: review?(n.nethercote)
Comment on attachment 8762194 [details] [diff] [review]
Add SurfacePipe::WritePixelsToRow().

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

This seems ok, but...

> We can simplify many WritePixels() callsites by simply adding a variant that
> returns after writing one row, which eliminates this redundant bounds
> checking on the caller side.

... you haven't changed any such callsites in the patch so it's hard for me to see this in action, which is a shame. Do you have any examples forthcoming in other bugs?
Attachment #8762194 - Flags: review?(n.nethercote) → feedback+
(In reply to Nicholas Nethercote [:njn] from comment #2)
> ... you haven't changed any such callsites in the patch so it's hard for me
> to see this in action, which is a shame. Do you have any examples
> forthcoming in other bugs?

Bug 1255107 and bug 1255105 will both make extensive use of WritePixelsToRow(); I'll get the patches uploaded today if possible.
Attachment #8762194 - Flags: feedback+ → review+
Pushed by mfowler@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e3a1a366bfa6
Add SurfacePipe::WritePixelsToRow(). r=njn
https://hg.mozilla.org/mozilla-central/rev/e3a1a366bfa6
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
You need to log in before you can comment on or make changes to this bug.