Created attachment 371826 [details] test case [cairo][SVG] If clipPath has multiple elements, clip-path doesn't work with opacity
The problem here is the translation (transform="translate(200,0)") in the second case. I've managed to fix this by using a cairo_pattern_t created for the given cairo_image_t and setting a transformation matrix. But that broke other test cases attached to bug #198701, so it needs some more work. It seems the matrix I'm using only works when current transformation matrix doesn't have a translation already. I'll continue investigating tomorrow.
My patch in Bug 198779 comment 2 solves this issue only in non high DPI display, but not fixed in high DPI display. Funny. It seems there is one more coordinate system bug around us.
Created attachment 371938 [details] WIP This fixes this test case without breaking the ones attached to bug #198701. It makes the code a lot simpler too. HiDPI is a different issue, we need to apply somehow the device scale factor to the mask surface. This patch only needs to include a test. Fujii why are you using html tests instead of svg?
Comment on attachment 371938 [details] WIP View in context: https://bugs.webkit.org/attachment.cgi?id=371938&action=review LGTM > Source/WebCore/platform/graphics/cairo/PlatformContextCairo.cpp:86 > + cairo_matrix_invert(&matrix); You can remove this cairo_matrix_invert by negating. cairo_matrix_init_translate(&matrix, -rect.x(), -rect.y());
(In reply to Fujii Hironori from comment #4) > Comment on attachment 371938 [details] > WIP > > View in context: > https://bugs.webkit.org/attachment.cgi?id=371938&action=review > > LGTM It breaks other tests, though. I think I have a new patch that would also fix bug #198779, running the tests now. > > Source/WebCore/platform/graphics/cairo/PlatformContextCairo.cpp:86 > > + cairo_matrix_invert(&matrix); > > You can remove this cairo_matrix_invert by negating. > cairo_matrix_init_translate(&matrix, -rect.x(), -rect.y()); Ok, even simpler indeed.
Created attachment 371942 [details] WIP This patch fixes this test case and also the HiDPI one with no regressions in layout tests. It just needs a couple of new tests.
Created attachment 371944 [details] WIP I forgot to remove the matrix_invert call
Created attachment 371946 [details] Patch
*** Bug 198779 has been marked as a duplicate of this bug. ***
Committed r246350: <https://trac.webkit.org/changeset/246350>
<rdar://problem/51665805>
The new test svg/clip-path/clip-hidpi.svg Added in https://trac.webkit.org/changeset/246350/webkit is timing out. History: https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=svg%2Fclip-path%2Fclip-hidpi.svg This test is effecting Mac wk1. It looks like EWS caught this timeout on Mac wk1 and Mac debug but was not allowed to finish running. It looks like this test should be corrected. Our policies state we should roll this out until a functioning test can be landed with the patch.
Additional a failure just occurred on Mac Debug wk2 Diff: https://build.webkit.org/results/Apple%20Mojave%20Debug%20WK2%20(Tests)/r246352%20(3028)/svg/clip-path/clip-hidpi-diffs.html
Reverted r246350 for reason: r246350 Introduced a failing and timing out test svg/clip-path/clip-hidpi.svg Committed r246354: <https://trac.webkit.org/changeset/246354>
(In reply to Truitt Savell from comment #12) > The new test svg/clip-path/clip-hidpi.svg > > Added in https://trac.webkit.org/changeset/246350/webkit > > is timing out. History: > https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard. > html#showAllRuns=true&tests=svg%2Fclip-path%2Fclip-hidpi.svg > > This test is effecting Mac wk1. It looks like EWS caught this timeout on Mac > wk1 and Mac debug but was not allowed to finish running. It looks like this > test should be corrected. Our policies state we should roll this out until a > functioning test can be landed with the patch. This timeout was also seen on Win10: https://build.webkit.org/results/Apple%20Win%2010%20Release%20(Tests)/r246350%20(2551)/results.html
Reopening due to the rollout.
Created attachment 372014 [details] Patch
Created attachment 372015 [details] Patch * Fixed timing out issue by using ->. * Fixed the bug id in ChangeLog
Comment on attachment 372015 [details] Patch Clearing flags on attachment: 372015 Committed r246391: <https://trac.webkit.org/changeset/246391>
All reviewed patches have been landed. Closing bug.
Great work! Good job, KaL. I'm still confused by transform matrix hell, but your solution looks consistent.
(In reply to Fujii Hironori from comment #18) > * Fixed timing out issue by using ->. Oops, my bad. This is a just syntax error. Filed in Bug 199313.