Skip to content

fix(info): surface RTSS-wired normal map textures (#510) - #512

Merged
fernandotonon merged 1 commit into
masterfrom
fix/510-info-rtss-textures
May 14, 2026
Merged

fix(info): surface RTSS-wired normal map textures (#510)#512
fernandotonon merged 1 commit into
masterfrom
fix/510-info-rtss-textures

Conversation

@fernandotonon

@fernandotonon fernandotonon commented May 13, 2026

Copy link
Copy Markdown
Owner

Summary

Test plan

  • `qtmesh info Rumba\ Dancing.fbx` → `Textures: Boss_diffuse.png, Boss_normal.png`.
  • JSON output includes both.
  • New unit test `ExtractMeshInfo_SurfacesRTSSNormalMapHint` builds a triangle entity, stashes the UOB without adding a TUS, asserts the texture surfaces.
  • Linux CI runs the test under Xvfb.

🤖 Generated with Claude Code

Summary by CodeRabbit

Release Notes

  • Bug Fixes

    • Mesh extraction now correctly identifies and recovers normal map textures stored in alternate binding locations, complementing the existing texture enumeration process.
  • Tests

    • Added test coverage to validate normal map texture recovery during mesh extraction for various material configurations.

Review Change Stack

@coderabbitai

coderabbitai Bot commented May 13, 2026

Copy link
Copy Markdown

Warning

Rate limit exceeded

@fernandotonon has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 33 minutes and 38 seconds before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, 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 have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 03d3efb2-a22a-4fff-957f-aa07a795eda8

📥 Commits

Reviewing files that changed from the base of the PR and between f0f4dd1 and 0f977af.

📒 Files selected for processing (2)
  • src/CLIPipeline.cpp
  • src/CLIPipeline_test.cpp
📝 Walkthrough

Walkthrough

Extended CLIPipeline::extractMeshInfo() to recover normal-map texture names from RTSS render-state user-object bindings (qtme.normal_map key) alongside regular CONTENT_NAMED texture units. Implementation guards type casting with std::bad_cast. New unit test validates that both diffuse and RTSS-bound normal maps are reported.

Changes

Normal-map recovery from RTSS user-object bindings

Layer / File(s) Summary
Normal-map recovery implementation and test
src/CLIPipeline.cpp, src/CLIPipeline_test.cpp
Refactored extractMeshInfo() texture enumeration to use a collectFromPass lambda that gathers CONTENT_NAMED texture unit names and additionally attempts to recover normal-map texture names from the qtme.normal_map user-object binding key via Ogre::Any casting with std::bad_cast guard. Deduplication is maintained across both paths. New ExtractMeshInfo_SurfacesRTSSNormalMapHint test verifies a material with a diffuse texture and qtme.normal_map UOB binding reports both textures in the extracted info.

🎯 3 (Moderate) | ⏱️ ~20 minutes

🐰 With eager paws I hop through textures twain,
Both diffuse and normal, no longer a pain!
The RTSS bindings now surface and shine,
In the mesh info report—a feature divine!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% 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 'fix(info): surface RTSS-wired normal map textures (#510)' clearly and specifically describes the main change: fixing the extractMeshInfo function to report normal map textures that are wired through RTSS side channels.
Description check ✅ Passed The PR description includes both required sections: Summary (explaining the problem and solution) and Technical Details with test plan, providing sufficient context about the fix.
Linked Issues check ✅ Passed The implementation meets the core requirements from #510: it recovers RTSS-wired normal map textures from the qtme.normal_map UOB hint, deduplicates textures, adds unit test coverage, and enables qtmesh info to report both diffuse and normal textures.
Out of Scope Changes check ✅ Passed All changes are directly within scope: CLIPipeline.cpp refactors extractMeshInfo() to handle RTSS-wired textures via UOB hints, and CLIPipeline_test.cpp adds test coverage for this specific functionality.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/510-info-rtss-textures

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

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

@fernandotonon
fernandotonon force-pushed the fix/510-info-rtss-textures branch from 0348743 to f0f4dd1 Compare May 14, 2026 00:08

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
src/CLIPipeline_test.cpp (1)

348-350: ⚡ Quick win

Strengthen this test to assert exact texture cardinality.

contains() checks are good, but this still passes if duplicates/unexpected textures are present. Add a size assertion to lock dedup behavior from both CONTENT_NAMED and UOB paths.

Suggested diff
     MeshInfo info = CLIPipeline::extractMeshInfo(entity, "with_normal.fbx");

+    EXPECT_EQ(info.textures.size(), 2);
     EXPECT_TRUE(info.textures.contains("Boss_diffuse.png"));
     EXPECT_TRUE(info.textures.contains("Boss_normal.png"))
         << "RTSS-wired normal map should surface via the qtme.normal_map UOB hint";
🤖 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 `@src/CLIPipeline_test.cpp` around lines 348 - 350, Test currently only checks
presence via EXPECT_TRUE(info.textures.contains(...)) which won't catch
duplicates or extra entries; add an exact cardinality assertion (e.g.
EXPECT_EQ(info.textures.size(), 2)) alongside the existing contains checks to
lock down dedup behavior for textures coming from CONTENT_NAMED and the
qtme.normal_map UOB hint so only "Boss_diffuse.png" and "Boss_normal.png" are
present.
🤖 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.

Nitpick comments:
In `@src/CLIPipeline_test.cpp`:
- Around line 348-350: Test currently only checks presence via
EXPECT_TRUE(info.textures.contains(...)) which won't catch duplicates or extra
entries; add an exact cardinality assertion (e.g.
EXPECT_EQ(info.textures.size(), 2)) alongside the existing contains checks to
lock down dedup behavior for textures coming from CONTENT_NAMED and the
qtme.normal_map UOB hint so only "Boss_diffuse.png" and "Boss_normal.png" are
present.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 28b05535-4f55-45ac-a2fb-1882f70e2423

📥 Commits

Reviewing files that changed from the base of the PR and between 33bb7cc and f0f4dd1.

📒 Files selected for processing (2)
  • src/CLIPipeline.cpp
  • src/CLIPipeline_test.cpp

extractMeshInfo only walked CONTENT_NAMED TextureUnitStates, which
covers every PBR slot MaterialProcessor binds as a plain TUS (diffuse,
albedo, metallic, roughness, ao, emissive) but misses the normal map.
The normal map is routed through RTSS's render-state side channel
(see RTShaderHelper::applyNormalMap), so it never appears as a base-
pass TUS, even though the editor renders it.

Recover it from the qtme.normal_map UOB hint the importer leaves on
the pass — the same hint slice #507 added so FBX export could round-
trip the normal map. The walk reads the Any via Ogre::any_cast and
adds the texture name to the dedup set, so qtmesh info / scan engine /
overlay all see it.

Unit test: build a triangle entity, stash the UOB hint without adding
a TUS, assert the texture surfaces.

Verified locally:
  $ qtmesh info Rumba\ Dancing.fbx
    Textures: Boss_diffuse.png, Boss_normal.png

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@fernandotonon
fernandotonon force-pushed the fix/510-info-rtss-textures branch from f0f4dd1 to 0f977af Compare May 14, 2026 00:35
@fernandotonon
fernandotonon merged commit 5ed1b5b into master May 14, 2026
13 checks passed
@fernandotonon
fernandotonon deleted the fix/510-info-rtss-textures branch May 14, 2026 00:38
@sonarqubecloud

Copy link
Copy Markdown

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.

feat(info): surface RTSS-wired normal/PBR textures in qtmesh info output (currently hidden)

1 participant