Bug 250384 - Potential Assertion Fix - newStartAngle >= 0 && newStartAngle < twoPiFloat
Summary: Potential Assertion Fix - newStartAngle >= 0 && newStartAngle < twoPiFloat
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Canvas (show other bugs)
Version: Safari Technology Preview
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2023-01-10 07:07 PST by Ahmad Saleem
Modified: 2023-10-30 02:45 PDT (History)
2 users (show)

See Also:

Attachments
Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Ahmad Saleem 2023-01-10 07:07:11 PST 
Hi Team,

While going through Blink's commit, I came across following potential assertion fix:

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

WebKit Source - https://github.com/WebKit/WebKit/blob/28de25b653f64ca3fc8e87fbf1d1a963329a5342/Source/WebCore/html/canvas/CanvasPath.cpp#L143

Unfortunately, it does not have any test case to confirm whether it is an issue in Safari / WebKit or not but just wanted to get an input.

Thanks!
Comment 1 Radar WebKit Bug Importer 2023-01-17 07:08:16 PST 
<rdar://problem/104332195>
Comment 2 Ahmad Saleem 2023-10-27 15:00:01 PDT 
This compiles:

tatic void normalizeAngles(float& startAngle, float& endAngle, bool anticlockwise)
{
    float newStartAngle = fmodf(startAngle, (2 * piFloat));
    if (newStartAngle < 0)
        newStartAngle += (2 * piFloat);
Comment 3 Ahmad Saleem 2023-10-27 15:03:33 PDT 
If we add this:

    constexpr auto twoPiFloat = 2 * piFloat;

we can simplify whole function:

static void normalizeAngles(float& startAngle, float& endAngle, bool anticlockwise)
{
    constexpr auto twoPiFloat = 2 * piFloat;
    float newStartAngle = fmodf(startAngle, twoPiFloat);
    if (newStartAngle < 0)
        newStartAngle += twoPiFloat;
    float delta = newStartAngle - startAngle;
    startAngle = newStartAngle;
    endAngle = endAngle + delta;
    ASSERT(newStartAngle >= 0 && (newStartAngle < twoPiFloat || WTF::areEssentiallyEqual<float>(newStartAngle, twoPiFloat)));
    if (anticlockwise && startAngle - endAngle >= twoPiFloat)
        endAngle = startAngle - twoPiFloat;
    else if (!anticlockwise && endAngle - startAngle >= twoPiFloat)
        endAngle = startAngle + twoPiFloat;
}
Comment 4 Ahmad Saleem 2023-10-29 06:33:57 PDT 
PR attempt - https://github.com/WebKit/WebKit/pull/19657
Comment 5 EWS 2023-10-30 02:45:03 PDT 
Committed 269925@main (34e0f2e73041): <https://commits.webkit.org/269925@main>

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