Re: Comments on clipping fix (118317)



Owen Taylor <otaylor redhat com> writes:

> Comments on the patch:
> 
> ===
>        gc_values.fill = GDK_TILED;
>        gc_values.tile = private->bg_pixmap;
> -      gc_values.ts_x_origin = - paint->x_offset;
> -      gc_values.ts_y_origin = - paint->y_offset;
> +      gc_values.ts_x_origin = -paint->x_offset;
> +      gc_values.ts_y_origin = -paint->y_offset;
> ===
>    
> Probably wouldn't complain if this was new code, but the spaces
> are definitely supposed to be there in the GTK+ coding style.
> (For unary minus, not for negative numbers, of course.)

Okay. I have changed it back in the new patch, and also fixed other
instances of this problem.

> The change in the x_offset/y_offset for the other paints doesn't 
> make sense to me - the offset is an offset between the backing
> pixmap and the window. It's the amount we offset our drawing 
> operations by.

Fixed in the new version.

> I think clipping the clear to the actual region is worthwhile - the
> regions are not infrequently very sparse, for an opaque move resize,
> we might frequently have a 500x500 region of which only 
> two 5x500 stripes are there.

Done.

> This change looks like compensation for the change I commented
> on above.

Yes.

> Having the paint_clipbox involved here looks wrong to me -
> I would expect:
> 
>     gdk_draw_drawable (tmp_pixmap, tmp_gc, paint->pixmap,
> 		       clipbox.x + paint->x_offset,
> 		       clipbox.y + paint->y_offset,
> 		       clipbox.x, clipbox.y,
> 		       clipbox.width, clipbox.height);

This was another complication from the change in paint offset. The new
version is much simpler.

One other change:

+  paint->pixmap =
+    gdk_pixmap_new (window,
+		    MAX (clip_box.width, 1), MAX (clip_box.height, 1), -1);

is necessary unless begin/end_paint() with an empty region is
illegal. (Only relevant for explicit begin/end_paints()).

Søren

? head-unchanged-gtk+-diff
? paintstack-2.patch
? patches
? demos/gtk-demo/geninclude.pl
Index: gdk/gdkwindow.c
===================================================================
RCS file: /cvs/gnome/gtk+/gdk/gdkwindow.c,v
retrieving revision 1.150
diff -u -r1.150 gdkwindow.c
--- gdk/gdkwindow.c	9 Jul 2003 18:00:21 -0000	1.150
+++ gdk/gdkwindow.c	20 Aug 2003 15:08:17 -0000
@@ -168,6 +168,11 @@
 static void gdk_window_init       (GdkWindowObject      *window);
 static void gdk_window_class_init (GdkWindowObjectClass *klass);
 static void gdk_window_finalize   (GObject              *object);
+static void gdk_window_clear_backing_rect (GdkWindow *window,
+					   gint       x,
+					   gint       y,
+					   gint       width,
+					   gint       height);
 
 static gpointer parent_class = NULL;
 
@@ -847,7 +852,8 @@
 }
 
 static GdkGC *
-gdk_window_get_bg_gc (GdkWindow *window, GdkWindowPaint *paint)
+gdk_window_get_bg_gc (GdkWindow      *window,
+		      GdkWindowPaint *paint)
 {
   GdkWindowObject *private = (GdkWindowObject *)window;
 
@@ -883,24 +889,6 @@
   return gdk_gc_new_with_values (paint->pixmap, &gc_values, gc_mask);
 }
 
-static void
-gdk_window_paint_init_bg (GdkWindow      *window,
-			  GdkWindowPaint *paint,
-			  GdkRegion      *init_region)
-{
-  GdkGC *tmp_gc;
-  
-  tmp_gc = gdk_window_get_bg_gc (window, paint);
-
-  gdk_region_offset (init_region,
-                     - paint->x_offset,
-                     - paint->y_offset);
-  gdk_gc_set_clip_region (tmp_gc, init_region);
-
-  gdk_draw_rectangle (paint->pixmap, tmp_gc, TRUE, 0, 0, -1, -1);
-  g_object_unref (tmp_gc);
-}
-
 #ifdef GDK_WINDOWING_X11
 #include "x11/gdkx.h"
 #endif
