Indexed multi-material batching for GLTF PBR and legacy materials

[RyeMutt]

Description

Extends the viewer's per-vertex texture-index batching — previously diffuse-only — so a single draw call can cover multiple materials, for both GLTF PBR and legacy Blinn-Phong materials, across the opaque, alpha-mask, rigged, shadow, and glow/emissive passes. Multi-material objects and meshes that previously emitted one draw call per material now batch into one, cutting draw-call count and CPU-side bind/setup overhead.

Mechanism. Each vertex carries a material slot index (reusing the existing texture_index packed in position.w). Hand-written indexed shaders select that slot's samplers via an if-chain gated on a GLTF_INDEXED_CHANNELS define (if-chain, not switch, for driver portability). Indexed draw infos coexist with scalar ones in the existing render passes — flagged by mGLTFMaterialList.size() > 1 (PBR) / mMaterialSlotList.size() > 1 (legacy) — so no new pass types are introduced and shadow/probe/cull coverage is unchanged. Per-slot material maps bind to texture units [s, N+s, 2N+s, 3N+s]; per-slot scalars/transforms arrive as uniform arrays indexed by the slot.

Scope, by phase (one commit each):

• 1 static opaque PBR
• 2 PBR alpha-mask (+ shadow)
• 3 PBR rigged (skinned + shadow)
• 4 profiling + shadow bind reduction
• 5a / 5b / 5c legacy materials — static / rigged / shadow alpha-mask (all normal/spec combinations)
• 6 glow/emissive — PBR PASS_GLTF_GLOW + legacy PASS_GLOW, static + rigged; plus an eGLTFIndexedMaps fast path (the glow sweep binds base-color + emissive only, skipping the normal/ORM binds and the normal-map discard-level fetch) and a shader-load robustness guard

Graceful degradation. Every indexed shader program is gated and kept out of the main shader-load success chain. If any fails to compile, that path falls back to the existing scalar rendering — never a crash, never disabling the core viewer. A PBR-opaque-indexed load failure additionally tears down the legacy indexed programs so the whole feature disables together (invariant: sIndexedLegacyMaterials => sIndexedGLTFChannels >= 2). Runtime A/B toggle: RenderGLTFPBRBatching (default on).

Related Issues

No tracking issue — internal rendering performance work.

Issue Link: n/a

---

Checklist

• I have provided a clear title and detailed description for this pull request.
• If useful, I have included media such as screenshots and video to show off my changes.
• I have tested the changes locally and verified they work as intended.
• All new and existing tests pass.
• Code follows the project's style guidelines.
• Documentation has been updated if needed.
• Any dependent changes have been merged and published in downstream modules
• I have reviewed the contributing guidelines.

---

Additional Notes

Verification status (runtime, visual A/B via RenderGLTFPBRBatching):

• PBR phases 1–4 and legacy 5a/5b/5c — verified working in-world.
• Phase 6 (glow/emissive) — committed; final runtime verification of multi-material glow (≥2 glowing materials, PBR + legacy, static + rigged) still recommended before merge.
• No unit tests added — this is GBuffer/draw-pool rendering; correctness is validated visually.

Reflection probes: verified to already capture indexed batches correctly — probe capture (display_cube_face → renderGeomDeferred → pool renderDeferred) shares the main GBuffer path and the same scalar/indexed split, so per-slot materials and cutouts are correct in probes with no probe-specific code.

Known limitation: a batch is capped at 8 materials by the fixed per-slot sampler switch; lifting that (and sharing units across slots) would require bindless or array textures — a separate, larger change. Documented as future work.

Reviewer notes: the indexed shaders mirror their scalar counterparts (pbropaqueIndexed*, materialIndexed*, pbrShadowAlphaMaskIndexed*, materialShadowIndexed*, pbrglowIndexed*, emissiveIndexed*). The shadow sweep for batched objects stays scalar by design except for alpha-mask, which has dedicated indexed programs so per-face cutouts cast correct shadows.

