Re: Thoughts about a new GdkPixbuf transform interface

[Owen Taylor <otaylor redhat com>]

On Wed, 2003-02-19 at 04:16, Oleg Klimov wrote:
> 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.

Sorry ... it didn't occur to me to mention your work because:

 - My mail was about a new API, and your patch closely matches
   the old API.

 - My mail was about API, not implementation

 - Even considering implementation, your patch implements only
   the NEAREST filter, and (no offense) that's the easy part.
   From doing pixops.c I'd estimate that filtering scaling is 
   easily 20-30 times as much work to implement as NEAREST.

   So, it didn't seem immediately reusable.

But I should have mentioned it anyways. My apologies.

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

As other people have said, that's an affine transform. :-) The 6 numbers
above would be used as:

 a00 * x + a01 * y + a02
 a10 * x + a11 * y + a12

They could be numbered differently, or passed in as an array of doubles.
The reason for allowing the affine transform to be specified explicitely
is that some callers might have it in that form if they are keeping
track of transform matrices themselves. (Also, some non-useful affines
aren't just rotations and scales.)(

> >/* 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?

If the edge mode is NEAREST or TILE, then the result doesn't have
a boundary. But frequently you know the portion that you want to
render. (If you are scaling an image without rotation, or with 
a rotation that is a mutiple of 90 degrees, you want to use an 
edge mode of NEAREST and the exact area, or you
will get fuzzy edges in the result with filtered scaling.)

If the edge mode is TRANSPARENT, then you can just pass
in 0, 0, -1, -1 if you want to render the whole thing.
(gdk-pixbuf can optimize internally to avoid rendering totally
transparent portions.) For arbitrary rotations, this is probably 
most useful.

>  - "remove alpha channel using dst", so we actualy do reverse 
> transformation?! Confusing.

I don't understand this.

> >/* 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 certainly not as needed as it was before GTK+ had a
gdk_draw_pixbuf() that handled alpha efficiently but
pixbuf-on-solid background is stillk a common case.

And gdk_draw_pixbuf() still is going to be faster in general 
if the pixbuf doesn't have an alpha channel. (In some 
cases, much faster ... on a remote server without the
RENDER extension.)

Passing in 0 is the same as what you want, and gdk-pixbuf
could certainly optimize that internally.

I could make an argument either way about whether it's
better to have the background color or not here.

> >/* 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_.

See Matthias's reference.

> >  /* 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?..

If you write down examples without testing them, sometimes they
have typos.

>>  /* 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

I'm not a big fan of adding lots of convenience functions. Your
better off if you can have a few easy to understand primitives, 
since then the application programmer can do anything once
they learn them.

It's best to wait to add convenience functions until people
are using the API and you know what gets done over and over
again.

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

I'm not sure what you mean by "real clipping area" - a rectangle list
would certainly be useful in some cases, though it doesn't actually
add any new capabilities ... you could just call the function once
for each rectangle. 

In some cases you want something different - you want to render 
through a polygonal mask. But that's out of scope for this API,
I think.

Regards,
                                       Owen