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

Status: RESOLVED FIXED

(Core :: Graphics: ImageLib, defect)

Description

[Seth Fowler [:seth] [:s2h]]

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.

Comment 1

[Seth Fowler [:seth] [:s2h]]

Attachment: Use the ImageContainer's size and not the intrinsic size when computing the transform in nsDisplayImage::ConfigureLayer.

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.

Comment 2

[Timothy Nikkel (: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?

Comment 3

[Timothy Nikkel (:tnikkel)]

nsDisplayXULImage and nsDisplayBackgroundImage suffer from the same problem.

Comment 4

[Seth Fowler [:seth] [:s2h]]

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.

Comment 5

[Seth Fowler [:seth] [:s2h]]

OK, the changes needed for nsDisplayXULImage and nsDisplayBackgroundImage are totally identical, so I'll go ahead and update them here.

Comment 6

[Seth Fowler [:seth] [:s2h]]

Attachment: Use the ImageContainer's size and not the intrinsic size when computing the transform in nsDisplayImage::ConfigureLayer.

Here's the updated version that covers the other image-related ConfigureLayer() ethods.

Comment 7

[Seth Fowler [:seth] [:s2h]]

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

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/b485a3749445