vurtun / nuklear

A single-header ANSI C gui library
13.69k stars 1.11k forks source link

Circles are not full circles #566

Open h33p opened 6 years ago

h33p commented 6 years ago

At least on SDL GL2, circles (non filled ones), when rendered at big size and low thickness have a noticeable missing vertice on the right side. (Not sure yet how they are drawn, will check out later) Either the point position calculation should use one less point inside, or an additional vertice should be added to render the circle. Will try to look into a way to fix it, and will also test on D3D9 if it's just a problem with SDL or not.

h33p commented 6 years ago

Okay, this has to do with sin/cos approximation. It is good for everything but circles. I looked for other approximation algorithms and stumbled on this post. The faster method fails miserably, but the more precise one works well. I really think it's costlier than the current method but it results in a nice circle: Also, the current approximation does not wrap the x value in the correct range but I guess, it's not needed for internal use.

h33p commented 6 years ago

Since other parts are completely fine with the default implementation, using a different sin/cos algorithm for all of them would result in performance degradation. I would propose a more accurate sin/cos implementation for parts that need more precision (e.g. this circle, inside nk_draw_list_path_arc_to). This would be NK_SINA and NK_COSA, and the following code would be run:

NK_INTERN float
nk_sina(float x)
{
    float sin = 0.f;

    if (x < -3.14159265f)
        x += 6.28318531f;
    else
        if (x >  3.14159265f)
            x -= 6.28318531f;

    if (x < 0)
    {
        sin = 1.27323954f * x + .405284735f * x * x;

        if (sin < 0)
            sin = .225f * (sin *-sin - sin) + sin;
        else
            sin = .225f * (sin * sin - sin) + sin;
    }
    else
    {
        sin = 1.27323954f * x - 0.405284735f * x * x;

        if (sin < 0)
            sin = .225f * (sin *-sin - sin) + sin;
        else
            sin = .225f * (sin * sin - sin) + sin;
    }
    return sin;
}

NK_INTERN float
nk_cosa(float x)
{
    return nk_sina(x + 1.57079632f);
}

This implementation also wraps the value inside -PI +PI range, if that's not needed, it can be removed.

h33p commented 6 years ago

Speaking of circles, an option to pass over segment count could be made, so that smaller circles wouldn't need to have that many polygons. Rendering 50+ variable sized circles would help drastically with performance.

vurtun commented 6 years ago

Hm I need to think about adding another sin and cos approximation. However what you could do is directly overwrite sin and cos by defining NK_SIN to sinf and NK_COS to cosf or your own sin and cos implementation.

Speaking of circles, an option to pass over segment count could be made, so that smaller circles wouldn't need to have that many polygons. Rendering 50+ variable sized circles would help drastically with performance.

You can define the default number of circle segments used to draw a circle by setting circle_segment_count inside nk_convert_configto a higher number. The reason why I don't have a segment count property for each circle so far was because all non-vertex backends have no need for it.

Not sure if it is worth it or not to add one. Since then each widget drawing a circle somehow has to guess how many vertexes it needs. Another solutions may be to have a configurable describing the number of segments, or a push/pop API for circle segment count. Problem is that all these solutions force each widget implementation into thinking about vertex count even if the backends does not use it.

I need to think about this.

h33p commented 6 years ago

I have already added variable segment count in my repo, basically, if the command has segment count set to 0, it uses the config's segment count, else, uses the supplied one.