Closed Bug 1279617 Opened 4 years ago Closed 4 years ago

When SurfacePipe::WritePixels() finishes writing early, zero out the rest of the surface

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)

A lambda passed to SurfacePipe::WritePixels() can return WriteState::FINISHED, which marks the surface as complete and will forbid further writes even if the lambda hasn't yet written to every pixel. This is a feature that's only useful in exception circumstances, such as when image decoder input is truncated.

In this scenario it'd be better to zero out the rest of the surface. There are two reasons why:

(1) Doing this allows us to be sure that SurfaceFilters that buffer a number of rows have generated all of the output that they can. If we don't do this, we could end up in a situation where e.g. we output fewer rows of a truncated image when it's downscaled than when it's decoded at its intrinsic size.

(2) Even if we finish early, we should still generate an invalidation for the entire surface, and writing empty rows ensures that we'll do this.
Here's the patch. The additional testing tests more than just this change; I
wanted to make sure we had better coverage for exiting early from WritePixels()
in general.
Attachment #8762185 - Flags: review?(n.nethercote)
Blocks: 1279117
Comment on attachment 8762185 [details] [diff] [review]
When SurfacePipe::WritePixels() finishes early, zero out the rest of the surface.

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

Looks good.

Would this have prevented bug 1217465?
Attachment #8762185 - Flags: review?(n.nethercote) → review+
Thanks for the review!

(In reply to Nicholas Nethercote [:njn] from comment #2)
> Looks good.
> 
> Would this have prevented bug 1217465?

Not by itself, no, because that's a case where the writes to the final pixels/rows simply never come. SurfacePipe doesn't have a mechanism for detecting this right now. It *will* be able to detect it once it owns the surface it's writing to and the caller has to explicitly take possession of it; the plan is that when the caller takes the surface, we'll check if it's finished and run this exact code if it isn't.
https://hg.mozilla.org/integration/mozilla-inbound/rev/35530c11c1631b62e2e02c59e071f49d877b1184
Bug 1279617 - When SurfacePipe::WritePixels() finishes early, zero out the rest of the surface. r=njn
https://hg.mozilla.org/mozilla-central/rev/35530c11c163
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.