Re: Redrawing patch

[Soeren Sandmann <sandmann daimi au dk>]

Owen Taylor <otaylor redhat com> writes:

> 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.

Probably not. It was useful to me trying to understand how the resize
- invalidation - expose process actually worked, but I guess, for the
purpose of determining how much of ones widget is redrawn, one color
should be enough.

The attached new patch just sets the clip_region.

>  - 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 only want to invalidate when MAPPED(), not REALIZED()

Ok, these are fixed in the attached patch. (Although I don't see the
point in being that careful about avoiding invalidationas long as we
are only talking about four small rectangles, not a potentially large
number of drawing calls (as would be the case if the entire child was
invalidated)).

>  - 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)

The first call to gtk_frame_invalidate_border() invalidates the
frame area before the change, and the second invalidates the frame
area after the change. This should take care of redrawing all areas of
the frame that needs to be redrawn, both when the frame changes
position and when it moves.

Or am I missing something?

>  - 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.

I don't understand why this has to be done. Presumably, if the child
is a NO_WINDOW/ widget, then if it is a REDRAW_ON_ALLOCATE,
gtk_widget_size_allocate() will invalidate the union of it old and new
allocations, and if not, the child itself will have to invalidate its
redraw regions itself. If the child is a WINDOW widget,

   gdk_window_invalidate_rectangle (..., FALSE)

won't invalidate it anyway.

>  - 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.

As far as I can tell, the entire toplevel is redrawn, but not
invalidated on resize. The point of not invalidating the child area of
a frame is that expose events don't propagate to more (NO_WINDOW)
children than necessary. (See below about gradients).

So while I must admit that I didn't even consider GtkAspectFrame, I
don't think the extra invalidation will matter much, because the extra
invalid area is mostly empty space which is drawn (to the backing
store at least) anyway by the nearest WINDOW ancestor.

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

Urgh. This means all output windows must be _completely_ invalidated
on _every_ resize, whether the widget is redraw_on_alloc or not -
otherwise a theme that uses gradients won't work.

This makes set_redraw_on_allocate completely pointless - the only time
it will ever make a difference is when a NO_WINDOW widget is resized
and its closest WINDOW ancestor is not - that happens very rarely, and
almost never in situations where drawing is a bottleneck.

In Gtk+ HEAD as of now, toplevels are not invalidated on resize, and
so gradient themes are not working. I can see four possible solutions
(in order of preference):

        - add API to allow themes to override redraw_on_allocate. Ie,
          allow themes on a widget-by-widget basis to say "ok, you
          don't want to be invalidated on resize, but I'll override
          that and make sure you are invalidated anyway".

          This would allow gradient themes to work and make the
          non-gradient case fast.

        - leave redraw_on_allocate in GTK and disregard everywhere
          (ie, always invalidate, no matter what the flag says). The
          idea would be to add the theme overriding API in 2.2.

        - remove redraw_on_allocate entirely.

        - live with gradient themes not working.

Søren

Index: gdk/gdkwindow.c
===================================================================
RCS file: /cvs/gnome/gtk+/gdk/gdkwindow.c,v
retrieving revision 1.124
diff -u -p -r1.124 gdkwindow.c
--- gdk/gdkwindow.c	2001/11/04 22:57:02	1.124
+++ gdk/gdkwindow.c	2001/11/15 21:57:30
@@ -2151,6 +2151,34 @@ gdk_window_invalidate_rect   (GdkWindow 
   gdk_region_destroy (region);
 }
 
