OBJ vertex normals (#1572) and per-material diffuse textures (#1573) - #1577
Merged
Conversation
Two gaps in the OBJ/MTL path, both of which made an OBJ model render worse than the same model imported from glTF. #1572 — the parser read `vn` and threw it away, so `lit: true` on an OBJ shaded against a fallback. Authored normals (`v//vn`, `v/vt/vn`) now reach the mesh, with a vertex shared between different normals split so hard edges stay hard. A file supplying none gets them generated from face geometry — area-weighted, accumulated, normalized — computed after the winding correction so they follow the final triangle orientation. Normals are stored raw; the axis bridge is applied at draw through the model matrix, as it is for glTF. #1573 — a multi-material model bound whichever material's `map_Kd` came first for the whole mesh, so a crate with wood sides and a steel lid rendered entirely in wood. Each material's `Kd` already composed correctly (baked per-vertex at construction), which made the asymmetry the confusing part. `Mesh` now resolves each material's own texture and reduces the result to the shortest list of index ranges that need switching (`mesh.textureGroups`); both GPU backends draw one indexed range per entry over the same buffers — `drawElements` at a byte offset on WebGL, `drawIndexed` with a `firstIndex` on WebGPU — covering the retained, accumulated and instanced paths. The collapses matter as much as the split: adjacent materials sharing a map merge, a material without a `map_Kd` keeps the mesh texture, and a model needing no split issues exactly the one draw call it always did. An explicit `texture:` pins one binding and opts out. A `map_Kd` naming an image that never loaded warns and falls back rather than throwing — only the first material's map used to be resolved at all, so a partially-preloaded model that rendered must keep rendering. Also fixed while demonstrating #1572: a lit mesh whose vertex normal is zero-length rendered black (a normalize of the zero vector is NaN). Both lit fragment shaders now guard the length and degrade to unlit shading, which is what a mesh with no usable normals should look like. That is the Camera2d case — `normals` is only populated on the Camera3d path — tracked separately as #1576. Tests: 23 new (15 mesh_texture_groups, 8 webgpu_texture_groups) over a four-group OBJ fixture exercising merge, switch and fallback in one model, plus 8 for OBJ normals; both no-split regression pins are explicit. New `materialTextures` example, verified identical on WebGL and WebGPU; AfterBurner is now lit, which is what surfaced the black degradation. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QVjYzf76AEU3wJk766JAQi
Three parallel reviews of eca097d found seven real defects. All are fixed here with regression tests; the two HIGH ones were reachable on ordinary assets. OBJ normals (#1572): - Index buffers stayed Uint16 unconditionally. Splitting a position per distinct normal can treble a flat-shaded model's unified vertex count, so a model that fit before can now exceed 65 535 — and every index past that wrapped mod 65 536 in silence, stitching the tail of the model to its head. The buffer widens to Uint32 past 65 536 vertices. - Generated normals were area-weighted, which is biased by how a polygon was triangulated: a fan hands its pivot corners two triangles' worth of one face and the others one, so a cube built from quads accumulated (1, 0.5, 0.5) at a corner instead of (1, 1, 1) — 19.5 degrees off, and asymmetric on a symmetric model. Now angle-weighted, which is triangulation-independent. - Normal values were read at face-parse time, so an out-of-range `vn` read past `sourceNormals` and wrote NaN (which the shader guard does not catch), and a `vn` declared after the face using it never resolved. Resolution now happens once the whole file is read. - Generation was gated on "the file declared any vn" rather than on whether a given vertex got one, leaving zero-filled normals for partially-normalled files, for `vn` blocks no face references, and for empty normal fields. The rule is now per-vertex. - Generation accumulated per unified vertex, so the per-material dedup scope creased the model along every `usemtl` boundary — contradicting the documented "smooth" claim. It accumulates per source position. - `Mesh` wrote the OBJ's normals back onto the caller's `settings` object: a frozen literal threw, and one object reused for two models gave the second mesh the first's normals. - The WebGL lit shader's zero-normal early return dropped the per-instance emissive term the lit path adds, diverging from WGSL. Per-material textures (#1573): - The WebGL pre-pass resolved every range's texture unit before drawing. Exhausting the unit budget makes the texture cache recycle from unit 0, which invalidates units already handed out, so the earlier ranges sampled whatever landed last. Bound per range instead, immediately before its own draw — safe because `flush()` returns at zero vertices without touching the vertex array, and correct because a later range recycling an earlier one's unit no longer matters. - `textureFilter` was written onto `mesh.texture` alone and the batcher's mipmap gate read `mesh.texture` rather than the range's own, so a pixel-art model rendered one material crisp and the rest through the renderer default, with an incoherent mag/min pair on the others. - A zero-index group (two `usemtl` in a row) could name the mesh-level texture, painting the model in a map no geometry uses and leaving `mesh.texture` outside the plan entirely. - WebGPU resolved the mesh-level material even on the split path, reserving a unit for a binding immediately overwritten. CHANGELOG: the "falls back rather than throwing" claim now says which resolution it covers, and the black-lit entry no longer reads as closing #1576, which stays open. Tests: +14 (10 OBJ normals, 4 per-material). The unit-exhaustion test asserts a cache reset actually fired, so it cannot pass vacuously. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QVjYzf76AEU3wJk766JAQi
`indexType === UNSIGNED_INT ? 4 : 2` was dead code when written — the OBJ parser only ever emitted Uint16. Widening past 65 536 vertices in the previous commit makes it live, so a large multi-material model now takes that branch. A hard-coded stride of 2 would draw the wrong triangles with no GL error. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QVjYzf76AEU3wJk766JAQi
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
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.
Two gaps in the OBJ/MTL path, both of which made an OBJ model render worse than the same model imported from glTF. Closes #1572, closes #1573.
#1572 — OBJ models carry vertex normals, so they can be lit
The parser read
vnand threw it away, solit: trueon an OBJ shaded against a fallback while the identical model imported from glTF lit correctly.v//vnandv/vt/vn) now reach the mesh, and a vertex shared between different normals is split so hard edges stay hard.s) are still ignored, so a model relying on them for hard edges reads softer than authored; supplyvnto control that precisely.#1573 — per-material diffuse textures
A multi-material model bound whichever material's
map_Kdcame first for the whole mesh, so a crate with wood sides and a steel lid rendered entirely in wood. Each material'sKdalready composed correctly (baked into a per-vertex colour buffer at construction), which made the asymmetry the confusing part.Meshnow resolves each material's own texture and reduces the result to the shortest list of index ranges that actually need switching, exposed asmesh.textureGroups. Both GPU backends draw one indexed range per entry over the same buffers —drawElementsat a byte offset on WebGL,drawIndexedwith afirstIndexon WebGPU — covering the retained, accumulated and instanced paths.The collapses matter as much as the split:
map_Kdof its own keeps the mesh-level texture;Kd-only multi-material one — issues exactly the one draw call it always did;texture:pins one binding over the whole model and opts out entirely;map_Kdnaming an image that never loaded warns and falls back to the mesh texture rather than throwing. Only the first material's map used to be resolved at all, so a partially-preloaded model that rendered before must keep rendering.The Canvas renderer is unaffected: it solid-fills multi-material meshes per triangle and never samples a texture.
Also fixed
A lit mesh whose vertex normal is zero-length rendered black — normalizing the zero vector is NaN. Both lit fragment shaders (GLSL + WGSL) now guard the length and degrade to unlit shading, which is what a mesh with no usable normals should look like. That is the Camera2d case, since
normalsis only populated on the Camera3d path; populating them there is tracked separately as #1576.Verification
mesh_texture_groups.spec.jsand 8 inwebgpu_texture_groups.spec.jsover a four-group OBJ fixture that exercises merge, switch and fallback in one model. Both no-split regression pins (single-material → exactly 1 draw, on the retained and accumulated paths) are explicit, as are the byte-offset arithmetic and the "does not leave the split bound for the next mesh" case.materialTexturesexample — a crate whose wood, steel and label materials each carry their own map. Verified pixel-identical on WebGL and WebGPU.tscclean.🤖 Generated with Claude Code
https://claude.ai/code/session_01QVjYzf76AEU3wJk766JAQi