Skip to content

Commit

Permalink
fix(troika-three-text): fix changing text length in ThreeJS r117+
Browse files Browse the repository at this point in the history
Fixes #69. This works around an issue introduced in Three r117 where a
given geometry uses the size of any InstancedBufferAttribute as a max
for `instanceCount`, and caches that for the lifetime of the geometry
even if the attribute is replaced with one of a different size.

It's unclear if this is a ThreeJS bug or a if our approach of "resizing"
by replacing the attribute is truly unsupported; the discussion in
mrdoob/three.js#19706 is ambiguous,
mrdoob/three.js#19595 implies it's not
allowed, and mrdoob/three.js#17418 implies it
should be allowed.
  • Loading branch information
lojjic committed Jul 19, 2020
1 parent 0650c59 commit a7ef945
Showing 1 changed file with 8 additions and 0 deletions.
8 changes: 8 additions & 0 deletions packages/troika-three-text/src/GlyphsGeometry.js
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,14 @@ function updateBufferAttr(geom, attrName, newArray, itemSize) {
attr.needsUpdate = true
} else {
geom.setAttribute(attrName, new InstancedBufferAttribute(newArray, itemSize))
// If the new attribute has a different size, we also have to (as of r117) manually clear the
// internal cached max instance count. See https://github.com/mrdoob/three.js/issues/19706
// It's unclear if this is a threejs bug or a truly unsupported scenario; discussion in
// that ticket is ambiguous as to whether replacing a BufferAttribute with one of a
// different size is supported, but https://github.com/mrdoob/three.js/pull/17418 strongly
// implies it should be supported. It's possible we need to
delete geom._maxInstanceCount //for r117+, could be fragile
geom.dispose() //for r118+, more robust feeling, but more heavy-handed than I'd like
}
} else if (attr) {
geom.deleteAttribute(attrName)
Expand Down

0 comments on commit a7ef945

Please sign in to comment.