Closed Bug 770617 Opened 7 years ago Closed 7 years ago

SnappyTree demo doesn't re-render until I switch tabs

Categories

(Core :: Graphics, defect)

x86_64
Linux
defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: dholbert, Assigned: mattwoodrow)

References

(Blocks 1 open bug, )

Details

(Keywords: regression)

Attachments

(1 file)

STR:
 1. Load https://developer.mozilla.org/en-US/demosdetail/snappytree/launch
 2. Click a preset that looks different from the initial rendering
   (e.g. the one with red leaves)
    --> Does it load?
 3. Switch to another tab, and switch back to the SnappyTree tab

ACTUAL RESULTS:
The preset doesn't load until I switch tabs & switch back.

VERSION INFO: Latest x86_64 linux nightly:
Mozilla/5.0 (X11; Linux x86_64; rv:16.0) Gecko/16.0 Firefox/16.0
Built from http://hg.mozilla.org/mozilla-central/rev/e61399f31505
Last good nightly: 2012-06-30
First bad nightly: 2012-07-01

Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=f08d285b63b0&tochange=d9d61d199b11

That pushlog has DLBI in it (Bug 539356) -- assuming this is a regression from that.
Blocks: dlbi
Keywords: regression
See also bug 770686 about a similar issue with Google Maps.

(Though -- note that in bug 770686, a window-resize will fix the problem. In this bug, window-resizes don't help -- they just resize the stale tree, instead of drawing a new tree.)
Hmm, DLBI is in https://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2012/06/2012-06-30-04-02-27-mozilla-inbound/ (at least the Mac build) but I can't reproduce this bug in that.
I can reproduce this in linux-x64/basic, but not win7/d3d10.  So a basic-layers bug.
Comment on attachment 666282 [details] [diff] [review]
Always update canvas contents for inactive layers

This patch can regress inactive frames with inactive canvases which
are being frequently invalidated outside the canvas area, but I don't
have any evidence that that's a problem.  Compositing already-painted
canvases is pretty cheap.

>diff --git a/gfx/layers/Layers.h b/gfx/layers/Layers.h
>--- a/gfx/layers/Layers.h
>+++ b/gfx/layers/Layers.h
>@@ -1385,16 +1385,36 @@ public:
> 
>   /**
>    * Notify this CanvasLayer that the canvas surface contents have
>    * changed (or will change) before the next transaction.
>    */
>   void Updated() { mDirty = true; SetInvalidRectToVisibleRegion(); }
> 
>   /**
>+   * Notify this CanvasLayer that the canvas surface contents have
>+   * been painted since the last change.
>+   */
>+  void Painted() { mDirty = false; }

I think this name is a little confusing, but it makes sense wrt
Updated() above.  Not your fault we chose this scheme.

>+  bool IsDirty() 
>+  { 
>+    // We can only tell if we are dirty if we're part of the
>+    // widget's retained layer tree.
>+    if (!mManager || !mManager->IsWidgetLayerManager()) {
>+      return;

Erm, I think you meant |return true|? ;)

r=me with that fix.
Attachment #666282 - Flags: review?(jones.chris.g) → review+
Are you leaving this open to write a test?
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Assignee: nobody → matt.woodrow
You need to log in before you can comment on or make changes to this bug.