@@ -958,95 +946,39 @@
   GdkWindowObject *private = (GdkWindowObject *)window;
   GdkRectangle clip_box;
   GdkWindowPaint *paint;
-  GdkRegion *init_region;
-  GdkGC *tmp_gc;
+  GSList *list;
   
   g_return_if_fail (window != NULL);
   g_return_if_fail (GDK_IS_WINDOW (window));
 
   if (GDK_WINDOW_DESTROYED (window))
     return;
-  
-  paint = g_new (GdkWindowPaint, 1);
 
-  paint->region = gdk_region_copy (region);
+  gdk_region_get_clipbox (region, &clip_box);
 
-  init_region = gdk_region_copy (region);
-  gdk_region_get_clipbox (paint->region, &clip_box);
+  paint = g_new (GdkWindowPaint, 1);
+  paint->region = gdk_region_copy (region);
+  paint->x_offset = clip_box.x;
+  paint->y_offset = clip_box.y;
+  paint->pixmap =
+    gdk_pixmap_new (window,
+		    MAX (clip_box.width, 1), MAX (clip_box.height, 1), -1);
 
-  if (private->paint_stack)
+  for (list = private->paint_stack; list != NULL; list = list->next)
     {
-      gint old_width, old_height;
-      GdkWindowPaint *tmp_paint = private->paint_stack->data;
-      GdkRectangle old_rect, new_rect;
-      GSList *tmp_list;
-
-      gdk_drawable_get_size (tmp_paint->pixmap, &old_width, &old_height);
-      old_rect.x = tmp_paint->x_offset;
-      old_rect.y = tmp_paint->y_offset;
-      old_rect.width = old_width;
-      old_rect.height = old_height;
-
-      gdk_rectangle_union (&clip_box, &old_rect, &new_rect);
-
-      if (new_rect.width > old_rect.width || new_rect.height > old_rect.height)
-	{
-	  paint->pixmap = gdk_pixmap_new (window,
-                                          new_rect.width, new_rect.height, -1);
-          tmp_gc = gdk_gc_new (paint->pixmap);
-	  gdk_draw_drawable (paint->pixmap, tmp_gc, tmp_paint->pixmap,
-			     0, 0,
-			     old_rect.x - new_rect.x, old_rect.y - new_rect.y,
-                             old_rect.width, old_rect.height);
-          g_object_unref (tmp_gc);
-	  g_object_unref (tmp_paint->pixmap);
-
-	  paint->x_offset = new_rect.x;
-	  paint->y_offset = new_rect.y;
-	  
-	  tmp_list = private->paint_stack;
-	  while (tmp_list)
-	    {
-	      tmp_paint = tmp_list->data;
-	      gdk_region_subtract (init_region, tmp_paint->region);
-
-	      tmp_paint->pixmap = paint->pixmap;
-	      tmp_paint->x_offset = paint->x_offset;
-	      tmp_paint->y_offset = paint->y_offset;
-              
-	      tmp_list = tmp_list->next;
-	    }
-	}
-      else
-	{
-	  paint->x_offset = tmp_paint->x_offset;
-	  paint->y_offset = tmp_paint->y_offset;
-	  paint->pixmap = tmp_paint->pixmap;
-
-	  tmp_list = private->paint_stack;
-	  while (tmp_list)
-	    {
-	      tmp_paint = tmp_list->data;
-	      gdk_region_subtract (init_region, tmp_paint->region);
+      GdkWindowPaint *tmp_paint = list->data;
 
-	      tmp_list = tmp_list->next;
-	    }
-	}
+      gdk_region_subtract (tmp_paint->region, paint->region);
     }
