Closed Bug 1240073 Opened 8 years ago Closed 8 years ago

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

Categories

(Core :: Layout, defect)

Product:
Core
Component:
Layout
Version:
37 Branch
Platform:
Unspecified
Windows
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla47
Tracking Flags:
Tracking Status
firefox47 --- fixed

People

(Reporter: scu, Assigned: mattwoodrow)

References

Details

(Keywords: regression, testcase)

Attachments

(7 files, 2 obsolete files)

testbottom.jpg
78.41 KB, image/jpeg
Details
testtop.jpg
50.60 KB, image/jpeg
Details
1240073.html
1.23 KB, text/html
Details
screencast-FF46.mp4
468.44 KB, video/mp4
Details
screencast-FF43.mp4
535.69 KB, video/mp4
Details
static testcase (no bug)
1.23 KB, text/html
Details
Use the normal AGR for nsDisplayTransform v3
7.73 KB, patch
tnikkel
: review+
Details | Diff | Splinter Review 
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: 8 years ago

          status-firefox47:
          --- → fixed
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.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: