3D transforms of images seems to have been broken

VERIFIED FIXED

Status

()

Core
Layout
VERIFIED FIXED
5 years ago
a year ago

People

(Reporter: ayg, Unassigned)

Tracking

({regression})

Trunk
x86
All
regression
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite ?

Firefox Tracking Flags

(firefox14+ verified, firefox15+ fixed)

Details

Attachments

(1 attachment)

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.

Comment 2

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

Comment 3

5 years ago
Sorry,

Please Ignore comment #2

Comment 4

5 years ago
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.
Created attachment 606110 [details] [diff] [review]
Make ChooseScaleAndSetTransform correctly detect active layers

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?

Comment 13

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

Updated

5 years ago
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
Last Resolved: 5 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Duplicate of this bug: 747381
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

Updated

5 years ago
tracking-firefox14: --- → +

Updated

5 years ago
status-firefox14: --- → fixed
status-firefox15: --- → fixed
tracking-firefox15: --- → +
status-firefox14: fixed → verified
Attachment #606110 - Flags: review?(roc)
You need to log in before you can comment on or make changes to this bug.