+static void
+draw_ugly_color (GdkWindow *window,
+		 GdkRegion *region)
+{
+  /* Draw ugly color all over the newly-invalid region */
+  GdkColor ugly_color = { 0, 50000, 10000, 10000 };
+  GdkGC *ugly_gc;
+  GdkRectangle clipbox;
+    
+  ugly_gc = gdk_gc_new (window);
+  gdk_gc_set_rgb_fg_color (ugly_gc, &ugly_color);
+  gdk_gc_set_clip_region (ugly_gc, region);
+
+  gdk_region_get_clipbox (region, &clipbox);
+  
+  gdk_draw_rectangle (window,
+		      ugly_gc,
+		      TRUE,
+		      clipbox.x, clipbox.y,
+		      clipbox.width, clipbox.height);
+  
+  g_object_unref (G_OBJECT (ugly_gc));
+
+  gdk_flush ();
+
+  g_usleep (10000);
+}      
+
 /**
  * gdk_window_invalidate_maybe_recurse:
  * @window: a #GdkWindow
@@ -2200,26 +2228,7 @@ gdk_window_invalidate_maybe_recurse (Gdk
   if (!gdk_region_empty (visible_region))
     {
       if (debug_updates)
-        {
-          /* Draw ugly color all over the newly-invalid region */
-          GdkRectangle ugly_rect;
-          GdkGC *ugly_gc;
-          GdkColor ugly_color = { 0, 60000, 10000, 10000 };
-          
-          ugly_gc = gdk_gc_new (window);
-
-          gdk_gc_set_rgb_fg_color (ugly_gc, &ugly_color);
-          
-	  gdk_region_get_clipbox (visible_region, &ugly_rect);
-
-          gdk_draw_rectangle (window,
-                              ugly_gc,
-                              TRUE,
-                              ugly_rect.x, ugly_rect.y,
-                              ugly_rect.width, ugly_rect.height);
-          
-          g_object_unref (G_OBJECT (ugly_gc));
-        }
+	draw_ugly_color (window, visible_region);
       
       if (private->update_area)
 	{
Index: gtk/gtkframe.c
===================================================================
RCS file: /cvs/gnome/gtk+/gtk/gtkframe.c,v
retrieving revision 1.41
diff -u -p -r1.41 gtkframe.c
--- gtk/gtkframe.c	2001/11/04 22:57:03	1.41
+++ gtk/gtkframe.c	2001/11/15 21:57:30
@@ -184,6 +184,8 @@ gtk_frame_class_init (GtkFrameClass *cla
 static void
 gtk_frame_init (GtkFrame *frame)
 {
+  gtk_widget_set_redraw_on_allocate (GTK_WIDGET (frame), FALSE);
+  
   frame->label_widget = NULL;
   frame->shadow_type = GTK_SHADOW_ETCHED_IN;
   frame->label_xalign = 0.0;
@@ -522,8 +524,10 @@ gtk_frame_paint (GtkWidget    *widget,
 	    xalign = 1 - frame->label_xalign;
 
 	  height_extra = MAX (0, child_requisition.height - widget->style->xthickness);
-	  y -= height_extra * (1 - frame->label_yalign);
-	  height += height_extra * (1 - frame->label_yalign);
+	  height_extra *= (1 - frame->label_yalign);
+
+	  y -= height_extra;
+	  height += height_extra;
 	  
 	  x2 = widget->style->xthickness + (frame->child_allocation.width - child_requisition.width - 2 * LABEL_PAD - 2 * LABEL_SIDE_PAD) * xalign + LABEL_SIDE_PAD;
 
@@ -594,31 +598,58 @@ gtk_frame_size_request (GtkWidget      *
 }
 
 static void
+gtk_frame_invalidate_border (GtkFrame *frame)
+{
+  GtkWidget *widget = GTK_WIDGET (frame);
+
+  if (GTK_WIDGET_MAPPED (widget))
+    {
+      GdkRegion *border;
+      GdkRegion *child_area;
+
+      border = gdk_region_rectangle (&(widget->allocation));
+      child_area = gdk_region_rectangle (&(frame->child_allocation));
+
+      gdk_region_subtract (border, child_area);
+
+      gdk_window_invalidate_region (widget->window, border, FALSE);
+
+      gdk_region_destroy (child_area);
+      gdk_region_destroy (border);
+    }
+}
+
+static void
 gtk_frame_size_allocate (GtkWidget     *widget,
 			 GtkAllocation *allocation)
 {
   GtkFrame *frame = GTK_FRAME (widget);
   GtkBin *bin = GTK_BIN (widget);
   GtkAllocation new_allocation;
+  gboolean need_invalidate = FALSE;
 
+  if (allocation->x != widget->allocation.x ||
+      allocation->y != widget->allocation.y ||
+      allocation->width != widget->allocation.width ||
+      allocation->height != widget->allocation.height)
+    {
+      need_invalidate = TRUE;
+    }
+
+  if (need_invalidate)
+    gtk_frame_invalidate_border (frame);
+  
   widget->allocation = *allocation;
 
   gtk_frame_compute_child_allocation (frame, &new_allocation);
   
-  /* If the child allocation changed, that means that the frame is drawn
-   * in a new place, so we must redraw the entire widget.
-   */
-  if (GTK_WIDGET_MAPPED (widget) &&
-      (new_allocation.x != frame->child_allocation.x ||
-       new_allocation.y != frame->child_allocation.y ||
-       new_allocation.width != frame->child_allocation.width ||
-       new_allocation.height != frame->child_allocation.height))
-    gdk_window_invalidate_rect (widget->window, &widget->allocation, FALSE);
-  
   if (bin->child && GTK_WIDGET_VISIBLE (bin->child))
     gtk_widget_size_allocate (bin->child, &new_allocation);
   
   frame->child_allocation = new_allocation;
+
+  if (need_invalidate)
+    gtk_frame_invalidate_border (frame);
   
   if (frame->label_widget && GTK_WIDGET_VISIBLE (frame->label_widget))
     {