[coderabbitai]

Warning

Review limit reached

@RyeMutt, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 23 minutes and 27 seconds. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 46b0362d-eece-4180-b64d-633590e646dc

📥 Commits

Reviewing files that changed from the base of the PR and between e6f7cac and d047980.

📒 Files selected for processing (7)

• indra/newview/app_settings/settings_alchemy.xml
• indra/newview/lldrawpool.cpp
• indra/newview/lldrawpoolmaterials.cpp
• indra/newview/lldrawpoolpbropaque.cpp
• indra/newview/lldrawpoolsimple.cpp
• indra/newview/llviewershadermgr.cpp
• indra/newview/llvovolume.cpp

📝 Walkthrough

Walkthrough

Adds indexed multi-material draw batching for GLTF PBR and legacy Blinn-Phong materials in the deferred renderer. New LLGLSLShader state fields gate the feature; LLDrawInfo gains per-slot material lists; indexed shader variants are compiled and conditionally loaded based on available texture units; genDrawInfo and registerFace classify and accumulate indexed batches; new scalar/indexed draw-pool dispatch methods replace unconditional scalar paths; and shadow rendering conditionally routes alpha-masked passes through indexed programs.

Changes

Indexed Multi-Material Batching

Layer / File(s)	Summary
State variables, data contracts, and public API declarations indra/llrender/llglslshader.cpp, indra/llrender/llglslshader.h, indra/newview/app_settings/settings_alchemy.xml, indra/newview/llspatialpartition.h, indra/newview/lldrawpool.h, indra/newview/llviewershadermgr.h	Declares sIndexedGLTFChannels and sIndexedLegacyMaterials statics; adds RenderGLTFPBRBatching setting; extends LLDrawInfo with mGLTFMaterialList, MaterialSlot, and mMaterialSlotList; introduces eGLTFIndexedMaps enum and all new LLRenderPass method declarations; adds extern shader handles for all indexed program variants.
Indexed shader globals, loading, and unloading indra/newview/llviewershadermgr.cpp	Adds global handles for all indexed shader programs; implements per-slot sampler-mapping helpers; computes sIndexedGLTFChannels from texture unit count; adds conditional load paths for GLTF opaque/glow/shadow, legacy GBuffer/emissive/shadow indexed programs with graceful fallback (zeroing channels or disabling legacy batching) on compilation failure; extends deferred unload to clean up indexed programs.
Face eligibility, batching classification, and genDrawInfo accumulation indra/newview/llvovolume.cpp	Introduces can_batch_gltf_material and can_batch_legacy_material eligibility helpers; extends registerFace to classify faces as GLTF-indexed or legacy-indexed and populate mGLTFMaterialList/mMaterialSlotList; updates merge and construction paths for indexed slot lists; adds rebuildGeom feature-gating flags; extends genDrawInfo with batch_gltf/batch_legacy parameters and per-face setTextureIndex(slot) assignment with compatibility enforcement.
Draw-pool scalar/indexed dispatch methods indra/newview/lldrawpool.cpp, indra/newview/lldrawpoolmaterials.cpp, indra/newview/lldrawpoolpbropaque.cpp, indra/newview/lldrawpoolsimple.cpp	Adds skip guards in existing mask-batch passes for multi-material draw infos; implements pushMaskBatchesIndexed, pushEmissiveBatchesScalar/Indexed, pushGLTFBatchesScalar/Indexed, pushGLTFBatchIndexed (per-slot texture/uniform binding and draw), and rigged counterparts; wires indexed paths into LLDrawPoolMaterials (new sMaterialShaderIdx table, pushMaterialBatchIndexed), LLDrawPoolGLTFPBR renderDeferred/renderPostDeferred, and LLDrawPoolGlow::renderPostDeferred.
Shadow-pass indexed batching integration indra/newview/pipeline.cpp	Conditionally binds indexed legacy shadow material programs for alpha-mask passes and routes them through pushMaskBatchesIndexed; introduces gltf_indexed check in GLTF shadow alpha-mask rendering to select indexed vs non-indexed programs and corresponding batch submission calls.

