Re: [PATCH] updates to canvas fixes

[Federico Mena Quintero <federico ximian com>]

Rusty Conover <rconover zootweb com> writes:

> The last patch I sent out that contained a fix for stroking the
> outlines of rectangle canvas items had a bug in it.  This patch
> superseces the previous one.

Looks good!  Please feel free to apply it when you take care of the
comments below :-)

> diff -u ../../gnome-libs-1.2.11/libgnomeui/gnome-canvas-line.c ./gnome-canvas-line.c
> --- ../../gnome-libs-1.2.11/libgnomeui/gnome-canvas-line.c	Sat Feb  3 18:42:07 2001
> +++ ./gnome-canvas-line.c	Sat Feb  3 18:32:38 2001
> @@ -771,6 +771,14 @@
>  		if (!item->canvas->aa)
>  			set_line_gc_foreground (line);
>  
> +
> +		if(line->first_svp) {
> +			gnome_canvas_item_request_redraw_svp(item, line->first_svp);
> +		}
> +		if(line->last_svp) {
> +			gnome_canvas_item_request_redraw_svp(item, line->last_svp);
> +		}

Please make these read

		if (line->first_svp)
			gnome_canvas_item_request_redraw_svp (item, line->first_svp);

		if (line->last_svp)
			gnome_canvas_item_request_redraw_svp (item, line->last_svp);

I.e. no braces for single-line clauses, space between the "if" and the
paren, and likewise between the function name and the paren.  Please
apply similar changes to the rest of the patch.  This is just me being
neurotic, but I want the code to be consistent :-)


> diff -u ../../gnome-libs-1.2.11/libgnomeui/gnome-canvas-rect-ellipse.c ./gnome-canvas-rect-ellipse.c
> --- ../../gnome-libs-1.2.11/libgnomeui/gnome-canvas-rect-ellipse.c	Sat Feb  3 18:42:08 2001
> +++ ./gnome-canvas-rect-ellipse.c	Sun Feb  4 18:01:48 2001
> @@ -860,6 +860,7 @@
>  	GnomeCanvasRE *re;
>  	ArtVpath vpath[11];
>  	ArtVpath *vpath2;
> +	ArtSVP *stroke_svp;
>  	double x0, y0, x1, y1;
>  	double dx, dy;
>  	double halfwidth;
> @@ -916,7 +917,55 @@
>  		} else
>  			gnome_canvas_item_update_svp (item, &re->fill_svp, NULL);
>  
> +
>  		if (re->outline_set) {
> +			/* The previous implementation here someone
> +			 * decided it would be better to do the
> +			 * stroking by hand, but it * dosen't work
> +			 * right when the canvas is zoomed, so * I'm
> +			 * letting libart do the stroking properly */
> +
> +			 /* If the item is filled, the vpath will already be built,
> +			    so whats the point in rebuilding it?  If its not already
> +			    built then lets build it. */
> +                       
> +			if(!re->fill_set) {
> +				vpath[0].code = ART_MOVETO;
> +				vpath[0].x = x0;
> +				vpath[0].y = y0;
> +				vpath[1].code = ART_LINETO;
> +				vpath[1].x = x0;
> +				vpath[1].y = y1;
> +				vpath[2].code = ART_LINETO;
> +				vpath[2].x = x1;
> +				vpath[2].y = y1;
> +				vpath[3].code = ART_LINETO;
> +				vpath[3].x = x1;
> +				vpath[3].y = y0;
> +				vpath[4].code = ART_LINETO;
> +				vpath[4].x = x0;
> +				vpath[4].y = y0;
> +				vpath[5].code = ART_END;
> +				vpath[5].x = 0;
> +				vpath[5].y = 0;
> +			}
> +
> +			vpath2 = art_vpath_affine_transform(vpath, affine);
> +			
> +			stroke_svp = art_svp_vpath_stroke (vpath2,
> +                                                           ART_PATH_STROKE_JOIN_MITER,
> +                                                           ART_PATH_STROKE_CAP_BUTT,
> +                                                           (re->width_pixels) ? re->width : (re->width * item->canvas->pixels_per_unit),
> +                                                           4,
> +                                                           0.25);
> +                        
> +                        gnome_canvas_item_update_svp_clip (item, &re->outline_svp, stroke_svp, clip_path);
> +                        art_free (vpath2);
> +			
> +			
> +
> +
> +#if 0
>  			/* We do the stroking by hand because it's simple enough
>  			   and could save time. */
>  
> @@ -981,6 +1030,7 @@
>  
>  			gnome_canvas_item_update_svp_clip (item, &re->outline_svp, art_svp_from_vpath (vpath2), clip_path);
>  			art_free (vpath2);
> +#endif

Gorgeous.  Feel free to remove the "#if 0"-ed part.

> +/**
> + * gnome_canvas_set_state_change_sensitive:
> + * @canvas: A canvas.
> + * @sensitivity: boolean indicating if this canvas should be sensitive to state changes, by forcing a repaint
> + * 
> + * Sets the sensitivity of the canvas widget to state changes, by forcing a full repaint.
> + **/
> +void gnome_canvas_set_state_change_sensitivity (GnomeCanvas *canvas, gboolean sensitive) {
> +	g_return_if_fail (canvas != NULL);
> +	g_return_if_fail (GNOME_IS_CANVAS (canvas));	
> +	canvas->state_change_sensitive = sensitive;
>  }

Should it redraw itself here?  But see below first.

> Only in .: gnome-canvas.c~
> diff -u ../../gnome-libs-1.2.11/libgnomeui/gnome-canvas.h ./gnome-canvas.h
> --- ../../gnome-libs-1.2.11/libgnomeui/gnome-canvas.h	Sat Feb  3 18:42:08 2001
> +++ ./gnome-canvas.h	Sat Feb  3 18:32:30 2001
> @@ -486,6 +486,10 @@
>  
>  	/* dither mode for aa drawing */
>  	unsigned int dither : 2;
> +
> +	/* Whether the canvas should be sensitive to state changes,
> +	   and if state changes force a rerender */
> +	unsigned int state_change_sensitive : 1;
>  };

I am not sure about this.  We already have 8 bits in bitfields, so
adding one more bit could potentially change the size of the padded
structure :-(

Maybe we could tag a private structure to the canvas as a piece of
GTK+ object data and retrieve that as needed.  ZVT did this for some
time; maybe it still does it.

Rusty, thanks a lot for your patches!  Please apply this thing except
for the sensitivity part, which we have to discuss a bit it seems.

  Federico