Re: Image transformations

[Alan Third]

On Wed, Jun 12, 2019 at 06:30:11PM +0300, Eli Zaretskii wrote:
> > >   . There are no tests, AFAICT.  We should have a simple manual test
> > >   which could be used to exercise all of the transformations,
> > >   separately and in combinations.  I wonder how did the people who
> > >   worked on the implementations for different platforns verify that
> > >   the results are consistent, especially given the lack of
> > >   documentation (e.g., is rotation of 90 deg clockwise or
> > >   counter-clockwise?).
> > 
> > Is there some example of how to write a manual test like this?
> 
> See test/manual/redisplay-testsuite.el, for example.
> 
> The idea is to show some instructions, and then let the user/tester
> invoke operations being tested and observe the results.  The epected
> results should be described as part of the instructions.

That’s what I imagined, but I didn’t know about the existing manual
tests. Good to know they’re there. Thanks!

> > My hope was that MS Windows support for affine transform matrices
> > would be forthcoming quite quickly in which case there would be no
> > need to differentiate between the different types of transform (if one
> > is supported, they all are), but perhaps that’s hoping for too much. :)
> 
> Well, they don't really queue up for the job of making this work on
> Windows, do they?

Indeed.

> Part of the reason for my message was that I tried to figure out what
> would it take to provide these capabilities on Windows, and quickly
> got lost.  I have no background in graphics programming, neither on
> Windows nor on any other platform, so for me good documentation is
> critical.
> 
> Having this function be a simple boolean, on the assumption that all
> GUI platforms provide all the sub-features, would actually mean that
> this function is just a fancy alias for display-graphic-p.  I don't
> think this is the best we can do.
> 
> In addition, my research indicates that the equivalent features on
> Windows will have to use APIs that aren't available on Windows 9X
> (unless we decide to transform on pixel level by our own code, which I
> think is way too gross).  So there will be cases where rotations will
> not be supported, even after the code to do that will have been
> written.
> 
> Finally, there's the ImageMagick case, where we support rotations by
> arbitrary angles, and it would be good to be able to make that
> distinction with this single function, instead of making additional
> tests.

To be frank the only reason I added this function is because having
XRender doesn’t guarantee every frame will be able to perform these
transforms. I imagine that most modern hardware (from the last 15
years or so) will be able to handle it, but older hardware may be
using graphics cards that don’t support transforms. I think this means
you can end up in the situation where a frame on monitor one, running
on graphics card one, will work, but a frame on monitor two on
graphics card two won’t.

I think this is potentially an even worse situation than on Windows,
but probably very rare.

However extending image-transforms-p, or coming up with another
implementation, makes a lot of sense.

> > I’ve been thinking a bit about the idea of returning some sort of
> > capabilities list and it seems quite neat. We could perhaps roll some
> > of the imagemagick-types stuff into it.
> 
> We have similar availability testing functions in gnutls.c.

Thanks, I’ll have a look.

> > +Cropping is performed after scaling but before rotation.
> 
> This sounds strange to me; are you sure?  I'd expect cropping to be
> done either before everything else or after everything else.  Is this
> so because that's how XRender does it?  At the very least, it begs the
> question whether the parameters of :crop are measured in units before
> or after scaling.

I agree, but this is how our imagemagick code does it and I didn’t
want to make my code behave differently, even though I think it makes
no sense.

It’s easy enough to re‐order the events. In the native transforms code
you simply reorder these function calls in lookup_image:

    image_set_size (img, transform_matrix);
    image_set_crop (img, transform_matrix);
    image_set_rotation (img, transform_matrix);

and reorder some of the code in imagemagick_load_image.

IMO the best order is probably crop, rotate and resize. I think
there’s an argument for putting resize before rotate, but rotating by
90 degrees after setting the size would mean :max-width and
:max-height affecting the wrong dimensions, as they do at the moment.

Alternatively we could split resizing so :max-width and :max-height
always operate last. Probably not worth it, though. Just put resizing
last.

