From 7818d05745f5ca44530aae774df13deafb35588a Mon Sep 17 00:00:00 2001 From: Jason Johnston Date: Fri, 11 Oct 2024 23:28:27 -0600 Subject: [PATCH] fix(troika-three-text): remove DoubleSide double draw hack in favor of forceSinglePass flag --- package-lock.json | 14 ++++---- packages/troika-examples/package.json | 2 +- .../troika-three-text/src/GlyphsGeometry.js | 35 +------------------ packages/troika-three-text/src/Text.js | 17 --------- .../src/TextDerivedMaterial.js | 3 ++ 5 files changed, 12 insertions(+), 59 deletions(-) diff --git a/package-lock.json b/package-lock.json index fc1bb06b..b697b63c 100644 --- a/package-lock.json +++ b/package-lock.json @@ -20,7 +20,7 @@ "react-dat-gui": "^4.0.0", "react-dom": "^16.14.0", "terser": "^5.14.2", - "three": "^0.147.0", + "three": "^0.149.0", "three-line-2d": "^1.1.6", "webgl-sdf-generator": "1.1.1", "yoga-layout-prebuilt": "1.9.6" @@ -22585,9 +22585,9 @@ "dev": true }, "node_modules/three": { - "version": "0.147.0", - "resolved": "https://registry.npmjs.org/three/-/three-0.147.0.tgz", - "integrity": "sha512-LPTOslYQXFkmvceQjFTNnVVli2LaVF6C99Pv34fJypp8NbQLbTlu3KinZ0zURghS5zEehK+VQyvWuPZ/Sm8fzw==" + "version": "0.149.0", + "resolved": "https://registry.npmjs.org/three/-/three-0.149.0.tgz", + "integrity": "sha512-tohpUxPDht0qExRLDTM8sjRLc5d9STURNrdnK3w9A+V4pxaTBfKWWT/IqtiLfg23Vfc3Z+ImNfvRw1/0CtxrkQ==" }, "node_modules/three-line-2d": { "version": "1.1.6", @@ -41254,9 +41254,9 @@ "dev": true }, "three": { - "version": "0.147.0", - "resolved": "https://registry.npmjs.org/three/-/three-0.147.0.tgz", - "integrity": "sha512-LPTOslYQXFkmvceQjFTNnVVli2LaVF6C99Pv34fJypp8NbQLbTlu3KinZ0zURghS5zEehK+VQyvWuPZ/Sm8fzw==" + "version": "0.149.0", + "resolved": "https://registry.npmjs.org/three/-/three-0.149.0.tgz", + "integrity": "sha512-tohpUxPDht0qExRLDTM8sjRLc5d9STURNrdnK3w9A+V4pxaTBfKWWT/IqtiLfg23Vfc3Z+ImNfvRw1/0CtxrkQ==" }, "three-line-2d": { "version": "1.1.6", diff --git a/packages/troika-examples/package.json b/packages/troika-examples/package.json index 21e177a2..58769f8f 100644 --- a/packages/troika-examples/package.json +++ b/packages/troika-examples/package.json @@ -21,7 +21,7 @@ "react": "^16.14.0", "react-dat-gui": "^4.0.0", "react-dom": "^16.14.0", - "three": "^0.147.0", + "three": "^0.149.0", "three-instanced-uniforms-mesh": "^0.50.0", "three-line-2d": "^1.1.6", "troika-2d": "^0.50.0", diff --git a/packages/troika-three-text/src/GlyphsGeometry.js b/packages/troika-three-text/src/GlyphsGeometry.js index 3e5f7c17..980a38c1 100644 --- a/packages/troika-three-text/src/GlyphsGeometry.js +++ b/packages/troika-three-text/src/GlyphsGeometry.js @@ -1,13 +1,9 @@ import { - Float32BufferAttribute, - BufferGeometry, PlaneGeometry, InstancedBufferGeometry, InstancedBufferAttribute, Sphere, Box3, - DoubleSide, - BackSide, } from 'three' const templateGeometries = {} @@ -15,29 +11,7 @@ const templateGeometries = {} function getTemplateGeometry(detail) { let geom = templateGeometries[detail] if (!geom) { - // Geometry is two planes back-to-back, which will always be rendered FrontSide only but - // appear as DoubleSide by default. FrontSide/BackSide are emulated using drawRange. - // We do it this way to avoid the performance hit of two draw calls for DoubleSide materials - // introduced by Three.js in r130 - see https://github.com/mrdoob/three.js/pull/21967 - const front = new PlaneGeometry(1, 1, detail, detail) - const back = front.clone() - const frontAttrs = front.attributes - const backAttrs = back.attributes - const combined = new BufferGeometry() - const vertCount = frontAttrs.uv.count - for (let i = 0; i < vertCount; i++) { - backAttrs.position.array[i * 3] *= -1 // flip position x - backAttrs.normal.array[i * 3 + 2] *= -1 // flip normal z - } - ['position', 'normal', 'uv'].forEach(name => { - combined.setAttribute(name, new Float32BufferAttribute( - [...frontAttrs[name].array, ...backAttrs[name].array], - frontAttrs[name].itemSize) - ) - }) - combined.setIndex([...front.index.array, ...back.index.array.map(n => n + vertCount)]) - combined.translate(0.5, 0.5, 0) - geom = templateGeometries[detail] = combined + geom = templateGeometries[detail] = new PlaneGeometry(1, 1, detail, detail).translate(0.5, 0.5, 0) } return geom } @@ -103,13 +77,6 @@ class GlyphsGeometry extends InstancedBufferGeometry { // No-op; we'll sync the boundingBox proactively when needed. } - // Since our base geometry contains triangles for both front and back sides, we can emulate - // the "side" by restricting the draw range. - setSide(side) { - const verts = this.getIndex().count - this.setDrawRange(side === BackSide ? verts / 2 : 0, side === DoubleSide ? verts : verts / 2) - } - set detail(detail) { if (detail !== this._detail) { this._detail = detail diff --git a/packages/troika-three-text/src/Text.js b/packages/troika-three-text/src/Text.js index 7696768e..129c21e8 100644 --- a/packages/troika-three-text/src/Text.js +++ b/packages/troika-three-text/src/Text.js @@ -1,7 +1,6 @@ import { Color, DoubleSide, - FrontSide, Matrix4, Mesh, MeshBasicMaterial, @@ -488,22 +487,6 @@ class Text extends Mesh { if (material.isTroikaTextMaterial) { this._prepareForRender(material) } - - // We need to force the material to FrontSide to avoid the double-draw-call performance hit - // introduced in Three.js r130: https://github.com/mrdoob/three.js/pull/21967 - The sidedness - // is instead applied via drawRange in the GlyphsGeometry. - material._hadOwnSide = material.hasOwnProperty('side') - this.geometry.setSide(material._actualSide = material.side) - material.side = FrontSide - } - - onAfterRender(renderer, scene, camera, geometry, material, group) { - // Restore original material side - if (material._hadOwnSide) { - material.side = material._actualSide - } else { - delete material.side // back to inheriting from base material - } } /** diff --git a/packages/troika-three-text/src/TextDerivedMaterial.js b/packages/troika-three-text/src/TextDerivedMaterial.js index fd9f4843..e40cfd79 100644 --- a/packages/troika-three-text/src/TextDerivedMaterial.js +++ b/packages/troika-three-text/src/TextDerivedMaterial.js @@ -258,6 +258,9 @@ export function createTextDerivedMaterial(baseMaterial) { // Force transparency - TODO is this reasonable? textMaterial.transparent = true + // Force single draw call when double-sided + textMaterial.forceSinglePass = true + Object.defineProperties(textMaterial, { isTroikaTextMaterial: {value: true},