ImDrawList::PathArcToAdaptive - path arc that automatically adjust precision according to arc radius

[thedmd]

This PR adds ImDrawList::PathArcToAdaptive(). An implementation of PathArcTo that automatically adjust number of used samples according to used arc radius.

Implementation use 32 element lookup table to fill arc content. On worst case scenario calculate two sampling points when start or end angles are not in lookup table. 32 samples cover radii up to 80 pixels.

To complete this PR decision to call PathArcToAdptive instead of PathArcTo by default in AddXXX functions should be made.

I'm looking for your feedback.

Node: Demo in DO NOT MERGE commit may not work out of the box.

Drawing results

Benchmark results

Notice a jump around 80 pixel radius. This is where fallback to PathArcTo kick in.

With PathArcToFast

With PathArcTo

[thedmd]

Edit: It turns out it is easy to make mistake. So I did.

I added PathArcToFast2 which also use lookup table with 32 samples. It outperform PathArcToFast for radii < 10.
PathArcToFast2 isn't outperforming PathArcToFast, but always generate smooth looking output.

void ImDrawList::PathArcToFast2(const ImVec2& center, float radius, int a_min, int a_max, int a_step)
{
    if (radius == 0.0f || (a_min >= a_max) || (a_step < 0))
    {
        _Path.push_back(center);
        return;
    }

    if (a_step == 0)
    {
        // Determine step size between samples. Small radii usually allow to skip more than
        // one sample, reduce geometry complexity without harming visual output.
        const float error_for_radius = IM_DRAWLIST_ADAPTIVE_ARC_RADIUS_ERROR_CALC(IM_DRAWLIST_ADAPTIVE_ARC_SAMPLES, radius);
        if (_Data->CircleSegmentMaxError > error_for_radius)
            // Reduce step, but never go lower than 2 samples per quarter of the circle.
            a_step = ImMin((int)ImSqrt(_Data->CircleSegmentMaxError / error_for_radius), IM_DRAWLIST_ADAPTIVE_ARC_SAMPLES / (4 * 2));
    }

    const int  distance = a_max - a_min;
    const bool extra_max_sample = ((a_max - a_min) % a_step) != 0;

    const int samples = (distance) / a_step + (extra_max_sample ? 1 : 0);

    _Path.reserve(_Path.Size + samples);

    for (int a = a_min; a <= a_max; a += a_step)
    {
        const ImVec2& c = _Data->AdaptiveArcVtx[a % IM_ARRAYSIZE(_Data->AdaptiveArcVtx)];
        _Path.push_back(ImVec2(center.x + c.x * radius, center.y + c.y * radius));
    }

    if (extra_max_sample)
    {
        const int a = a_max;
        const ImVec2& c = _Data->AdaptiveArcVtx[a % IM_ARRAYSIZE(_Data->AdaptiveArcVtx)];
        _Path.push_back(ImVec2(center.x + c.x * radius, center.y + c.y * radius));
    }
}

[thedmd]

Few tweaks and PathArcToFast2, integrating adaptive arc with PathArcTo (used when num_segments == 0), bumping number of samples in table to 288 and I think it is a decent replacement.
Note: PathArcToFast is now mapped to PathArcToFast2.

Everything is sampled 10000 times, repeated 10 times and then averaged.

PathArcToFast always output same amount of geometry
PathArcToFast2 adjust amount geometry according to radius, so it will be slower for larger arcs.

Most common radii are around 10.

What is interesting PathArcTo is as performant as PathArcToFast2.

To try it by yourself replace yoursimgui_demo.cpp with one from this branch. You will be able to see performance of current version. Or just build this PR and you will see performance of proposed version. Keep in mind that this version emit more geometry for larger arcs and less for small arcs.

You can checkout spreadsheet with data, there are also more charts inside (file is in ods format):
bench_stats_cmp.zip

Feedback will be appreciated.

[thedmd]

Rebased and squashed to make it easier to view.

[thedmd]

Rebased on master. It is green now.

[ocornut]

Thank you very much. I'm hoping to adopt that shortly after 1.80.

[ocornut]

Quick question:
you added in PathArcTo():

    if (a_min >= a_max)
        return;

What's the reasoning for it?

[ocornut]

if (a_min >= a_max)
return;

That matches the equivalent test in PathArcToFast(). But it's technically an API breakage which I don't mind if necessary but seems like we could just swap values? Otherwise need to document as breaking.

[thedmd]

Values can be swapped. It was a guard to prevent invalid input values.

[thedmd]

PR was rebased on #3808, cleaned up and grew a need for feedback.

At the moment I introduced PathArcTo2 and PathArcToFast2, leaving current implementation untouched for benchmarking purposes. Pushing math work in #3808 was beneficial and allowed me to simplify implementation and get rid of "work on stardust" factor.

One thing I do want to address and need the feedback the most is the bool anti_aliased function parameter. Anti-Aliased geometry use smaller max error value to produce smoother shapes. This is also true for arcs. PathXXX primitives does not have knowledge how they're about to be used. AddXXX functions on the other hand do.
The question is, how can I get rid of this parameter? Should I use same max error value everywhere? Should AA flags in ImDrawList be unified into one? Some other solution? Please share your thoughts.

Also updated benchmark results. There will be more after technical issued get addressed.

[thedmd]

I made a test using ImDrawList::PathRect(). Made a copy of it called PathRect2 and replaced calls to PathArcToFast with PathArcToFast2 which is an adaptive version. PathRect call arc function four times. Rounding radius is always maximum possible value, which is worst case scenario for rounded rect. This also make them look like circles:

The Benchmark

Each PathXXX function was run 10000 times, after that PathStroke or PathFill were called also 10000. After that was done, whole operation was repeated 50 times and results averaged. So for each radius test each function was called half a milion times. Benchmark is restricted to single core. And there are the results:

Results

We have four graphs in this order:

Graph	Comment
PathRect2 - using _PathArcToFastEx	Outlined, Adaptive rounding
PathRect - current	Outlined, Each corner emit 4 points
PathRect2 - using _PathArcToFastEx	Filled, Adaptive rounding
PathRect - current	Filled, Each corner emit 4 points
(Sorry for not naming them differently.)

Worst case

Worst case scenario: rounding radius = 0.25 * rect_width
(Image is rotated to avoid excessive scrolling).

More realistic case

Less cruel example, where rounding is halved. rounding radius = 0.25 * rect_width

Conclusions

Overall, adaptive arcs are on pair with current implementation. They are faster for small radii, on pair with most commonly used ones and became slower as number of vertices grow. Please take a note that we're in microseconds (1e-6) realm while reading graphs.

Arc generation to PathStroke/PathConvexFill ratio is 1:2. And it as number of vertices grow ratio approach 1:5, this is for radius 116.

Summary

I'm open for any feedback you can have. Branch currently have a benchmark code embedded in image_demo.cpp. You can build it and test it yourself. There are more test cases for arc in the code, but disabled. Demo can generate CSV file for enabled benchmarks.

After few days waiting for feedback I will move forward to wrap up this PR by replacing current implementation.

Thanks!

Edit:
One extra perf graph for 90° arc length. Most commonly used by rounded rect.

[thedmd]

Commits 42923c6 and d836189 are the essence of this PR. Remaining two are benchmark harness and fixup after replacing original implementation.
First two can be squashed together.

I think, PR is ready to be reviewed.

[ocornut]

Merged (had lots of privates discussions leading to this), thanks for your amazing work.

I'll close this. As you know part of the demo/test bed may be worth including in either Demo or our Test Facilities in the future.