Closed Bug 735373 Opened 13 years ago Closed 13 years ago

3D transforms of images seems to have been broken

Categories

(Core :: Layout, defect)

Product:
Core
Component:
Layout
Platform:
x86
All
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
VERIFIED FIXED
Tracking Flags:
Tracking Status
firefox14 + verified
firefox15 + fixed

People

(Reporter: ayg, Unassigned)

References

Details

(Keywords: regression)

Attachments

(1 file)

Make ChooseScaleAndSetTransform correctly detect active layers
7.69 KB, patch
Details | Diff | Splinter Review
data:text/html,<!DOCTYPE html> <img style="-moz-transform:rotatex(30deg)" src='http://www.mozilla.org/img/download-logo-sub.png'> <img style="-moz-transform:scale3d(2,2,2)" src='http://www.mozilla.org/img/download-logo-sub.png'> The first image is translated down, not squished vertically as one would expect. The second disappears instead of being scaled. Something seems to be badly wrong here.
Better test-case for the latter transform: data:text/html,<!DOCTYPE html> <img style="-moz-transform:scale3d(-2,1,2)" src='http://www.mozilla.org/img/download-logo-sub.png'> This translates the image toward the right. It seems like bitmap images affected by 3D transforms are being translated for some reason, instead of being transformed in the manner they should. Another example: data:text/html,<!DOCTYPE html> <img style="-moz-transform:perspective(100px) scale(-2,1)" src='http://www.mozilla.org/img/download-logo-sub.png'> Was this really always here? It must be a regression, it's too crazy for no one to have noticed it until now.
Works: http://hg.mozilla.org/integration/mozilla-inbound/rev/6799a5e6912f Mozilla/5.0 (Windows NT 6.1; WOW64; rv:13.0) Gecko/20120312 Firefox/13.0a1 ID:20120312224507 Fails: http://hg.mozilla.org/integration/mozilla-inbound/rev/4bd80687cb32 Mozilla/5.0 (Windows NT 6.1; WOW64; rv:13.0) Gecko/20120312 Firefox/13.0a1 ID:20120312225607 Pushlog: http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=6799a5e6912f&tochange=4bd80687cb32
Sorry, Please Ignore comment #2
Regression window(m-c) Works: http://hg.mozilla.org/mozilla-central/rev/cd4853b0b94a Mozilla/5.0 (Windows NT 6.1; WOW64; rv:13.0) Gecko/20120226 Firefox/13.0a1 ID:20120226031015 Fails: http://hg.mozilla.org/mozilla-central/rev/b98fc24ac54b Mozilla/5.0 (Windows NT 6.1; WOW64; rv:13.0) Gecko/20120226 Firefox/13.0a1 ID:20120226151249 Pushlog: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=cd4853b0b94a&tochange=b98fc24ac54b Regression window(m-i) Works: http://hg.mozilla.org/integration/mozilla-inbound/rev/c928dc7c9dff Mozilla/5.0 (Windows NT 6.1; WOW64; rv:13.0) Gecko/20120225 Firefox/13.0a1 ID:20120225011949 Fails: http://hg.mozilla.org/integration/mozilla-inbound/rev/7dcff354aeae Mozilla/5.0 (Windows NT 6.1; WOW64; rv:13.0) Gecko/20120225 Firefox/13.0a1 ID:20120225014049 Pushlog: http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=c928dc7c9dff&tochange=7dcff354aeae Suspected: 7dcff354aeae Aryeh Gregor — Bug 724614 - 3D scaling makes text blurry. r=mattw
Blocks: 724614
Keywords: regression
OS: Linux → All
Um, oops. Matt, any idea on what the problem is here? I more or less just put something in my patch that looked like it might work, and it seemed to, so I figured it was okay. I don't know what would have caused this regression. If you don't have any ideas offhand on how to fix it properly, I can write a patch to just back out the change.
Aryeh: Looks like your patch just exposed a different bug. ChooseScaleAndSetTransform was using AreLayerMarkedActive to check for an active transform layer, which doesn't accurately reflect the result of GetLayerState(). What this meant was that we were removing the 2x2 scale from the transform, with the intent of applying it to the content and then never doing the latter.
Attachment #606110 - Flags: review?(roc)
(In reply to Matt Woodrow (:mattwoodrow) from comment #6) > What this meant was that we were removing the 2x2 scale from the transform, > with the intent of applying it to the content and then never doing the > latter. This confuses me. Why were we never applying it? The scale factors in the ContainerParameters returned by ChooseScaleAndSetTransform must *always* be applied. If they're not being applied in some cases, we should fix that.
Why only apply it if mInActiveTransformedSubtree is set to tree, which it wasn't previously. This patch fixes that.
Seems to me that that code in PopThebesLayerData should be unconditional instead of testing mInActiveTransformedSubtree, no? If there are scale factors, they should always be applied. Arguably ConfigureLayer should take mParameters as a parameter and do the adjustment itself.
I'm not actually sure why, but being conditional on active transforms is correct. I guess because inactive layers are being drawn into a context with the scale already applied? This bug only affects 3d transforms (since they are always active), there's no bug with 2d equivalents. My patch just fixes the active layer detection code to be correct, but I can look into other solutions if you'd like.
(In reply to Matt Woodrow (:mattwoodrow) from comment #10) > I'm not actually sure why, but being conditional on active transforms is > correct. I guess because inactive layers are being drawn into a context with > the scale already applied? Probably. I believe my proposal in comment #9 is correct and will fix this bug. It leads to simpler code since the scale factors are always applied instead of only being applied in some cases. So we should do that. Am I wrong?
So this is a standing regression from Firefox 13. If a proper fix isn't going to land soon, shouldn't we just back out bug 724614?
Fixed window(m-c): Bad: http://hg.mozilla.org/mozilla-central/rev/c00a9c1940c5 Mozilla/5.0 (Windows NT 6.1; WOW64; rv:15.0) Gecko/15.0 Firefox/15.0a1 ID:20120515185430 Fixed: http://hg.mozilla.org/mozilla-central/rev/bfc259be3fa2 Mozilla/5.0 (Windows NT 6.1; WOW64; rv:15.0) Gecko/15.0 Firefox/15.0a1 ID:20120516020729 Pushlog: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=c00a9c1940c5&tochange=bfc259be3fa2 Fixed window(m-c): Bad: http://hg.mozilla.org/integration/mozilla-inbound/rev/b69d000b726a Mozilla/5.0 (Windows NT 6.1; WOW64; rv:15.0) Gecko/15.0 Firefox/15.0a1 ID:20120514231326 Fixed: http://hg.mozilla.org/integration/mozilla-inbound/rev/dd3c873674fc Mozilla/5.0 (Windows NT 6.1; WOW64; rv:15.0) Gecko/15.0 Firefox/15.0a1 ID:20120515030522 Pushlog: http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=b69d000b726a&tochange=dd3c873674fc It seems fixed in Firefox14 and Firefox15 by Bug 750334
Blocks: 745523
(In reply to Aryeh Gregor from comment #12) > So this is a standing regression from Firefox 13. If a proper fix isn't > going to land soon, shouldn't we just back out bug 724614? Yes. This bug should have been tracked for FF13 :-(
(In reply to Alice0775 White from comment #13) > Fixed window(m-c): > Bad: > http://hg.mozilla.org/mozilla-central/rev/c00a9c1940c5 > Mozilla/5.0 (Windows NT 6.1; WOW64; rv:15.0) Gecko/15.0 Firefox/15.0a1 > ID:20120515185430 > Fixed: > http://hg.mozilla.org/mozilla-central/rev/bfc259be3fa2 > Mozilla/5.0 (Windows NT 6.1; WOW64; rv:15.0) Gecko/15.0 Firefox/15.0a1 > ID:20120516020729 > Pushlog: > http://hg.mozilla.org/mozilla-central/ > pushloghtml?fromchange=c00a9c1940c5&tochange=bfc259be3fa2 > > Fixed window(m-c): > Bad: > http://hg.mozilla.org/integration/mozilla-inbound/rev/b69d000b726a > Mozilla/5.0 (Windows NT 6.1; WOW64; rv:15.0) Gecko/15.0 Firefox/15.0a1 > ID:20120514231326 > Fixed: > http://hg.mozilla.org/integration/mozilla-inbound/rev/dd3c873674fc > Mozilla/5.0 (Windows NT 6.1; WOW64; rv:15.0) Gecko/15.0 Firefox/15.0a1 > ID:20120515030522 > Pushlog: > http://hg.mozilla.org/integration/mozilla-inbound/ > pushloghtml?fromchange=b69d000b726a&tochange=dd3c873674fc > > It seems fixed in Firefox14 and Firefox15 by Bug 750334 Thanks, Alice! You're awesome. :) Confirmed works for me now. We should add a test, though. (I have some in the CSS Transforms tests I'm submitting, so if those are ever imported that should suffice.)
Status: NEW → RESOLVED
Closed: 13 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
http://jsbin.com/3/ugipen/5/edit?html,css,live - no difference if removing the background color data:text/html,<!DOCTYPE html> <img style="-moz-transform:rotatex(30deg)" src='http://www.mozilla.org/img/download-logo-sub.png'> <img style="-moz-transform:scale3d(2,2,2)" src='http://www.mozilla.org/img/download-logo-sub.png'> The first image is squashed vertically as expect and the second one doesn't disappear. data:text/html,<!DOCTYPE html> <img style="-moz-transform:scale3d(-2,1,2)" src='http://www.mozilla.org/img/download-logo-sub.png'> The image is squashed vertically. Verified fixed on FF 14b9 on Win 7, Ubuntu 12.04 and Mac OS X 10.6.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: