Skip to content

Fix #5512 Give icosahedron normals & texcoords#5650

Closed
hamoid wants to merge 1 commit intoopenframeworks:masterfrom
hamoid:master
Closed

Fix #5512 Give icosahedron normals & texcoords#5650
hamoid wants to merge 1 commit intoopenframeworks:masterfrom
hamoid:master

Conversation

@hamoid
Copy link
Copy Markdown
Contributor

@hamoid hamoid commented Jun 5, 2017

In the previous version, icosphere() could be used to produce
icosahedrons with normals and texture coordinates, while
icosahedron() would produce just the faces with missing normals
and texture coordinates.

The dependency between icosphere and icosahedron has been
reversed, and icosahedron() now provides normals and texture
coordinates.

In the previous version, icosphere() could be used to produce
icosahedrons with normals and texture coordinates, while 
icosahedron() would produce just the faces with missing normals
and texture coordinates.

The dependency between icosphere and icosahedron has been
reversed, and icosahedron() now provides normals and texture 
coordinates.
@bakercp
Copy link
Copy Markdown
Member

bakercp commented Jun 6, 2017

Nice work. Confirmed working on master w/ macOS.

@arturoc
Copy link
Copy Markdown
Member

arturoc commented Jun 6, 2017

that looks good but i think the normals should be flat for an icosahedron and smooth for an icosphere. perhaps there should be a common private function for icosphere and icosahedron for the geometry and then each would add the correct normals. otherwise the faces look strange when seen with lighting

@hamoid
Copy link
Copy Markdown
Contributor Author

hamoid commented Jun 6, 2017

Would it make sense to add a flatNormals() method to ofMesh, which would duplicate vertices to get the expected non-smooth icosahedron? Such method could be used with other primitives like cylinder or any random mesh.

@arturoc
Copy link
Copy Markdown
Member

arturoc commented Jun 6, 2017

yeah that would make sense

@hamoid
Copy link
Copy Markdown
Contributor Author

hamoid commented Jun 8, 2017

I implemented a method to set flat shading at #5651

@arturoc
Copy link
Copy Markdown
Member

arturoc commented Jun 8, 2017

perfect, once that's merged i think this should be setting flat normals for the icoshaedron by default

@hamoid
Copy link
Copy Markdown
Contributor Author

hamoid commented Jun 8, 2017

I agree. I'll wait for the merge and set it flat by default.

@arturoc
Copy link
Copy Markdown
Member

arturoc commented Jul 4, 2017

Just merged the other PR, if you want to add the flat by default for icoshaedron i'll merge this too

@hamoid
Copy link
Copy Markdown
Contributor Author

hamoid commented Jul 4, 2017

I'll close this PR. I created a new one with normals and default flat shading for the icosahedron

@hamoid hamoid closed this Jul 4, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants