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)
Tracking
()
RESOLVED
FIXED
mozilla39
People
(Reporter: atirip, Assigned: seth)
References
Details
(Keywords: regression)
Attachments
(2 files)
78.77 KB,
application/zip
|
Details | |
1.28 KB,
patch
|
roc
:
review+
|
Details | Diff | Splinter Review |
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•9 years ago
|
||
I ran into this as well
Comment 2•9 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•9 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)
Comment 5•9 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•9 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•9 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•9 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•9 years ago
|
Assignee: nobody → seth
Status: NEW → ASSIGNED
Assignee | ||
Comment 9•9 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•9 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•9 years ago
|
||
Thanks for the review! Pushed to inbound since try looks green: https://hg.mozilla.org/integration/mozilla-inbound/rev/f778ed142145
Comment 12•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/f778ed142145
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
Reporter | ||
Comment 13•9 years ago
|
||
Thank you!
Assignee | ||
Comment 14•9 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!
You need to log in
before you can comment on or make changes to this bug.
Description
•