Add 3D text shape support with instance recycling#477
Merged
tracygardner merged 6 commits intomainfrom Mar 26, 2026
Merged
Conversation
- Fix create_3d_text generator to use block.id-based meshId (userVar__blockId) so the mesh can be found/recycled across re-runs - Fix create3DText API to extract blockKey from modelId, set metadata.blockKey, call _recycleOldestByKey/_registerInstance for proper lifecycle management - Add create_3d_text to addmeshes.js so text appears/disappears live when the block is added or removed from the workspace - Add SIZE/DEPTH geometry change handling in handlePrimitiveGeometryChange so editing block inputs updates the mesh live; width scales proportionally - Add create_3d_text case in scale gizmo onDragEndObservable to sync SIZE (from world height) and DEPTH back to the block; position/rotation gizmos and colour picker sync work automatically via existing metadata.blockKey plumbing https://claude.ai/code/session_01Evm7NrABMXnUnDekVLQ7XJ
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughDeterministic 3D-text IDs flow end-to-end: generator emits user-based IDs, UI creates Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as ui/addmeshes.js
participant Generator as generators-text.js
participant API as api/shapes.js
participant Scene as BabylonScene
User->>UI: Create 3D Text block
UI->>Generator: getVariableInfo(ID_VAR)
Generator-->>UI: userVariableName, block.id
UI->>UI: Resolve COLOR (→ colorOrMaterial, alpha), read TEXT/SIZE/DEPTH
UI->>API: flock.create3DText(modelId: userVar__blockId, color, alpha, size, depth, position)
API->>API: Parse modelId → meshId, blockKey
API->>API: flock._recycleOldestByKey(blockKey)
API->>Scene: Check for existing mesh named meshId
alt mesh exists
API->>API: meshId += flock.scene.getUniqueId()
end
API->>Scene: CreateText(...) with meshId and material.alpha = toAlpha(alpha)
API->>API: Bake uniform XY scale so rendered height == requested size
API->>API: mesh.metadata.blockKey = blockKey
API->>API: flock._registerInstance(blockKey, mesh.name)
API-->>UI: return meshId
UI->>UI: map block.id → mesh
UI->>Scene: gizmo/scale events update mesh
Scene->>UI: onDragEnd -> write SIZE/DEPTH back to block
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@generators/generators-text.js`:
- Around line 2-8: The ui_button generator calls generateUniqueId() but the
current import from mesh-state.js only brings in meshMap and meshBlockIdMap;
update the import from mesh-state.js to also import generateUniqueId so
generateUniqueId is available to the ui_button code (search for ui_button and
the call at line ~174 to verify). Ensure the import statement in
generators/generators-text.js includes generateUniqueId alongside meshMap and
meshBlockIdMap.
In `@ui/addmeshes.js`:
- Around line 631-656: The create_3d_text case calls resolveColorOrMaterial but
only extracts color; destructure alpha as well (e.g., { colorOrMaterial: color,
alpha }) from resolveColorOrMaterial and pass that alpha into the options when
calling flock.create3DText (add alpha: alpha or similar) so 3D text supports
transparency; also ensure the flock.create3DText implementation (api/shapes.js)
accepts and applies an alpha parameter.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2189507f-ae2c-4d22-9bf0-c38b6029fba4
📒 Files selected for processing (5)
api/shapes.jsgenerators/generators-text.jsui/addmeshes.jsui/blockmesh.jsui/gizmos.js
blocks/text.js: switch create_3d_text from handleBlockCreateEvent to handleBlockChange, matching all other shape blocks (create_box, etc.). This enables colour picker changes to propagate to the mesh in the scene, and also lets SIZE/DEPTH/X/Y/Z edits in the block update the mesh live. ui/gizmos.js: fix the scale gizmo → block sync for create_3d_text. Using h (bounding box world height) as SIZE is wrong because the font em-height (SIZE parameter) != cap height (bounding box height, ~70% of em). Instead use currentSize * mesh.scaling.y so the block value after a gizmo drag is consistent with what create3DText will reproduce when code re-runs. Same fix applied to DEPTH using mesh.scaling.z. Pattern matches how create_cylinder reads its current DIAMETER_TOP/DIAMETER_BOTTOM values. https://claude.ai/code/session_01Evm7NrABMXnUnDekVLQ7XJ
Deploying flockdev with
|
| Latest commit: |
316a01d
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://c27abe88.flockdev.pages.dev |
| Branch Preview URL: | https://claude-add-3d-text-gizmo-syn.flockdev.pages.dev |
- Uniform diamond and X/Y handles scale SIZE (XY proportional, Z locked) - Z handle scales DEPTH only (XY locked to 1) - Track active axis via per-sub-gizmo onDragStartObservable to avoid delta-detection feedback loops https://claude.ai/code/session_01Evm7NrABMXnUnDekVLQ7XJ
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ui/gizmos.js`:
- Around line 940-946: The scale-gizmo drag-start observers are being
re-registered each time toggleGizmo("scale") runs
(sg.xGizmo/.yGizmo/.zGizmo/.uniformScaleGizmo →
dragBehavior.onDragStartObservable), causing duplicate handlers; update the code
to register the observer only once by either clearing existing observers (e.g.,
call clear() or removeObservers on each dragBehavior.onDragStartObservable
before adding) or use a one-time registration API (addOnce) or store and dispose
previous observer references, then add the new observer that sets textScaleAxis
to "x"/"y"/"z"/"uniform".
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
Font cap-height is ~70% of em-height, so after creation the bounding box height != SIZE in the block. When the scale gizmo is released and handlePrimitiveGeometryChange fires with mesh.scaling != (1,1,1), setAbsoluteSize would use newSize (em-height*S) vs currentH (cap-height*S), causing a ~43% height jump. Fix: bake a uniform XY normalization scale immediately after mesh creation so bounding_box_height = size always, making currentH = newSize on drag end. https://claude.ai/code/session_01Evm7NrABMXnUnDekVLQ7XJ
- generators-text.js: restore generateUniqueId import from mesh-state.js
(needed by ui_button generator at line 174)
- addmeshes.js + api/shapes.js: pass alpha through create3DText so
transparency works consistently with other shape blocks
- gizmos.js: guard scale-gizmo axis observers with _textAxisObserversRegistered
flag to prevent duplicate registration on repeated toggleGizmo("scale") calls
https://claude.ai/code/session_01Evm7NrABMXnUnDekVLQ7XJ
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
api/shapes.js (1)
742-815:⚠️ Potential issue | 🟠 MajorDispose partial meshes before falling back or rejecting.
If the Manifold branch throws after Line 762 creates
mesh, the inner catch falls back to the standard path without cleaning up the scene object it already inserted. The outer catch has the same leak for later failures. That leaves orphaned meshes/materials behind and can make futuregetMeshByName(meshId)checks hit a half-built mesh.🧹 Possible fix
if (useManifold) { // Use Manifold-based text generation for guaranteed watertight geometry try { // Get TTF/OTF font URL - convert JSON font path to TTF if needed let fontUrl = font; @@ // Flip faces to ensure correct orientation for CSG operations mesh.flipFaces(); } catch (manifoldError) { + mesh?.material?.dispose(); + mesh?.dispose(); + mesh = null; console.warn( "[create3DText] Manifold approach failed, falling back to standard:", manifoldError.message, ); useManifold = false; @@ } catch (error) { + mesh?.material?.dispose(); + mesh?.dispose(); console.error(`Error creating 3D text '${meshId}':`, error); reject(error); }Also applies to: 895-897
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/shapes.js` around lines 742 - 815, The Manifold branch can create a partial Babylon mesh (mesh) before an exception occurs, so modify the try/catch around createManifoldTextMesh/vertexData.applyToMesh/mesh.flipFaces to detect and dispose any partially-created mesh and its material when catching manifoldError (and similarly in the outer catch path referenced around the later fallback), e.g. if mesh is truthy call mesh.dispose() and dispose mesh.material if present and ensure getMeshByName(meshId) won't return a half-built mesh before proceeding to the standard path; update the error handling around createManifoldTextMesh, vertexData.applyToMesh, and mesh.flipFaces to perform this cleanup before setting useManifold = false or rethrowing.ui/gizmos.js (1)
938-958:⚠️ Potential issue | 🟠 MajorGuard all scale observers, not just the axis drag-start handlers.
The
_textAxisObserversRegisteredflag guards the per-axis drag-start handlers (lines 943–946), buttoggleGizmo("scale")still registers freshonAttachedToMeshObservable,onDragObservable,onDragStartObservable, andonDragEndObservablelisteners on every invocation (lines 950, 963, 1001, 1026). There is no cleanup mechanism anywhere in the file. After multiple scale toggles, each drag operation triggers the accumulated listeners, causing the bottom-anchor correction and SIZE/DEPTH sync to run multiple times. Register these observers once, or remove old listeners before re-adding them.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/gizmos.js` around lines 938 - 958, The scale gizmo registers multiple observables every time toggleGizmo("scale") runs (gizmoManager.onAttachedToMeshObservable, sg.xGizmo/.yGizmo/.zGizmo/.uniformScaleGizmo onDragObservable/onDragStartObservable/onDragEndObservable), but only the axis drag-start handlers are guarded by sg._textAxisObserversRegistered; fix by ensuring all scale-related observers are registered exactly once or explicitly disposed before re-registering: extend the existing sg._textAxisObserversRegistered pattern (or create a sg._observersRegistered flag) to gate registration of onAttachedToMeshObservable, onDragObservable, onDragStartObservable, and onDragEndObservable in configureScaleGizmo (and/or store and call observer.dispose() on previously saved observer references) so repeated toggleGizmo("scale") calls do not accumulate listeners.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@api/shapes.js`:
- Around line 725-735: Create a per-blockKey in-flight token map and use it to
serialize builds and drop stale results: when beginning create3DText for a given
blockKey (derived from modelId), generate a unique token and store it in
something like flock._pendingBuilds[blockKey] before calling
_recycleOldestByKey; run the async build; on completion compare the token with
flock._pendingBuilds[blockKey] and if they differ (meaning a newer build
started) do not call _registerInstance(...) or overwrite the ready promise;
finally clear the token entry when the accepted build finishes. Reference
symbols: blockKey, modelId, create3DText, _recycleOldestByKey,
_registerInstance, and add flock._pendingBuilds map to track tokens.
In `@ui/gizmos.js`:
- Around line 943-946: The observer block for the gizmo drag start handlers is
not formatted consistently with the file; run Prettier (or reformat) the four
add(...) calls so they match surrounding style. Locate the
sg.xGizmo.dragBehavior.onDragStartObservable.add,
sg.yGizmo.dragBehavior.onDragStartObservable.add,
sg.zGizmo.dragBehavior.onDragStartObservable.add, and
sg.uniformScaleGizmo.dragBehavior.onDragStartObservable.add calls and reformat
the inline arrow handlers (spacing, indentation, and semicolon placement) to
conform to the project's Prettier settings, then stage the updated ui/gizmos.js
so CI formatting checks pass.
---
Outside diff comments:
In `@api/shapes.js`:
- Around line 742-815: The Manifold branch can create a partial Babylon mesh
(mesh) before an exception occurs, so modify the try/catch around
createManifoldTextMesh/vertexData.applyToMesh/mesh.flipFaces to detect and
dispose any partially-created mesh and its material when catching manifoldError
(and similarly in the outer catch path referenced around the later fallback),
e.g. if mesh is truthy call mesh.dispose() and dispose mesh.material if present
and ensure getMeshByName(meshId) won't return a half-built mesh before
proceeding to the standard path; update the error handling around
createManifoldTextMesh, vertexData.applyToMesh, and mesh.flipFaces to perform
this cleanup before setting useManifold = false or rethrowing.
In `@ui/gizmos.js`:
- Around line 938-958: The scale gizmo registers multiple observables every time
toggleGizmo("scale") runs (gizmoManager.onAttachedToMeshObservable,
sg.xGizmo/.yGizmo/.zGizmo/.uniformScaleGizmo
onDragObservable/onDragStartObservable/onDragEndObservable), but only the axis
drag-start handlers are guarded by sg._textAxisObserversRegistered; fix by
ensuring all scale-related observers are registered exactly once or explicitly
disposed before re-registering: extend the existing
sg._textAxisObserversRegistered pattern (or create a sg._observersRegistered
flag) to gate registration of onAttachedToMeshObservable, onDragObservable,
onDragStartObservable, and onDragEndObservable in configureScaleGizmo (and/or
store and call observer.dispose() on previously saved observer references) so
repeated toggleGizmo("scale") calls do not accumulate listeners.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a153ec3d-039c-460b-a37f-13f8bce2e9d6
📒 Files selected for processing (4)
api/shapes.jsgenerators/generators-text.jsui/addmeshes.jsui/gizmos.js
🚧 Files skipped from review as they are similar to previous changes (2)
- generators/generators-text.js
- ui/addmeshes.js
Race condition: overlapping create3DText calls for the same blockKey could both pass the _recycleOldestByKey gate, both complete async, and both call _registerInstance leaving duplicate meshes. Fix: stamp each build with a Symbol token in flock._pendingTextBuilds; on completion, drop the mesh if a newer build has overwritten the token. Formatting: run Prettier on gizmos.js to fix CI warning. https://claude.ai/code/session_01Evm7NrABMXnUnDekVLQ7XJ
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR adds support for creating 3D text shapes in the visual programming environment and implements instance recycling to manage mesh memory efficiently.
Key Changes
3D Text Shape Creation: Added
create_3d_textblock support across the UI and code generation layers, allowing users to create customizable 3D text with configurable size, depth, font, and colorInstance Recycling System: Implemented
_recycleOldestByKey()and_registerInstance()methods to track and reuse mesh instances, preventing memory leaks when shapes are recreatedMesh Identification: Enhanced mesh naming with a dual-key system using
__separator (e.g.,3dtext__blockId) to distinguish between logical block keys and unique mesh identifiers, enabling proper instance tracking and recyclingMetadata Tracking: Added
blockKeymetadata to mesh objects to maintain the association between mesh instances and their source blocksUI Integration:
addmeshes.jsblockmesh.jsfor real-time property updatesgizmos.jsCode Generation: Updated text generator to use the new dual-key naming scheme and removed dependency on
generateUniqueId()in favor of block IDsImplementation Details
The mesh identification system now supports both a
blockKey(for recycling/tracking) and ameshId(for unique Babylon.js mesh names). When a mesh with the same name already exists in the scene, a unique suffix is appended to prevent conflicts. This allows multiple instances of the same logical shape to coexist while maintaining proper cleanup through the recycling system.https://claude.ai/code/session_01Evm7NrABMXnUnDekVLQ7XJ
Summary by CodeRabbit
New Features
Bug Fixes / Improvements
UX