Bug 250382
| Summary: | Check for non-finite points before transforming them instead of after | ||
|---|---|---|---|
| Product: | WebKit | Reporter: | Ahmad Saleem <ahmad.saleem792> |
| Component: | Canvas | Assignee: | Nobody <webkit-unassigned> |
| Status: | RESOLVED FIXED | ||
| Severity: | Normal | CC: | dino, mmaxfield, sabouhallawa, webkit-bug-importer |
| Priority: | P2 | Keywords: | InRadar |
| Version: | Safari Technology Preview | ||
| Hardware: | Unspecified | ||
| OS: | Unspecified | ||
Ahmad Saleem
Hi Team,
While going through Blink's commit, I came across potential assertion fix and also to improve handling NaN cases:
Blink Commit - https://src.chromium.org/viewvc/blink?revision=183090&view=revision
WebKit Source - https://github.com/WebKit/WebKit/blob/28de25b653f64ca3fc8e87fbf1d1a963329a5342/Source/WebCore/html/canvas/CanvasRenderingContext2DBase.cpp#L520
Unfortunately it does not have any test case to identify whether it is causing assert or failing or something but I just wanted to get input.
Thanks!
| Attachments | ||
|---|---|---|
| Add attachment proposed patch, testcase, etc. |
Radar WebKit Bug Importer
<rdar://problem/104331898>
Said Abou-Hallawa
The layout tests:
1) https://github.com/WebKit/WebKit/blob/28de25b653f64ca3fc8e87fbf1d1a963329a5342/LayoutTests/fast/canvas/canvas-path-isPointInPath.html
2) https://github.com/WebKit/WebKit/blob/28de25b653f64ca3fc8e87fbf1d1a963329a5342/LayoutTests/fast/canvas/canvas-path-isPointInStroke.html
test passing NaN values to isPointInPath() and isPointInStroke().
But we are fine because the complier ensures the math with NaN values are handled correctly all the way. However I think we should check
if (!std::isfinite(x) || !std::isfinite(y))
return false;
In CanvasRenderingContext2DBase::isPointInPathInternal() and CanvasRenderingContext2DBase::isPointInStrokeInternal() instead of checking the values of the transformedPoint.
We do not need to check whether the values transformedPoint.x() and transformedPoint.y() are finite because AffineTransform::mapPoint() does it math in double and then it clamp it to float.
Ahmad Saleem
PR - https://github.com/WebKit/WebKit/pull/10673
EWS
Committed 260946@main (90fc0225ff34): <https://commits.webkit.org/260946@main>
Reviewed commits have been landed. Closing PR #10673 and removing active labels.