18694 2008-04-23 02:24:16 -0700 [CAIRO] Problem with rotation in a given matrix in SVG 2008-08-01 12:26:57 -0700 1 1 1 Unclassified WebKit WebKitGTK 528+ (Nightly build) PC All RESOLVED FIXED Cairo P2 Normal --- 1 krit webkit-unassigned jasa.david oldest_to_newest 78634 0 krit 2008-04-23 02:24:16 -0700 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. 78635 1 20768 krit 2008-04-23 02:47:29 -0700 Created attachment 20768 SVGCairoRotation see comment above 78797 2 krit 2008-04-24 12:53:04 -0700 The values are given correctly to AffineTransform (in SVG-specification): like matrix(a,b,c,d,tx,ty) 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); 79088 3 krit 2008-04-27 23:08:06 -0700 (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 > reinterpret_cast. You can replace reinterpret_cast: const cairo_matrix_t* matrix = reinterpret_cast<const cairo_matrix_t*>(&transform); cairo_transform(cr, matrix); to cairo_matrix_t matrix = cairo_matrix_t(transform); cairo_transform(cr, &matrix); 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. 79089 4 krit 2008-04-27 23:19:51 -0700 > have to change b to a and a to b in AffineTransform. I mean b to c and c to b. 79329 5 20768 eric 2008-04-30 23:49:56 -0700 Comment on attachment 20768 SVGCairoRotation 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! 79570 6 jasa.david 2008-05-03 07:02:02 -0700 Maybe WK could use this test as layout test: http://www.carto.net/papers/svg/samples/matrix.shtml 79587 7 krit 2008-05-03 13:02:20 -0700 (In reply to comment #6) > Maybe WK could use this test as layout test: > http://www.carto.net/papers/svg/samples/matrix.shtml > But not a good test in this case, since WebKitGtk doesn't support the <!ENTITY ...> -element (or at least partial). 83273 8 21690 krit 2008-06-13 15:13:45 -0700 Created attachment 21690 SVGCairoRotation 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). 83298 9 21694 krit 2008-06-13 23:55:47 -0700 Created attachment 21694 SVGCairoRotation forgot b(), setB() and c(), setC() 83316 10 21700 krit 2008-06-14 07:00:43 -0700 Created attachment 21700 LayoutTest 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. 83897 11 krit 2008-06-19 11:17:55 -0700 The patch solves problems with transformed images on http://paulbakaus.com/lab/js/coverflow/ too. 84507 12 21694 eric 2008-06-26 12:01:08 -0700 Comment on attachment 21694 SVGCairoRotation 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. :( 87052 13 mrowe 2008-07-26 23:02:48 -0700 Landed in r35399. 87593 14 21700 eric 2008-08-01 12:15:16 -0700 Comment on attachment 21700 LayoutTest 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.) 87602 15 21700 eric 2008-08-01 12:26:57 -0700 Comment on attachment 21700 LayoutTest 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. 20768 2008-04-23 02:47:29 -0700 2008-04-30 23:49:56 -0700 SVGCairoRotation SVGCairoRotation.patch text/plain 1244 krit SW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvQ2hhbmdlTG9n CShyZXZpc2lvbiAzMjQzMCkKKysrIFdlYkNvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBA IC0xLDMgKzEsMTMgQEAKKzIwMDgtMDQtMjMgIERpcmsgU2NodWx6ZSAgPHNldCBFTUFJTF9BRERS RVNTIGVudmlyb25tZW50IHZhcmlhYmxlPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAo T09QUyEpLgorCisgICAgICAgIEZpeCBTVkcgZGlzcGxheWluZyBpc3N1ZXMgaW4gQ2Fpcm8sIGlm IHRoZSBTVkctb2JqZWN0IGlzIHJvdGF0ZWQgd2l0aCBtYXRyaXgoKQorICAgICAgICBpbnN0ZWFk IG9mIHJvdGF0ZSgpLiAKKworICAgICAgICAqIHBsYXRmb3JtL2dyYXBoaWNzL2NhaXJvL0FmZmlu ZVRyYW5zZm9ybUNhaXJvLmNwcDoKKyAgICAgICAgKFdlYkNvcmU6OkFmZmluZVRyYW5zZm9ybTo6 QWZmaW5lVHJhbnNmb3JtKToKKwogMjAwOC0wNC0yMiAgRGFuIEJlcm5zdGVpbiAgPG1pdHpAYXBw bGUuY29tPgogCiAgICAgICAgIFJldmlld2VkIGJ5IEFuZGVycyBDYXJsc3Nvbi4KSW5kZXg6IFdl YkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3MvY2Fpcm8vQWZmaW5lVHJhbnNmb3JtQ2Fpcm8uY3BwCj09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT0KLS0tIFdlYkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3MvY2Fpcm8vQWZmaW5lVHJhbnNm b3JtQ2Fpcm8uY3BwCShyZXZpc2lvbiAzMjQyOCkKKysrIFdlYkNvcmUvcGxhdGZvcm0vZ3JhcGhp Y3MvY2Fpcm8vQWZmaW5lVHJhbnNmb3JtQ2Fpcm8uY3BwCSh3b3JraW5nIGNvcHkpCkBAIC00MSw3 ICs0MSw3IEBAIEFmZmluZVRyYW5zZm9ybTo6QWZmaW5lVHJhbnNmb3JtKCkKIAogQWZmaW5lVHJh bnNmb3JtOjpBZmZpbmVUcmFuc2Zvcm0oZG91YmxlIGEsIGRvdWJsZSBiLCBkb3VibGUgYywgZG91 YmxlIGQsIGRvdWJsZSB0eCwgZG91YmxlIHR5KQogewotICAgIGNhaXJvX21hdHJpeF9pbml0KCZt X3RyYW5zZm9ybSwgYSwgYywgYiwgZCwgdHgsIHR5KTsKKyAgICBjYWlyb19tYXRyaXhfaW5pdCgm bV90cmFuc2Zvcm0sIGEsIGIsIGMsIGQsIHR4LCB0eSk7CiB9CiAKIEFmZmluZVRyYW5zZm9ybTo6 QWZmaW5lVHJhbnNmb3JtKGNvbnN0IGNhaXJvX21hdHJpeF90ICZtYXRyaXgpCgo= 21690 2008-06-13 15:13:45 -0700 2008-06-13 23:55:47 -0700 SVGCairoRotation CairoRotation.patch text/plain 1669 krit SW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvQ2hhbmdlTG9n CShyZXZpc2lvbiAzNDUyMykKKysrIFdlYkNvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBA IC0xLDMgKzEsMTYgQEAKKzIwMDgtMDYtMTMgIERpcmsgU2NodWx6ZSAgPHZiczg1QGdteC5kZT4K KworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBodHRwOi8v YnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xODY5NAorICAgICAgICBbQ0FJUk9dIFBy b2JsZW0gd2l0aCByb3RhdGlvbiBpbiBhIGdpdmVuIG1hdHJpeCBpbiBTVkcKKworICAgICAgICBG aXhlZCB2YWx1ZXMgZ2l2ZW4gdG8gQWZmaW5lVHJhbnNmb3JtLgorCisgICAgICAgICogcGxhdGZv cm0vZ3JhcGhpY3MvY2Fpcm8vQWZmaW5lVHJhbnNmb3JtQ2Fpcm8uY3BwOgorICAgICAgICAoV2Vi Q29yZTo6QWZmaW5lVHJhbnNmb3JtOjpBZmZpbmVUcmFuc2Zvcm0pOgorICAgICAgICAoV2ViQ29y ZTo6QWZmaW5lVHJhbnNmb3JtOjpzZXRNYXRyaXgpOgorCiAyMDA4LTA2LTEzICBEYXJpbiBBZGxl ciAgPGRhcmluQGFwcGxlLmNvbT4KIAogICAgICAgICBSZXZpZXdlZCBieSBKb2huIFN1bGxpdmFu LgpJbmRleDogV2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy9jYWlyby9BZmZpbmVUcmFuc2Zvcm1D YWlyby5jcHAKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PQotLS0gV2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy9jYWlyby9B ZmZpbmVUcmFuc2Zvcm1DYWlyby5jcHAJKHJldmlzaW9uIDM0NTE3KQorKysgV2ViQ29yZS9wbGF0 Zm9ybS9ncmFwaGljcy9jYWlyby9BZmZpbmVUcmFuc2Zvcm1DYWlyby5jcHAJKHdvcmtpbmcgY29w eSkKQEAgLTQxLDcgKzQxLDcgQEAgQWZmaW5lVHJhbnNmb3JtOjpBZmZpbmVUcmFuc2Zvcm0oKQog CiBBZmZpbmVUcmFuc2Zvcm06OkFmZmluZVRyYW5zZm9ybShkb3VibGUgYSwgZG91YmxlIGIsIGRv dWJsZSBjLCBkb3VibGUgZCwgZG91YmxlIHR4LCBkb3VibGUgdHkpCiB7Ci0gICAgY2Fpcm9fbWF0 cml4X2luaXQoJm1fdHJhbnNmb3JtLCBhLCBjLCBiLCBkLCB0eCwgdHkpOworICAgIGNhaXJvX21h dHJpeF9pbml0KCZtX3RyYW5zZm9ybSwgYSwgYiwgYywgZCwgdHgsIHR5KTsKIH0KIAogQWZmaW5l VHJhbnNmb3JtOjpBZmZpbmVUcmFuc2Zvcm0oY29uc3QgY2Fpcm9fbWF0cml4X3QgJm1hdHJpeCkK QEAgLTUxLDcgKzUxLDcgQEAgQWZmaW5lVHJhbnNmb3JtOjpBZmZpbmVUcmFuc2Zvcm0oY29uc3Qg YwogCiB2b2lkIEFmZmluZVRyYW5zZm9ybTo6c2V0TWF0cml4KGRvdWJsZSBhLCBkb3VibGUgYiwg ZG91YmxlIGMsIGRvdWJsZSBkLCBkb3VibGUgdHgsIGRvdWJsZSB0eSkKIHsKLSAgICBjYWlyb19t YXRyaXhfaW5pdCgmbV90cmFuc2Zvcm0sIGEsIGMsIGIsIGQsIHR4LCB0eSk7CisgICAgY2Fpcm9f bWF0cml4X2luaXQoJm1fdHJhbnNmb3JtLCBhLCBiLCBjLCBkLCB0eCwgdHkpOwogfQogCiB2b2lk IEFmZmluZVRyYW5zZm9ybTo6bWFwKGRvdWJsZSB4LCBkb3VibGUgeSwgZG91YmxlKiB4MiwgZG91 YmxlKiB5MikgY29uc3QKCg== 21694 2008-06-13 23:55:47 -0700 2008-06-26 12:01:08 -0700 SVGCairoRotation CairoRotation.patch text/plain 2157 krit SW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvQ2hhbmdlTG9n CShyZXZpc2lvbiAzNDUyMykKKysrIFdlYkNvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBA IC0xLDMgKzEsMTYgQEAKKzIwMDgtMDYtMTMgIERpcmsgU2NodWx6ZSAgPHZiczg1QGdteC5kZT4K KworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBodHRwOi8v YnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xODY5NAorICAgICAgICBbQ0FJUk9dIFBy b2JsZW0gd2l0aCByb3RhdGlvbiBpbiBhIGdpdmVuIG1hdHJpeCBpbiBTVkcKKworICAgICAgICBD aGFuZ2VkIHZhbHVlcyBnaXZlbiB0byBBZmZpbmVUcmFuc2Zvcm0uCisKKyAgICAgICAgKiBwbGF0 Zm9ybS9ncmFwaGljcy9jYWlyby9BZmZpbmVUcmFuc2Zvcm1DYWlyby5jcHA6CisgICAgICAgIChX ZWJDb3JlOjpBZmZpbmVUcmFuc2Zvcm06OkFmZmluZVRyYW5zZm9ybSk6CisgICAgICAgIChXZWJD b3JlOjpBZmZpbmVUcmFuc2Zvcm06OnNldE1hdHJpeCk6CisKIDIwMDgtMDYtMTMgIERhcmluIEFk bGVyICA8ZGFyaW5AYXBwbGUuY29tPgogCiAgICAgICAgIFJldmlld2VkIGJ5IEpvaG4gU3VsbGl2 YW4uCkluZGV4OiBXZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL2NhaXJvL0FmZmluZVRyYW5zZm9y bUNhaXJvLmNwcAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09Ci0tLSBXZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL2NhaXJv L0FmZmluZVRyYW5zZm9ybUNhaXJvLmNwcAkocmV2aXNpb24gMzQ1MTcpCisrKyBXZWJDb3JlL3Bs YXRmb3JtL2dyYXBoaWNzL2NhaXJvL0FmZmluZVRyYW5zZm9ybUNhaXJvLmNwcAkod29ya2luZyBj b3B5KQpAQCAtNDEsNyArNDEsNyBAQCBBZmZpbmVUcmFuc2Zvcm06OkFmZmluZVRyYW5zZm9ybSgp CiAKIEFmZmluZVRyYW5zZm9ybTo6QWZmaW5lVHJhbnNmb3JtKGRvdWJsZSBhLCBkb3VibGUgYiwg ZG91YmxlIGMsIGRvdWJsZSBkLCBkb3VibGUgdHgsIGRvdWJsZSB0eSkKIHsKLSAgICBjYWlyb19t YXRyaXhfaW5pdCgmbV90cmFuc2Zvcm0sIGEsIGMsIGIsIGQsIHR4LCB0eSk7CisgICAgY2Fpcm9f bWF0cml4X2luaXQoJm1fdHJhbnNmb3JtLCBhLCBiLCBjLCBkLCB0eCwgdHkpOwogfQogCiBBZmZp bmVUcmFuc2Zvcm06OkFmZmluZVRyYW5zZm9ybShjb25zdCBjYWlyb19tYXRyaXhfdCAmbWF0cml4 KQpAQCAtNTEsNyArNTEsNyBAQCBBZmZpbmVUcmFuc2Zvcm06OkFmZmluZVRyYW5zZm9ybShjb25z dCBjCiAKIHZvaWQgQWZmaW5lVHJhbnNmb3JtOjpzZXRNYXRyaXgoZG91YmxlIGEsIGRvdWJsZSBi LCBkb3VibGUgYywgZG91YmxlIGQsIGRvdWJsZSB0eCwgZG91YmxlIHR5KQogewotICAgIGNhaXJv X21hdHJpeF9pbml0KCZtX3RyYW5zZm9ybSwgYSwgYywgYiwgZCwgdHgsIHR5KTsKKyAgICBjYWly b19tYXRyaXhfaW5pdCgmbV90cmFuc2Zvcm0sIGEsIGIsIGMsIGQsIHR4LCB0eSk7CiB9CiAKIHZv aWQgQWZmaW5lVHJhbnNmb3JtOjptYXAoZG91YmxlIHgsIGRvdWJsZSB5LCBkb3VibGUqIHgyLCBk b3VibGUqIHkyKSBjb25zdApAQCAtMTQ3LDIyICsxNDcsMjIgQEAgdm9pZCBBZmZpbmVUcmFuc2Zv cm06OnNldEEoZG91YmxlIGEpCiAKIGRvdWJsZSBBZmZpbmVUcmFuc2Zvcm06OmIoKSBjb25zdAog ewotICAgIHJldHVybiBtX3RyYW5zZm9ybS54eTsKKyAgICByZXR1cm4gbV90cmFuc2Zvcm0ueXg7 CiB9CiAKIHZvaWQgQWZmaW5lVHJhbnNmb3JtOjpzZXRCKGRvdWJsZSBiKQogewotICAgIG1fdHJh bnNmb3JtLnh5ID0gYjsKKyAgICBtX3RyYW5zZm9ybS55eCA9IGI7CiB9CiAKIGRvdWJsZSBBZmZp bmVUcmFuc2Zvcm06OmMoKSBjb25zdAogewotICAgIHJldHVybiBtX3RyYW5zZm9ybS55eDsKKyAg ICByZXR1cm4gbV90cmFuc2Zvcm0ueHk7CiB9CiAKIHZvaWQgQWZmaW5lVHJhbnNmb3JtOjpzZXRD KGRvdWJsZSBjKQogewotICAgIG1fdHJhbnNmb3JtLnl4ID0gYzsKKyAgICBtX3RyYW5zZm9ybS54 eSA9IGM7CiB9CiAKIGRvdWJsZSBBZmZpbmVUcmFuc2Zvcm06OmQoKSBjb25zdAoK 21700 2008-06-14 07:00:43 -0700 2008-08-01 12:26:57 -0700 LayoutTest rotate-matrix.svg image/svg+xml 461 krit PD94bWwgdmVyc2lvbj0iMS4wIiBlbmNvZGluZz0iVVRGLTgiIHN0YW5kYWxvbmU9Im5vIj8+Cjxz dmcgd2lkdGg9IjI1MHB4IiBoZWlnaHQ9IjIwMHB4Ig0KICAgICB4bWxucz0iaHR0cDovL3d3dy53 My5vcmcvMjAwMC9zdmciIHhtbG5zOnhsaW5rPSJodHRwOi8vd3d3LnczLm9yZy8xOTk5L3hsaW5r Ij4KICAgIDxyZWN0IGZpbGw9InJlZCIgd2lkdGg9IjEwMCIgaGVpZ2h0PSIxMDAiIHg9IjIwIiB5 PSIzMCIgdHJhbnNmb3JtPSJyb3RhdGUoLTEwKSIgLz4KICAgIDxyZWN0IGZpbGw9ImdyZWVuIiB3 aWR0aD0iMTAwIiBoZWlnaHQ9IjEwMCIgeD0iMjAiIHk9IjMwIiB0cmFuc2Zvcm09Im1hdHJpeCgw Ljk4NDgwOCwtMC4xNzM2NDgsMC4xNzM2NDgsMC45ODQ4MDgsMCwwKSIgLz4KICAgIDx0ZXh0IHg9 IjEwIiB5PSIxODAiPllvdSBzaG91bGQgc2VlIGEgcm90YXRlZCBncmVlbiBib3g8L3RleHQ+Cjwv c3ZnPgo=