Closed Bug 1251806 Opened 4 years ago Closed 4 years ago
Fix the check in Raster
Image::Get Frame Internal() that determines if the frame covers the entire surface
In RasterImage::GetFrameInternal() we're checking whether the frame covers the entire surface, but the check is wrong; we check whether the frame has the size that was passed in to GetFrameInternal() - |aSize| - but if substitution occurred when doing the SurfaceCache lookup, the size might be different than |aSize| and yet the frame could still cover the entire surface. This bug causes us to call CopyFrame() and uselessly copy surfaces that don't need to be copied; when we hit it, it has a significant performance cost.
Here's the patch. It's straightforward; if the frame doesn't cover the actual surface, it'd have either an offset (which is unchanged from before this patch) or its actual size would not match its logical size - i.e., imgFrame::GetSize() and imgFrame::GetImageSize() would return different values. The old code compared imgFrame::GetSize() and |aSize|, which is wrong if substitution occurred.
Attachment #8724359 - Flags: review?(tnikkel)
Attachment #8724359 - Flags: review?(tnikkel) → review+
Are all GetFrame and GetFrameAtSize callers coded to deal with getting a surface of any possible size? Maybe we should audit.
(In reply to Timothy Nikkel (:tnikkel) from comment #2) > Are all GetFrame and GetFrameAtSize callers coded to deal with getting a > surface of any possible size? Maybe we should audit. We probably should. A lot of them pass FLAG_SYNC_DECODE, in which case they won't hit this issue. But for ones that don't, they need to not assume that the size of the surface they receive matches the size of the imgIContainer. Ironically enough this is probably actually simpler in most cases, but the code does need to be updated.
https://hg.mozilla.org/integration/mozilla-inbound/rev/8c056908523ec29f08db9d304bff71f5485a14f2 Bug 1251806 - In RasterImage::GetFrameInternal(), check if the frame covers the actual surface size rather than the requested surface size. r=tn
You need to log in before you can comment on or make changes to this bug.