Skip to content

Conversation

Mugen87
Copy link
Collaborator

@Mugen87 Mugen87 commented Jun 2, 2018

More Geometry removal...

@Mugen87
Copy link
Collaborator Author

Mugen87 commented Jun 2, 2018

@WestLangley
Copy link
Collaborator

if ( geo.isGeometry ) geo = new THREE.BufferGeometry().fromGeometry( geo );

If the user passes in Geometry, won't this generate a plot with face and vertex indices different from the original? I expect that would be quite confusing.

@WestLangley
Copy link
Collaborator

Your plots appear to be overprinting triangles on the right edge with two different sets of indices.

@Mugen87
Copy link
Collaborator Author

Mugen87 commented Jun 2, 2018

Your plots appear to be overprinting triangles on the right edge with two different sets of indices.

You're right. I'll have a look.

If the user passes in Geometry, won't this generate a plot with face and vertex indices different from the original? I expect that would be quite confusing.

When converting to BufferGeometry, geometry data become non-indexed so the result will be different. We could stop supporting Geometry to prevent this...

@WestLangley
Copy link
Collaborator

We could stop supporting Geometry to prevent this...

I think the only useful solution is to add direct support for both Geometry and BufferGeometry to this utility.

@Mugen87
Copy link
Collaborator Author

Mugen87 commented Jun 2, 2018

Okay, done. UVDebug handles now Geometry and BufferGeometry without conversions.

@Mugen87 Mugen87 changed the title UVDebug: Moved to BufferGeometry UVDebug: Process BufferGeometry without conversion Jun 2, 2018
@WestLangley
Copy link
Collaborator

Notice how in the Icosahedron case, the triangles straddle the seam. That is what the special logic was for. :)

@Mugen87
Copy link
Collaborator Author

Mugen87 commented Jun 2, 2018

What do you mean with "triangles straddle the seam"? On what logic do you refer to? Sry, I'm a bit lost^^.

@WestLangley
Copy link
Collaborator

For IcosahedronGeometry( size, 1 ), the triangles on the right side of the uv plot straddle the uv seam, that is, they continue onto the left side of the plot. The geometry will only render with a texture correctly if wrapS = THREE.RepeatWrapping. That is what the "// wrap x" inline comments are all about.

@Mugen87
Copy link
Collaborator Author

Mugen87 commented Jun 2, 2018

Um, the visual result is exactly like before...

@Mugen87
Copy link
Collaborator Author

Mugen87 commented Jun 2, 2018

This is how the visualization of IcosahedronGeometry looks in production:

image

@WestLangley
Copy link
Collaborator

I was not being critical. I was just making what I thought was an interesting point. I was reminded of #3299. Sorry I did not make myself more clear.

@WestLangley
Copy link
Collaborator

@mrdoob This PR is good-to-go.

@Mugen87
Copy link
Collaborator Author

Mugen87 commented Jun 3, 2018

@WestLangley Thanks for you help in this PR! 👍 😊

@mrdoob mrdoob added this to the r94 milestone Jun 16, 2018
@mrdoob mrdoob merged commit a3538ec into mrdoob:dev Jun 16, 2018
@mrdoob
Copy link
Owner

mrdoob commented Jun 16, 2018

Thanks guys!

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