-  else
-    {
-      paint->x_offset = clip_box.x;
-      paint->y_offset = clip_box.y;
-      paint->pixmap = gdk_pixmap_new (window, 
-                                      clip_box.width, clip_box.height, -1);
-    }
-
-  if (!gdk_region_empty (init_region))
-    gdk_window_paint_init_bg (window, paint, init_region);
-  
-  gdk_region_destroy (init_region);
   
   private->paint_stack = g_slist_prepend (private->paint_stack, paint);
+
+  if (!gdk_region_empty (region))
+    {
+      gdk_window_clear_backing_rect (window,
+				     clip_box.x, clip_box.y,
+				     clip_box.width, clip_box.height);
+    }
 #endif /* USE_BACKING_STORE */
 }
 
@@ -1096,29 +1028,16 @@
   _gdk_windowing_window_get_offsets (window, &x_offset, &y_offset);
 
   gdk_gc_set_clip_region (tmp_gc, paint->region);
-  gdk_gc_set_clip_origin (tmp_gc, -x_offset, -y_offset);
+  gdk_gc_set_clip_origin (tmp_gc, - x_offset, - y_offset);
 
   gdk_draw_drawable (private->impl, tmp_gc, paint->pixmap,
                      clip_box.x - paint->x_offset,
                      clip_box.y - paint->y_offset,
                      clip_box.x - x_offset, clip_box.y - y_offset,
                      clip_box.width, clip_box.height);
-  g_object_unref (tmp_gc);
-
-  if (private->paint_stack)
-    {
-      GSList *tmp_list = private->paint_stack;
-      while (tmp_list)
-	{
-	  GdkWindowPaint *tmp_paint = tmp_list->data;
-	  gdk_region_subtract (tmp_paint->region, paint->region);
-          
-	  tmp_list = tmp_list->next;
-	}
-    }
-  else
-    g_object_unref (paint->pixmap);
 
+  g_object_unref (tmp_gc);
+  g_object_unref (paint->pixmap);
   gdk_region_destroy (paint->region);
   g_free (paint);
 #endif /* USE_BACKING_STORE */
@@ -1430,52 +1349,22 @@
                                    gint        *composite_y_offset)
 {
   GdkWindowObject *private = (GdkWindowObject *)drawable;
-  GdkWindowPaint *paint;
-  GdkRegion *buffered_region;
-  GSList *tmp_list;
-  GdkPixmap *buffer;
+  GSList *list;
   GdkPixmap *tmp_pixmap;
   GdkRectangle rect;
-  GdkRegion *rect_region;
   GdkGC *tmp_gc;
-  gint windowing_x_offset, windowing_y_offset;
-  gint buffer_x_offset, buffer_y_offset;
+  gboolean overlap_buffer;
 
+  _gdk_windowing_window_get_offsets (drawable,
+				     composite_x_offset,
+				     composite_y_offset);
+  
   if ((GDK_IS_WINDOW (drawable) && GDK_WINDOW_DESTROYED (drawable))
       || private->paint_stack == NULL)
     {
       /* No backing store */
-      _gdk_windowing_window_get_offsets (drawable,
-                                         composite_x_offset,
-                                         composite_y_offset);
-      
       return g_object_ref (drawable);
     }
-  
-  buffered_region = NULL;
-  buffer = NULL;
-
-  /* All GtkWindowPaint structs have the same pixmap and offsets, just
-   * get the first one. (should probably be cleaned up so that the
-   * pixmap is stored in the window)
-   */
-  paint = private->paint_stack->data;
-  buffer = paint->pixmap;
-  buffer_x_offset = paint->x_offset;
-  buffer_y_offset = paint->y_offset;
-  
-  tmp_list = private->paint_stack;
-  while (tmp_list != NULL)
-    {
-      paint = tmp_list->data;
-      
-      if (buffered_region == NULL)
-        buffered_region = gdk_region_copy (paint->region);
-      else
-        gdk_region_union (buffered_region, paint->region);
-
-      tmp_list = g_slist_next (tmp_list);
-    }
 
   /* See if the buffered part is overlapping the part we want
    * to get
@@ -1485,65 +1374,64 @@
   rect.width = width;
   rect.height = height;
 
-  rect_region = gdk_region_rectangle (&rect);
+  overlap_buffer = FALSE;
   
-  gdk_region_intersect (buffered_region, rect_region);
-
-  gdk_region_destroy (rect_region);
-
-  if (gdk_region_empty (buffered_region))
+  for (list = private->paint_stack; list != NULL; list = list->next)
     {
-      gdk_region_destroy (buffered_region);
+      GdkWindowPaint *paint = list->data;
+      GdkOverlapType overlap;
 
-      _gdk_windowing_window_get_offsets (drawable,
-                                         composite_x_offset,
-                                         composite_y_offset);
+      overlap = gdk_region_rect_in (paint->region, &rect);
 
-      return g_object_ref (drawable);
+      if (overlap == GDK_OVERLAP_RECTANGLE_IN)
+	{
+	  *composite_x_offset = paint->x_offset;
+	  *composite_y_offset = paint->y_offset;
+	  
+	  return g_object_ref (paint->pixmap);
+	}
+      else if (overlap == GDK_OVERLAP_RECTANGLE_PART)
+	{
+	  overlap_buffer = TRUE;
+	  break;
+	}
     }
-  
-  tmp_pixmap = gdk_pixmap_new (drawable,
-                               width, height,
-                               -1);
 
+  if (!overlap_buffer)
+    return g_object_ref (drawable);
+
+  tmp_pixmap = gdk_pixmap_new (drawable, width, height, -1);
   tmp_gc = gdk_gc_new (tmp_pixmap);
 
-  _gdk_windowing_window_get_offsets (drawable,
-                                     &windowing_x_offset,
-                                     &windowing_y_offset);
-  
   /* Copy the current window contents */
   gdk_draw_drawable (tmp_pixmap,
                      tmp_gc,
                      private->impl,
-                     x - windowing_x_offset,
-                     y - windowing_y_offset,
+                     x - *composite_x_offset,
+                     y - *composite_y_offset,
                      0, 0,
                      width, height);
 
-  /* Make buffered_region relative to the tmp_pixmap */
-  gdk_region_offset (buffered_region,
-                     - x,
-                     - y);
-  
-  /* Set the clip mask to avoid drawing over non-buffered areas of
-   * tmp_pixmap. 
-   */
-  
-  gdk_gc_set_clip_region (tmp_gc, buffered_region);
-  gdk_region_destroy (buffered_region);
-  
-  /* Draw backing pixmap onto the tmp_pixmap, offsetting
-   * appropriately.
-   */
-  gdk_draw_drawable (tmp_pixmap,
-                     tmp_gc,
-                     buffer,
-                     x - buffer_x_offset,
-                     y - buffer_y_offset,
-                     0, 0,
-                     width, height);
-  
+  /* paint the backing stores */
+  for (list = private->paint_stack; list != NULL; list = list->next)
+    {
+      GdkWindowPaint *paint = list->data;
+      GdkRegion *clip_region;
+
+      clip_region = gdk_region_rectangle (&rect);
+      gdk_region_intersect (clip_region, paint->region);
+      gdk_region_offset (clip_region, - x, - y);
+
+      gdk_gc_set_clip_region (tmp_gc, clip_region);
+      
+      gdk_draw_drawable (tmp_pixmap, tmp_gc, paint->pixmap,
+			 x - paint->x_offset,
+			 y - paint->y_offset,
+			 0, 0, width, height);
+
+      gdk_region_destroy (clip_region);
+    }
+
   /* Set these to location of tmp_pixmap within the window */
   *composite_x_offset = x;
   *composite_y_offset = y;
@@ -1781,7 +1669,6 @@
   RESTORE_GC (gc);
 }
 
