Closed Bug 1128467 Opened 9 years ago Closed 9 years ago

CSS transform with rotate and scale corrupts transformed image

Categories

(Core :: Graphics: Layers, defect)

35 Branch
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla39
Tracking Status
firefox36 --- affected
firefox37 --- affected
firefox38 --- affected
firefox39 --- fixed
firefox-esr31 --- unaffected
firefox-esr38 --- ?

People

(Reporter: atirip, Assigned: seth)

References

Details

(Keywords: regression)

Attachments

(2 files)

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_10_2) AppleWebKit/600.3.18 (KHTML, like Gecko) Version/8.0.3 Safari/600.3.18

Steps to reproduce:

I applied scale, transform and rotate to image.
demo page: http://devteam.ixara.com/public/bugs/mozrot.html

Upper image is rotated, scaled, transformed, lower image is only scaled and transformed.





Actual results:

Image contents seems to be crop of very small part of that image, scaled to fill. Matrix transform is applied correctly. If i modify transform matrix by zeroing b & c components, then image is fine.


Expected results:

Image contents should not change.
I ran into this as well
This is interesting - at a guess, I'd say that the image is being sampled from its pre-transformed state, where it's 1x1 pixel. Definitely not expected behaviour in this case, though I wonder if fixing it involves altering/breaking other transform optimisations...

(I may be totally wrong as to the cause, complete conjecture here)
Status: UNCONFIRMED → NEW
Component: Untriaged → Graphics: Layers
Ever confirmed: true
OS: Mac OS X → All
Product: Firefox → Core
Hardware: x86 → All
Version: 35 Branch → Trunk
(playing with the test, it looks like my assumption is correct)

Matt, I remember you talking some time about how we choose to render layers with transforms (i.e. either pre or post transform, I guess?) Any ideas on whether this is fixable?

I'd have expected the element to have been rendered post-transform, as the transform is 2d and not animated? And don't we factor out scale in some cases to avoid this sort of problem specifically?
Flags: needinfo?(matt.woodrow)
Apparently this worked in FF33
So in the test case we have this structure:

<div with transform>
  <div>
    <img width=1 height=1 with transform>

The outer div's transform is irrelevant. This is really about our behavior when the computed size of an <img> element is very small (1x1 here) and we then scale it back up to a larger size using a transform.

Notwithstanding the regression range, the problem is this check:

https://dxr.mozilla.org/mozilla-central/source/layout/base/nsLayoutUtils.cpp?from=ComputeSnappedImageDrawingParameters&case=true#5787

Because of that code, we ignore the context's matrix when computing the drawing parameters that we're going to use to draw the image. That makes us decide that we should draw the image at a target size of 1x1, which unsurprisingly doesn't look great when scaled back up to ~101x49.

The straightforward fix here is to consider the context's matrix when computing the drawing parameters, even if we decide not to snap. In practice that'd mean taking the matrix into consideration when passing a size into OptimalImageSizeForDest.
Actually, there is another straightforward solution - just don't draw with high quality scaling when there's a rotation on the context matrix, since if we don't do any scaling, it doesn't matter what the target size is. I don't prefer that, though, because that'd also prevent downscale-during-decode from working and could thus have significant memory usage consequences.
The solution in comment 6 seems to work just fine. It completely fixes the
problem for me.
Attachment #8580952 - Flags: review?(roc)
Assignee: nobody → seth
Status: NEW → ASSIGNED
Try job here:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=c94968ea2fea

Clearing the needinfo request for Matt since I think this is figured out at this point.
Flags: needinfo?(matt.woodrow)
(BTW, I guess I could call ScaleFactors() all the time, but just grabbing the matrix components directly should be a little faster in the common case.)
Thanks for the review! Pushed to inbound since try looks green:

https://hg.mozilla.org/integration/mozilla-inbound/rev/f778ed142145
https://hg.mozilla.org/mozilla-central/rev/f778ed142145
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
Thank you!
(In reply to Priit Pirita from comment #13)
> Thank you!

I'm glad to help! Sorry it took a while to get fixed!
Depends on: 1157542
You need to log in before you can comment on or make changes to this bug.