Skip to content

Attempt manifold 3D text mesh creation with verbose logging and fallback #524

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.

Sign up for GitHub

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

Closed
tracygardner wants to merge 8 commits into from
Closed

Attempt manifold 3D text mesh creation with verbose logging and fallback #524

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
e94aa6e
Add create3DText path logging for manifold fallback
tracygardner Apr 11, 2026
70c0434
Add subtractMeshes path diagnostics for CSG flow
tracygardner Apr 11, 2026
364ce0b
Tag subtractMeshes logs with trace id and step
tracygardner Apr 11, 2026
c503569
Reduce subtractMeshes logs to initial step per trace
tracygardner Apr 11, 2026
5fd2ff4
Trim CSG trace output to entry/path and failures
tracygardner Apr 12, 2026
105da91
Fix manifold 3D text shading by keeping original face winding
tracygardner Apr 12, 2026
46a4f81
Restore manifold text visibility and recalculate normals
tracygardner Apr 12, 2026
5b0d1a6
Tone down manifold text shininess for readability
tracygardner Apr 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 52 additions & 6 deletions api/csg.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -708,6 +708,9 @@
const { modelId: resolvedModelId, blockKey } =
resolveCsgModelIdentity(modelId);
modelId = resolvedModelId;
const traceId = options.traceId || "no-trace-id";
const traceStep = Number.isFinite(options.traceStep) ? options.traceStep : 1;
const initialTrace = traceStep === 1;

const collectMaterialMeshesDeep = (root) => {
const out = [];
Expand Down Expand Up @@ -751,6 +754,11 @@
};

