There is a problem with rotation of objects, if you use matrix() instead of rotate(). I opened the bug just to be sure not to forget anything.
You can fix the problem for svg with:
AffineTransform::AffineTransform(double a, double b, double c, double d, double tx, double ty)
cairo_matrix_init(&m_transform, a, b, c, d, tx, ty);
Is there anything else to do? Perhaps setB(), b() and setC(), c() ?
In cairo a matrix is initialised with:
cairo_matrix_init(cairo_matrix_t *matrix, xx, yx, xy, yy, x0, y0);
So setB(), b() and setC() and c() could be wrong initialised.
Created attachment 20768 [details]
see comment above
The values are given correctly to AffineTransform (in SVG-specification):
If you transform an SVG-object with Cairo, b and c are interchanged. That means:
matrix(a,c,b,d,tx,ty) is the same on cairo like the matrix above on SVG. Thats why the interchange of c and b in AffineTransform should be correct. But the SVG-object is displayed wrong with it.
I searched in GraphicsContestCairo and found the method
void GraphicsContext::concatCTM(const AffineTransform& transform)
In this method "transform" is changed back to a cairo_matrix_t with reinterpret_cast.
I asked in IRC and it COULD be that this transformation interchange b and c in the background while transforming "transform" to cairo_matrix_t.
Perhaps, thats why cairo_matrix_init(&m_transform, a, c, b, d, tx, ty); doesn't work correctly.
So you should use a other possibility to change the type to cairo_matrix_t or you use cairo_matrix_init(&m_transform, a, b, c, d, tx, ty);
(In reply to comment #2)
> I searched in GraphicsContestCairo and found the method
> void GraphicsContext::concatCTM(const AffineTransform& transform)
> In this method "transform" is changed back to a cairo_matrix_t with
You can replace reinterpret_cast:
const cairo_matrix_t* matrix = reinterpret_cast<const cairo_matrix_t*>(&transform);
cairo_matrix_t matrix = cairo_matrix_t(transform);
The code don't use reinterpret_cast but it doesn't solve the problem. You still have to change b to a and a to b in AffineTransform.
I searched through the source-code of webkit with grep and SVG is the only one that use this method. I shoulnd't be a problem to fix it that way.
> have to change b to a and a to b in AffineTransform.
I mean b to c and c to b.
Comment on attachment 20768 [details]
This needs a layout test. Doesn't it already affect some layout test result? Also, your email is missing from your changelog. Otherwise the patch looks great!
Maybe WK could use this test as layout test: http://www.carto.net/papers/svg/samples/matrix.shtml
(In reply to comment #6)
> Maybe WK could use this test as layout test:
But not a good test in this case, since WebKitGtk doesn't support the <!ENTITY ...> -element (or at least partial).
Created attachment 21690 [details]
I misunderstood the documentation of CGAffineTransform. The CGAffineTransform is the transpose of cairo_matrix. That means
a is xx on cairo, [1,1] on CGAffineTransform
b is yx on cairo, [1,2] on CGAffineTransform
c is xy on cairo, [2,1] on CGAffineTransform
d is yy on cairo, [2,2] on CGAffineTransform
and so on.
That means the handover of the values have to be AffineTransform(a, b, c, d, e, f).
Created attachment 21694 [details]
forgot b(), setB() and c(), setC()
Created attachment 21700 [details]
I created a test because I haven't found a test case for rotation with a matrix in layouttests. This one draws two boxes. The red box is rotated with the attribute 'rotate' and the green box with a matrix (both with the same angle). You should only see the green box.
The patch solves problems with transformed images on http://paulbakaus.com/lab/js/coverflow/ too.
Comment on attachment 21694 [details]
The patch looks fine. Of course Dirk informed me over IRC that GTK doesn't actually have DumpRenderTree yet... so run-webkit-tests (And thus layout tests) don't work. :(
Landed in r35399.
Comment on attachment 21700 [details]
this test could be much better. If you want to test that certain rotations work as expected, you should put a second red box behind the green one, the green one would only ever cover all of the red if your bug was fixed (i.e. the rotations for those particular matrices were calculated correctly.)
Comment on attachment 21700 [details]
My bad. Looks fine. Even better would be to just show the green box at 0,0 unrotated (no text below it). That would require these to be in a <g> with another rotate(-10) applied.