Re: Redrawing patch

[Owen Taylor <otaylor redhat com>]

Soeren Sandmann <sandmann daimi au dk> writes:

> Here is a patch that
> 
>     - makes --gtk-debug=updates more entertaining to look at and
>       only draw on the exact region that is being redrawn

Is redrawing every rectangle of the redraw region in a different
color really needed? It would be more compact to simply 
set the region as the clip on the GC and draw a single rectangle...
I don't want to waste _too_ much code on --gtk-debug=updates.
 
>     - fixes what I think is a typo in gdk_invalidate_maybe_recurse().
>       Instead of actually recursing, the function called
>       gdk_window_invalidate_region() with invalidate_children=TRUE
> 
>       The patch makes the function actually recurse down the window
>       tree, based on the child_func(). If it is really intended to
>       just "recurse" one level and then invalidate everything from
>       there, I think the documentation should have a note about it

Yes, this change looks right.
 
>     - makes GtkFrame !redraw_on_allocate and handle invalidations
>       itself in ->size_allocation()

Hmm, this part of it has some problems:

 - You don't want to redraw on every size allocation, just when
   things change ... size_allocate() will be called recursively
   down from the toplevel on every queue resize on any subwidget.

 - You don't handle invalidation when the widget moves (you
   may not have noticed that I took your advice on that and
   don't handle this automatically for no-window widgets
   if they are !redraw_on_allocate)

 - You only want to invalidate when MAPPED(), not REALIZED()

 - It's not invalidating enough ... if the child moves/resizes,
   we have to invalidate the union of the space used to draw
   the child before and the space afterwards.

 - It's invalidating probably more than you intended for
   GtkAspectFrame, where the drawn portion of the widget
   is shrinked wrapped around the child.

As long as we are invalidating the entire toplevel when we
resize that, I don't know if I see much of a point in
trying to micromanage the area of the frame that we
redraw.

(Not redrawing the entire toplevel would prevent themes
that use a gradient across entire toplevel windows.)

>       This uncovered a bug where there would sometimes be rounding
>       errors in a calculation in gtk_frame_paint(). I think this fix
>       should go in, even if the rest of the patch is rejected.

Yes, this fix looks fine.
 
>     - Makes GtkViewport !redraw_on_allocate. This is a really simple
>       change that makes the viewport not invalidate all of its child
>       if the child is a NO_WINDOW widget. 

Hmmm, it might be worth here invalidating only the 4 rectangles
of the outside area since we don't handle child clipping in
invalidation and this is a pretty egregious case where we'll
be redrawing more than we need to.

This change looks like it will make a big change to the speed
of the "resize the testgtk main window" benchmark, though I 
don't know if it matters that much for real apps, since 
GtkViewport isn't used that much. 

[ GtkViewport really should be NO_WINDOW since there is no point
in having a separate window to draw the shadow in.... ]

Regards,
                                        Owen