CSS transform with rotate and scale corrupts transformed image

RESOLVED FIXED in Firefox 39

Status

()

Core
Graphics: Layers
RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: Priit Pirita, Assigned: seth)

Tracking

({regression})

35 Branch
mozilla39
regression
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

3 years ago
Created attachment 8557850 [details]
demo case html provided

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.

Comment 1

2 years ago
I ran into this as well

Comment 2

2 years ago
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

Comment 3

2 years ago
(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
Keywords: regressionwindow-wanted

Comment 5

2 years ago
[Tracking Requested - why for this release]:

Pushlog:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=786737c759d7&tochange=fa25a75b4ad9

Triggered by: Bug 1060200
Blocks: 1060200
status-firefox36: --- → affected
status-firefox37: --- → affected
status-firefox38: --- → affected
status-firefox39: --- → affected
status-firefox-esr31: --- → unaffected
status-firefox-esr38: --- → ?
Keywords: regressionwindow-wanted → regression
Version: Trunk → 35 Branch
(Assignee)

Comment 6

2 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

2 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

2 years ago
Created attachment 8580952 [details] [diff] [review]
Take scale factors into account when computing image size even if we aren't snapping

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

Updated

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

Comment 9

2 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

2 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.)
Attachment #8580952 - Flags: review?(roc) → review+
(Assignee)

Comment 11

2 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: 2 years ago
status-firefox39: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
(Reporter)

Comment 13

2 years ago
Thank you!
(Assignee)

Comment 14

2 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

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