Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor Pen Tool to use triangles instead of triangle strips for rendering #2992

Merged
merged 11 commits into from Sep 14, 2020

Conversation

InfiniteLee
Copy link
Contributor

@InfiniteLee InfiniteLee commented Sep 9, 2020

Three.js as of r112 has removed drawMode from meshes, thus deprecating support for triangle-strip rendering. This updates networked-drawing and sharedbuffergeometry to use indexed triangles instead.

This also moves the serialize drawing button on the drawing hover menu so that it doesn't overlap with the new media refresh button.

Copy link
Contributor

@johnshaughnessy johnshaughnessy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not as familiar with this code as I'd like to be - but these changes lgtm

this.current.attributes[key].array[pos++] = this.current.attributes[key].array[i];
if (key === "index") {
//index needs to be handled specially
this.current.index.array[pos++] = this.current.index.array[i] - idx.position - 1;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does this line do / why are indices handled differently? I'm having a hard time keeping track of idx, prevIdx, this.idx and all of their .index attributes

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unlike position, normals or colors, indexes are not accessible on the geometry.attributes property, they're instead accessed via geometry.index directly, or via getIndex() or setIndex()

src/components/tools/networked-drawing.js Outdated Show resolved Hide resolved
@InfiniteLee InfiniteLee merged commit 963a4ac into master Sep 14, 2020
@InfiniteLee InfiniteLee deleted the feature/drawing-triangles-refactor branch September 14, 2020 20:47
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.

None yet

2 participants