-/* Fixme - this is just like gdk_window_paint_init_bg */
 static void
 gdk_window_clear_backing_rect (GdkWindow *window,
 			       gint       x,
@@ -1795,10 +1682,16 @@
 
   if (GDK_WINDOW_DESTROYED (window))
     return;
-  
+
   tmp_gc = gdk_window_get_bg_gc (window, paint);
+
+  gdk_region_offset (paint->region, paint->x_offset, paint->y_offset);
+  gdk_gc_set_clip_region (tmp_gc, paint->region);
+  gdk_region_offset (paint->region, - paint->x_offset, - paint->y_offset);
+  
   gdk_draw_rectangle (paint->pixmap, tmp_gc, TRUE,
 		      x - paint->x_offset, y - paint->y_offset, width, height);
+
   g_object_unref (tmp_gc);
 }
 
@@ -2393,7 +2286,7 @@
 
 		  /* This copy could be saved with a little more complexity */
 		  child_region = gdk_region_copy (visible_region);
-		  gdk_region_offset (child_region, -x, -y);
+		  gdk_region_offset (child_region, - x, - y);
 		  
 		  gdk_window_invalidate_maybe_recurse ((GdkWindow *)child, child_region, child_func, user_data);
 		  
Index: tests/testgtk.c
===================================================================
RCS file: /cvs/gnome/gtk+/tests/testgtk.c,v
retrieving revision 1.336
diff -u -r1.336 testgtk.c
--- tests/testgtk.c	10 Aug 2003 23:37:27 -0000	1.336
+++ tests/testgtk.c	20 Aug 2003 15:09:28 -0000
@@ -2346,8 +2346,8 @@
       GtkWidget *sw;
       GtkWidget *src;
       GtkWidget *snap;
-      GtkWidget *vbox;
       GtkWidget *hbox;
+      GtkWidget *vbox;
       GtkWidget *button;
       struct GetImageData *gid;
 
@@ -2368,9 +2368,9 @@
                               gid,
                               g_free);
       
-      vbox = gtk_vbox_new (FALSE, 0);
+      hbox = gtk_hbox_new (FALSE, 0);
       
-      gtk_container_add (GTK_CONTAINER (window), vbox);
+      gtk_container_add (GTK_CONTAINER (window), hbox);
       
       sw = gtk_scrolled_window_new (NULL, NULL);
       gtk_scrolled_window_set_policy (GTK_SCROLLED_WINDOW (sw),
@@ -2394,11 +2394,11 @@
       gtk_scrolled_window_add_with_viewport (GTK_SCROLLED_WINDOW (sw),
                                              src);
       
-      gtk_box_pack_start (GTK_BOX (vbox),
+      gtk_box_pack_start (GTK_BOX (hbox),
                           sw, TRUE, TRUE, 0);                          
 
 
-      hbox = gtk_hbox_new (FALSE, 3);
+      vbox = gtk_vbox_new (FALSE, 3);
 
       snap = gtk_widget_new (GTK_TYPE_IMAGE, NULL);
 
@@ -2412,7 +2412,7 @@
       
       gtk_scrolled_window_add_with_viewport (GTK_SCROLLED_WINDOW (sw), snap);
 
-      gtk_box_pack_end (GTK_BOX (hbox), sw, FALSE, FALSE, 5);
+      gtk_box_pack_end (GTK_BOX (vbox), sw, FALSE, FALSE, 5);
 
       button = gtk_button_new_with_label ("Get image from drawable");
 
@@ -2421,9 +2421,9 @@
                         G_CALLBACK (take_snapshot),
                         gid);
       
-      gtk_box_pack_start (GTK_BOX (hbox), button, FALSE, FALSE, 0);
+      gtk_box_pack_start (GTK_BOX (vbox), button, FALSE, FALSE, 0);
 
-      gtk_box_pack_end (GTK_BOX (vbox), hbox, FALSE, FALSE, 0);
+      gtk_box_pack_end (GTK_BOX (hbox), vbox, FALSE, FALSE, 0);
       
       gtk_widget_show_all (window);
     }


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]