Closed Bug 1251804 Opened 5 years ago Closed 5 years ago

Use the ImageContainer size, not the intrinsic size, when computing the transform in nsDisplayImage::ConfigureLayer

Categories

(Core :: ImageLib, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla48
Tracking Status
firefox48 --- fixed

People

(Reporter: seth, Assigned: seth)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

It's possible (for example, due to downscale-during-decode) that the ImageContainer an ImageLayer is holding has a different size from the intrinsic size of the image. For this reason we should compute the transform using the ImageContainer's size rather than the image's intrinsic size.


In reality, since the size of the ImageContainer may change asynchronously, this is not enough. Bug 1183378 will provide a more complete fix, but this solution is safe in more cases than simply relying on the intrinsic size.

I'm making this block bug 1225934 because this will help ensure that we do the right thing even if the patches in that bug have missed some obscure case.
Here's the patch. The ImageContainer knows its current size, so we just grab
that information and use it for the calculation.

Earlier in the function we also grab the imgIContainer's intrinsic size and bail
if it doesn't have one. I suspect we did that in part because we used that
information in this calculation, and now we don't, but in the interest of not
changing error handling behavior as part of this patch I haven't removed that
check.
Attachment #8724358 - Flags: review?(tnikkel)
Comment on attachment 8724358 [details] [diff] [review]
Use the ImageContainer's size and not the intrinsic size when computing the transform in nsDisplayImage::ConfigureLayer.

Do we even need to get imageWidth/Height at all in this function then?
Attachment #8724358 - Flags: review?(tnikkel) → review+
nsDisplayXULImage and nsDisplayBackgroundImage suffer from the same problem.
Thanks for the review!

(In reply to Timothy Nikkel (:tnikkel) from comment #2)
> Do we even need to get imageWidth/Height at all in this function then?

As mentioned in comment 1, we actually don't, but I didn't want to change the error handling behavior as part of this bug. I'll file a followup.

(In reply to Timothy Nikkel (:tnikkel) from comment #3)
> nsDisplayXULImage and nsDisplayBackgroundImage suffer from the same problem.

Thanks. If they look identical I'll go ahead and update the patch in this bug to fix them; if there are any meaningful difficulties we'll take care of it in a followup bug.
OK, the changes needed for nsDisplayXULImage and nsDisplayBackgroundImage are totally identical, so I'll go ahead and update them here.
Blocks: 1254360
Here's the updated version that covers the other image-related ConfigureLayer() ethods.
Attachment #8724358 - Attachment is obsolete: true
https://hg.mozilla.org/integration/mozilla-inbound/rev/b485a37494452b6cedacd95f0aa673d48ef449b5
Bug 1251804 - Use the ImageContainer's size and not the intrinsic size when computing the transform in nsDisplayImage::ConfigureLayer. r=tn
https://hg.mozilla.org/mozilla-central/rev/b485a3749445
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Depends on: 1255362
Depends on: 1256211
You need to log in before you can comment on or make changes to this bug.