[BUG FIX] Correctly render assets that pair a base color with an emissive map. - #3088
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 333940aba9
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
39a7d00 to
7c45e43
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7c45e4367b
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
40568be to
c8955cb
Compare
|
@codex review |
|
Codex Review: Didn't find any major issues. Swish! Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
1 participant
Description
get_rgba(the packed-RGBA albedo consumed by the rasterizer and batch renderer) now prefers the authored base-color texture on every surface (Glass/Metal/Plastic/BSDF), and falls back to the emissive texture only when the base color is absent or fully black. The selection is centralized in a single_resolve_albedohelper instead of being duplicated per surface, soGlassandEmissionnow share the baseget_rgba. Per-environment (batched) textures resolve this fallback independently for each environment, pairing base and emissive batches by least common multiple to match how the RGBA combiner handles batch dimensions.surface_uvs_to_trimesh_visual) bakes a surface's emissive into the material, so emissive is composited additively on top of the base color across every rasterizer path (rigid and deformable alike) through the commonfrom_trimeshconversion. It is added only when the base color is the packed albedo; emissive thatget_rgbaalready packs as albedo (emission-only assets, a black base color, or anEmissionsurface) is not added a second time.mesh.frag(the factor was already folded into the sampled emissive value).KHR_materials_unlitassets, whose imagery is surfaced through emissive, no longer render as the default white base; the base is given a black factor so the emissive is resolved as the albedo.Related Issue
Resolves #3046
Resolves #2935
Supersedes #3038 (original base-color precedence fix by @NoahLinckeScout), extending it to all surfaces, additive emissive compositing across renderers, and glTF UV-set / unlit handling.
Motivation and Context
Some assets author a base color alongside an emissive map (e.g. vehicles); others zero their base-color factor and store the visible imagery in emissive (e.g. HLOD terrain tiles). The packed RGBA previously gave emissive unconditional precedence, so ordinary assets lost their base color, and assets with a sparse near-black emissive rendered almost entirely black in the rasterizer and batch renderer (only the ray tracer, which applies emission separately, was correct). A naive flip to always prefer diffuse would instead break the zero-base-factor tiles. Preferring a non-black base color with an emissive fallback handles both cases, and the rasterizer now reproduces glTF's additive emissive rather than silently discarding it.
How Has This Been / Can This Be Tested?
tests/rendering/test_offscreen.py::test_emissive_composites_over_base_color_without_double_counting- renders one rasterizer scene with entities added through the public API and separated per entity by segmentation. A rigid GLB (base atlas + emissive atlas) and a deformable (FEM) entity with a base color and an emissive each keep their base color dominant while the emissive lifts the blue channel (covering both the rigid and non-rigid render paths); a black-base emissive surface renders at the same brightness as the equivalent plain-color surface (its emissive, already packed as albedo, is not counted twice). This covers the base-over-emissive selection and its black-base fallback end to end.tests/parsers/test_mesh.py::test_glb_uv_set_and_unlit_albedo_resolution- parses a GLB with three materials and checks the baked UV set (a base atlas owns it; a flat base defers it to the emissive atlas) and that an unlit material resolves its imagery as the albedo rather than the white default.tests/rendering/test_offscreen.py::test_flat_base_with_image_emissive_keeps_uvs- checks the built visual keeps the UVs for a flat base color with an image emissive (a uniform atlas has no cheap rendered observable, so the visual is checked directly).tests/core/test_misc.py::test_packed_rgba_resolves_batched_fallback_per_environment- checks that per-environment (batched) textures resolve the fallback independently for each entry; asserted on the packed RGBA because only the batch renderer consumes them.Both render and parser tests fail on
mainand pass with this change. Programmatic GLB asset generation lives in the module-localconftest.pyfixtures.The batch renderer (gs-madrona) has no emissive field in its material schema, so it renders base-color-only; it still benefits from the base-color fix and no longer turns black.
Checklist:
Submitting Code Changessection of CONTRIBUTING document.