CSS transform with rotate and scale corrupts transformed image

RESOLVED FIXED in Firefox 39

Status

()

defect
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: atirip, Assigned: seth)

Tracking

({regression})

35 Branch
mozilla39
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox36 affected, firefox37 affected, firefox38 affected, firefox39 fixed, firefox-esr31 unaffected, firefox-esr38 ?)

Details

Attachments

(2 attachments)

(Reporter)

Description

4 years ago
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)
(Assignee)

Comment 6

4 years ago
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.
(Assignee)

Comment 7

4 years ago
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.
(Assignee)

Comment 8

4 years ago
The solution in comment 6 seems to work just fine. It completely fixes the
problem for me.
Attachment #8580952 - Flags: review?(roc)
(Assignee)

Updated

4 years ago
Assignee: nobody → seth
Status: NEW → ASSIGNED
(Assignee)

Comment 9

4 years ago
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)
(Assignee)

Comment 10

4 years ago
(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.)
(Assignee)

Comment 11

4 years ago
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
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
(Reporter)

Comment 13

4 years ago
Thank you!
(Assignee)

Comment 14

4 years ago
(In reply to Priit Pirita from comment #13)
> Thank you!

I'm glad to help! Sorry it took a while to get fixed!

Updated

4 years ago
Depends on: 1157542
You need to log in before you can comment on or make changes to this bug.