Sequence Diagram(s)

sequenceDiagram
  participant LLPipeline
  participant LLVolumeGeometryManager
  participant LLDrawInfo
  participant LLViewerShaderMgr
  participant LLRenderPass

  rect rgba(100, 149, 237, 0.5)
    Note over LLVolumeGeometryManager,LLDrawInfo: Geometry Build Phase
    LLVolumeGeometryManager->>LLVolumeGeometryManager: can_batch_gltf_material / can_batch_legacy_material
    LLVolumeGeometryManager->>LLDrawInfo: setTextureIndex(slot) per face
    LLVolumeGeometryManager->>LLDrawInfo: populate mGLTFMaterialList or mMaterialSlotList
  end

  rect rgba(60, 179, 113, 0.5)
    Note over LLViewerShaderMgr: Shader Load Phase
    LLViewerShaderMgr->>LLViewerShaderMgr: compute sIndexedGLTFChannels = clamp(units/4, 1, 8)
    LLViewerShaderMgr->>LLViewerShaderMgr: load gDeferredPBROpaqueIndexedProgram
    LLViewerShaderMgr->>LLViewerShaderMgr: map per-slot samplers
    LLViewerShaderMgr->>LLGLSLShader: sIndexedLegacyMaterials = true
  end

  rect rgba(210, 105, 30, 0.5)
    Note over LLPipeline,LLRenderPass: Render Phase
    LLPipeline->>LLRenderPass: pushGLTFBatchesScalar (skip multi-slot)
    LLPipeline->>LLRenderPass: bind gDeferredPBROpaqueIndexedProgram
    LLPipeline->>LLRenderPass: pushGLTFBatchesIndexed
    LLRenderPass->>LLRenderPass: pushGLTFBatchIndexed (bind per-slot textures + uniforms, draw)
    LLPipeline->>LLRenderPass: pushMaskBatchesIndexed (shadow pass)
  end

Loading

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~120 minutes

Suggested labels

c/cpp, llrender, viewer

Poem

🐇 Hop hop, the draw calls shrink today,
Per-slot uniforms packed in array,
GLTF channels counted by four,
Legacy batches batch even more—
One indexed pass where there were many,
The rabbit saves each precious penny!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 39.02% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely summarizes the main change: extending batching to support multiple materials for both GLTF PBR and legacy materials, which is the core objective of this changeset.
Description check	✅ Passed	The description comprehensively covers motivation, mechanism, scope, degradation strategy, and verification status. All required checklist items are addressed, and the related issues section is properly filled.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 6

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

indra/newview/llvovolume.cpp (1)

6343-6350: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Enable legacy indexing for the no-map material bucket.

Line 6234 sends legacy materials with neither normal nor specular maps to sSimpleFaces, but only the normal/spec/normspec buckets receive batch_legacy=true. Those valid PASS_MATERIAL / mask candidates stay scalar even when indexed legacy programs are available; pass the flag through and let can_batch_legacy_material() filter non-material simple faces.

⚡ Proposed fix

-        geometryBytes += genDrawInfo(group, simple_mask | extra_mask, sSimpleFaces[i], simple_count[i], false, batch_textures, rigged);
+        geometryBytes += genDrawInfo(group, simple_mask | extra_mask, sSimpleFaces[i], simple_count[i], false, batch_textures, rigged, false, legacy_batch_enabled);
         geometryBytes += genDrawInfo(group, fullbright_mask | extra_mask, sFullbrightFaces[i], fullbright_count[i], false, batch_textures, rigged);
         geometryBytes += genDrawInfo(group, alpha_mask | extra_mask, sAlphaFaces[i], alpha_count[i], alpha_sort, batch_textures, rigged);
         geometryBytes += genDrawInfo(group, bump_mask | extra_mask, sBumpFaces[i], bump_count[i], false, false, rigged);

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@indra/newview/llvovolume.cpp` around lines 6343 - 6350, The simple_mask
genDrawInfo call for sSimpleFaces is missing the legacy_batch_enabled parameter
that should be passed to enable legacy indexing for the no-map material bucket.
Update the first genDrawInfo call (simple_mask) to include legacy_batch_enabled
as the final parameter, matching the pattern used in the norm_mask, spec_mask,
and normspec_mask calls just below it, so that valid PASS_MATERIAL candidates
can be properly indexed when legacy programs are available.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@indra/newview/app_settings/settings_alchemy.xml`:
- Line 1241: The description string in the setting at line 1241 incorrectly
states that batching applies to "static opaque only," but the toggle now gates
broader indexed batching paths including rigged materials, alpha-masked
materials, and legacy indexed eligibility. Update the string description to
accurately reflect the broader scope of batching that this setting controls,
removing the misleading "static opaque only" limitation.

In `@indra/newview/lldrawpool.cpp`:
- Around line 1015-1033: The code uses fixed-size 8-slot arrays (roughness,
metallic, min_alpha, emissive, bc_xform, nm_xform, mr_xform, em_xform) without
validating that the input size respects this limit. Add guard assertions or
checks before array writes across four locations to ensure no buffer overruns.
In indra/newview/lldrawpool.cpp lines 1015-1033, validate that n (from
mGLTFMaterialList.size()) does not exceed both 8 and
LLGLSLShader::sIndexedGLTFChannels before the main loop. In
indra/newview/lldrawpool.cpp lines 593-607, validate the mask slot count before
writing to the min_alpha[8] array. In indra/newview/lldrawpool.cpp lines
673-680, validate the emissive slot count before binding slot texture units. In
indra/newview/lldrawpoolmaterials.cpp lines 135-153, validate the material slot
count (from mMaterialSlotList.size()) before writing per-slot arrays and binding
texture unit offsets.

In `@indra/newview/lldrawpoolpbropaque.cpp`:
- Around line 61-92: The indexed flag is being used to skip scalar
multi-material rendering before verifying that all indexed program variants are
complete, which can cause incomplete indexed programs to be used. In
indra/newview/lldrawpoolpbropaque.cpp L61-L92, add completeness checks for both
gDeferredPBROpaqueIndexedProgram.isComplete() and its rigged variant before the
indexed conditional that calls pushGLTFBatchesScalar and
pushRiggedGLTFBatchesScalar. In indra/newview/lldrawpoolpbropaque.cpp L114-L143,
add the gPBRGlowIndexedProgram rigged variant completeness check before the
indexed conditional that routes rigged glow. In
indra/newview/lldrawpoolsimple.cpp L63-L96, add the
gDeferredEmissiveIndexedProgram rigged variant completeness check before setting
glow_indexed for both static and rigged legacy glow paths. Ensure all indexed
program completeness is verified before allowing scalar sweeps to skip
multi-material infos.

In `@indra/newview/llviewershadermgr.cpp`:
- Around line 1406-1467: The static flag LLGLSLShader::sIndexedLegacyMaterials
can retain a true value from a previous successful shader load, causing a stale
state when reloading. If the outer condition (LLGLSLShader::sIndexedGLTFChannels
>= 2) is false or material_indexed_ok fails, the flag remains true while the
indexed material programs are unloaded or never compiled, creating an
inconsistent state. Reset LLGLSLShader::sIndexedLegacyMaterials to false before
the material_indexed_ok block starts (at the beginning of the outer if
statement) to ensure a clean state for each load attempt.

