[BUG FIX] Preserve PBR base color in packed RGBA - #3038
Conversation
b6c2b05 to
0ef742e
Compare
0ef742e to
7eb24b3
Compare
duburcqa
changed the title
![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: fc85491dae
ℹ️ 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".
duburcqa
changed the title
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 931ce42991
ℹ️ 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".
1911484 to
f74bd0f
Compare
209eac2 to
beafdf8
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: beafdf8142
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 318a2a3436
ℹ️ 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".
| if base_texture is not None and not base_texture.is_black: | ||
| return base_texture |
There was a problem hiding this comment.
When a GLB uses KHR_materials_unlit, parse_glb_material explicitly moves the base color into emissive_texture and clears color_texture, but Surface.update_texture then installs the default white ColorTexture as the primary texture. This new resolver treats that injected white as an authored nonblack base and returns it, so get_rgba() packs white instead of the unlit/emissive texture. This regresses unlit assets and other emissive-only fallback cases unless the fallback can distinguish an absent base from the defaulted one.
Useful? React with 👍 / 👎.
| if material.emissiveTexture.texCoord is not None and (color_texture is None or color_texture.is_black): | ||
| uvs_used = material.emissiveTexture.texCoord |
There was a problem hiding this comment.
When the base color is only a factor (ColorTexture) and the emissive channel is an image, the base albedo does not consume UVs, but this condition still prevents emissiveTexture.texCoord from becoming uvs_used. A GLB with baseColorFactor plus an emissive map on TEXCOORD_1 will therefore bake TEXCOORD_0 and sample the emissive map with the wrong coordinates. The base UV should win only for base textures that actually require UVs.
Useful? React with 👍 / 👎.
| material = trimesh.visual.material.PBRMaterial( | ||
| baseColorFactor=color_f32_to_u8(texture.color), **emissive_kwargs | ||
| ) | ||
| visual = trimesh.visual.TextureVisuals(material=material) |
There was a problem hiding this comment.
When texture is a ColorTexture but emission is an ImageTexture, emissive_kwargs contains an emissiveTexture, yet this branch creates TextureVisuals without uv=uvs. Mesh._get_trimesh_props then sends no texcoord_0 to pyrender, while materials with HAS_EMISSIVE_TEX sample uv_0 in mesh.frag, so the emissive map is lost or can fail to render for solid-base materials with textured emission. Pass the available UVs whenever the PBR material includes a texture.
Useful? React with 👍 / 👎.
duburcqa
changed the title
8143285 to
7eb24b3
Compare
|
Superseded by #3088, which consolidates and extends this fix (all surfaces, additive emissive across renderers, and glTF UV-set / unlit handling). Thanks @NoahLinckeScout for the original fix - it is preserved as the first change there. |
2 participants
Why
Some GLBs, including HLOD tiles, use a zero base-color factor while putting their imagery in an emissive texture. The previous packed-RGBA path treated the mere presence of a diffuse texture as authoritative, producing black terrain. Conversely, preferring emissive whenever present loses the intended PBR base color of ordinary assets such as vehicles.
What changed
Impact / safety properties
This only affects the packed RGBA representation consumed by renderers; it leaves the original material textures and shading inputs intact.
Validation
Constructed both material shapes under Genesis initialization and verified the selected RGBA pixels.
Fixes #3046.