> > +/* image_set_rotation, image_set_crop, image_set_size and
> > +   image_set_transform use affine transformation matrices to perform
> > +   various transforms on the image.  The matrix is a 2D array of
> > +   doubles.  It is laid out like this:
> > +
> > +   m[0][0] = m11 | m[0][1] = m12 | m[0][2] = tx
> > +   --------------+---------------+-------------
> > +   m[1][0] = m21 | m[1][1] = m22 | m[1][2] = ty
> > +   --------------+---------------+-------------
> > +   m[2][0] = 0   | m[2][1] = 0   | m[2][2] = 1
> 
> Looking at the code, it seems that the matrix is actually defined like
> this:
> 
>    m[0][0] = m11 | m[0][1] = m12 | m[0][2] = 0
>    --------------+---------------+-------------
>    m[1][0] = m21 | m[1][1] = m22 | m[1][2] = 0
>    --------------+---------------+-------------
>    m[2][0] = tx  | m[2][1] = ty  | m[2][2] = 1
> 
> If not, I think the Cairo code takes the wrong components for its
> implementation.

You’re right.

> > +   tx and ty represent translations, m11 and m22 represent scaling
> > +   transforms and m21 and m12 represent shear transforms.
> 
> Can you please add the equations used to perform this affine
> transformation, i.e. how x' and y' are computed from x and y?  I think
> it will go a long way towards clarifying the processing.

I’ll add some further explanations of how to use the affine
transformation matrices, but I don’t know that I’ll be able to do a
very good job of explaining exactly how they work. I would suggest
that if someone is interested they look it up elsewhere, however I
also don’t think it’s necessary to fully understand the maths to be
able to use them.

I’ll provide an updated patch soon.

> Also, as long as I have your attention: could you please tell what you
> get in the elements of matrix in image_set_size just before you pass
> the values to XRenderSetPictureTransform, when you evaluate this in
> *scratch*:
> 
>   (insert-image (create-image "splash.svg" 'svg nil :rotation 90))
> 
> I stepped through the code trying to figure out how to map these
> features to the equivalent Windows APIs, and I saw some results that
> confused me.  After you show me the values you get in this use case, I
> might have a few follow-up questions about the code, to clarify my
> understanding of what the code does and what we expect from the
> backend when we hand it the matrix computed here.

Using this code at the top of image_set_transform:

  fprintf(stderr, "matrix:\n");
  fprintf(stderr, "%f %f %f\n",matrix[0][0], matrix[1][0], matrix[2][0]);
  fprintf(stderr, "%f %f %f\n",matrix[0][1], matrix[1][1], matrix[2][1]);
  fprintf(stderr, "%f %f %f\n",matrix[0][2], matrix[1][2], matrix[2][2]);

I get this printed (on macOS, but it should be the same everywhere):

matrix:
0.000000 1.000000 0.000000
-1.000000 0.000000 232.000000
0.000000 0.000000 1.000000

I don’t know exactly what that means. The 1 and -1 are shearing the
image in the x and y dimensions. The 232 is moving the image in the y
dimension. It’s actually moving the origin, so a positive value moves
the origin downwards.

The full process of rotating an image is to move the origin to the
centre of the image (width/2, height/2); perform the rotation, which
is some combination of shearing and resizing, but appears in this case
to not involve any actual resizing; and finally move the origin back
to the top left of the image, which may now be a different corner.

This is done by creating a matrix for each of those actions, then
multiplying the transformation matrix (the one we pass into each
function) by each of those matrices in order (matrix multiplication is
not commutative). After we’ve done that we can use our modified
transformation matrix to transform points. I believe we take the x and
y coordinates and convert them into a 3x1 matrix and multiply that by
the transformation matrix and it gives us a new set of coordinates.

    [x]   [m11 m12 m13]
    [y] X [m21 m22 m23] = [x’ y’ 0]
    [0]   [m31 m32 m33]

Luckily we don’t have to worry about the last step as the graphics
toolkit will do it for us.

Cropping is easier as we just move the origin to the top left of where
we want to crop and set the width and height accordingly. The matrices
don’t know anything about width and height.

Scaling is also simple as you can set m11 to scale x, m22 to scale y
and, I think, m33 to scale both by the same value. My code ignores
m33.

And of course you have to set the width and height accordingly again.

It’s possible to pre‐calculate the matrix multiplications and just
generate one transform matrix that will do everything we need in a
single step, but the maths for each element is much more complex and I
thought it was better to lay out the steps separately.

(perhaps I should just put the above into the comment in image.c)
-- 
Alan Third