In `@indra/newview/llvovolume.cpp`:
- Around line 5558-5567: The legacy slot key only includes material and diffuse
texture, but the slot data includes mSpecColor and mEnvIntensity which are
derived from the TE shiny value (via spec_lut[shiny & TEM_SHINY_MASK]). This
causes faces with the same material and diffuse but different shiny values to
reuse the same slot, overwriting earlier vertex data. Include the TE shiny value
in the legacy slot key generation at both affected locations (around lines
5558-5567 and 6742-6761) to ensure each unique combination of material, diffuse,
and shiny gets its own slot.
- Around line 5526-5533: The scalar rendering path needs to reset the texture
index sentinel before registerFace infers indexed mode. Currently, the code
checks if index is less than FACE_DO_NOT_BATCH_TEXTURES to determine indexed
batching (in both the gltf_indexed and legacy_indexed boolean assignments), but
scalar faces from a prior indexed rebuild may retain a material slot. Clear the
sentinel value for scalar faces before these index comparisons occur in
registerFace to ensure that scalar rendering properly degrades instead of
incorrectly populating indexed material lists when RenderGLTFPBRBatching is
disabled or during shader fallback.

---

Outside diff comments:
In `@indra/newview/llvovolume.cpp`:
- Around line 6343-6350: The simple_mask genDrawInfo call for sSimpleFaces is
missing the legacy_batch_enabled parameter that should be passed to enable
legacy indexing for the no-map material bucket. Update the first genDrawInfo
call (simple_mask) to include legacy_batch_enabled as the final parameter,
matching the pattern used in the norm_mask, spec_mask, and normspec_mask calls
just below it, so that valid PASS_MATERIAL candidates can be properly indexed
when legacy programs are available.

🪄 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: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: be82a551-51c5-405b-a4b9-66bcfada856e

📥 Commits

Reviewing files that changed from the base of the PR and between dfc10be and e6f7cac.

⛔ Files ignored due to path filters (12)

• indra/newview/app_settings/shaders/class1/deferred/emissiveIndexedF.glsl is excluded by !**/*.glsl
• indra/newview/app_settings/shaders/class1/deferred/emissiveIndexedV.glsl is excluded by !**/*.glsl
• indra/newview/app_settings/shaders/class1/deferred/materialIndexedF.glsl is excluded by !**/*.glsl
• indra/newview/app_settings/shaders/class1/deferred/materialIndexedV.glsl is excluded by !**/*.glsl
• indra/newview/app_settings/shaders/class1/deferred/materialShadowIndexedF.glsl is excluded by !**/*.glsl
• indra/newview/app_settings/shaders/class1/deferred/materialShadowIndexedV.glsl is excluded by !**/*.glsl
• indra/newview/app_settings/shaders/class1/deferred/pbrShadowAlphaMaskIndexedF.glsl is excluded by !**/*.glsl
• indra/newview/app_settings/shaders/class1/deferred/pbrShadowAlphaMaskIndexedV.glsl is excluded by !**/*.glsl
• indra/newview/app_settings/shaders/class1/deferred/pbrglowIndexedF.glsl is excluded by !**/*.glsl
• indra/newview/app_settings/shaders/class1/deferred/pbrglowIndexedV.glsl is excluded by !**/*.glsl
• indra/newview/app_settings/shaders/class1/deferred/pbropaqueIndexedF.glsl is excluded by !**/*.glsl
• indra/newview/app_settings/shaders/class1/deferred/pbropaqueIndexedV.glsl is excluded by !**/*.glsl

📒 Files selected for processing (13)

• indra/llrender/llglslshader.cpp
• indra/llrender/llglslshader.h
• indra/newview/app_settings/settings_alchemy.xml
• indra/newview/lldrawpool.cpp
• indra/newview/lldrawpool.h
• indra/newview/lldrawpoolmaterials.cpp
• indra/newview/lldrawpoolpbropaque.cpp
• indra/newview/lldrawpoolsimple.cpp
• indra/newview/llspatialpartition.h
• indra/newview/llviewershadermgr.cpp
• indra/newview/llviewershadermgr.h
• indra/newview/llvovolume.cpp
• indra/newview/pipeline.cpp