389224 - Clipped objects are sometimes not fully redrawn after clipping

Status: RESOLVED FIXED

(Core :: Layout: Positioned, defect)

Description

[Daniel Kirsch]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; de; rv:1.8.1.5) Gecko/20070713 Firefox/2.0.0.5
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; de-DE; rv:1.9a6pre) Gecko/20070627

When dynamically setting the clip regions of a HTML element the element sometimes isn't shown completely. The change of the clipping regions is done by an interval to have some simple animation effect.

The problem seems to depend on garbage collection this way:
I create complex html:body content dynamically using javascript holding a lot of references to JavaScript objects and back to the DOM elements. I remove all those elements and create others again. Removing and recreating is one step. The clipping animation immidiately starts after such a step. The animation often
hang and is sometimes not rendered completely although the css properties are set correctly. Moving other windows over the incompletely rendered object shows that it's there but not fully redrawn.

My guess is that GC runs after the page recreation step pausing further execution and also timers or at least redrawing. If GC starts before the animation is finished and stops after, the animation step in between is not rendered.

Reproducible: Sometimes

Comment 1

[Daniel Kirsch]

Attachment: Screenshot

The screenshot shows three different states.
1. The original picture including a red border
2. The picture after the clipping animation is done
3. The picture after moving another window over the not drawn area to the left

It seems that object changes done by a script that is running during GC are not drawn after GC finished.

Comment 2

[Daniel Kirsch]

See also bug 388951 for the garbage collection issue this problem might depend on.

Comment 3

[Daniel Kirsch]

The problem can be forced by doing an clipping animation, and move a dependent window during animation.

Comment 4

[Daniel Kirsch]

Attachment: Testcase

To reproduce the bug, reload the window to start animation again (button "Reload"), and move the window or at least keep the mouse pressed on the titlebar for about 2 seconds. Now the left part of the animation is not drawn completely. 

In this testcase you can see that the text gets redrawn but not the surrounding blank element.

NOTE: This is not reproducable with official Firefox releases but with the current XULRunner trunk.

Comment 5

[Martijn Wargers (dead)]

Well, I can reliably reproduce this with the testcase.
It regressed between 2007-03-25 and 2007-03-27:
http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2007-03-25+04&maxdate=2007-03-27+09&cvsroot=%2Fcvsroot
I guess a regression from bug 372037, somehow.

Comment 6

[Daniel Kirsch]

(In reply to comment #5)
> I guess a regression from bug 372037, somehow.

Yes, seems possible. Thanks for testing.

As far as I can see, GC is not the problem but one possible situation to cause this behaviour.

Comment 7

[Eli Friedman]

I'll take this for the moment.

Comment 8

[Eli Friedman]

I was sure I had addressed this when I was writing my patch for bug 372037... we really need some sort of invalidation testing framework.

The issue here is rather straightforward: the transition from clip:rect(...) to clip:auto isn't handled correctly.

Comment 9

[Daniel Kirsch]

Attachment: Updated testcase

The new testcase shows, that the clipping bug is easily reproducable without pausing script execution. I simply set the clipping region once and back to default or auto after a short timeout. This seems to be related with the "overflow" property set to "visible".

Comment 10

[Daniel Kirsch]

Another problem in this context is that style changes seems to be buffered before rendering. For problems like this bug I usually used

 var d = elm.style.display;
 elm.style.display = "none";
 elm.style.display = d;

This forced the element to be redrawn. On the current trunk, this doesn't work anymore. I think that mozilla now buffers style changes and renders them at once (and in this case no real change happens). While this might be a big plus in performance it makes workarounds like described before impossible.
Is there a way to force a redraw at least in chrome environment?

Comment 11

[Martijn Wargers (dead)]

If you put an elm.offsetHeight inbetween, it should work.

Comment 12

[Daniel Kirsch]

Thanks!
If I have an object with that rendering bug visible and then get offsetHeigh, the rendering doesn't get updated. However in combination with the three script lines above it works.
Why is that? And is my assumption right, that mozilla uses an internal style buffer?

Comment 13

[Martijn Wargers (dead)]

I don't know why that happens. This was implemented in bug 230170 (batching of style reresolutions). Boris put a lot of work into it.

Comment 14

[Eli Friedman]

(In reply to comment #12)
> Thanks!
> If I have an object with that rendering bug visible and then get offsetHeigh,
> the rendering doesn't get updated. However in combination with the three script
> lines above it works.
> Why is that? And is my assumption right, that mozilla uses an internal style
> buffer?

Yes, style changes are buffered internally, and normally style resolution is done off of an internal event.  However, querying offsetHeight forces the browser to do style resolution and layout immediately, because the value of offsetHeight depends on the layout.  If the style resolution and layout is already up-to-date, querying offsetHeight simply calculates the value and returns.

Comment 15

[Eli Friedman]

Attachment: Patch

We should really consider changing how invalidation works... but this is good enough, and I don't think it'll be a significant perf hit in common cases.

Comment 16

[Eli Friedman]

Comment on attachment 278089 [details] [diff] [review]
Patch

This is a low-risk fix for a 1.9 regression.

Comment 17

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 278089 [details] [diff] [review]
Patch

a1.9=dbaron

Comment 18

[Eli Friedman]

Checked in.