return new Promise((resolve) => {
if (initialTrace) {
console.info(
`[subtractMeshes][path] trace=${traceId} step=${traceStep} approach=merge modelId=${modelId} base=${baseMeshName} tools=${meshNames.length}`,
);
}
flock.whenModelReady(baseMeshName, (baseMesh) => {
if (!baseMesh) return resolve(null);
let actualBase = baseMesh.metadata?.modelName
Expand Down Expand Up @@ -783,7 +791,6 @@
// Check if mesh itself has valid geometry (e.g., manifold text meshes)
const meshHasGeometry =
mesh.getTotalVertices && mesh.getTotalVertices() > 0;

if (parts.length > 0) {
const partClones = parts.map((p, i) =>
cloneForCSG(p, `temp_${meshIndex}_${i}`),
Expand Down Expand Up @@ -822,7 +829,6 @@
subtractDuplicates.push(clone);
}
});

subtractDuplicates.forEach((m, idx) => {
try {
const meshCSG = flock.BABYLON.CSG2.FromMesh(m, false);
Expand All @@ -832,6 +838,9 @@
`[subtractMeshesMerge] Subtraction ${idx} failed:`,
e.message,
);
console.info(
`[subtractMeshes][merge] trace=${traceId} subtractionIndex=${idx} status=failed tool=${m.name}`,
);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}
});

Expand Down Expand Up @@ -902,6 +911,9 @@
const { modelId: resolvedModelId, blockKey } =
resolveCsgModelIdentity(modelId);
modelId = resolvedModelId;
const traceId = options.traceId || "no-trace-id";
const traceStep = Number.isFinite(options.traceStep) ? options.traceStep : 1;
const initialTrace = traceStep === 1;

const collectMaterialMeshesDeep = (root) => {
const out = [];
Expand All @@ -922,6 +934,11 @@
};

return new Promise((resolve) => {
if (initialTrace) {
console.info(
`[subtractMeshes][path] trace=${traceId} step=${traceStep} approach=individual modelId=${modelId} base=${baseMeshName} tools=${meshNames.length}`,
);
}
flock.whenModelReady(baseMeshName, (baseMesh) => {
if (!baseMesh) return resolve(null);
let actualBase = baseMesh;
Expand Down Expand Up @@ -958,7 +975,7 @@

let outerCSG = flock.BABYLON.CSG2.FromMesh(baseDuplicate, false);
const allToolParts = [];
validMeshes.forEach((mesh) => {
validMeshes.forEach((mesh, meshIndex) => {

Check failure on line 978 in api/csg.js

View workflow job for this annotation

GitHub Actions / eslint 

'meshIndex' is defined but never used. Allowed unused args must match /^_/u
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 12, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Fix ESLint violation: unused meshIndex parameter.

The pipeline is failing because meshIndex is defined but never used. Per the project's ESLint configuration, unused arguments must be prefixed with _.

🔧 Proposed fix 
-          validMeshes.forEach((mesh, meshIndex) => {
+          validMeshes.forEach((mesh, _meshIndex) => {

Alternatively, if the index is not needed at all:

-          validMeshes.forEach((mesh, meshIndex) => {
+          validMeshes.forEach((mesh) => {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
validMeshes.forEach((mesh, meshIndex) => {
validMeshes.forEach((mesh, _meshIndex) => {
🧰 Tools
🪛 GitHub Actions: ESLint

[error] 978-978: ESLint (no-unused-vars): 'meshIndex' is defined but never used. Allowed unused args must match /^_/u

🪛 GitHub Check: eslint

[failure] 978-978:
'meshIndex' is defined but never used. Allowed unused args must match /^_/u

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@api/csg.js` at line 978, The forEach callback currently declares an unused
parameter named meshIndex which violates ESLint; update the callback signature
used on validMeshes.forEach to either remove the unused second parameter or
rename it to start with an underscore (e.g., _meshIndex) so it satisfies the
project's lint rule; locate the validMeshes.forEach(...) callback and change the
parameter accordingly in that anonymous function.

const parts = collectMaterialMeshesDeep(mesh);
parts.forEach((p) => {
const dup = p.clone("partDup", null, true);
Expand All @@ -968,12 +985,15 @@
});
});

allToolParts.forEach((part) => {
allToolParts.forEach((part, index) => {
try {
const partCSG = flock.BABYLON.CSG2.FromMesh(part, false);
outerCSG = outerCSG.subtract(partCSG);
} catch (e) {
console.warn(e);
console.info(
`[subtractMeshes][individual] trace=${traceId} subtractionIndex=${index} status=failed tool=${part.name}`,
);
}
});

Expand Down Expand Up @@ -1058,16 +1078,42 @@
typeof optionsOrApproach === "string"
? optionsOrApproach
: options.approach || "merge";
if (!flock._subtractTraceSteps) flock._subtractTraceSteps = new Map();
const traceKey = typeof modelId === "string" ? modelId : String(modelId);
const nextStep = (flock._subtractTraceSteps.get(traceKey) || 0) + 1;
flock._subtractTraceSteps.set(traceKey, nextStep);
Comment on lines +1081 to +1084
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 11, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Bound trace-state map to prevent unbounded memory growth.

flock._subtractTraceSteps only grows and is never pruned. In long-running sessions with many unique modelIds, this can accumulate indefinitely.

♻️ Suggested bounded-cache fix 
 if (!flock._subtractTraceSteps) flock._subtractTraceSteps = new Map();
 const traceKey = typeof modelId === "string" ? modelId : String(modelId);
+const TRACE_STEP_CACHE_LIMIT = 1000;
+const isNewTraceKey = !flock._subtractTraceSteps.has(traceKey);
+if (isNewTraceKey && flock._subtractTraceSteps.size >= TRACE_STEP_CACHE_LIMIT) {
+  const oldestKey = flock._subtractTraceSteps.keys().next().value;
+  flock._subtractTraceSteps.delete(oldestKey);
+}
 const nextStep = (flock._subtractTraceSteps.get(traceKey) || 0) + 1;
 flock._subtractTraceSteps.set(traceKey, nextStep);
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@api/csg.js` around lines 1122 - 1125, flock._subtractTraceSteps is an
unbounded Map and can grow indefinitely for new traceKey values; change it to a
bounded cache by replacing the plain Map with a size-limited structure (e.g., an
LRU or simple FIFO-evicting Map) so when adding a new traceKey in the block that
references flock._subtractTraceSteps and traceKey you evict the oldest entry
once a maxEntries threshold is reached (use a constant like
MAX_SUBTRACT_TRACE_STEPS) and then set the new nextStep; update the
initialization (where flock._subtractTraceSteps is created) and the increment
logic that uses nextStep to maintain that bound.

const traceId =
(options && options.traceId) ||
`${traceKey.split("__")[0]}:${traceKey.slice(-6)}`;
const tracedOptions = {
...options,
traceId,
traceStep: nextStep,
};
if (nextStep === 1) {
console.info(
`[subtractMeshes][entry] trace=${traceId} step=${nextStep} requestedApproach=${approach} modelId=${modelId} base=${baseMeshName} tools=${meshNames.length}`,
);
} else if (nextStep === 2) {
console.info(
`[subtractMeshes][entry] trace=${traceId} repeated_steps_detected=true suppressing_repetitive_logs=true`,
);
}

if (approach === "individual") {
return this.subtractMeshesIndividual(
modelId,
baseMeshName,
meshNames,
options,
tracedOptions,
);
} else {
return this.subtractMeshesMerge(modelId, baseMeshName, meshNames, options);
return this.subtractMeshesMerge(
modelId,
baseMeshName,
meshNames,
tracedOptions,
);
}
},
intersectMeshes(modelId, meshList) {
Expand Down
34 changes: 34 additions & 0 deletions api/shapes.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -721,8 +721,16 @@ export const flockShapes = {
try {
let mesh;
let fontReferenceHeight = null;
const manifoldRequested = useManifold;

console.info(
`[create3DText][path] meshId=${meshId} blockKey=${blockKey} manifoldRequested=${manifoldRequested}`,
);

if (useManifold) {
console.info(
`[create3DText][path] meshId=${meshId} attempting=manifold`,
);
try {
let fontUrl = font;
if (font.endsWith(".json")) {
Expand Down Expand Up @@ -777,6 +785,26 @@ export const flockShapes = {
vertexData.positions = centeredPositions;
vertexData.applyToMesh(mesh);
mesh.flipFaces();
const positionsForNormals = mesh.getVerticesData(
flock.BABYLON.VertexBuffer.PositionKind,
);
const indicesForNormals = mesh.getIndices();
if (positionsForNormals && indicesForNormals) {
const recalculatedNormals = [];
flock.BABYLON.VertexData.ComputeNormals(
positionsForNormals,
indicesForNormals,
recalculatedNormals,
);
mesh.setVerticesData(
flock.BABYLON.VertexBuffer.NormalKind,
recalculatedNormals,
true,
);
}
console.info(
`[create3DText][path] meshId=${meshId} used=manifold`,
);
} catch (manifoldError) {
console.warn(
"[create3DText] Manifold approach failed, falling back to standard:",
Expand All @@ -787,6 +815,12 @@ export const flockShapes = {
}

if (!useManifold) {
const fallbackReason = manifoldRequested
? "manifold_failed"
: "manifold_disabled";
console.info(
`[create3DText][path] meshId=${meshId} used=standard reason=${fallbackReason}`,
);
const fontData = await (await fetch(font)).json();
mesh = flock.BABYLON.MeshBuilder.CreateText(
meshId,
Expand Down
Loading