Last Comment Bug 334411 - Implement paint flashing
: Implement paint flashing
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Graphics (show other bugs)
: Trunk
: x86 Linux
: -- normal with 1 vote (vote)
: mozilla11
Assigned To: Bas Schouten (:bas.schouten)
:
Mentors:
: 723716 (view as bug list)
Depends on:
Blocks: 102062 273293 334720
  Show dependency treegraph
 
Reported: 2006-04-17 16:47 PDT by Boris Zbarsky [:bz]
Modified: 2014-04-25 15:16 PDT (History)
18 users (show)
bzbarsky: blocking1.9-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Introduce paint flashing (1.32 KB, patch)
2011-11-13 15:39 PST, Bas Schouten (:bas.schouten)
no flags Details | Diff | Splinter Review
Introduce paint flashing v2 (3.07 KB, patch)
2011-11-14 20:12 PST, Bas Schouten (:bas.schouten)
roc: review+
Details | Diff | Splinter Review
Add nglayout.debug.paint_flashing to all.js (924 bytes, patch)
2012-02-05 20:32 PST, Robert O'Callahan (:roc) (Exited; email my personal email if necessary)
bas: review+
roc: checkin+
Details | Diff | Splinter Review

Description Boris Zbarsky [:bz] 2006-04-17 16:47:20 PDT
Someone who understands what regions cairo is actually repainting needs to adjust the patch from bug 220953 to work in cairo builds.
Comment 1 Boris Zbarsky [:bz] 2006-10-05 15:35:25 PDT
Not gonna block 1.9 on this.

roc, could we possibly do this in the viewmanager instead of widgets?  That would also flash the area we really are repainting...
Comment 2 David Baron :dbaron: ⌚️UTC+2 (mostly busy through August 4; review requests must explain patch) 2006-10-05 15:42:39 PDT
...and would be cross-platform, which would mean we'd stop having to rewrite it.
Comment 3 Vladimir Vukicevic [:vlad] [:vladv] 2006-10-05 18:06:12 PDT
With our newfangled alpha support, we could even draw the actual update region in a translucent color, overlaying the page content.  It would be a permanent thing on the rendered display, as opposed to just a flash, but I think that would actually be preferable.  How's that sound?
Comment 4 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2006-10-05 19:51:51 PDT
(In reply to comment #3)
> With our newfangled alpha support, we could even draw the actual update region
> in a translucent color, overlaying the page content.  It would be a permanent
> thing on the rendered display, as opposed to just a flash, but I think that
> would actually be preferable.  How's that sound?

Wouldn't that mean that if I paint area A and then area B, both A and B have the "update" overlay, so I can't tell which one was the last one to be repainted?

It's hard to do a temporary flash from non-widget code because that stuff's all double buffered in the normal paint path. We could create a rendering context and do an immediate draw but I'd actually like to eliminate drawing outside of the paint event. I'm not sure what the right way to go is.
Comment 5 Boris Zbarsky [:bz] 2006-10-05 20:31:56 PDT
> It would be a permanent thing on the rendered display

That's not really desirable, because then we'd just end up with the whole window showing it...  In my experience paint flashin is most useful for things like scrolling, DHTML, UI interaction, where every so often the whole thing gets invalidated.
Comment 6 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2006-10-05 20:50:47 PDT
For Xgl there's a little tool that does this for any and all applications (probably works in aiglx too). The only problem is that it shows damage due to scrolling as well as invalidation, which I don't want, but that may not worry other people. I think OSX has something similar.

A related question is whether we want the app to pause while flashing. Currently it does, which can be good and bad...
Comment 7 Mike Shaver (:shaver -- probably not reading bugmail closely) 2010-11-02 23:51:12 PDT
Do we really still not have paint flashing on X?
Comment 8 Boris Zbarsky [:bz] 2010-11-03 00:01:09 PDT
Last I checked, we do not.

Nor do we really have it on Mac, though QuartzDebug can help with that a bit.
Comment 9 Joe Drew (not getting mail) 2011-10-07 13:27:20 PDT
Paint flashing belongs more in layers now than in Cairo drawing code; IIRC we have something approximating it too.
Comment 10 Jet Villegas (:jet) 2011-10-18 16:59:53 PDT
I don't see anything approximating this in the code. I'm going to try hacking this up as a learning exercise. I don't see a logical bottleneck in the Layer classes and my plan is to tack it on to gfxContext::Paint(). Will that work?
Comment 11 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2011-10-18 18:11:26 PDT
This probably needs to be done as part of bug 690064.
Comment 12 Bas Schouten (:bas.schouten) 2011-11-13 15:39:49 PST
Created attachment 574190 [details] [diff] [review]
Introduce paint flashing

This patch introduces 'paint flashing' what is does is it draws a transparent 'shade' in a random color (varying per paint) over the area that was invalidated.
Comment 13 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2011-11-13 15:56:43 PST
Comment on attachment 574190 [details] [diff] [review]
Introduce paint flashing

Review of attachment 574190 [details] [diff] [review]:
-----------------------------------------------------------------

::: layout/base/FrameLayerBuilder.cpp
@@ +2130,5 @@
>      aContext->Restore();
>    }
> +
> +#ifdef DEBUG
> +  if (Preferences::GetBool("nglayout.debug.paint_flashing", false)) {

Use AddBoolVarCache and make this non-#ifdef DEBUG
Comment 14 Bas Schouten (:bas.schouten) 2011-11-14 20:12:45 PST
Created attachment 574521 [details] [diff] [review]
Introduce paint flashing v2

Updated as per review comments.
Comment 15 Ed Morley [:emorley] 2011-11-15 11:51:42 PST
https://hg.mozilla.org/mozilla-central/rev/6235e0c97c3b

To save time/mistakes when merging, please could you set assignee + target milestone when landing on inbound (https://wiki.mozilla.org/Tree_Rules/Inbound#Please_do_the_following_after_pushing_to_inbound).

Thanks :-)
Comment 16 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2012-02-05 20:32:18 PST
Created attachment 594628 [details] [diff] [review]
Add nglayout.debug.paint_flashing to all.js

We should have done this originally
Comment 17 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2012-02-07 15:18:32 PST
https://hg.mozilla.org/integration/mozilla-inbound/rev/677bf71d3030
Comment 18 Ed Morley [:emorley] 2012-02-08 09:01:56 PST
https://hg.mozilla.org/mozilla-central/rev/677bf71d3030
Comment 19 Ed Morley [:emorley] 2012-02-10 12:29:29 PST
*** Bug 723716 has been marked as a duplicate of this bug. ***

Note You need to log in before you can comment on or make changes to this bug.