Skip to content

fix(ir): preserve geometry referencing Unknown/Unresolved colors during baking#65

Open
bkfunk wants to merge 2 commits into
segfault87:mainfrom
bkfunk:fix/color-resolution-baking
Open

fix(ir): preserve geometry referencing Unknown/Unresolved colors during baking#65
bkfunk wants to merge 2 commits into
segfault87:mainfrom
bkfunk:fix/color-resolution-baking

Conversation

@bkfunk
Copy link
Copy Markdown

@bkfunk bkfunk commented May 13, 2026

Summary

PartBufferBundleBuilder::query_mesh in ir/src/part.rs previously matched only ColorReference::Current, ColorReference::Complement, and ColorReference::Color, returning None for the Unknown and Unresolved variants. As a result, any geometry that referenced a color whose definition wasn't (yet) in the catalog was silently dropped during bake_part_from_*, even when a ColorCatalog was provided.

This patch folds Unknown and Unresolved into the same arm as Color, so their geometry is preserved in a dedicated mesh buffer and can be reified later via Part::resolve_colors.

Repro (before the fix)

Baking a part that contains a reference to a color not in the supplied ColorCatalog produces a Part with missing geometry — no warning, no error.

Change

-            (ColorReference::Color(_), _) => {
+            (ColorReference::Color(_) | ColorReference::Unknown(_) | ColorReference::Unresolved(_), _) => {
                 Some(self.colored_meshes.entry(group.clone()).or_default())
             }
-            _ => None,
         }

The wildcard arm is no longer reachable, so it's removed (compiles cleanly because ColorReference only has these four constructors).

Test plan

  • cargo check --workspace passes
  • Manual: bake a model that references an out-of-catalog color and confirm its geometry is present

Copilot AI review requested due to automatic review settings May 13, 2026 16:20
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes IR part baking so geometry that references ColorReference::Unknown(_) or ColorReference::Unresolved(_) is no longer silently dropped during baking. This aligns with the codebase’s existing ability to later reify color references (e.g., via Part::resolve_colors) once a ColorCatalog is available.

Changes:

  • Treat ColorReference::Unknown(_) and ColorReference::Unresolved(_) the same as ColorReference::Color(_) when selecting a destination mesh buffer during baking.
  • Remove the now-unreachable fallback match arm in PartBufferBundleBuilder::query_mesh.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread ir/src/part.rs Outdated
Comment on lines 260 to 270
(ColorReference::Color(_), _) => {
(ColorReference::Color(_) | ColorReference::Unknown(_) | ColorReference::Unresolved(_), _) => {
Some(self.colored_meshes.entry(group.clone()).or_default())
}
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch — addressed in f36398c. query_mesh now returns &mut MeshBuffer directly, the call site is simplified accordingly, and the dead Skipping unknown color group_key branch is gone.

After the previous commit, query_mesh's match is exhaustive over
ColorReference and always returns Some(...). Drop the Option wrapper
and simplify the call site (the "Skipping unknown color group_key"
branch was dead code).

Per review feedback on segfault87#65.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants