WebKit Bugzilla
New
Browse
Search+
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
198746
[cairo][SVG] If clipPath has multiple elements, clip-path doesn't work with transform
https://bugs.webkit.org/show_bug.cgi?id=198746
Summary
[cairo][SVG] If clipPath has multiple elements, clip-path doesn't work with t...
Fujii Hironori
Reported
2019-06-10 23:11:39 PDT
Created
attachment 371826
[details]
test case [cairo][SVG] If clipPath has multiple elements, clip-path doesn't work with opacity
Attachments
test case
(846 bytes, text/html)
2019-06-10 23:11 PDT
,
Fujii Hironori
no flags
Details
WIP
(2.98 KB, patch)
2019-06-12 02:25 PDT
,
Carlos Garcia Campos
no flags
Details
Formatted Diff
Diff
WIP
(3.21 KB, patch)
2019-06-12 05:11 PDT
,
Carlos Garcia Campos
no flags
Details
Formatted Diff
Diff
WIP
(3.18 KB, patch)
2019-06-12 05:15 PDT
,
Carlos Garcia Campos
no flags
Details
Formatted Diff
Diff
Patch
(8.91 KB, patch)
2019-06-12 06:04 PDT
,
Carlos Garcia Campos
no flags
Details
Formatted Diff
Diff
Patch
(21.25 KB, patch)
2019-06-12 19:05 PDT
,
Fujii Hironori
fujii.hironori
: commit-queue-
Details
Formatted Diff
Diff
Patch
(9.29 KB, patch)
2019-06-12 19:08 PDT
,
Fujii Hironori
no flags
Details
Formatted Diff
Diff
Show Obsolete
(5)
View All
Add attachment
proposed patch, testcase, etc.
Carlos Garcia Campos
Comment 1
2019-06-11 07:35:31 PDT
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.
Fujii Hironori
Comment 2
2019-06-11 23:09:50 PDT
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.
Carlos Garcia Campos
Comment 3
2019-06-12 02:25:29 PDT
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?
Fujii Hironori
Comment 4
2019-06-12 03:28:04 PDT
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());
Carlos Garcia Campos
Comment 5
2019-06-12 04:45:40 PDT
(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.
Carlos Garcia Campos
Comment 6
2019-06-12 05:11:24 PDT
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.
Carlos Garcia Campos
Comment 7
2019-06-12 05:15:03 PDT
Created
attachment 371944
[details]
WIP I forgot to remove the matrix_invert call
Carlos Garcia Campos
Comment 8
2019-06-12 06:04:50 PDT
Created
attachment 371946
[details]
Patch
Carlos Garcia Campos
Comment 9
2019-06-12 06:41:02 PDT
***
Bug 198779
has been marked as a duplicate of this bug. ***
Carlos Garcia Campos
Comment 10
2019-06-12 06:43:43 PDT
Committed
r246350
: <
https://trac.webkit.org/changeset/246350
>
Radar WebKit Bug Importer
Comment 11
2019-06-12 06:44:16 PDT
<
rdar://problem/51665805
>
Truitt Savell
Comment 12
2019-06-12 09:06:45 PDT
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.
Truitt Savell
Comment 13
2019-06-12 09:18:23 PDT
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
Truitt Savell
Comment 14
2019-06-12 09:22:02 PDT
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
>
Ryan Haddad
Comment 15
2019-06-12 09:40:14 PDT
(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
Michael Catanzaro
Comment 16
2019-06-12 10:17:08 PDT
Reopening due to the rollout.
Fujii Hironori
Comment 17
2019-06-12 19:05:39 PDT
Created
attachment 372014
[details]
Patch
Fujii Hironori
Comment 18
2019-06-12 19:08:01 PDT
Created
attachment 372015
[details]
Patch * Fixed timing out issue by using ->. * Fixed the bug id in ChangeLog
WebKit Commit Bot
Comment 19
2019-06-12 21:10:39 PDT
Comment on
attachment 372015
[details]
Patch Clearing flags on attachment: 372015 Committed
r246391
: <
https://trac.webkit.org/changeset/246391
>
WebKit Commit Bot
Comment 20
2019-06-12 21:10:41 PDT
All reviewed patches have been landed. Closing bug.
Fujii Hironori
Comment 21
2019-06-12 21:18:04 PDT
Great work! Good job, KaL. I'm still confused by transform matrix hell, but your solution looks consistent.
Fujii Hironori
Comment 22
2019-06-28 00:27:49 PDT
(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
.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug