cinder / Cinder

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

Icosphere::subdivisions is very unlike Sphere::subdivisions #1059

Open richardeakin opened 9 years ago

richardeakin commented 9 years ago

I'm not sure if this is a problem, but I thought I'd bring up that while the following produces a nicely triangulated sphere:

geom::Sphere().subdivisions( 20 );

The following will cause modern computers to gag as it is creating a zillion vertices:

geom::Icossphere().subdivisions( 20 );

Put differently, while a Sphere() subdivided by 4 looks like:

screenshot 2015-08-11 01 24 02

subdividing an Icosphere by 4 looks like:

screenshot 2015-08-11 01 23 10

(default icossphere looks like:)

screenshot 2015-08-11 01 24 38

So Icosphere treats the parameter as the number of recursive subdivisions, while Sphere treats it as horizontal and vertical strips. I realize the two are constructed quite differently, but I'm wondering if we can't make Icosphere's subdivisions param to mean something like 'number of triangles in each dimension' or something... or at least make the names different to avoid confusion (always last resort suggestion!).

richardeakin commented 9 years ago

Also somewhat related, I take geom::Icosphere().subdivisions( 0 ) to mean in it's current form 'an Icosphere with no subdivisions, which I think is an Icosehedron (from Blender docs). Though it looks to me like it has one level of subdivision with this setting:

screenshot 2015-08-11 01 42 37

Also found a nice description of Blender's parameters with an image of the subdivisions of Sphere vs Icosphere.

notlion commented 9 years ago

Hmm. That subdivisions(0) icosphere should definitely be an icosahedron. Currently it's what you'd call a V2 in geodesic dome terminology. Changing the icosphere subdivision to mean subdivisions along each edge of an icosahedron would make it more similar to the sphere. This would also allow for finer control of triangle count.

paulhoux commented 9 years ago

The reason that subdivisions(0) creates a "V2", is that the icosahedron does not have vertices at the extremes: (0, 1, 0), (1, 0, 0), (0, 0, 1), (0, -1, 0), (-1, 0, 0) and (0, 0, -1). Subdividing the icosahedron once, then normalizing the vertices, produces vertices at all these extremes, causing it to fit nicely in a unit cube.

It's a choice I made, and I am still behind that choice. But I am open to suggestions.

notlion commented 9 years ago

I'm probably not going to be a super heavy user of this functionality, so feel free to disregard my opinions on this. Anyhow.. here are my thoughts:

Having vertices at the extremes seems like an arbitrary restriction. The only restriction i'd expect would be that the vertices are unit distance from the origin. I feel that if the user is allowed to specify the subdivision level, they should be able to choose anything in the range of possible values. That would mean 0 for icosahedron, and then positive integers expressed as the number of non-recursive edge subdivisions on the icosahedron. If that was the case, we could generate V0 – Vn icospheres.

paulhoux commented 9 years ago

That's 2 against 1 :) I'll make the change tomorrow.

paulhoux commented 9 years ago

In the following links, they use the word "level" instead of subdivision: http://donhavey.com/blog/tutorials/tutorial-3-the-icosahedron-sphere/ http://www.sciencedirect.com/science/article/pii/S0377042706006522

, where level 0 is the icosahedron, level 1 will split each edge into 2 edges and level 2 will split each edge into 3 edges. Would "level" be a good name?

I will also have to change the subdivision algorithm to accommodate for the higher decree of control. Right now, we only subdivide in steps of 2: 2, 4, 8, 16, etc. Basically, the complete implementation will have to change, as we need vertices at the poles, even at level 0.

richardeakin commented 9 years ago

We spoke a bit here too, I was suggesting that we use the method subdivision() consistently across all the geoms to mean how many times the geom is subdivided linear in space, as a general concept. For Icosphere, yes I think a separate method such as level(), degree(), or order() would clear things up.

Maybe we should update the api's for the clearer terminology, and then do the finer tuned level / degree subdividing as a separate feature? My thinking is in getting the api right / consistent in preparation for the upcoming release.

notlion commented 9 years ago

Hmm, I agree that the word "subdivisions" suggests recursion rather than linear division. The question then is if having two different scales of detail refinement actually gets us anything. In the case of the sphere and icosphere, the possible results are a subset of those possible with linear division. Say there were two functions divisions() (or level, detail, etc) and subdivisions(). subdivisions(3) can also be expressed as divisions(8).

