Bug 249764

Summary: [SVG] Fix ellipse hit testing in the non-circle case
Product: WebKit Reporter: Ahmad Saleem <ahmad.saleem792>
Component: SVGAssignee: Yusuke Suzuki <ysuzuki>
Status: RESOLVED FIXED    
Severity: Normal CC: sabouhallawa, webkit-bug-importer, ysuzuki, zalan, zimmermann
Priority: P2 Keywords: InRadar
Version: Safari Technology Preview   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
GitHub Desktop Patch none

Description Ahmad Saleem 2022-12-22 01:38:55 PST 
Hi Team,

While going through I came across another failing test in Safari 16.2 & STP160, while the test case passes in Chrome Canary 110 only. Even Firefox Nightly 110 fail it with single point (dot).

Test Case - https://jsfiddle.net/54b80mLc/show

^ Make sure to run it in /show mode full width to see the results else on small window even Chrome Canary 111 will fail.

Blink Commit - https://chromium.googlesource.com/chromium/blink/+/fb46751341e395b641b691670c1c70e90bae30d4

It might be possible to merge this 1-1 but I haven't looked completely but from quick glance, it seems that it is possible.

Just wanted to raise bug so in future, I can give it a try.

Thanks!
Comment 1 Radar WebKit Bug Importer 2022-12-29 01:39:16 PST 
<rdar://problem/103756227>
Comment 2 Ahmad Saleem 2023-01-15 18:02:33 PST 
It might have some impact on WPT as well:

https://wpt.fyi/results/svg/interact/scripted/ellipse-hittest.html?label=experimental&label=master&aligned
Comment 3 Ahmad Saleem 2023-03-27 21:31:14 PDT 
Follow-up commit as well - https://src.chromium.org/viewvc/blink?view=revision&revision=189271
Comment 4 Ahmad Saleem 2023-03-27 21:58:05 PDT 
Created attachment 465627 [details]
GitHub Desktop Patch

I took Blink's commit and applied without comment changes and it compiles. We only have to remove one reference variable, since now it would be unused.

Also on local testing on WebKit ToT (262194@main) with above leads to passing all WPT tests. Hence, I added WPTImpact tag.

Although, I am not sure whether we would have similar performance impacts which was fixed in the follow-up fix but just wanted to keep it here for someone to confirm.
Comment 5 Ahmad Saleem 2023-10-19 15:04:13 PDT 
Pre-requisite - https://src.chromium.org/viewvc/blink?view=revision&revision=186650
Comment 6 Yusuke Suzuki 2023-10-19 16:33:42 PDT 
Right. This is exactly what I pointed at https://github.com/WebKit/WebKit/pull/18740#discussion_r1353911754
I'll create a simple patch which makes this fixed.
Comment 7 Yusuke Suzuki 2023-10-19 16:50:16 PDT 
Pull request: https://github.com/WebKit/WebKit/pull/19320
Comment 8 EWS 2023-10-19 20:32:30 PDT 
Committed 269554@main (339f49b72a95): <https://commits.webkit.org/269554@main>

Reviewed commits have been landed. Closing PR #19320 and removing active labels.