Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Improve on automatic circle segment count calculation #3808

Closed
wants to merge 3 commits into from

Conversation

thedmd
Copy link
Contributor

@thedmd thedmd commented Feb 13, 2021

This PR improves on automatic number of circle segment calculation for anti-aliased and non anti-aliased shapes.

The Old

After analyzing equation in IM_DRAWLIST_CIRCLE_AUTO_SEGMENT_CALC few issues were discovered:

  • number of segments N was two times bigger than necessary (error was applied to half of an angle spanned by the segment)
    This is event somewhat acknowledged in the code, because small circles received too many segments:
    FIXME: the minimum number of auto-segment may be undesirably high for very small radiuses (e.g. 1.0f).
  • N was floored, but should be ceiled, this was reducing quality but cannot be spotted due to first issue
  • CircleSegmentMaxError claim to be expressed in pixels, but default value 1.6 seems to not reflect that

The New

Equation for number of circle segments was derived from scratch using method described in this stackoverflow post. Which lead to discoveries listed above about old method.

New method:

  • Defaults CircleSegmentMaxError to 0.5f, which is half of a pixel
  • N is ceiled up and rounded to next even value
  • N is different for anti-aliased and not anti-aliased shapes

More details

Number of segments (N) is calculated using equation:

             /          pi         \
   N = ceil | --------------------- |     where r > 0, error <= r
             \ acos(1 - error / r) /

Note:

Equation is significantly simpler that one in the post thanks for choosing segment
that is perpendicular to X axis. Follow steps in the article from this starting condition
and you will get this result.

Rendering circles with an odd number of segments, while mathematically correct will produce
asymmetrical results on the raster grid. Therefore we're rounding N to next even number.
(7 became 8, 11 became 12, but 8 will still be 8).

Error value is expressed in pixels and defaults to 0.5f, half a pixel. Which produce
non anti-aliased circles where segments cannot be spotted.
Anti-aliasing work on much finer grid than pixels which make segments visible. To mitigate this
issue error is scaled by IM_DRAWLIST_CIRCLE_AUTO_SEGMENT_AA_ERROR_SCALE_FACTOR defaulting to 0.5f.
This make anti-aliased circles use 0.25f as an error value, making segments unnoticeable.

Derivation

N equation was derived by following steps in method described in this stackoverflow post. What I did differently is I choose different A and B points to make calculations simpler.

image

Comparison

At previews you can see actual maximum error value used.

Old vs. New

Anti-Aliased

Old
image
New
image

Non Anti-Aliased

Notice there are gaps on the circumference of the circle N 27 in the Old. They appear if mesh is too dense.

Old
image
New
image

New Method: AA vs. Non AA

Maximum error: 0.5

Notice that you can spot line segments when circle is drawn with same number of segments for AA and not-AA circles.

Non AA
image
AA
image

Maximum error: 0.25

Notice that non anti-aliased primitives has gaps due to too dense mesh (first circle), but anti-aliased drawing give the desired result.

Non AA
image
AA
image

@thedmd thedmd force-pushed the feature/draw_path_circle_error branch from eb71b82 to 92a0005 Compare February 13, 2021 13:09
@ocornut
Copy link
Owner

ocornut commented Feb 13, 2021

Going to have to create new certificates like the one in #3627 ..

@ocornut
Copy link
Owner

ocornut commented Feb 17, 2021

Merged squashed (f107693) + amends (fb15d8c)
Thank you again for all those details and iterations and pms :)

@ocornut ocornut closed this Feb 17, 2021
@thedmd thedmd deleted the feature/draw_path_circle_error branch March 10, 2021 16:21
off_x += 10.0f + rad * 2.0f;
const float rad = RAD_MIN + (RAD_MAX - RAD_MIN) * (float)n / (9.0f - 1.0f);

const int segment_count = draw_list->_CalcCircleAutoSegmentCount(rad);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

imgui_demo never relies on private APIs, except this one new case.

imgui/imgui.h

Lines 2440 to 2447 in 00d570e

// [Internal helpers]
IMGUI_API void _ResetForNewFrame();
IMGUI_API void _ClearFreeMemory();
IMGUI_API void _PopUnusedDrawCmd();
IMGUI_API void _OnChangedClipRect();
IMGUI_API void _OnChangedTextureID();
IMGUI_API void _OnChangedVtxOffset();
IMGUI_API int _CalcCircleAutoSegmentCount(float radius) const;

When porting over the demo, I had to skip this line:

oprypin/crystal-imgui@7381050#diff-03b057dd391cd708d0edc96be17b8790d6471aac4adf0ec433745795795a99d3R5588

I think this should be fixed

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is documented as private but available in imgui.h and we use it to provide info about tessellation in the demo. I think that's fine, and very obviously we don't expect people to copy/reuse this overly specific tessellation display code.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please consider that previously it would be 100% true to say "absolutely everything in imgui_demo can be done without internal calls". I think that's worth something. But now you cannot say this.

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants