641599 - Layer is not cleaned after image zooming with Ctrl+

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[gadjo]

User-Agent:       Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b13pre) Gecko/20110313 Firefox/4.0b13pre
Build Identifier: 

When the image is resized with Ctrl+/Ctrl- the layer containing the previous size is not cleaned up, see the screenshot

Reproducible: Always

Steps to Reproduce:
1. Open any image in the browser, i.e. http://i.imgur.com/3HImp.png
2. Click on the image to fit it in the screen
3. Press Ctrl+ few times
Actual Results:  
Parts of the smaller size are still visible

Expected Results:  
Only actual size should be visible

Comment 1

[gadjo]

Attachment: screenshot

Comment 2

[gadjo]

2011-03-11 - good
2011-03-12 - bad

regression range http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=823105711a3b&tochange=d8fe8514d7e6

Comment 3

[Timothy Nikkel (:tnikkel)]

http://hg.mozilla.org/mozilla-central/rev/24c25a9f87fa or http://hg.mozilla.org/mozilla-central/rev/3570861040e9 maybe?

Comment 4

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Bug 637822 seems likely.

Comment 5

[Alice0775 White]

Regression window:
Works:
http://hg.mozilla.org/mozilla-central/rev/e00e8ee0aeb7
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b13pre) Gecko/20110310 Firefox/4.0b13pre ID:20110310221006
Fails:
http://hg.mozilla.org/mozilla-central/rev/3570861040e9
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b13pre) Gecko/20110311 Firefox/4.0b13pre ID:20110311000203
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=e00e8ee0aeb7&tochange=3570861040e9
Regressed by:
3570861040e9	Chris Jones — Bug 637Regression window:
Works:
http://hg.mozilla.org/mozilla-central/rev/e00e8ee0aeb7
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b13pre) Gecko/20110310 Firefox/4.0b13pre ID:20110310221006
Fails:
http://hg.mozilla.org/mozilla-central/rev/3570861040e9
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b13pre) Gecko/20110311 Firefox/4.0b13pre ID:20110311000203
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=e00e8ee0aeb7&tochange=3570861040e9
Regressed by:
3570861040e9	Chris Jones — Bug 637822: Translate invalidated content by the offset at which it was made valid. r=roc a=b822: Translate invalidated content by the offset at which it was made valid. r=roc a=b

Comment 6

[Alice0775 White]

Sorry bug spam, paste miss in comment#5,

Regression window:
Works:
http://hg.mozilla.org/mozilla-central/rev/e00e8ee0aeb7
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b13pre) Gecko/20110310
Firefox/4.0b13pre ID:20110310221006
Fails:
http://hg.mozilla.org/mozilla-central/rev/3570861040e9
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b13pre) Gecko/20110311
Firefox/4.0b13pre ID:20110311000203
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=e00e8ee0aeb7&tochange=3570861040e9
Regressed by:
3570861040e9    Chris Jones — Bug 637822: Translate invalidated content by the
offset at which it was made valid. r=roc a=b

Comment 7

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

I think the problem here is that zooming changes the appunits-to-device-pixel ratio, which effectively invalidates all the cached information on layers that's expressed in appunits.

Comment 8

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

Attachment: fix

Writing a test for this seems rather difficult.

Comment 9

[Timothy Nikkel (:tnikkel)]

Comment on attachment 519323 [details] [diff] [review]
fix

Is it enough to just invalid the contents all thebes layers? What about some of the properties we store?

Comment 10

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

ThebesLayerInvalidRegionProperty is removed by this call. ThebesLayerLastPaintOffsetProperty is irrelevant if there is no ThebesLayerInvalidRegionProperty. I think that's all we have in appunits.

Comment 11

[Timothy Nikkel (:tnikkel)]

Ok. Things that are cached in any unit could need to be recalculated though, no?

Comment 12

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

There will be a complete restyle and reflow, which should take care of most things. The issue here is that the meaning of appunits has changed. Appunits stored in the frame tree will be updated by the reflow, but these cached appunit properties will not.

