Skip to content

Conversation

@rotu
Copy link
Contributor

@rotu rotu commented Nov 24, 2024

Description

#29696 introduced a crash condition where calling BufferGeometry.setFromPoints would cause a crash like three.module.js:10894 Uncaught TypeError: Cannot read properties of undefined (reading 'x') if trying to set fewer points than is in the position attribute.

@rotu
Copy link
Contributor Author

rotu commented Nov 24, 2024

It may be appropriate to also emit a warning or to also set the draw range to only the newly set positions.

@github-actions
Copy link

github-actions bot commented Nov 24, 2024

📦 Bundle size

Full ESM build, minified and gzipped.

Before After Diff
WebGL 339.14
78.99
339.14
78.99
+0 B
+0 B
WebGPU 483.15
134.15
483.15
134.15
+0 B
+0 B
WebGPU Nodes 482.62
134.06
482.62
134.06
+0 B
+0 B

🌳 Bundle size after tree-shaking

Minimal build including a renderer, camera, empty scene, and dependencies.

Before After Diff
WebGL 464.59
111.96
464.62
111.98
+25 B
+17 B
WebGPU 552.25
149.49
552.28
149.51
+25 B
+17 B
WebGPU Nodes 508.13
139.2
508.16
139.22
+25 B
+17 B

@Mugen87 Mugen87 added this to the r171 milestone Nov 25, 2024
@Mugen87
Copy link
Collaborator

Mugen87 commented Nov 25, 2024

It may be appropriate to also emit a warning or to also set the draw range to only the newly set positions.

If the developer intentionally wants to update just a part of the position buffer but rendering the entire range, updating draw range or emitting a warning would be confusing though. I think the PR is fine as it is.

@Mugen87 Mugen87 merged commit add7f9b into mrdoob:dev Nov 25, 2024
12 checks passed
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.

2 participants