Re: Thoughts about a new GdkPixbuf transform interface

[Oleg Klimov <quif land ru>]

Owen Taylor wrote:

> - No support for rotation (or other arbitrary transforms)
I'm sorry to say this, but you could mention already working 
implementtion of arbitrary transforms:
http://bugzilla.gnome.org/show_bug.cgi?id=105794

Anyway, I'm glad there's some attention.

> What I was thinking of:
>
> GdkPixbufTransform *gdk_pixbuf_transform_new (GdkPixbuf *source);
>
> void       gdk_pixbuf_transform_rotate          (GdkPixbufTransform *transform,
> 						 gdouble             angle);
> void       gdk_pixbuf_transform_scale           (GdkPixbufTransform *transform,
> 						 gdouble             scale_x,
> 						 gdouble             scale_y);
> void       gdk_pixbuf_transform_offset          (GdkPixbufTransform *transform,
> 						 gdouble             offset_x,
> 						 gdouble             offset_y);
That's fine for me.

> void       gdk_pixbuf_transform_set_affine      (GdkPixbufTransform *transform,
> 						 double              a00,
> 						 double              a01,
> 						 double              a02,
> 						 double              a10,
> 						 double              a11,
> 						 double              a12);
What is "affine" anyway?
Natural values for arbitrary transform is offset + 2x2 matrix, as in 
formula:

| dest_x |   | offset_x |     | m00 m01 | | src_x |
| dest_y | = | offset_y |  +  | m10 m11 |*| src_y |

all above operations are easy to represent in this terms, and combine.
(rotate, scale, offset)

    | cos(angle) -sin(angle) |
m *= | sin(angle)  cos(angle) |

m *= scale

         | add_offset_x |
offset += | add_offset_y |

Not only easier implementation, it's easier to understand.

> /* Composite onto a pixbuf */
> void       gdk_pixbuf_transform_composite       (GdkPixbufTransform *transform,
> 						 GdkPixbuf          *dest,
> 						 int                 render_x,
> 						 int                 render_y,
> 						 int                 render_width,
> 						 int                 render_height);
What's this? I just don't get it.
if "Composite" stands for
- "render", "do the transform", then why we need any more arguments? 
OK, some
constrains. How can I get the bounds of the result?
- "remove alpha channel using dst", so we actualy do reverse 
transformation?! Confusing.

> /* Convenience function to create a pixbuf from the bounding box of the
> * transformed source and composite_color() on it.
> */
> GdkPixbuf *gdk_pixbuf_transform_output          (GdkPixbufTransform *transform,
> 						 guint32             bg_color);
I'd prefer _rotate_simple, _scale_simple, etc.
Why anyone need bg_color? Window background works fine. Faster in some 
circumstances?
It's no good to inculde every very special case in API.

> typedef enum GdkPixbufEdgeMode {
>  GDK_PIXBUF_EDGE_NEAREST,
>  GDK_PIXBUF_EDGE_TRANSPARENT,
>  GDK_PIXBUF_EDGE_TILE
> };
>
> /* Specify what to do for source pixels off the edge */
> void gdk_pixbuf_transform_set_edge_mode (GdkPixbufTransform *transform,
> 					 GdkPixbufEdgeMode   mode);
What's this?!
Absolutly no idea what this is all _about_.

>  /* Double the size of a pixbuf and rotate by 90 degrees
>   */
>  GdkPixbufTransform *transform = gdk_pixbuf_transform_new ();
>  gdk_pixbuf_transform_scale (transform, 2., 2.);
>  gdk_pixbuf_transform_rotate (transform, 90.);
>  GdkPixbufTransform *result = gdk_pixbuf_transform_output (transform);
>  g_object_unref (transform);
Just confusing... Result is Transform, not Pixbuf?.. What for?..

>  /* Render a pixbuf at half size in 'dest', offset by 100 pixels
>   */
>  GdkPixbufTransform *transform = gdk_pixbuf_transform_new ();
>  gdk_pixbuf_transform_scale (transform, 0.5, 0.5);
>  gdk_pixbuf_transform_offset (transform, 100., 100.);
>  gdk_pixbuf_transform_composite (transform, dest,
> 				  100, 100, width/2, height/2);
>  g_object_unref (transform);
Fine. Here's another convinience function: rotate using src center 
coords to dst_xy

> Notable differences from the old API:
<..>

> - Checks feature of composite_color() removed. Was a silly demo-app thing,
>   and won't be significantly faster than just filling a destination
>   pixmap.
My suggestions:
- Remove any bg_color, composite, etc. It is very special-case.
I see no reason why someone wants to replace alpha channel with color
or checkerboard. Checkerboard effect: load one cell pixbuf, clone it on
another pixbuf large enough, transform the image, here you are. 20 lines 
of code.
- Think of adding real clipping area, instead of silly 
render_x/render_width
(if I get it right). This will make real impact on game-like stuff, improves
double-buffer code.