Skip to content

Fix textToModel face normals for extruded text #8091

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

Open
wants to merge 6 commits into
base: dev-2.0
Choose a base branch
from
+132 −48
Open

Fix textToModel face normals for extruded text #8091

Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
5e557e5
Fix textToModel face normals for extruded text
VANSH3104 Sep 15, 2025
9a7e956
Merge branch 'dev-2.0' into fix-textToModel
VANSH3104 Sep 25, 2025
3e567bf
Fix textToModel extrusion and degenerate face issues
VANSH3104 Sep 29, 2025
4c91d6e
Fix textToModel extrusion and degenerate face issues
VANSH3104 Sep 29, 2025
a68bd7b
Fix text extrusion with proper vertex deduplication, edge detection, …
VANSH3104 Oct 28, 2025
db20daa
Merge branch 'dev-2.0' into fix-textToModel
davepagurek Oct 29, 2025
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
180 changes: 132 additions & 48 deletions src/type/p5.Font.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { textCoreConstants } from './textCore';
import * as constants from '../core/constants';
import { UnicodeRange } from '@japont/unicode-range';
import { unicodeRanges } from './unicodeRanges';
import { Vector } from '../math/p5.Vector';

/*
API:
Expand Down Expand Up @@ -542,63 +543,146 @@ export class Font {
textToModel(str, x, y, width, height, options) {
({ width, height, options } = this._parseArgs(width, height, options));
const extrude = options?.extrude || 0;
const contours = this.textToContours(str, x, y, width, height, options);

let contours = this.textToContours(str, x, y, width, height, options);
if (!Array.isArray(contours[0][0])) {
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like right now this is being used to create multiple begin/endShape calls. Do we need that? I assumed it would be ok to just get a list of contours and put them all in one shape.

Also I believe textToContours should always be returning an array of array of points (a list of contours, each contour a list of points), so I also wonder if we always go down the same branch here anyway.

contours = [contours];
}

const geom = this._pInst.buildGeometry(() => {
if (extrude === 0) {
const prevValidateFaces = this._pInst._renderer._validateFaces;
this._pInst._renderer._validateFaces = true;
const prevValidateFaces = this._pInst._renderer._validateFaces;
this._pInst._renderer._validateFaces = true;
this._pInst.push();
this._pInst.stroke(0);
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this do the same thing as noStroke()?


contours.forEach(glyphContours => {
this._pInst.beginShape();
this._pInst.normal(0, 0, 1);
for (const contour of contours) {
for (const contour of glyphContours) {
this._pInst.beginContour();
for (const { x, y } of contour) {
this._pInst.vertex(x, y);
}
contour.forEach(({ x, y }) => this._pInst.vertex(x, y, 0));
this._pInst.endContour(this._pInst.CLOSE);
}
this._pInst.endShape();
this._pInst._renderer._validateFaces = prevValidateFaces;
} else {
const prevValidateFaces = this._pInst._renderer._validateFaces;
this._pInst._renderer._validateFaces = true;

// Draw front faces
for (const side of [1, -1]) {
this._pInst.beginShape();
for (const contour of contours) {
this._pInst.beginContour();
for (const { x, y } of contour) {
this._pInst.vertex(x, y, side * extrude * 0.5);
}
this._pInst.endContour(this._pInst.CLOSE);
}
this._pInst.endShape();
}
this._pInst._renderer._validateFaces = prevValidateFaces;

// Draw sides
for (const contour of contours) {
this._pInst.beginShape(this._pInst.QUAD_STRIP);
for (const v of contour) {
for (const side of [-1, 1]) {
this._pInst.vertex(v.x, v.y, side * extrude * 0.5);
}
}
this._pInst.endShape();
}
}
this._pInst.endShape(this._pInst.CLOSE);
});
this._pInst.pop();
this._pInst._renderer._validateFaces = prevValidateFaces;
});
if (extrude !== 0) {
geom.computeNormals();
for (const face of geom.faces) {
if (face.every(idx => geom.vertices[idx].z <= -extrude * 0.5 + 0.1)) {
for (const idx of face) geom.vertexNormals[idx].set(0, 0, -1);
face.reverse();
}

if (extrude === 0) {
return geom;
}

const vertexIndices = {};
const vertexId = v => `${v.x.toFixed(6)}-${v.y.toFixed(6)}-${v.z.toFixed(6)}`;
Copy link
Contributor

Choose a reason for hiding this comment

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

I had to remind myself that this was necessary because tessellation produces triangles with no shared vertices, so we need to deduplicate them here to find connected faces. Would you mind leaving a comment explaining this for future contributors' context?

const newVertices = [];
const newVertexIndex = [];

for (const v of geom.vertices) {
const id = vertexId(v);
if (!(id in vertexIndices)) {
const index = newVertices.length;
vertexIndices[id] = index;
newVertices.push(v.copy());
}
newVertexIndex.push(vertexIndices[id]);
}

// Remap faces to use deduplicated vertices
const newFaces = geom.faces.map(f => f.map(i => newVertexIndex[i]));

//Find outer edges (edges that appear in only one face)
const seen = {};
for (const face of newFaces) {
for (let off = 0; off < face.length; off++) {
const a = face[off];
const b = face[(off + 1) % face.length];
const id = `${Math.min(a, b)}-${Math.max(a, b)}`;
if (!seen[id]) seen[id] = [];
seen[id].push([a, b]);
}
}
const validEdges = [];
for (const key in seen) {
if (seen[key].length === 1) {
validEdges.push(seen[key][0]);
}
}
return geom;

console.log(`Found ${validEdges.length} outer edges from ${Object.keys(seen).length} total edges`);
Copy link
Contributor

Choose a reason for hiding this comment

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

Just a reminder to take this out before merging to not clog up the console


// Step 5: Create extruded geometry
const extruded = this._pInst.buildGeometry(() => {});
const half = extrude * 0.5;
extruded.vertices = [];
extruded.faces = [];
extruded.edges = []; // INITIALIZE EDGES ARRAY

// Add side face vertices (separate for each edge for flat shading)
for (const [a, b] of validEdges) {
const vA = newVertices[a];
const vB = newVertices[b];
// Skip if vertices are too close (degenerate edge)
const dist = Math.sqrt(
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this still passes if we have a triangle that has collinear edges. I think in computeNormals, we check basically that the magnitude of the cross product is over a certain size. maybe we should match that?

Math.pow(vB.x - vA.x, 2) +
Math.pow(vB.y - vA.y, 2) +
Math.pow(vB.z - vA.z, 2)
);
if (dist < 0.0001) continue;
// Front face vertices
const frontA = extruded.vertices.length;
extruded.vertices.push(new Vector(vA.x, vA.y, vA.z + half));
const frontB = extruded.vertices.length;
extruded.vertices.push(new Vector(vB.x, vB.y, vB.z + half));
const backA = extruded.vertices.length;
extruded.vertices.push(new Vector(vA.x, vA.y, vA.z - half));
const backB = extruded.vertices.length;
extruded.vertices.push(new Vector(vB.x, vB.y, vB.z - half));

extruded.faces.push([frontA, backA, backB]);
extruded.faces.push([frontA, backB, frontB]);
extruded.edges.push([frontA, frontB]);
extruded.edges.push([backA, backB]);
extruded.edges.push([frontA, backA]);
extruded.edges.push([frontB, backB]);
}

// Add front face (with unshared vertices for flat shading)
const frontVertexOffset = extruded.vertices.length;
for (const v of newVertices) {
extruded.vertices.push(new Vector(v.x, v.y, v.z + half));
}
for (const face of newFaces) {
if (face.length < 3) continue;
const mappedFace = face.map(i => i + frontVertexOffset);
extruded.faces.push(mappedFace);

// ADD EDGES FOR FRONT FACE
for (let i = 0; i < mappedFace.length; i++) {
const nextIndex = (i + 1) % mappedFace.length;
extruded.edges.push([mappedFace[i], mappedFace[nextIndex]]);
}
}

// Add back face (reversed winding order)
const backVertexOffset = extruded.vertices.length;
for (const v of newVertices) {
extruded.vertices.push(new Vector(v.x, v.y, v.z - half));
}

for (const face of newFaces) {
if (face.length < 3) continue;
const mappedFace = [...face].reverse().map(i => i + backVertexOffset);
extruded.faces.push(mappedFace);

// ADD EDGES FOR BACK FACE
for (let i = 0; i < mappedFace.length; i++) {
const nextIndex = (i + 1) % mappedFace.length;
extruded.edges.push([mappedFace[i], mappedFace[nextIndex]]);
}
}

extruded.computeNormals();
return extruded;
}

variations() {
Expand Down
Loading