Closed Bug 1359155 Opened 3 years ago Closed 3 years ago

Get rid of gfxASurface::FormatStrideForWidth

Categories

(Core :: Graphics, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: jwatt, Assigned: jwatt)

Details

Attachments

(2 files, 1 obsolete file)

We can now reimplement the functionality of gfxASurface::FormatStrideForWidth that we care about in a Moz2D helper and remove our dependence on cairo there. None of the current consumers depend on particulars of the cairo functionality (support for A1 surfaces).
Assignee: nobody → jwatt
Attachment #8861128 - Flags: review?(mstange)
Attachment #8861128 - Attachment is obsolete: true
Attachment #8861128 - Flags: review?(mstange)
Attachment #8861129 - Flags: review?(mstange)
Attachment #8861129 - Flags: review?(mstange) → review+
Attachment #8861130 - Flags: review?(mstange) → review+
Pushed by jwatt@jwatt.org:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b695217bbfa7
part 1 - Implement a StrideForFormatAndWidth Moz2D helper. r=mstange
https://hg.mozilla.org/integration/mozilla-inbound/rev/2e2e2ba5ecb2
part 2 - Convert gfxASurface::FormatStrideForWidth callers to Moz2D's StrideForFormatAndWidth. r=mstange
Comment on attachment 8861129 [details] [diff] [review]
part 1 - implement some Moz2D functionality

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

::: gfx/2d/DataSurfaceHelpers.cpp
@@ +31,5 @@
> +  if (aWidth >= (INT32_MAX - alignment) / bpp) {
> +    return -1; // too big
> +  }
> +
> +  return (bpp * aWidth + alignment-1) & ~alignment;

Of course this should be:

  return (bpp * aWidth + alignment-1) & ~(alignment-1);

Getting that wrong could cause us to return a stride that is smaller than necessary. However it seems unlikely to me that this would cause the types of failures there showed up on my landing.
I really don't see anything else wrong. Given that this doesn't [reliably?] repro on Try I'll land it in more incremental pieces to minimize backout churn if the failures show up again.
Pushed by jwatt@jwatt.org:
https://hg.mozilla.org/integration/mozilla-inbound/rev/89815b99de27
part 1 - Implement a StrideForFormatAndWidth Moz2D helper. r=mstange
Pushed by jwatt@jwatt.org:
https://hg.mozilla.org/integration/mozilla-inbound/rev/9b1cf8c44079
part 2 - Convert MediaEngineTabVideoSource::Draw to use Moz2D's StrideForFormatAndWidth. r=mstange
Pushed by jwatt@jwatt.org:
https://hg.mozilla.org/integration/mozilla-inbound/rev/769a435f3525
part 3 - Convert BasicPlanarYCbCrImage::CopyData to use Moz2D's StrideForFormatAndWidth. r=mstange
https://hg.mozilla.org/integration/mozilla-inbound/rev/0498fdd7a14a
part 4 - Remove gfxASurface::FormatStrideForWidth. r=mstange
You need to log in before you can comment on or make changes to this bug.