Added the missing platonic solid and their wireframe version.

[CCortex]

Hi,

I added some geometry : Tetrahedron Octahedron Dodecahedron

The only missing thing is the "right" uv text. Tell me guys how you want it implemented.

[paulhoux]

Oh hey, thanks for the pull request. I guess it's kind of hard to come up with a good mapping algorithm for these solids, again fueling the discussion we had earlier about separating the UV-mapping from the primitives completely.

For now, we could simply map the solids in a similar way as the icosahedron, where each triangle is mapped independently.

[CCortex]

Correction done. For the uv mapping, we'll just leave it like this for now until the separation is made then.

[richardeakin]

Separating UV-mapping from geom::Sources? One of their main utilities is providing texcoords, so I don't see why we'd ever want to remove that functionality. We could possibly think of a way to offer more flexible mappings, actually I think this is already possible with a geom::Modifier but perhaps we could add something built in for primitives that could use more complex mappings.

[richardeakin]

The implementations look good to me, although I'm seeing many white space irregularities. Please use spaces within brackets (like here) and around math operations (like here and most calls within that function). Of course there is the more complete style guide: CONTRIBUTING.md.

[paulhoux]

To explain a bit more what I mean with separating UV-mapping: we've discussed having a geom::Mapper class of sorts, providing mappers for spherical, cylindrical, cube and planar mapping, to name just a few. The primitives themselves would not have code in them to provide texcoords, but by applying one of the mappers you could easily add them. As you mentioned, geom::Mapper would probably extend geom::Modifier.

[paulhoux]

I think the code inside the geom namespace will likely change in the future, as I can see a lot of duplicated functionality in there. For instance, a lot of primitives can be constructed by extruding a 2D shape (e.g. a circle) along a line (to create a cylinder) or a curve (to create a torus). Having a few basic functions and using them from within the respective classes would reduce the code tremendously while making it easier to add primitives later. I will not tackle this in the near future, though, too much stuff on my plate already.

[paulhoux]

For the uv mapping, we'll just leave it like this for now until the separation is made then.

I'd much prefer to have the new primitives support some form of mapping, otherwise they stand out as the only ones that don't. It would feel incomplete. @CCortex Do you think you can add texcoords?

[richardeakin]

I agree that any new primitives should support at least some sort of uv mapping, or else when you try to apply a texture to them (or just expect texcoords in the shader) it will appear broken. I should also say thanks for the contribution, having more geoms to play with is always nice.

(sorry, derailing on geom refactor below)

Re geom::Mapper: if you're talking using this as an implementation detail and a way to improve code organization, I can see how that can be an improvement. However if it means that from then on out, you need to remember an extra call in order to get basic tex coords (like say, on a Cube) that seems not as nice to me as having the texcoords built into the geom. To me, a geom::Source should always be able to provide some sort of tex coords as is, but maybe that could mean using a Mapper internally.

So I guess my concerns are about API - for example I would much rather have a Cylinder class than having to construct a cylinder by extruding a circle, even though you can do that of course if you wanted to. Likewise, I'd like that Cylinder to have a default texcoord mapping. Though I can see the gain by implementing these higher level geom classes using reusable components.

[paulhoux]

Oh yes, I totally mean to keep a Cylinder class, but its internals could be rewritten to make use of more generic algorithms, instead of having very similar code for each of the primitives (the way it is now).

I agree that the geom::Mapper should be optional, with uv-coordinates being available by default. Not entirely sure how to code this, but we'll figure it out.

[paulhoux]

Hi @CCortex ,

I just tried the new primitives in the Geometry sample, but there are a few issues:

• The Octahedron and Dodecahedron have missing texture coordinates.
• The WireDodecahedron should be clean, without triangulation.
• The Dodecahedron does not fit inside a 2x2x2 cube, which is what we chose to be the default.
• The Tetrahedron does not align well with the coordinate frame. This makes it hard to use the primitive in applications. Preferably, it should be aligned with the y-axis, where the y-axis goes through one vertex (the 'top') and through the middle of the opposite polygon.

These issues need to be fixed before we can accept the pull request.