Comment 13

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

I can't reproduce this on Windows 7 - is it specific to certain drivers?

Comment 14

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

We'd take this on branch, but I don't think we'd hold Firefox 4 for this as it's not 100% reproducible for everyone. (Several people tested, some saw it, most didn't)

Do we know why that is?

Comment 15

[Alice0775 White]

(In reply to comment #13)
>  is it specific to certain drivers?
I do not think so. 
This happens on Windows 7,WindowsXP and Ubuntu10.04.

*Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b13pre) Gecko/20110315 Firefox/4.0b13pre ID:20110315030415
Graphics
  Adapter Description: ATI Radeon HD 4300/4500 Series
  Vendor ID: 1002
  Device ID: 954f
  Adapter RAM: 512
  Adapter Drivers: aticfx64 aticfx64 aticfx32 aticfx32 atiumd64 atidxx64
atiumdag atidxx32 atiumdva atiumd6a atitmm64
  Driver Version: 8.821.0.0
  Driver Date: 1-26-2011
  Direct2D Enabled: true
  DirectWrite Enabled: true (6.1.7601.17514, font cache n/a)
  WebGL Renderer: Google Inc. -- ANGLE -- OpenGL ES 2.0 (ANGLE 0.0.0.541)
  GPU Accelerated Windows: 1/1 Direct3D 10
 
*Mozilla/5.0 (Windows NT 5.1; rv:2.0b13pre) Gecko/20110315 Firefox/4.0b13pre ID:20110315030415
Graphics
  Adapter Description: VMware SVGA II
  Vendor ID: 15ad
  Device ID: 0405
  Adapter RAM: Unknown
  Adapter Drivers: vmx_fb
  Driver Version: 11.6.0.35
  Driver Date: 4-21-2010
  Direct2D Enabled: Blocked on your graphics card because of unresolved driver issues.
  DirectWrite Enabled: false (0.0.0.0, font cache n/a)
  WebGL Renderer: Google Inc. -- ANGLE -- OpenGL ES 2.0 (ANGLE 0.0.0.541)
  GPU Accelerated Windows: 0/1

*Mozilla/5.0 (X11; Linux i686; rv:2.0b13pre) Gecko/20110315 Firefox/4.0b13pre ID:20110315030415
Graphics
   GPU Accelerated Windows0/2

Comment 16

[Timothy Nikkel (:tnikkel)]

(In reply to comment #8)
> Created attachment 519323 [details] [diff] [review]
> fix
> 
> Writing a test for this seems rather difficult.

Because it would have to combine enablePrivilege (to set nsIMarkupDocumentViewer.fullZoom) with invalidation testing like in the reftest harness?

Comment 17

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

Exactly.

Comment 18

[Timothy Nikkel (:tnikkel)]

(In reply to comment #12)
> There will be a complete restyle and reflow, which should take care of most
> things. The issue here is that the meaning of appunits has changed. Appunits
> stored in the frame tree will be updated by the reflow, but these cached
> appunit properties will not.

Yeah, I mainly meant any other properties stored by FrameLayerBuilder or in the layer tree.

Comment 19

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

The layer tree doesn't have any appunits stuff in it, by design.

FrameLayerBuilder's properties other than ThebesLayerInvalidRegionProperty and ThebesLayerLastPaintOffsetProperty don't use appunits.

So for those other properties and the layer tree, a zoom isn't any different from any other complete restyle and reflow.

Comment 21

[Daniel Veditz [:dveditz]]

There doesn't seem to be enough certainty in this bug to make a 4.0.1 release. Get it into FF5 and we can then think about whether it's worth it.

Comment 24

[(no longer active)]

http://hg.mozilla.org/projects/cedar/rev/1a53dc7c5c20

Comment 25

[(no longer active)]

http://hg.mozilla.org/mozilla-central/rev/1a53dc7c5c20