Maybe as a quick fix, just the naming of the functions could change.. geometries that are linearly divided get a divisions() method and recursive geometries get a subdivisions()?

paulhoux commented 9 years ago

I've thought long and hard about this and would like to propose the following. Bear with me, because it requires some explanation.

The geom namespace currently contains a Subdivide modifier that can handle any source mesh. It subdivides each triangle into 4 new ones. My first proposal is to change the implementation a bit to allow users to subdivide into 9, 16, 25 etc. triangles, allowing more precise control over the subdivision.

The TriMesh class already has a subdivide algorithm that does precisely this. Its implementation is kind of horrible (and I am allowed to say that because I wrote it), but it is efficient and works and could serve as a template for the changes to Subdivide. Since TriMesh itself is a geom::Source, it could easily use the same Subdivide modifier mentioned above. This way, we can get rid of a lot of duplicate code in Cinder.

An icosphere is created by taking an icosahedron, subdivide it and then normalize the vertex positions. The latter is sometimes called "to spherize". I'd like to add a Spherize modifier that takes an optional radius and which can handle all geom::Source classes. That way, you can also easily create a sphere based on a cube ( Image: (c)2008 Inigo Quilez ):

gfx02

Next, I'd like to propose to rewrite part of the IcoSphere code in such a way that it uses the Subdivision and Spherize modifiers internally to calculate its vertex attributes. We will keep the code that removes the texturing artefacts (seams and zig-zags).

With all these changes, we get rid of a lot of duplicate code and create more freedom for the user. What y'all think?

paulhoux commented 9 years ago

Bummer, the subdivision algorithm currently implemented in TriMesh has a problem: it does not share newly created vertices between polygons. Take for instance a rectangle made of 2 triangles. It would suffice to create 5 new vertices: 1 for each of the edges and 1 for the diagonal. The latter can then be shared. Yet the current algorithm creates 2 identical vertices on the diagonal, for a total of 6.

This is getting complicated. To do this properly, I'd need to create a different representation of the data (e.g. half-edge adjacency structure).

Ah well, I could also decide to accept the fact we're not sharing the vertices for now. So far, nobody has complained about it, haha.

richardeakin commented 9 years ago

Seems like nice future work to look into, but really the goal of opening this issue on my end was to come up with a way to unify the terminology so that geom functions with similar names act consistently.

Concerning the common function subdivisions(), this was a word that @andrewfb originally came up with when we were adding geom::Plane(), and noticing that many of the geoms had a similar method but used a variety of names and arguments. I didn't think of it as meaning recursive division at the time, but I guess you could interpret it that way depending on where you're coming from. More importantly, I just think we should pick one term to mean 'linear segmentation' and use it throughout, since the goal is usually do control how much geometry detail depending on how you are going to treat the object. I'd be fine with divisions() or segments() as well. Once we document what this word means, I think it would be fine for any of the three, as long as we keep it consistent.

I also still like the more mathematically correct methods, such as the one for Icossphere that would define whether it is a V0, V1, V2, etc. @paulhoux brought up the term level( int ) or levels( int ), which I think is pretty clear.

araid commented 8 years ago

I'm very interested in this as I'm making heavy use of the geom classes right now. In particular, making low-poly shapes can sometimes feel a bit confusing with the current controls.

In Sphere, for example, the number of rings is calculated with *numRings = ( *numSegments >> 1 ) + 1;, so the user has no control over that.

For reference, Three.js seems to have a nice api for that http://threejs.org/docs/#Reference/Extras.Geometries/SphereGeometry

Anyways, thanks for the work @paulhoux !

richardeakin commented 8 years ago

I like the thetaStart and thetaLenght params. Though I think the harder, more controversial api choices to decide on are those of Icosphere, specifically on what subdivisions() means there in order for it not to be surprising.

paulhoux commented 8 years ago

I am open to suggestions and really like the clean Three.js API. Is it consistent when it comes to terminology? Can we adopt it?

richardeakin commented 8 years ago

Here's the api for Icosahedron, it's pretty basic. One thing interesting is that he chose the word 'detail' instead of subdivisions (looks like he uses the term 'segments' for the UV Sphere that @araid linked).