Re: canvas change (dither support) again

[Federico Mena Quintero <federico helixcode com>]

mike engber <engber eazel com> writes:

> Below is a revised patch - now it includes the doc changes Miguel
> suggested.

Thanks!  A few comments interspersed in the patch:

> Index: gnome-canvas.c
> ===================================================================
> RCS file: /cvs/gnome/gnome-libs/libgnomeui/gnome-canvas.c,v
> retrieving revision 1.93.4.13
> diff -p -u -r1.93.4.13 gnome-canvas.c
> --- gnome-canvas.c	2000/06/29 04:20:38	1.93.4.13
> +++ gnome-canvas.c	2000/11/27 22:04:18
> @@ -3272,7 +3272,7 @@ paint (GnomeCanvas *canvas)
>  							    (draw_y1 - DISPLAY_Y1 (canvas)
>  							     + canvas->zoom_yofs),
>  							    width, height,
> -							    GDK_RGB_DITHER_NONE,
> +							    canvas->dither,
>  							    buf.buf,
>  							    IMAGE_WIDTH_AA * 3);
>  				}
> @@ -4136,4 +4136,39 @@ gnome_canvas_set_stipple_origin (GnomeCa
>  	g_return_if_fail (gc != NULL);
>  
>  	gdk_gc_set_ts_origin (gc, -canvas->draw_xofs, -canvas->draw_yofs);
> +}
> +
> +/**
> + * gnome_canvas_set_dither:
> + * @canvas: A canvas.
> + * @dither: Type of dither used to render an aa canvas.
> + *
> + * Controls dithered rendering for aa canvases. The value of dither
> + * should be; GDK_RGB_DITHER_NONE, GDK_RGB_DITHER_NORMAL, or
> + * GDK_RGB_DITHER_MAX. The default canvas setting is GDK_RGB_DITHER_NONE.
> + **/

Please put a hash (#) in front of the constant names so that gtk-doc
will be able to create references to their descriptions in the (ha!
nonexistent!) documentation in GTK+.

(It may actually pick up references to the description in the
gdk-pixbuf docs...)

Also, substitute "aa" for "antialiased".

> +/**
> + * gnome_canvas_get_dither:
> + * @canvas: A canvas.
> + *
> + * Returns the type of dithering used to render an aa canvas.
> + * 
> + * Return value: The dither setting.
> + **/
> +GdkRgbDither
> +gnome_canvas_get_dither (GnomeCanvas *canvas)
> +{
> +	g_return_if_fail (canvas != NULL);
> +	g_return_if_fail (GNOME_IS_CANVAS (canvas));
> +
> +	return canvas->dither;
>  }

This is wrong.  Please use g_return_val_if_fail() with
GDK_RGB_DITHER_NONE as the return value.

Also, in gnome_canvas_init() please make the default dither type be
GDK_RGB_DITHER_NORMAL.  This will make it do the right thing most of
the time even if you do not set the dither mode by hand.  Also, make
sure you change the description of the default value in the inline
docs for gnome_canvas_set_dither().

I guess with these changes I will not have any objections to the
patch.  Can you please mail the next revision to gnome-hackers just so
I can take the final look?

Thanks for working on this!

  Federico