cinder / Cinder

Cinder is a community-developed, free and open source library for professional-quality creative coding in C++.
http://libcinder.org
Other
5.34k stars 944 forks source link

Fix missing "slice" when drawing large circles with geom::Circle #2283

Closed afrancois closed 2 years ago

afrancois commented 2 years ago

If rendering a large solid circle with geom::circle, the circle doesn't go all the way around. This makes it so that the last point outside the circle is at the same location as the first outer point, thus ensuring the circle is complete.

The following class will test the circle.

#include "cinder/app/App.h"
#include "cinder/app/RendererGl.h"
#include "cinder/gl/gl.h"

using namespace ci;
using namespace ci::app;
using namespace std;

class CircleTestApp : public App {
  public:
    void setup() override;
    void mouseDown( MouseEvent event ) override;
    void update() override;
    void draw() override;

    ci::gl::BatchRef mRenderBatch;
    ci::gl::GlslProgRef mShaderProg;
};

void CircleTestApp::setup()
{
    mShaderProg = ci::gl::getStockShader(ci::gl::ShaderDef().color());
}

void CircleTestApp::mouseDown( MouseEvent event )
{
}

void CircleTestApp::update()
{
}

void CircleTestApp::draw()
{
    //a big radius
    auto radius = 1920*2;

    //set the center so that the right edge is visible 
    auto center = getWindowCenter();
    center.x =  -radius + center.x;

    gl::clear( Color( 0, 0, 0 ) ); 

    //draw a circle in white using drawSolidCircle. This works fine!
    gl::color(Color(1, 1, 1));
    gl::drawSolidCircle(center, radius);

    //make a red circle over top of the white one

    gl::color(Color(1, 0, 0));
    auto theCircle = ci::geom::Circle().radius(radius).center(center);

    if (mRenderBatch) mRenderBatch->replaceVboMesh(ci::gl::VboMesh::create(theCircle));
    else mRenderBatch = ci::gl::Batch::create(theCircle,mShaderProg);

    //draw the renderbatch circle. The result should be a solid red circle.
    //without the fix you'll get a small white sliver showing through the red circle.
    mRenderBatch->draw();
}

CINDER_APP( CircleTestApp, RendererGl )
andrewfb commented 2 years ago

Thanks for this, and especially a repro case. I think this may actually be due to float vs double precision - can you verify that efe951e9fd02937cbdf4be8e1f73ae36bd3357de addresses your case?

afrancois commented 2 years ago

@andrewfb Your solution in https://github.com/cinder/Cinder/commit/efe951e9fd02937cbdf4be8e1f73ae36bd3357de moves the problem out father but a sufficiently large circle is still going to exhibit the issue. At the end of the day, I think the that last point of the outside circle and the first point of the outside of the circle need to be the same.

andrewfb commented 2 years ago

The current implementation does make the first and last outside points the same, i.e. t=0.0 and t=1.0 are the first and last points, but it's using accumulation to get to 1.0 for the dubious benefit of eliminating a couple of inner-loop multiplies. Take a look at fbbe5aacc96b6c952f3d80b156db72dd5fde610d which I think should address it however. The problem with forcing the last point to match the t=0 point is that it pushes all the floating point accumulation error to the last triangle, resulting in a larger triangle than all the others.

andrewfb commented 2 years ago

Closing in favor of #2288