641770 - Image element with position:fixed is drawn on scroll bar

Status: VERIFIED FIXED

(Core :: Graphics, defect)

Description

[Alice0775 White]

Attachment: sample html

Build Identifier:
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b13pre) Gecko/20110314 Firefox/4.0b13pre ID:20110314030412

This was originally reported by http://bugzilla.mozilla.gr.jp/show_bug.cgi?id=6787

Image element with position:fixed is drawn on scroll bar.

Reproducible: Always

Steps to Reproduce:
1. Start Minefield with new profile
2. Open attached sample html
3.

Actual Results:
 Image element with position:fixed is drawn on scroll bar

Expected Results:
 Image element with position:fixed sgould not be drawn on scroll bar

Regression window:
Works:
http://hg.mozilla.org/mozilla-central/rev/16d3af58499c
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b10pre) Gecko/20110116 Firefox/4.0b10pre ID:20110117133144
Fails:
http://hg.mozilla.org/mozilla-central/rev/aa611c72c337
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b10pre) Gecko/20110115 Firefox/4.0b10pre ID:20110117140637
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=16d3af58499c&tochange=aa611c72c337

Comment 1

[Alice0775 White]

Attachment: screenshot

Comment 2

[Alice0775 White]

In local build,
build from d41e933f589a : fails
build from 16d3af58499c : works
Regressed by:
d41e933f589a	Matt Woodrow — Bug 586683 - Part 4 - Move images into a separate ImageLayer when they are the only item in a ThebesLayer. r=roc a=blocking2.0

Comment 3

[Boris Zbarsky [:bzbarsky]]

roc, can we ship with this?

Comment 4

[Mike Beltzner [:beltzner, not reading bugmail]]

I think we can ship with this, but should fix it!

Comment 5

[Mike Beltzner [:beltzner, not reading bugmail]]

Roc: renominate this, please, if you disagree with our assertion, but we didn't think it was bad enough to block. Joe thought it might represent a deeper and worse bug in Layers, though.

Comment 6

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I agree we can ship with this. It's almost certainly not a deep layers bug.

Comment 7

[Robert O'Callahan (:roc) (email my personal email if necessary)]

The bug is simply that we don't clip a layerized image, at all. Oops. Fortunately it can't escape from the Web page because the page is in a container layer that is clipped.

Comment 8

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: Fix MarkLeafLayersCoveredByOpaque

My reftest for this bug exposed this other layers bug.

Comment 9

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: Set clip rect on layerized images

Comment 10

[Timothy Nikkel (:tnikkel)]

Comment on attachment 519524 [details] [diff] [review]
Fix MarkLeafLayersCoveredByOpaque

>+// aClipRect and aRegoin are in that global coordinate system.

aRegoin

Comment 11

[Timothy Nikkel (:tnikkel)]

Comment on attachment 519525 [details] [diff] [review]
Set clip rect on layerized images

 nsRefPtr<ImageContainer>
 ContainerState::ThebesLayerData::CanOptimizeImageLayer(LayerManager* aManager)
 {
-  if (!mImage) {
+  if (!mImage || !mImageClip.mRoundedClipRects.IsEmpty()) {
     return nsnull;
   }
 
   return mImage->GetContainer(aManager);
 }

Should we do the IsRectClippedByRoundedCorner & RemoveRoundedCorners dance here?
 
 void
 ContainerStat

     if (imageContainer) {
       nsRefPtr<ImageLayer> imageLayer = CreateOrRecycleImageLayer();
       imageLayer->SetContainer(imageContainer);
       data->mImage->ConfigureLayer(imageLayer);
+      if (data->mImageClip.mHaveClipRect) {
+        nsPresContext* presContext = mContainerFrame->PresContext();
+        nscoord appUnitsPerDevPixel = presContext->AppUnitsPerDevPixel();
+        nsIntRect clip = data->mImageClip.mClipRect.ToNearestPixels(appUnitsPerDevPixel);
+        imageLayer->IntersectClipRect(
+          data->mImageClip.mClipRect.ToNearestPixels(appUnitsPerDevPixel));
+      }
       layer = imageLayer;

Assert no rounded rects in the clip here?

Comment 12

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(In reply to comment #11)
> Should we do the IsRectClippedByRoundedCorner & RemoveRoundedCorners dance
> here?

Let's not bother. When layers can clip to rounded corners, we won't be doing this anyway.

>  void
>  ContainerStat
> 
>      if (imageContainer) {
>        nsRefPtr<ImageLayer> imageLayer = CreateOrRecycleImageLayer();
>        imageLayer->SetContainer(imageContainer);
>        data->mImage->ConfigureLayer(imageLayer);
> +      if (data->mImageClip.mHaveClipRect) {
> +        nsPresContext* presContext = mContainerFrame->PresContext();
> +        nscoord appUnitsPerDevPixel = presContext->AppUnitsPerDevPixel();
> +        nsIntRect clip =
> data->mImageClip.mClipRect.ToNearestPixels(appUnitsPerDevPixel);
> +        imageLayer->IntersectClipRect(
> +          data->mImageClip.mClipRect.ToNearestPixels(appUnitsPerDevPixel));
> +      }
>        layer = imageLayer;
> 
> Assert no rounded rects in the clip here?

OK.

Comment 13

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: v2

Comment 14

[Bas Schouten (:bas.schouten)]

Comment on attachment 519524 [details] [diff] [review]
Fix MarkLeafLayersCoveredByOpaque

>From: Robert O'Callahan <[e-mail]>
>Bug 641770. Ensure that the cliprect for a layer is interpreted in the coordinate system of its container in MarkLeafLayersCoveredByOpaque. r=bas
>
>diff --git a/gfx/layers/basic/BasicLayers.cpp b/gfx/layers/basic/BasicLayers.cpp
>--- a/gfx/layers/basic/BasicLayers.cpp
>+++ b/gfx/layers/basic/BasicLayers.cpp
>@@ -1237,42 +1237,49 @@ TransformIntRect(nsIntRect& aRect, const
>   gfxRect gr = gfxRect(aRect.x, aRect.y, aRect.width, aRect.height);
>   gr = aMatrix.TransformBounds(gr);
>   aRect = (*aRoundMethod)(gr);
> }
> 
> // This implementation assumes that GetEffectiveTransform transforms
> // all layers to the same coordinate system. It can't be used as is
> // by accelerated layers because of intermediate surfaces.
>+// aClipRect and aRegoin are in that global coordinate system.

typo nit: aRegion

Comment 15

[Robert O'Callahan (:roc) (email my personal email if necessary)]

http://hg.mozilla.org/mozilla-central/rev/2622ae93dfe8
http://hg.mozilla.org/mozilla-central/rev/c96c353c9d5e

Comment 16

[George Carstoiu]

Verified with  Mozilla/5.0 (Windows NT 6.1; rv:2.2a1pre) Gecko/20110404 Firefox/4.2a1pre