Closed Bug 1240073 Opened 5 years ago Closed 5 years ago

Sliding one image over another using css translate causes distorted bottom image while moving

Categories

(Core :: Layout, defect)

37 Branch
Unspecified
Windows
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla47
Tracking Status
firefox47 --- fixed

People

(Reporter: scu, Assigned: mattwoodrow)

References

(Depends on 1 open bug)

Details

(Keywords: regression, testcase)

Attachments

(7 files, 2 obsolete files)

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:43.0) Gecko/20100101 Firefox/43.0
Build ID: 20160105164030

Steps to reproduce:

<!DOCTYPE html>
<html>
<head>
<title>Test Translation</title>
<meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1">
<script>
var moving=0;
function cardOpenClose() {
	var cardback = document.getElementById("CardBack");
	if(!moving) {
		moving=1;
		setTimeout(function(){  
			cardback.style.transform = "translateY(237px)";
			cardback.style.webkitTransform = "translateY(237px)";
		}, 250);
		setTimeout(function(){  
			cardback.style.transform = "translateY(0px)";
			cardback.style.webkitTransform = "translateY(0px)";
		}, 6250);
		setTimeout(function(){  
			moving=0;
		}, 8250);
	}
}
</script>
<style>
#CardBack {
	position: absolute;
	left: 500px;
	top: 105px;
	-webkit-transition: -webkit-transform 2s;
	transition: transform 3s;
}
#CardFront {
	position: absolute;
	left: 494px;
	top: 10px;
	-webkit-transition: -webkit-transform 2s;
	transition: transform 3;
}
</style>
</head>
<body>
	<a href="javascript:cardOpenClose();">
		<img id = "CardBack" src="testbottom.jpg" border="0" alt="" > 
		<img id = "CardFront" src="testtop.jpg" border="0" alt="" > 
	</a>

</body>
</html>



Actual results:

Card bottom slides out from under Card top, but bottom image becomes distorted as it is moving.  When the movement stops, the image looks correct again.


Expected results:

No distortion with IE or Chrome.  I tried to attach the two image files, but your interface will only allow one attached file.  testtop.jpg is 377x332   testbottom.jpg is 367x432
Product: Firefox → Core
Please, attach both images one by one to the bug report.
Flags: needinfo?(scu)
Attached image testbottom.jpg
Flags: needinfo?(scu)
Attached image testtop.jpg
Attached file 1240073.html
Maybe a duplicate.

Reg range:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=035a951fc24a&tochange=f1f48ccb2d4e
Component: Untriaged → Layout
Keywords: regression, testcase
What kind of distortion is it? I couldn't see a problem in the testcase.
Attached video screencast-FF46.mp4
Attached video screencast-FF43.mp4
Ah, I can reproduce on Windows. And if I make Firefox us d3d9 the problem goes away. I think this is due to https://hg.mozilla.org/mozilla-central/rev/823227372483 bug 1107297.
Blocks: 1107297
Status: UNCONFIRMED → NEW
Ever confirmed: true
Yes that's it. Many regressions are already linked to this bug maybe a dupe.
OS: Unspecified → Windows
Version: 43 Branch → 37 Branch
(In reply to Timothy Nikkel (:tnikkel) from comment #10)
> Ah, I can reproduce on Windows. And if I make Firefox us d3d9 the problem
> goes away. I think this is due to
> https://hg.mozilla.org/mozilla-central/rev/823227372483 bug 1107297.

Looks like layout isn't accurately passing the damaged rect, I've seen bugs similar to this. Matt might know where they are.
Flags: needinfo?(matt.woodrow)
This is a bug with the visible rect layout computes for the animated layer.

I constructed this static test case for the mid point of the animation.

With the static test case, the layer we construct looks like this:

ContainerLayerComposite () [shadow-clip=(x=0, y=79, w=1280, h=734)] [shadow-transform=[ 1 0; 0 1; 500 284; ]] [shadow-visible=< (x=0, y=137, w=367, h=286); >] [clip=(x=0, y=79, w=1280, h=734)] [transform=[ 1 0; 0 1; 500 284; ]] [visible=< (x=0, y=137, w=367, h=286); >]

Yet when we cross approximately the same transform with the original testcase we get this:

ContainerLayerComposite () [shadow-clip=(x=0, y=79, w=1280, h=734)] [shadow-transform=[ 1 0; 0 1; 500 284.859; ]] [shadow-visible=< (x=0, y=159, w=367, h=264); >] [clip=(x=0, y=79, w=1280, h=734)] [transform=[ 1 0; 0 1; 500 284.859; ]] [visible=< (x=0, y=159, w=367, h=264); >]

The rest of the layers trees are identical, and the transform is within 1px, yet the visible region is missing the top 22 pixels in the animated test case.

An incorrect visible region means that we fail to invalidate all the pixels that actually changed, and we get this corruption.
Flags: needinfo?(matt.woodrow)
The bug is in TryUpdateTransformOnly/SetBaseTransformForNextTransaction.

This code detects that the transform change is only a translation difference, and shortcuts the entire layer building process and just modifies the transform directly on the layer.

In this case the translation causes more of the layer to become visible, so we need to update the visible region on the layer too, but we don't.

This code is pretty hacky (it was added to specifically to optimize the old b2g homescreen side-scrolling), and I can't think of an easy to way to make it understand visible regions properly.

It mainly exists to optimize cases where translations are being adjusted, but not declaratively (css animations/transitions), since we have OMTA for the latter.

I think we should just get rid of it.
Now that I've said that, I can't see how it works with OMTA either.

The AnimatedGeometryRoot for an nsDisplayTransform is currently set to the AGR for content *outside* the transform.

This means that ContainerState::PostprocessRetainedLayers will use the same OpaqueRegionEntry for the transformed ContainerLayer as well as other un-transformed content on top of it. 

This is why we're treating the transformed layer as occluded, and reducing the visible region.

I can't see a reason why the AGR for nsDisplayTransform shouldn't be mFrame, and this fixes the occlusion calculations.
Assignee: nobody → matt.woodrow
Attachment #8708861 - Flags: review?(tnikkel)
Comment on attachment 8708861 [details] [diff] [review]
Use the normal AGR for nsDisplayTransform

Hmm, I think you're right. The fact that we had to add those hacks probably should have been a sign that we were on the wrong track.

I reserve the right to change my mind if this try run isn't green though :)

https://treeherder.mozilla.org/#/jobs?repo=try&revision=2d057dba6bd8
Attachment #8708861 - Flags: review?(tnikkel) → review+
Looks like a bunch of fuzz differences, which suggests that layerization has changed, I'll investigate.
I think I saw the same set of failures when I was working on AGRs a few months ago, if I recall the problem was this:

If we had two transforms nested, say with transform items (frames) outerItem (outerFrame) and innerItem (innerFrame), then we call RequiredLayerStateForChildren(..., expectedAGRForChildren = outerFrame) for outerItem, and the AGR for the innerItem is outerFrame. So the AGRs allow it to be inactive.

With your patch we call RequiredLayerStateForChildren(..., expectedAGRForChildren = outerFrame) for outerItem, but the AGR for the innerItem is innerFrame. So the AGRs force it active.
The problem was that the unique AGR meant that inactive transforms always got their own PaintedLayer, rather than being put into the same layers as surrounding content.
Attachment #8708861 - Attachment is obsolete: true
Attachment #8709238 - Flags: review?(tnikkel)
Fixed a bug with scroll clips being applied.

AGRs are much too complicated...
Attachment #8709238 - Attachment is obsolete: true
Attachment #8709238 - Flags: review?(tnikkel)
Attachment #8710260 - Flags: review?(tnikkel)
Attachment #8710260 - Flags: review?(tnikkel) → review+
https://hg.mozilla.org/mozilla-central/rev/90f7eaeca546
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
Depends on: 1325117
Depends on: 1328022
You need to log in before you can comment on or make changes to this bug.