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 943 forks source link

Add more intuitive geom::Combine() syntax #700

Open sansumbrella opened 9 years ago

sansumbrella commented 9 years ago

The upstream + source behavior of geom::Combine() is confusing.

Consider adding a geom::Combine() method that is more intuitive:

auto cube = geom::Cube();
auto sphere = geom::Sphere();
// Proposed
auto together = geom::Combine(&cube, &sphere);
// Current (I think)
auto together = cube >> geom::Combine(&sphere);

It would be extra cool if it was written with variadic parameters, so you could combine as many things as you like.

Alternatively, consider renaming geom::Combine() to geom::Append() to more clearly describe what it is doing: adding a piece of geometry at the end of the stream (right?).

sansumbrella commented 9 years ago

I will try putting together a PR for this, but I'm not yet familiar with the inner workings of the geom pipeline.

EDIT: looks like what I'm thinking of should be a Source, not a Modifier. Starting down the road of an Aggregate Source.

andrewfb commented 9 years ago

I'm open to this but maybe it's worth considering a name change rather than adding additional code. It seems like a new Source type is not adding any functionality we don't already have, in the sense that the example you cite above (proposed and current) are indeed equivalent. Part of the advantage of the current design is that the Combine()d geometry is secondary. This is important when deciding what to do when one of the sources lacks attributes the other does not, and is part of why I went with this design.

sansumbrella commented 9 years ago

That makes sense.

How do you feel about changing to geom::Append() then?

heisters commented 9 years ago

Apologies if this is not related, but I would love to see something that would allow using geom::Combine() with a source argument by value, and return a new source. I've been trying to turn this:

    auto b1 = geom::Cube() >> geom::Rotate( quat( randVec3f() * (float)M_PI * 2.f ) ) >> geom::Scale( randVec3f() );
    auto b2 = geom::Cube() >> geom::Rotate( quat( randVec3f() * (float)M_PI * 2.f ) ) >> geom::Scale( randVec3f() );
    auto b3 = geom::Cube() >> geom::Rotate( quat( randVec3f() * (float)M_PI * 2.f ) ) >> geom::Scale( randVec3f() );
    auto b4 = geom::Cube() >> geom::Rotate( quat( randVec3f() * (float)M_PI * 2.f ) ) >> geom::Scale( randVec3f() );
    auto b5 = geom::Cube() >> geom::Rotate( quat( randVec3f() * (float)M_PI * 2.f ) ) >> geom::Scale( randVec3f() );
    auto b6 = geom::Cube() >> geom::Rotate( quat( randVec3f() * (float)M_PI * 2.f ) ) >> geom::Scale( randVec3f() );
    auto b7 = geom::Cube() >> geom::Rotate( quat( randVec3f() * (float)M_PI * 2.f ) ) >> geom::Scale( randVec3f() );

    gl::VboMesh::create( b1 >> geom::Combine( &b2 ) >> geom::Combine( &b3 ) >> geom::Combine( &b4 ) >> geom::Combine( &b5 ) >> geom::Combine( &b6 ) >> geom::Combine( &b7 ) );

into something like this (which doesn't compile):

    auto boxy = geom::Cube();
    for ( size_t i = 0; i < 10; ++i ) {
        auto b = geom::Cube() >> geom::Rotate( quat( randVec3f() * (float)M_PI * 2.f ) ) >> geom::Scale( randVec3f() );
        boxy = b >> geom::Combine( &boxy );
    }
    gl::VboMesh::create( boxy );

I've tried a bunch of other possibilities, but they all fail at compile time or runtime. The latter, ostensibly because the argument to geom::Combine() is out of scope when the target requests the geometry.