505896 - Screen flashes when moving puzzle pieces

Status: VERIFIED FIXED

(Core Graveyard :: Plug-ins, defect, P2)

Description

[Jim Jeffery not reading bug-mail 1/2/11]

Visit the URL and pick any puzzle.  Once you start moving the pieces around to solve the puzzle the screen flickers/flashes.  Up until today's build JigZone has been working normally.  

Still present with latest hourly:
Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.2a1pre) Gecko/20090722 Minefield/3.6a1pre (.NET CLR 3.5.30729) ID:20090722135815
changeset: 
http://hg.mozilla.org/mozilla-central/rev/62d18b867afb

Comment 1

[Jim Jeffery not reading bug-mail 1/2/11]

Regressed with the landing of roc's phase 1 huge patch.

good in changeset:

build: 7/21/09 1856hrs
changeset: http://hg.mozilla.org/mozilla-central/rev/913714436524  OK

bad:
build: 7/21/09 20:56 Roc's huge patch
changeset: http://hg.mozilla.org/mozilla-central/rev/5b0d9f36c0b3

Comment 2

[(mostly gone) XtC4UaLL [:xtc4uall]]

it's the plugin (here: npjp2.dll) frame that flickers. fallout of Bug 339548?

Comment 3

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

You're talking about a white flash that appears periodically?

Comment 4

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

What seems to be happening here is that at least in some cases the puzzle applet is not double-buffering. At various times we're calling nsWindow::ConfigureChildren on the parent which calls Resize() on the plugin widget with aRepaint set to true, which asks the applet to repaint itself completely, which causes a flicker since the applet doesn't double-buffer.

The fix is to avoid that repainting as much as possible. But while testing that fix I found a related problem that needs more investigation.

Comment 5

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

Attachment: fix

The main change here is in nsWindow::ConfigureChildren, to only Resize with aInvalidate true if the size actually changed. Otherwise we can just Move, unless we didn't even move in which case we can do nothing.

But this exposed a problem in Scroll with SW_SCROLLCHILDREN, where SW_SCROLLCHILDREN sometimes moves children without repainting/blitting all of the child, if the child extends outside the area that's being blitted. So we need to detect that situation and explicitly invalidate.

Comment 6

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

Attachment: testcase

I've been using this file to manually test plugin scrolling where one plugin that overlaps the scrolled area is not moving, but another one is.

Unfortunately we have no way of automatically testing the effects of ScrollWindowEx and other Windows HWND manipulation, and I can't think of a good way to do it.

Comment 7

[Johnny Stenback (:jst)]

Seems like a good thing to get in for 1.9.2.

Comment 8

[Jim Mathies [:jimm]]

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

win code looks fine. 

However, I'd appreciate you moving that static function into the class, we've been trying to get away from that.

Comment 9

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

Why are you trying to get away from static functions? They're more efficient than class static member functions. For example, when they're only called once, gcc will inline the function no matter how big it is. And if it's not used at all, gcc warns.

Comment 10

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

http://hg.mozilla.org/mozilla-central/rev/da522ed1b640

Comment 11

[Jim Jeffery not reading bug-mail 1/2/11]

Verified fixed using hourly build:
Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.2a1pre) Gecko/20090727 Minefield/3.6a1pre (.NET CLR 3.5.30729) ID:20090727014139

changeset:
http://hg.mozilla.org/mozilla-central/rev/9d5e247b5052

Comment 12

[Sylvain Pasche]

This caused bug 507402

Comment 13

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

Mass change: adding fixed1.9.2 keyword

(This bug was identified as a mozilla1.9.2 blocker which was fixed before the mozilla-1.9.2 repository was branched (August 13th, 2009) as per this query: http://is.gd/2ydcb - if this bug is not actually fixed on mozilla1.9.2, please remove the keyword. Apologies for the bugspam)