fix(pbr): honor per-texture texCoord (UV1) for all channels#397
Open
RaananW wants to merge 2 commits into
Open
fix(pbr): honor per-texture texCoord (UV1) for all channels#397RaananW wants to merge 2 commits into
RaananW wants to merge 2 commits into
Conversation
glTF materials can point each texture at a different UV set via `texCoord` (0 or 1), but Babylon-Lite only wired the second UV set (uv2 / TEXCOORD_1) for the occlusion texture. baseColor / emissive / normal / metallicRoughness / specGloss with texCoord:1 were incorrectly sampled on UV0. Root cause (two-fold): 1. pbr-renderable `hasAnyUv2` gate only checked `occlusionTexCoord === 1`, so the uv2 attribute/varying was never threaded when another channel used UV1. 2. The PBR ext template only routed UV1 to the occlusion sample; every other channel hard-coded `input.uv`. Fix: a compact per-channel `_uv2Mask` integer (baseColor=1, orm=2, normal=4, emissive=8, specGloss=16, occlusion=32). It is encoded ONCE on the lazy glTF slow path (gltf-pbr-builder-ext) — the only place a texture can carry texCoord:1 — so the always-loaded fast path just reads `mat._uv2Mask`. The ext template decodes the mask and selects `input.uv2` per channel; the bindings and compose cache keys fold in the mask so distinct UV routings get distinct shaders. Verified: cx20/gltf-test #3 MultiUVTest now renders correctly (baseColor on UV0 overlaid by the emissive texture on UV1), matching Babylon.js. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes Babylon-Lite’s PBR glTF handling so each texture channel honors its own glTF texCoord (UV0 vs UV1) by introducing and threading a compact per-material _uv2Mask bitmask through shader composition, template generation, and bindings. This addresses incorrect sampling when channels like baseColor/emissive/normal request texCoord: 1.
Changes:
- Encode a per-channel UV1 selection mask on the glTF slow path and use it to enable UV2 plumbing (
PBR2_HAS_UV2) and selectinput.uv2per channel inpbr-template-ext. - Fold
_uv2Maskinto PBR compose/bindings cache keys (and thread it through geometry/MRT composition paths). - Tighten occlusion texture binding to require
occlusionTexCoord === 1, and regenerate per-scene bundle manifests.
Reviewed changes
Copilot reviewed 94 out of 94 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/babylon-lite/src/material/pbr/pbr-template-ext.ts | Decode _uv2Mask and route per-channel UV selection (uv vs uv2) in the lazy PBR template ext. |
| packages/babylon-lite/src/material/pbr/pbr-renderable.ts | Update UV2 gating and fold _uv2Mask into compose/bindings keys when building PBR renderables. |
| packages/babylon-lite/src/material/pbr/pbr-pipeline.ts | Require occlusionTexCoord === 1 before binding occlusionTexture. |
| packages/babylon-lite/src/material/pbr/pbr-material.ts | Add _uv2Mask to material props and compute PBR2_HAS_UV2 from it. |
| packages/babylon-lite/src/material/pbr/pbr-geometry-renderable.ts | Thread _uv2Mask through the geometry/MRT view composition path. |
| packages/babylon-lite/src/material/pbr/pbr-geometry-output-shader.ts | Extend geometry shader composer to pass _uv2Mask into PBR composition. |
| packages/babylon-lite/src/material/pbr/pbr-flags.ts | Extend fragment context with _uv2Mask for lazy fragments that need UV2 semantics. |
| packages/babylon-lite/src/material/pbr/pbr-compose.ts | Extend composer signature/cache key to include _uv2Mask and pass it to the template ext + frag ctx. |
| packages/babylon-lite/src/material/pbr/no-color-view.ts | Formatting-only change to createMaterialView call. |
| packages/babylon-lite/src/material/pbr/fragments/reflectance-fragment.ts | Use _uv2Mask bit 32 to decide occlusion-on-UV1 behavior instead of PBR2_HAS_UV2. |
| packages/babylon-lite/src/loader-gltf/gltf-pbr-builder-ext.ts | Encode _uv2Mask from glTF texture infos on the slow path and attach it to PbrMaterialProps. |
| lab/public/bundle/manifest/scene1.json | Regenerated runtime chunk manifest after bundle rebuild. |
| lab/public/bundle/manifest/scene5.json | Regenerated runtime chunk manifest after bundle rebuild. |
| lab/public/bundle/manifest/scene6.json | Regenerated runtime chunk manifest after bundle rebuild. |
| lab/public/bundle/manifest/scene7.json | Regenerated runtime chunk manifest after bundle rebuild. |
| lab/public/bundle/manifest/scene8.json | Regenerated runtime chunk manifest after bundle rebuild. |
| lab/public/bundle/manifest/scene10.json | Regenerated runtime chunk manifest after bundle rebuild. |
| lab/public/bundle/manifest/scene11.json | Regenerated runtime chunk manifest after bundle rebuild. |
| lab/public/bundle/manifest/scene12.json | Regenerated runtime chunk manifest after bundle rebuild. |
| lab/public/bundle/manifest/scene13.json | Regenerated runtime chunk manifest after bundle rebuild. |
| lab/public/bundle/manifest/scene14.json | Regenerated runtime chunk manifest after bundle rebuild. |
| lab/public/bundle/manifest/scene17.json | Regenerated runtime chunk manifest after bundle rebuild. |
| lab/public/bundle/manifest/scene19.json | Regenerated runtime chunk manifest after bundle rebuild. |
| lab/public/bundle/manifest/scene20.json | Regenerated runtime chunk manifest after bundle rebuild. |
| lab/public/bundle/manifest/scene21.json | Regenerated runtime chunk manifest after bundle rebuild. |
| lab/public/bundle/manifest/scene22.json | Regenerated runtime chunk manifest after bundle rebuild. |
| lab/public/bundle/manifest/scene23.json | Regenerated runtime chunk manifest after bundle rebuild. |
| lab/public/bundle/manifest/scene26.json | Regenerated runtime chunk manifest after bundle rebuild. |
| lab/public/bundle/manifest/scene27.json | Regenerated runtime chunk manifest after bundle rebuild. |
| lab/public/bundle/manifest/scene28.json | Regenerated runtime chunk manifest after bundle rebuild. |
| lab/public/bundle/manifest/scene29.json | Regenerated runtime chunk manifest after bundle rebuild. |
| lab/public/bundle/manifest/scene30.json | Regenerated runtime chunk manifest after bundle rebuild. |
| lab/public/bundle/manifest/scene31.json | Regenerated runtime chunk manifest after bundle rebuild. |
| lab/public/bundle/manifest/scene32.json | Regenerated runtime chunk manifest after bundle rebuild. |
| lab/public/bundle/manifest/scene33.json | Regenerated runtime chunk manifest after bundle rebuild. |
| lab/public/bundle/manifest/scene34.json | Regenerated runtime chunk manifest after bundle rebuild. |
| lab/public/bundle/manifest/scene35.json | Regenerated runtime chunk manifest after bundle rebuild. |
| lab/public/bundle/manifest/scene37.json | Regenerated runtime chunk manifest after bundle rebuild. |
| lab/public/bundle/manifest/scene39.json | Regenerated runtime chunk manifest after bundle rebuild. |
| lab/public/bundle/manifest/scene41.json | Regenerated runtime chunk manifest after bundle rebuild. |
| lab/public/bundle/manifest/scene47.json | Regenerated runtime chunk manifest after bundle rebuild. |
| lab/public/bundle/manifest/scene73.json | Regenerated runtime chunk manifest after bundle rebuild. |
| lab/public/bundle/manifest/scene99.json | Regenerated runtime chunk manifest after bundle rebuild. |
| lab/public/bundle/manifest/scene104.json | Regenerated runtime chunk manifest after bundle rebuild. |
| lab/public/bundle/manifest/scene105.json | Regenerated runtime chunk manifest after bundle rebuild. |
| lab/public/bundle/manifest/scene111.json | Regenerated runtime chunk manifest after bundle rebuild. |
| lab/public/bundle/manifest/scene112.json | Regenerated runtime chunk manifest after bundle rebuild. |
| lab/public/bundle/manifest/scene114.json | Regenerated runtime chunk manifest after bundle rebuild. |
| lab/public/bundle/manifest/scene115.json | Regenerated runtime chunk manifest after bundle rebuild. |
| lab/public/bundle/manifest/scene116.json | Regenerated runtime chunk manifest after bundle rebuild. |
| lab/public/bundle/manifest/scene141.json | Regenerated runtime chunk manifest after bundle rebuild. |
| lab/public/bundle/manifest/scene144.json | Regenerated runtime chunk manifest after bundle rebuild. |
| lab/public/bundle/manifest/scene146.json | Regenerated runtime chunk manifest after bundle rebuild. |
| lab/public/bundle/manifest/scene147.json | Regenerated runtime chunk manifest after bundle rebuild. |
| lab/public/bundle/manifest/scene148.json | Regenerated runtime chunk manifest after bundle rebuild. |
| lab/public/bundle/manifest/scene149.json | Regenerated runtime chunk manifest after bundle rebuild. |
| lab/public/bundle/manifest/scene152.json | Regenerated runtime chunk manifest after bundle rebuild. |
| lab/public/bundle/manifest/scene157.json | Regenerated runtime chunk manifest after bundle rebuild. |
| lab/public/bundle/manifest/scene158.json | Regenerated runtime chunk manifest after bundle rebuild. |
| lab/public/bundle/manifest/scene164.json | Regenerated runtime chunk manifest after bundle rebuild. |
| lab/public/bundle/manifest/scene171.json | Regenerated runtime chunk manifest after bundle rebuild. |
| lab/public/bundle/manifest/scene174.json | Regenerated runtime chunk manifest after bundle rebuild. |
| lab/public/bundle/manifest/scene175.json | Regenerated runtime chunk manifest after bundle rebuild. |
| lab/public/bundle/manifest/scene176.json | Regenerated runtime chunk manifest after bundle rebuild. |
| lab/public/bundle/manifest/scene177.json | Regenerated runtime chunk manifest after bundle rebuild. |
| lab/public/bundle/manifest/scene178.json | Regenerated runtime chunk manifest after bundle rebuild. |
| lab/public/bundle/manifest/scene179.json | Regenerated runtime chunk manifest after bundle rebuild. |
| lab/public/bundle/manifest/scene210.json | Regenerated runtime chunk manifest after bundle rebuild. |
| lab/public/bundle/manifest/scene211.json | Regenerated runtime chunk manifest after bundle rebuild. |
| lab/public/bundle/manifest/scene212.json | Regenerated runtime chunk manifest after bundle rebuild. |
| lab/public/bundle/manifest/scene215.json | Regenerated runtime chunk manifest after bundle rebuild. |
| lab/public/bundle/manifest/scene216.json | Regenerated runtime chunk manifest after bundle rebuild. |
| lab/public/bundle/manifest/scene217.json | Regenerated runtime chunk manifest after bundle rebuild. |
| lab/public/bundle/manifest/scene218.json | Regenerated runtime chunk manifest after bundle rebuild. |
| lab/public/bundle/manifest/scene219.json | Regenerated runtime chunk manifest after bundle rebuild. |
| lab/public/bundle/manifest/scene229.json | Regenerated runtime chunk manifest after bundle rebuild. |
| lab/public/bundle/manifest/scene240.json | Regenerated runtime chunk manifest after bundle rebuild. |
| lab/public/bundle/manifest/scene241.json | Regenerated runtime chunk manifest after bundle rebuild. |
| lab/public/bundle/manifest/scene242.json | Regenerated runtime chunk manifest after bundle rebuild. |
| lab/public/bundle/manifest/scene243.json | Regenerated runtime chunk manifest after bundle rebuild. |
| lab/public/bundle/manifest/scene244.json | Regenerated runtime chunk manifest after bundle rebuild. |
| lab/public/bundle/manifest/scene245.json | Regenerated runtime chunk manifest after bundle rebuild. |
| lab/public/bundle/manifest/scene246.json | Regenerated runtime chunk manifest after bundle rebuild. |
| lab/public/bundle/manifest/scene247.json | Regenerated runtime chunk manifest after bundle rebuild. |
| lab/public/bundle/manifest/scene248.json | Regenerated runtime chunk manifest after bundle rebuild. |
| lab/public/bundle/manifest/scene249.json | Regenerated runtime chunk manifest after bundle rebuild. |
| lab/public/bundle/manifest/scene251.json | Regenerated runtime chunk manifest after bundle rebuild. |
| lab/public/bundle/manifest/scene253.json | Regenerated runtime chunk manifest after bundle rebuild. |
| lab/public/bundle/manifest/scene254.json | Regenerated runtime chunk manifest after bundle rebuild. |
| lab/public/bundle/manifest/scene255.json | Regenerated runtime chunk manifest after bundle rebuild. |
| lab/public/bundle/manifest/scene257.json | Regenerated runtime chunk manifest after bundle rebuild. |
| lab/public/bundle/manifest/scene258.json | Regenerated runtime chunk manifest after bundle rebuild. |
| lab/public/bundle/manifest/scene259.json | Regenerated runtime chunk manifest after bundle rebuild. |
| lab/public/bundle/manifest/scene260.json | Regenerated runtime chunk manifest after bundle rebuild. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+208
to
214
| // Per-channel UV set selection (glTF texCoord). `_uv2Mask` is precomputed once at glTF build | ||
| // time by the lazy slow-path loader (gltf-pbr-builder-ext) — the only place a texture can carry | ||
| // texCoord:1 (occlusion included) — so the always-loaded fast path pays just one read here. | ||
| // Any channel on UV1 needs the uv2 vertex attribute + varying threaded through. | ||
| if ((mat as { _uv2Mask?: number })._uv2Mask) { | ||
| features2 |= PBR2_HAS_UV2; | ||
| } |
Comment on lines
128
to
132
| hasAnyUvTransform ||= !!mat._hasUvTx; | ||
| // UV2 only counts when occlusion samples texcoord 1. | ||
| hasAnyUv2 ||= !!m._gpu.uv2Buffer && mat.occlusionTexCoord === 1; | ||
| // UV2 counts when ANY PBR channel samples texCoord 1 (occlusion included) — precomputed as | ||
| // `_uv2Mask` on the material by the glTF slow path (0/undefined on the fast path). | ||
| hasAnyUv2 ||= !!m._gpu.uv2Buffer && !!(mat as { _uv2Mask?: number })._uv2Mask; | ||
| hasAnyVertexColor ||= !!m._gpu.colorBuffer; |
|
|
||
| const composed = composePbr(features, features2, meshFeatures, sceneFeatures, lightMode, singleLightType, esmShadowDepthCode, vbLayout, vbKey); | ||
| const bindings = getOrCreatePbrBindings(engine, features, features2, meshFeatures, sceneFeatures, composed, `${lightMode}:${singleLightType}${vbKey}`, mat.stencil ?? null); | ||
| const uv2Mask = (mat as { _uv2Mask?: number })._uv2Mask ?? 0; |
| const source = view.source as PbrMaterialProps; | ||
| const vbLayout = (source as unknown as { _vbLayout?: import("../../mesh/mesh.js").MeshVbLayout })._vbLayout; | ||
| const vbKey = ""; | ||
| const uv2Mask = (source as { _uv2Mask?: number })._uv2Mask ?? 0; |
Comment on lines
+2
to
+3
| "rawKB": 112.4, | ||
| "gzipKB": 45.6, |
Comment on lines
+2
to
+3
| "rawKB": 108.8, | ||
| "gzipKB": 46, |
Comment on lines
+2
to
+3
| "rawKB": 97.5, | ||
| "gzipKB": 41, |
…ved)
Provisional, minimal-slack ceiling bumps to cover the per-channel texCoord
fix's measured raw-byte deltas (user-approved, option (a)):
- scene144 112 -> 112.42 (+420B, GENUINE bugfix: dragon normal map now
correctly sampled on TEXCOORD_1)
- scene39 108.7 -> 108.81 (+103B, marginal generalized-decode plumbing)
- scene243 97.4 -> 97.49 (+84B, marginal generalized-decode plumbing)
- scene37 97.3 -> 97.31 (+1B, at-ceiling jitter)
Each ceiling is rounded up just enough to cover its current measured rawKB
(no extra slack); values will be reconciled to exact post-rebase numbers once
this branch rebases onto PR3 (#395), which edits the shared pbr-template-ext.ts.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
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.
Root cause
glTF materials can point each texture at a different UV set via
texCoord(0 or 1). Babylon-Lite only wired the second UV set (uv2 / TEXCOORD_1) for the occlusion texture (+ iridescence). WhenbaseColor/emissive/normal/metallicRoughness/specGlossspecifiedtexCoord:1, Lite incorrectly sampled UV0.Two-fold cause:
pbr-renderable.tshasAnyUv2gate only checkedocclusionTexCoord === 1, so the uv2 vertex attribute + varying were never threaded when another channel used UV1.pbr-template-ext.ts) only routed UV1 to the occlusion sample; every other channel hard-codedinput.uv.In cx20/gltf-test #3 MultiUVTest this is visually obvious: the baseColor texture on UV0 literally reads "Multiple UVs not supported in this viewer" and the emissive texture that should overlay via texCoord:1 was never sampled.
Fix
A compact per-channel
_uv2Maskinteger with a private bit contract:baseColor=1, orm=2, normal=4, emissive=8, specGloss=16, occlusion=32.gltf-pbr-builder-ext.assemblePbrPropsExt) — the only place a texture can carrytexCoord:1.pbr-material.ts,pbr-renderable.ts) just readsmat._uv2Mask— no new fields threaded throughMaterialRenderFeatures, keeping the hot chunk byte-neutral.input.uv2per channel (falling back toinput.uvwhen the uv2 attribute isn't present).pbr-pipeline.tsocclusion binding now also requiresocclusionTexCoord === 1, so a baseColor-UV1 material doesn't falsely bind an occlusion texture.This reuses the existing occlusion/iridescence texCoord plumbing as the pattern, rather than adding new shader permutations.
Files touched
loader-gltf/gltf-pbr-builder-ext.ts— encode_uv2Mask(single source of truth, slow path)material/pbr/pbr-material.ts— fast-path reads_uv2Mask→PBR2_HAS_UV2material/pbr/pbr-renderable.ts—hasAnyUv2gate + thread mask into compose/bindings keysmaterial/pbr/pbr-template-ext.ts— decode mask, per-channel base-UV selection (also edited by PR3 — see note below)material/pbr/pbr-compose.ts— thread mask, fold into cache key + fragCtxmaterial/pbr/fragments/reflectance-fragment.ts— occlusion-on-UV1 from mask bit 32material/pbr/pbr-pipeline.ts— occlusion binding requiresocclusionTexCoord === 1material/pbr/pbr-flags.ts,pbr-geometry-renderable.ts,pbr-geometry-output-shader.ts,no-color-view.ts— thread mask through geometry/MRT pathlab/public/bundle/manifest/scene*.json— regenerated per-scene bundle manifestsscene-config.json— 4 approved ceiling bumps (below)Bundle-size impact (maintainer/user-approved)
Four scenes needed a ceiling bump, each raised by the minimum to cover its measured raw-byte delta (no extra slack). Bundle-size spec: 207 passed.
uv2Mask=0) paying for the generalized decode code; shader output byte-identical to masterThese are provisional: values will be reconciled to exact post-rebase numbers once this branch rebases onto PR3 (which edits the shared shader and may shift these sizes).
Validation
skipParityOnCI:true, unrelated.)Model fixed
cx20/gltf-test #3 MultiUVTest — https://raw.githubusercontent.com/cx20/gltf-test/master/tutorialModels/MultiUVTest/glTF/MultiUVTest.gltf
This edits
pbr-template-ext.ts, which is also edited by PR3 (normal/tangent fixes, #395). PR3 should land first; this branch will then rebase onto master. Conflicts inpbr-template-ext.tsshould keep both PR3's backface-normal/tangent changes and this PR's per-channel texCoord/UV-selection logic (they touch different concerns). After the rebase, bundle-size + full parity will be re-run and the 4 ceilings reconciled to exact numbers. This PR should merge last.