feat: AI PBR map synthesis from albedo (ONNX) (#404) - #738
Merged
Conversation
First step of AI PBR map synthesis (#404): land ONNX Runtime as an optional dependency behind ENABLE_ONNX (OFF by default; release/test builds will turn it on in a later commit). - cmake/OnnxRuntime.cmake downloads the official prebuilt ONNX Runtime 1.20.1 release archive per-platform (verified SHA256) and exposes it as the imported SHARED target `qtmesh_onnx`, mirroring cmake/Libsodium.cmake's download pattern. macOS uses the universal2 archive so there is no per-arch trap (the libsodium-built-x86_64 lesson); CoreML EP ships inside it, CPU EP is always present. Linux x64/aarch64 selected via CMAKE_SYSTEM_PROCESSOR. Windows is mapped but ENABLE_ONNX stays off on the MinGW path (MSVC-built archive won't link) — the feature degrades gracefully. - .gitignore: broaden the cmake-module negation from the single Libsodium.cmake to cmake/*.cmake so tracked CMake modules aren't swallowed by the *.cmake rule. No consumers yet — configures as a no-op until AIAssistManager lands. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- PbrMapSynth (Ogre-free, unit-testable like NormalMapGenerator): NCHW tensor packing, overlapping-tile inference with feathered seam blend, normal/height tensor decoding (strength + OpenGL/DirectX invertG), and a low-frequency roughness heuristic from albedo luminance. The ONNX inference (synthesize()) is guarded by ENABLE_ONNX; the building blocks compile unconditionally. Input channel count + output names/shapes are discovered from the model at runtime rather than hardcoded, so a permissive UNet whose convention differs from DeepBump still works (normal, height, or both). - AIAssistManager (QML_SINGLETON, SDManager-pattern): wraps PbrMapSynth, resolves the model under AppData/ai_models/pbr, downloads it on first use via ModelDownloader (URL via QSettings ai/pbrModelUrl or QTMESH_PBR_MODEL_URL env; empty default until a permissive host is chosen), caches outputs next to the source albedo, and derives normal-from-height via the existing Sobel NormalMapGenerator when the model emits only height. Synchronous (ONNX is fast) with progress/completed/error signals for the GUI. Sentry breadcrumb ai.assist.pbr_synth. - CMake: compile both into the app + UnitTests; link qtmesh_onnx and copy the ONNX Runtime shared lib next to each binary (POST_BUILD) when ENABLE_ONNX. Verified: builds + links against ONNX Runtime 1.20.1 on macOS arm64. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
GL-free, ONNX-free tests for the synthesis building blocks: NCHW packing (RGB + luminance), nchwToRgb round-trip, flat-normal→blue decode, invertG green flip, height scaling, and the roughness heuristic (dark>bright, flat-luminance level). Run on any build under Xvfb in CI. Verified locally via a standalone harness (the gtest main aborts without GL on macOS). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
CLI surface for PBR map synthesis:
qtmesh material --texture albedo.png --generate-pbr [<mesh>] [-o out] \
[--tile-size N] [--no-normal] [--no-roughness] [--no-height]
Synchronous (no SD-style event loop): runs AIAssistManager::synthesizePbrMaps,
writes normal/roughness/height PNGs next to the albedo, and — when a mesh is
given — imports it, binds normal_map/roughness into the slice-E canonical slots
via RTShaderHelper::wirePbrSlotsForFFP, and re-exports. ENABLE_ONNX-guarded;
exit 1 with a "rebuild with -DENABLE_ONNX" message otherwise.
Also fixes PbrMapSynth::synthesize ordering so a roughness-only request
(--no-normal --no-height) succeeds offline — roughness is a pure heuristic and
must not require the ONNX model. Verified: roughness-only writes the PNG with no
model present; a full request fails cleanly when the model is missing.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
MCP surface for PBR map synthesis: registers + advertises generate_pbr_maps (ENABLE_ONNX-guarded). Takes albedo_path (required) + normal/roughness/height/ tile_size/overwrite; runs AIAssistManager::synthesizePbrMaps, and when a mesh is selected binds normal_map/roughness into the canonical slice-E slots via RTShaderHelper::wirePbrSlotsForFFP. Returns the output map paths + boundSubmeshes. Without ENABLE_ONNX it returns the standard "rebuild with -DENABLE_ONNX" error. Sentry breadcrumb ai.assist.pbr_synth. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Material Editor surface: a button in the Texture Properties panel (next to "Save Texture As…", shown only on an ONNX build via aiPbrAvailable()) that runs MaterialEditorQML::generatePbrFromDiffuse() on the current texture. It resolves the diffuse's on-disk path via the existing group-agnostic preview resolver, calls AIAssistManager::synthesizePbrMaps, and binds normal_map/roughness into the active material's canonical slots (RTShaderHelper::wirePbrSlotsForFFP + compile). pbrSynthStarted/Completed/Error signals drive a small status label. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
) - CLIPipeline_cmdmaterial_coverage_test.cpp: --generate-pbr cases — missing --texture (2/1), out-of-range --tile-size (2/1), roughness-only success offline (writes _roughness.png, ENABLE_ONNX) and the no-model/full-request clean failure (exit 1, no maps). Each branches on ENABLE_ONNX. - deploy.yml: add -DENABLE_ONNX=ON to the Linux coverage/test build and the Linux + macOS release builds (macOS universal2 archive is safe under -DCMAKE_OSX_ARCHITECTURES). Windows MinGW intentionally left off. - CLAUDE.md: document the CLI subcommands + a full architecture entry under the AI-Assisted Authoring epic (#397), including the GPL-model rationale, the empty production-URL TODO, and the Windows-MinGW deferral. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
More reviews will be available in 21 minutes and 10 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 To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (10)
📝 WalkthroughWalkthroughThis PR introduces end-to-end AI PBR map synthesis from albedo textures using ONNX Runtime. It adds a new ChangesONNX PBR Map Synthesis Feature
Sequence Diagram(s)sequenceDiagram
actor User
participant TexturePropertiesPanel
participant MaterialEditorQML
participant AIAssistManager
participant PbrMapSynth
participant OnnxRuntime
User->>TexturePropertiesPanel: click "Generate PBR maps from diffuse"
TexturePropertiesPanel->>MaterialEditorQML: generatePbrFromDiffuse()
MaterialEditorQML->>MaterialEditorQML: resolve diffuse file path
MaterialEditorQML->>AIAssistManager: synthesizePbrMaps(albedoPath, opts)
AIAssistManager->>AIAssistManager: check disk cache
alt cache miss
AIAssistManager->>PbrMapSynth: synthesize(albedo, modelPath, opts)
PbrMapSynth->>OnnxRuntime: session.Run(tiled input)
OnnxRuntime-->>PbrMapSynth: normal/height planes
PbrMapSynth-->>AIAssistManager: Result (normal/roughness/height QImage)
AIAssistManager->>AIAssistManager: write output PNGs
end
AIAssistManager-->>MaterialEditorQML: PbrMapSynthResult
MaterialEditorQML->>MaterialEditorQML: bind textures into Ogre material, wire PBR slots
MaterialEditorQML->>TexturePropertiesPanel: emit pbrSynthCompleted(result)
TexturePropertiesPanel->>User: update status label
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (2)
src/PbrMapSynth_test.cpp (1)
42-52: ⚡ Quick winAdd regression tests for invalid channel-count handling and zero-valued outputs.
Please add cases for unsupported
toNCHW(..., channels)values (e.g., 2/4) and for “valid but all-zero” output presence semantics, so these edge paths stay covered.Also applies to: 137-146
🤖 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/PbrMapSynth_test.cpp` around lines 42 - 52, The test ToNchwGrayscaleLuma currently only covers valid single-channel conversion. Add regression test cases to cover edge cases: test the toNCHW function with unsupported channel counts (such as 2 and 4) to verify proper error handling or expected behavior for invalid inputs, and add test cases that verify the function correctly handles inputs that produce all-zero outputs to ensure the edge case for zero-valued output presence semantics is properly covered. These additional assertions should be included in the existing test function to ensure these error paths remain covered.src/CLIPipeline.cpp (1)
3791-3792: ⚡ Quick winAdd file import/export breadcrumbs for the mesh branch.
The PBR mesh path imports and exports assets but only records the AI breadcrumb, so file I/O is missing from telemetry.
Proposed fix
if (!initOgreHeadless()) return 1; + SentryReporter::addBreadcrumb(QStringLiteral("file.import"), + QStringLiteral("Importing file %1").arg(meshFi.absoluteFilePath())); MeshImporterExporter::importer({meshFi.absoluteFilePath()}); @@ Ogre::SceneNode* node = entity->getParentSceneNode(); + SentryReporter::addBreadcrumb(QStringLiteral("file.export"), + QStringLiteral("Exporting file %1").arg(outFi.absoluteFilePath())); if (MeshImporterExporter::exporter(node, outFi.absoluteFilePath(), formatForExtension(outputPath)) != 0) {As per coding guidelines, all user-facing actions and significant operations must be tracked with
SentryReporter::addBreadcrumb(category, message), usingfile.import/file.exportfor I/O operations.Also applies to: 3834-3836
🤖 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.cpp` around lines 3791 - 3792, The MeshImporterExporter::importer call at line 3792 is missing a telemetry breadcrumb to track the file import operation. Add a SentryReporter::addBreadcrumb call with category "file.import" and a descriptive message (e.g., including the file path) immediately before or after the MeshImporterExporter::importer invocation to ensure this user-facing file I/O operation is properly recorded in telemetry. Apply the same fix to the corresponding export operations mentioned at lines 3834-3836.Source: Coding guidelines
🤖 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 `@CLAUDE.md`:
- Around line 72-73: The documentation for the qtmesh material command examples
needs clarification on mesh behavior and output files. Update the first example
to explicitly include a mesh argument and clarify that output is bound to the
mesh, or remove the "bound to mesh" description if no mesh is provided. Update
the second example to document that in addition to writing the PNGs, a .material
sidecar file is also generated. Ensure both examples clearly document their
respective output behaviors so users understand the difference between the
mesh-bound and mesh-less workflows.
In `@cmake/OnnxRuntime.cmake`:
- Around line 37-40: The elseif(WIN32) block at line 37 doesn't exclude MinGW
systems even though the file documents MinGW as unsupported. Since MinGW sets
WIN32=TRUE, it will enter this branch and attempt to download MSVC-built
artifacts, causing link-time failures. Add a MinGW exclusion guard to the
elseif(WIN32) condition by checking for the absence of MinGW (using CMake's
built-in MinGW detection variable) so MinGW builds properly skip this block and
degrade gracefully as intended.
In `@src/CLIPipeline_cmdmaterial_coverage_test.cpp`:
- Around line 372-381: The test `GeneratePbrNoModelFailsCleanly` expects the
command to fail due to a missing PBR model, but the AIAssistManager can resolve
the cached pbr_unet.onnx model from the user's AppData directory, causing the
assertion on the expected failure to be invalidated in environments where the
model is already cached. Isolate this test from the global model cache by either
setting up a temporary empty model location that the AIAssistManager will use
instead of AppData, or by mocking/overriding the model readiness state before
executing the cmdMaterial call to ensure the model resolution fails as expected
regardless of what is cached in the system.
In `@src/CLIPipeline.cpp`:
- Around line 3788-3817: The generated map files are being referenced by
basename in the bindSlot lambda but the actual files remain next to the albedo
path rather than beside the exported mesh. Before calling bindSlot with the
generated map paths (the parameters passed to bindSlot in the lines following
the resource group manager setup), copy those generated texture files from their
current location to the output directory specified by outputPath to ensure the
exported material package includes all necessary texture files alongside the
mesh.
- Around line 3332-3334: The pbrTileSize assignment uses QString().toInt() which
silently converts invalid non-numeric input to 0, bypassing subsequent
validation checks since 0 is treated as a valid value. Add validation after the
conversion to check if the string conversion was successful using the boolean
parameter form of toInt() method, and reject non-numeric arguments with an error
message rather than allowing them to silently default to 0.
In `@src/MCPServer.cpp`:
- Around line 1701-1706: The tile_size parameter handling in the
PbrMapSynth::Options configuration does not validate bounds, unlike the CLI. Add
bounds validation for the tile_size argument to ensure it is either 0 or falls
within the range of 32 to 4096 before assigning it to opts.tileSize. This
prevents invalid values from being passed to the synthesis function that could
cause excessive allocation or work. Check the CLI implementation for the exact
validation logic to mirror, then apply the same bounds check in the MCP handler
where opts.tileSize is currently set without validation.
In `@src/PbrMapSynth.cpp`:
- Around line 305-309: The accHeight vector is only allocated when
opts.generateHeight is true, but the normal derivation logic at lines 375-377
may require height data to be present even when generateHeight is disabled.
Additionally, the haveHeight and haveNormal flags are inferred from non-zero
data which incorrectly treats valid all-zero maps as missing. Fix this by always
allocating accHeight regardless of opts.generateHeight value, and update the
haveHeight and haveNormal inference logic (around lines 363-378) to track
whether data was actually generated or processed rather than relying solely on
whether the data contains non-zero values.
- Around line 60-74: The `toNCHW` function assumes that any channel count other
than 1 must be 3, and unconditionally writes three planes to the output vector
in the else block. However, `runModelOnce` can pass `channels == 2`, which
causes out-of-bounds writes to the output vector. Fix this by validating that
channels actually equals 3 before writing all three planes in the else branch,
or by adding a separate case to properly handle `channels == 2` with only two
planes written to the vector.
- Around line 236-244: In the code block where out.normal.assign() and
out.height.assign() are called, add validation before copying data to ensure the
output tensor's spatial dimensions match the input dimensions and sufficient
data exists. Specifically, after getting the shape with info.GetShape(), verify
that sh[2] equals the input height (h) and sh[3] equals the input width (w),
then use GetElementCount() to confirm the tensor has enough elements before
proceeding with the assign() calls for both the normal and height cases. This
prevents buffer over-read or under-read errors when the model outputs different
H/W dimensions.
---
Nitpick comments:
In `@src/CLIPipeline.cpp`:
- Around line 3791-3792: The MeshImporterExporter::importer call at line 3792 is
missing a telemetry breadcrumb to track the file import operation. Add a
SentryReporter::addBreadcrumb call with category "file.import" and a descriptive
message (e.g., including the file path) immediately before or after the
MeshImporterExporter::importer invocation to ensure this user-facing file I/O
operation is properly recorded in telemetry. Apply the same fix to the
corresponding export operations mentioned at lines 3834-3836.
In `@src/PbrMapSynth_test.cpp`:
- Around line 42-52: The test ToNchwGrayscaleLuma currently only covers valid
single-channel conversion. Add regression test cases to cover edge cases: test
the toNCHW function with unsupported channel counts (such as 2 and 4) to verify
proper error handling or expected behavior for invalid inputs, and add test
cases that verify the function correctly handles inputs that produce all-zero
outputs to ensure the edge case for zero-valued output presence semantics is
properly covered. These additional assertions should be included in the existing
test function to ensure these error paths remain covered.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: b94c97ae-5bbb-4d37-be6e-a850e3183700
📒 Files selected for processing (19)
.github/workflows/deploy.yml.gitignoreCLAUDE.mdCMakeLists.txtcmake/OnnxRuntime.cmakeqml/TexturePropertiesPanel.qmlsrc/AIAssistManager.cppsrc/AIAssistManager.hsrc/CLIPipeline.cppsrc/CLIPipeline.hsrc/CLIPipeline_cmdmaterial_coverage_test.cppsrc/CMakeLists.txtsrc/MCPServer.cppsrc/MCPServer.hsrc/MaterialEditorQML.cppsrc/MaterialEditorQML.hsrc/PbrMapSynth.cppsrc/PbrMapSynth.hsrc/PbrMapSynth_test.cpp
Comment on lines
+72
to
+73
| qtmesh material --texture albedo.png --generate-pbr -o out.fbx # AI PBR map synthesis (normal/roughness/height) from a diffuse → maps next to it + bound to mesh (needs ONNX build + first-run model download; roughness works offline) | ||
| qtmesh material --texture albedo.png --generate-pbr --no-height --tile-size 512 # selective maps + larger model tiles; omit the mesh to just write the PNGs |
There was a problem hiding this comment.
Clarify the optional mesh behavior in these examples.
The first example says the output is “bound to mesh” even though no <mesh> argument is passed, and the no-mesh path also writes a .material sidecar in addition to the PNGs. Please make one example include a mesh and update the other to reflect the sidecar.
Proposed doc tweak
-qtmesh material --texture albedo.png --generate-pbr -o out.fbx # AI PBR map synthesis (normal/roughness/height) from a diffuse → maps next to it + bound to mesh (needs ONNX build + first-run model download; roughness works offline)
-qtmesh material --texture albedo.png --generate-pbr --no-height --tile-size 512 # selective maps + larger model tiles; omit the mesh to just write the PNGs
+qtmesh material --texture albedo.png --generate-pbr mesh.fbx -o out.fbx # AI PBR map synthesis (normal/roughness/height) from a diffuse → maps next to it + bound to mesh (needs ONNX build + first-run model download; roughness works offline)
+qtmesh material --texture albedo.png --generate-pbr --no-height --tile-size 512 # selective maps + larger model tiles; omit the mesh to just write the PNGs and a .material sidecar📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| qtmesh material --texture albedo.png --generate-pbr -o out.fbx # AI PBR map synthesis (normal/roughness/height) from a diffuse → maps next to it + bound to mesh (needs ONNX build + first-run model download; roughness works offline) | |
| qtmesh material --texture albedo.png --generate-pbr --no-height --tile-size 512 # selective maps + larger model tiles; omit the mesh to just write the PNGs | |
| qtmesh material --texture albedo.png --generate-pbr mesh.fbx -o out.fbx # AI PBR map synthesis (normal/roughness/height) from a diffuse → maps next to it + bound to mesh (needs ONNX build + first-run model download; roughness works offline) | |
| qtmesh material --texture albedo.png --generate-pbr --no-height --tile-size 512 # selective maps + larger model tiles; omit the mesh to just write the PNGs and a .material sidecar |
🤖 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 `@CLAUDE.md` around lines 72 - 73, The documentation for the qtmesh material
command examples needs clarification on mesh behavior and output files. Update
the first example to explicitly include a mesh argument and clarify that output
is bound to the mesh, or remove the "bound to mesh" description if no mesh is
provided. Update the second example to document that in addition to writing the
PNGs, a .material sidecar file is also generated. Ensure both examples clearly
document their respective output behaviors so users understand the difference
between the mesh-bound and mesh-less workflows.
Owner
Author
There was a problem hiding this comment.
Will tidy the CLAUDE.md wording — the first --generate-pbr example omits yet says 'bound to mesh'; the no-mesh path writes the PNGs (and the line should say maps-next-to-albedo, not bound). Minor doc-only; addressing in a follow-up doc pass.
There was a problem hiding this comment.
@fernandotonon, understood — the fix plan sounds right:
- First example: either add
mesh.fbxas an argument, or drop "bound to mesh" and say "maps written next to albedo". - Second example: note that only PNGs are written (no mesh binding) since no mesh argument is provided.
Would you like me to open a GitHub issue to track this doc-only follow-up so it doesn't slip through?
Comment on lines
+236
to
+244
| auto info = ov.GetTensorTypeAndShapeInfo(); | ||
| auto sh = info.GetShape(); | ||
| const int oc = (sh.size() == 4) ? static_cast<int>(sh[1]) : 1; | ||
| const float* d = ov.GetTensorData<float>(); | ||
| if (oc >= 3 && out.normal.empty()) { | ||
| out.normal.assign(d, d + plane * 3); | ||
| } else if (oc == 1 && out.height.empty()) { | ||
| out.height.assign(d, d + plane); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify current output-copy path uses input plane without output-size guard.
rg -n -C4 'GetTensorTypeAndShapeInfo|GetShape|GetTensorData<float>|assign\(d, d \+ plane' src/PbrMapSynth.cppRepository: fernandotonon/QtMeshEditor
Length of output: 1148
🏁 Script executed:
#!/bin/bash
# Get the full function containing lines 236-244
rg -n -B30 'GetTensorTypeAndShapeInfo' src/PbrMapSynth.cpp | head -80Repository: fernandotonon/QtMeshEditor
Length of output: 2809
🏁 Script executed:
#!/bin/bash
# Check for other tensor element count operations in PbrMapSynth.cpp
rg -n 'GetElementCount|GetTensorData|assign|size()' src/PbrMapSynth.cpp | head -40Repository: fernandotonon/QtMeshEditor
Length of output: 2574
🏁 Script executed:
#!/bin/bash
# Search for ONNX API usage patterns and GetElementCount calls
rg -n 'GetElementCount|GetTypeInfo|TensorTypeAndShapeInfo' src/PbrMapSynth.cppRepository: fernandotonon/QtMeshEditor
Length of output: 196
🏁 Script executed:
#!/bin/bash
# Check the full context of output tensor processing
sed -n '235,245p' src/PbrMapSynth.cppRepository: fernandotonon/QtMeshEditor
Length of output: 518
🏁 Script executed:
#!/bin/bash
# Look for other ONNX files or includes to understand API
fd -t f '\.(h|hpp)$' | xargs rg -l 'ort::|Ort::' | head -5Repository: fernandotonon/QtMeshEditor
Length of output: 122
🏁 Script executed:
#!/bin/bash
# Search for ONNX includes and see what methods are available
rg -n '`#include`.*onnx|ort\.h' src/PbrMapSynth.cppRepository: fernandotonon/QtMeshEditor
Length of output: 106
🌐 Web query:
ONNX Runtime C++ API GetTensorTypeAndShapeInfo GetElementCount method
💡 Result:
In the ONNX Runtime C++ API, the GetElementCount method is a member of the Ort::TensorTypeAndShapeInfo class [1][2]. It returns the total number of elements in a tensor [1][3]. Key technical details: - Function Signature: size_t GetElementCount const [1][2]. - Functionality: It acts as a wrapper for the underlying C API function OrtApi::GetTensorShapeElementCount [1][2]. - Return Value: It returns a size_t representing the product of the tensor's dimensions [1][4]. If the tensor has no shape (i.e., HasShape is false), it typically returns 0 [5][6]. - Usage Context: It is commonly used to determine the total number of elements in a tensor, which is helpful for memory allocation or iteration when accessing underlying tensor data [7][8][4]. Example usage: auto type_info = tensor.GetTensorTypeAndShapeInfo; size_t element_count = type_info.GetElementCount; std::cout << "Total elements: " << element_count << std::endl; [8]
Citations:
- 1: https://onnxruntime.ai/docs/api/c/struct_ort_1_1detail_1_1_tensor_type_and_shape_info_impl.html
- 2: https://onnxruntime.ai/docs/api/c/struct_ort_1_1_tensor_type_and_shape_info.html
- 3: https://tomwildenhain-microsoft.github.io/onnxruntime/docs/api/c/struct_ort_1_1_tensor_type_and_shape_info.html
- 4: https://github.com/microsoft/onnxruntime/blob/main/js/react_native/cpp/TensorUtils.cpp
- 5: https://github.com/microsoft/onnxruntime/blob/master/onnxruntime/core/framework/tensor_type_and_shape.cc
- 6: microsoft/onnxruntime@149d208
- 7: https://github.com/jaoming-affable/onnxruntime/blob/4090d0d0de040794f0ca181f780600f3b2b9834b/csharp/test/Microsoft.ML.OnnxRuntime.EndToEndTests.Capi/CXX_Api_Sample.cpp
- 8: https://microsoft-onnxruntime-40.mintlify.app/inference/c-cpp-api
🌐 Web query:
ONNX Runtime tensor shape validation H W dimensions NCHW format
💡 Result:
ONNX Runtime enforces tensor shape and rank validation as a core part of its execution process to ensure model integrity and compatibility [1][2]. Key aspects of ONNX Runtime tensor shape validation for NCHW and other formats include: 1. Mandatory Format Compliance: The ONNX specification natively uses the NCHW (Batch, Channel, Height, Width) data layout for most spatial operators, such as convolution and pooling [3][4]. If your model uses NHWC or another layout, ONNX Runtime generally expects these to be converted to NCHW, typically via Transpose operations, as it does not natively support NHWC in standard spatial operators [3][5][4]. 2. Rank and Shape Validation: - ONNX Runtime validates both the rank (number of dimensions) and the specific dimension sizes of input tensors against the model's metadata [6][7]. - If the input provided at runtime does not match the expected rank or dimensions defined in the ONNX model, an INVALID_ARGUMENT error is typically raised, terminating execution [6][8][7]. - While some versions or configurations may allow for flexible or symbolic dimensions (e.g., dynamic batch sizes), the rank must usually remain consistent with the model's definition [9][10][11]. - Scalar inputs (rank 0) may sometimes bypass strict rank validation in specific versions/scenarios, though this is considered inconsistent behavior [8]. 3. Inference-Time Checks: Validation occurs during the session's execution phase [12][2]. When running a model, ONNX Runtime verifies that the input tensor matches the shape constraints specified in the graph. Similarly, output shape validation is performed to ensure that the computed results conform to expected dimensions [12][2]. 4. Troubleshooting: If you encounter shape validation errors, ensure that: - Your input tensor dimensions are in the correct order (NCHW for spatial inputs) [3][5]. - The rank (number of axes) of your input exactly matches the model's requirements (e.g., a 2D matrix [N, C] vs. a 1D vector [N]) [6][7]. - You are using tools like Netron to inspect the actual input/output shapes and names defined in your ONNX model file to confirm they match your expected data [7]. In scenarios where custom execution providers (EPs) might transform tensors in ways that deviate from the standard model definition, configuration options such as session.disable_input_validation or session.disable_output_validation may be used, though these are discouraged as they bypass safety checks and place the responsibility of tensor sanity on the user [2].
Citations:
- 1: Enforce shape validation. microsoft/onnxruntime#1716
- 2: Strict Input Output Tensor Validation microsoft/onnxruntime#27986
- 3: https://oliviajain.github.io/onnxruntime/docs/performance/tune-performance.html
- 4: Conv and KERAS storage_order does not match - is this right onnx/onnx#4099
- 5: Resize bilinear + NHWC layout requires scales to be specified microsoft/onnxruntime#2687
- 6: https://onnxruntime.ai/docs/api/python/auto_examples/plot_common_errors.html
- 7: unclear error message: INVALID_ARGUMENT : Unexpected input data type. Actual: (N11onnxruntime17PrimitiveDataTypeIlEE) , expected: (N11onnxruntime17PrimitiveDataTypeIiEE) microsoft/onnxruntime#4787
- 8: InferenceSession.run does not validate rank of scalar inputs microsoft/onnxruntime#19434
- 9: Shape inference fails microsoft/onnxruntime#11614
- 10: onnx and onnxruntime disagree on input with no known rank microsoft/onnxruntime#11891
- 11: https://onnx.ai/onnx/repo-docs/IR.html
- 12: Output Tensor Shape Validation b/w ONNX inference and ORT microsoft/onnxruntime#7252
Validate ONNX output tensor shape and element count before copying data.
Lines 241 and 243 copy from the output tensor using plane (derived from input tile dimensions) without verifying the output tensor's spatial dimensions match. If the model outputs a different H/W, this causes buffer over-read or under-read, leading to memory corruption or crashes.
Validate that output shape matches input (sh[2] == h && sh[3] == w) and use GetElementCount() to ensure sufficient data exists before calling assign().
🤖 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/PbrMapSynth.cpp` around lines 236 - 244, In the code block where
out.normal.assign() and out.height.assign() are called, add validation before
copying data to ensure the output tensor's spatial dimensions match the input
dimensions and sufficient data exists. Specifically, after getting the shape
with info.GetShape(), verify that sh[2] equals the input height (h) and sh[3]
equals the input width (w), then use GetElementCount() to confirm the tensor has
enough elements before proceeding with the assign() calls for both the normal
and height cases. This prevents buffer over-read or under-read errors when the
model outputs different H/W dimensions.
…ts (#404) unit-tests-linux failed to link MaterialEditorQML_test: that target (in tests/CMakeLists.txt, distinct from the main UnitTests) keeps its OWN explicit source list, which compiled MaterialEditorQML.cpp / MCPServer.cpp / CLIPipeline.cpp — all of which now reference AIAssistManager/PbrMapSynth under ENABLE_ONNX (on in the CI coverage build) — without the two new .cpp files, giving "undefined reference to AIAssistManager::instance()" etc. Add PbrMapSynth.cpp + AIAssistManager.cpp to the qtmesh_test_common source list and link qtmesh_onnx into TEST_SUPPORT_LIBRARIES when ENABLE_ONNX, mirroring the ENABLE_LOCAL_LLM / ENABLE_SENTRY pattern. Verified locally (coverage config, ENABLE_ONNX=ON): both compile into qtmesh_test_common and the AIAssistManager undefined references are gone. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Adopt PBRify_Remix (CC0-1.0, Kim2091) as the model — three separate per-map SPAN models (Normal/Roughness/Height), the realistic permissive choice after DeepBump (GPL) was rejected. Verified end-to-end: the .pth models export to ONNX and produce correct maps via ONNX Runtime 1.20.1 (normal mean RGB ~(127,131,247) tangent-space blue; roughness/height proper grayscale). - scripts/export-pbrify-onnx.py: one-time offline dev tool (NOT shipped) that downloads the CC0 .pth models and exports each to ONNX (spandrel + torch.onnx.export, opset 18, dynamo=False, dynamic H/W). - PbrMapSynth: split the tiling/inference into runTiledModel() returning the full-res planar RGB result for ONE 3ch SPAN model; add decodeGrayscaleFromRgb (luminance of the RGB output, for roughness/height which emit RGB). synthesize() kept as a thin wrapper for the existing tests. - AIAssistManager: per-map model files/URLs (Map enum), download any missing via ModelDownloader from a configurable base URL (ai/pbrModelBaseUrl / QTMESH_PBR_MODEL_BASE_URL; empty until hosted). Each requested map runs its own model; roughness falls back to the offline luminance heuristic when its model is absent so roughness-only always works. - Tests: decodeGrayscaleFromRgb luminance case. - Docs: record the CC0 model choice, the .pth→ONNX export step, and that hosting the exported files is the only remaining gap. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@scripts/export-pbrify-onnx.py`:
- Around line 32-38: The BASE_URL variable uses raw/main which is mutable and
not pinned to a specific commit, and the download function writes unverified
content directly to disk without integrity checks. Replace the mutable raw/main
reference in BASE_URL with a specific commit hash, then add checksum
verification to the download function that validates the downloaded file against
a known hash before writing it to disk. Apply these same changes to the similar
download code referenced at lines 82-87 to ensure all model artifacts are pinned
and verified.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1bb2a2d6-4fe1-4ec8-b004-4e1e8a0390b5
📒 Files selected for processing (9)
.github/workflows/deploy.ymlCLAUDE.mdscripts/export-pbrify-onnx.pysrc/AIAssistManager.cppsrc/AIAssistManager.hsrc/PbrMapSynth.cppsrc/PbrMapSynth.hsrc/PbrMapSynth_test.cpptests/CMakeLists.txt
✅ Files skipped from review due to trivial changes (1)
- CLAUDE.md
🚧 Files skipped from review as they are similar to previous changes (4)
- src/PbrMapSynth.h
- src/AIAssistManager.h
- src/PbrMapSynth_test.cpp
- .github/workflows/deploy.yml
Comment on lines
+32
to
+38
| BASE_URL = "https://github.com/Kim2091/PBRify_Remix/raw/main/Models/{name}.pth" | ||
|
|
||
|
|
||
| def download(name: str, dest: str) -> None: | ||
| url = BASE_URL.format(name=name) | ||
| print(f" downloading {url}") | ||
| urllib.request.urlretrieve(url, dest) |
There was a problem hiding this comment.
Pin and verify model artifacts before export.
Line 32 pulls from raw/main (mutable), and Line 38 writes unverified content to disk. This makes exports non-reproducible and opens a supply-chain tampering path for generated ONNX assets.
Suggested hardening patch
@@
import argparse
+import hashlib
import os
import sys
import urllib.request
@@
-BASE_URL = "https://github.com/Kim2091/PBRify_Remix/raw/main/Models/{name}.pth"
+# Pin to an immutable commit/tag rather than `main`.
+BASE_URL = "https://github.com/Kim2091/PBRify_Remix/raw/<PINNED_REF>/Models/{name}.pth"
+
+# Fill with known-good hashes for reproducible exports.
+MODEL_SHA256 = {
+ "1x-PBRify_NormalV3": "<sha256>",
+ "1x-PBRify_RoughnessV2": "<sha256>",
+ "1x-PBRify_Height": "<sha256>",
+}
@@
def download(name: str, dest: str) -> None:
@@
urllib.request.urlretrieve(url, dest)
+ expected = MODEL_SHA256[name]
+ h = hashlib.sha256()
+ with open(dest, "rb") as f:
+ for chunk in iter(lambda: f.read(1024 * 1024), b""):
+ h.update(chunk)
+ actual = h.hexdigest()
+ if actual != expected:
+ raise RuntimeError(f"SHA256 mismatch for {name}: {actual} != {expected}")Also applies to: 82-87
🧰 Tools
🪛 Ruff (0.15.17)
[error] 38-38: Audit URL open for permitted schemes. Allowing use of file: or custom schemes is often unexpected.
(S310)
🤖 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 `@scripts/export-pbrify-onnx.py` around lines 32 - 38, The BASE_URL variable
uses raw/main which is mutable and not pinned to a specific commit, and the
download function writes unverified content directly to disk without integrity
checks. Replace the mutable raw/main reference in BASE_URL with a specific
commit hash, then add checksum verification to the download function that
validates the downloaded file against a known hash before writing it to disk.
Apply these same changes to the similar download code referenced at lines 82-87
to ensure all model artifacts are pinned and verified.
Source: Linters/SAST tools
Owner
Author
There was a problem hiding this comment.
Fixed in 0998529 — the script now pins to a specific PBRify_Remix commit (190db53…) instead of mutable main, making exports reproducible. (Dev-only tool, not shipped; export still verifies the ONNX loads+runs in ORT before writing.)
There was a problem hiding this comment.
@fernandotonon — Pinning to the immutable commit 190db53 is a solid fix for the mutable-ref concern; that's the higher-severity of the two issues raised since a raw GitHub URL at a specific commit is effectively content-addressed.
One remaining gap: the SHA256 verification of the downloaded .pth files themselves is still absent. The ORT functional check (load + run) confirms the model is structurally valid ONNX, but doesn't guard against a corrupted or tampered download that happens to be a valid ONNX file. Given this is a dev-only offline tool, the practical risk is low — but if you'd like belt-and-suspenders hygiene, adding a known-SHA256 table for the pinned .pth artifacts (similar to what cmake/OnnxRuntime.cmake does for the ONNX Runtime archives) would close the loop completely.
…-404 # Conflicts: # .github/workflows/deploy.yml
Makes PBR synthesis work out-of-the-box: - kDefaultModelBaseUrl now points at the CC0 ONNX models hosted on Hugging Face (fernandotonon/QtMeshEditor-models — an ONNX re-export of Kim2091/PBRify_Remix, CC0-1.0, credited in the model card). Overridable via ai/pbrModelBaseUrl / QTMESH_PBR_MODEL_BASE_URL. - AIAssistManager::ensureModelBlocking(Map): downloads a missing per-map model via ModelDownloader and blocks on a local QEventLoop until it lands, so the synchronous synthesize path triggers a first-run download from CLI/MCP/GUI alike (previously nothing called ensureModel, so the model never downloaded). synthesizePbrMaps ensures each requested map's model before running; roughness still falls back to the offline heuristic if its download fails. - CLAUDE.md: record the UltraSharpV2 license due-diligence — repo LICENSE is CC0 and README says "exclusively CC0 ambientCG"; OpenModelDB's "ambientCG + UltraSharpV2" note conflicts (UltraSharpV2 is cc-by-nc-sa), and we ship on the author's authoritative CC0 claim. Verified end-to-end: from an empty model dir, `qtmesh material --texture x.png --generate-pbr` downloads all 3 models from HF and produces normal/roughness/ height. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@src/AIAssistManager.cpp`:
- Around line 116-148: The ensureModelBlocking function uses a QEventLoop
without a timeout mechanism, which can cause indefinite blocking if the network
request hangs. Add a QTimer with a reasonable timeout duration (e.g., 60
seconds) that will quit the event loop if the download does not complete within
that time. Create the QTimer instance, connect its timeout signal to call
loop.quit(), configure it as single-shot, and start it before calling
loop.exec(). Update the ok variable initialization or add additional tracking to
distinguish between successful completion, error, and timeout cases, then return
false if a timeout occurs. Ensure QTimer include is present at the top of the
file.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 15988af2-12ba-4957-8f07-09739bd433d7
📒 Files selected for processing (6)
.github/workflows/deploy.ymlCLAUDE.mdCMakeLists.txtsrc/AIAssistManager.cppsrc/AIAssistManager.hsrc/CMakeLists.txt
✅ Files skipped from review due to trivial changes (1)
- CLAUDE.md
🚧 Files skipped from review as they are similar to previous changes (4)
- CMakeLists.txt
- .github/workflows/deploy.yml
- src/CMakeLists.txt
- src/AIAssistManager.h
) unit-tests-linux failed: after wiring first-run auto-download into the synchronous synthesize path, CLIPipelineCmdMaterialCoverageTest's --generate-pbr cases blocked on a Hugging Face download in CI (no/limited network), hung on the QEventLoop until the per-suite wall-clock cap SIGKILLed it (signal 9), and that cascade also killed the next suite (SelectionBoxObjectTest). - AIAssistManager::ensureModelBlocking() now honours a QTMESH_PBR_NO_DOWNLOAD env guard: when set it never touches the network and returns false (the map then reports the graceful "model not available" error, or roughness falls back to the offline heuristic). Doubles as an offline-mode escape hatch. - The PBR coverage-test fixture sets QTMESH_PBR_NO_DOWNLOAD in SetUp (unset in TearDown), so the tests exercise the no-model contract without hanging. Verified locally: with the guard + empty model dir, a normal request fails fast (~0.85s, exit 1) instead of hanging, and roughness-only still succeeds via the heuristic. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ding (#404) Adds an expandable "PBR Texture Slots" section to the Material Editor's Texture Properties panel: for PBR-named materials it lists every texture unit of the current pass (albedo/normal_map/roughness/…), each with a 48px preview, the bound texture name, a dropdown to pick a loaded texture, and a "Load file…" button — so the per-slot state (and Generate-PBR's output) is actually visible. New per-unit C++ accessors (address a unit by index without mutating the global "current" selection): isPbrMaterial(), textureNameForUnit, texturePreviewPathForUnit, setTextureForUnit (binds + RTSS rewire), loadTextureFileForUnit, plus a textureUnitsChanged() signal. getTextureUnitAt() resolves the live TUS from the selected pass (not the cacheable m_texUnitMap). Fixes two issues found while testing Generate PBR: - The slot list didn't update after generation: generatePbrFromDiffuse bound the new normal_map/roughness units but never rebuilt m_textureUnitList or emitted the change signals, so the grid (and isPbrMaterial) never saw them. Now it runs updateTechniqueList()/updateTextureUnitList() and emits both signals. - No visible change on the model: it only called wirePbrSlotsForFFP (which just marks the normal unit non-FFP); RTSS never sampled the normal map. Now it also calls RTShaderHelper::applyNormalMap so the SRS_NORMALMAP sub-render-state is wired and the map actually perturbs viewport shading. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@qml/TexturePropertiesPanel.qml`:
- Line 337: The visibility binding in the PBR section is not reactive to texture
unit changes because isPbrMaterial() is a Q_INVOKABLE method without a NOTIFY
signal, so the binding doesn't re-evaluate when the units property changes in
the Connections block. Replace the visibility binding from
MaterialEditorQML.isPbrMaterial() to a binding that directly depends on the
units property instead, so that the visibility automatically updates whenever
units are added or removed at runtime.
In `@src/MaterialEditorQML.cpp`:
- Around line 2024-2048: The loadTextureFileForUnit() method is a user-visible
file import operation that lacks instrumentation with breadcrumbs for production
tracing. Add SentryReporter::addBreadcrumb() calls at the start of the function
with category 'file.import' and a descriptive message indicating the texture
loading operation, and optionally add additional breadcrumbs at key checkpoints
(such as when the texture file is validated or when it's set for the unit) to
help trace the import flow in production. Follow the same instrumentation
pattern for the other mentioned method generatePbrFromDiffuse() which is also a
user-facing operation.
- Around line 1996-2008: The texturePreviewPathForUnit method caches preview
paths using only the texture name as the key, while getTexturePreviewPath uses a
group-aware key format of group + '\n' + name. This causes stale thumbnail
results when different material groups reuse the same texture basename. Modify
the cache operations in texturePreviewPathForUnit (the
m_previewPathCache.constFind and m_previewPathCache.insert calls) to construct
and use a group-aware cache key in the same format used by
getTexturePreviewPath. You will need to obtain the group context for the current
material/unit and prepend it to the texture name when accessing or storing
values in the cache.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: a7acbba6-67a4-47a8-9246-bcbfacb11c3e
📒 Files selected for processing (13)
CLAUDE.mdCMakeLists.txtqml/TexturePropertiesPanel.qmlsrc/AIAssistManager.cppsrc/CLIPipeline.cppsrc/CLIPipeline.hsrc/CLIPipeline_cmdmaterial_coverage_test.cppsrc/CMakeLists.txtsrc/MCPServer.cppsrc/MCPServer.hsrc/MaterialEditorQML.cppsrc/MaterialEditorQML.htests/CMakeLists.txt
🚧 Files skipped from review as they are similar to previous changes (10)
- tests/CMakeLists.txt
- CMakeLists.txt
- src/CLIPipeline.h
- src/CLIPipeline_cmdmaterial_coverage_test.cpp
- src/MCPServer.h
- src/CMakeLists.txt
- CLAUDE.md
- src/MCPServer.cpp
- src/AIAssistManager.cpp
- src/CLIPipeline.cpp
CodeRabbit findings on the PBR-synth + multi-slot UI work: Critical: - PbrMapSynth::toNCHW: clamp channel count to 1 or 3 so a stray channels==2 can't allocate 2 planes while writing 3 (OOB). - runModelOnce: validate the ONNX output tensor's element count before copying plane*3 / plane (guards a non-1x / mismatched-shape model output). Major: - TexturePropertiesPanel: PBR-slots visibility binds to the reactive `units` property instead of the non-NOTIFY Q_INVOKABLE isPbrMaterial(), so it updates when slots change at runtime. - CLI cmdMaterialGeneratePbr: copy generated maps beside the -o output mesh before binding by basename, so the exported material's refs resolve. - MCP generate_pbr_maps: mirror the CLI tile_size bounds (0 or 32..4096). - cmake/OnnxRuntime.cmake: guard WIN32 with NOT MINGW. - Breadcrumbs: generatePbrFromDiffuse (ui.action) + loadTextureFileForUnit (file.import). Minor: - texturePreviewPathForUnit: same group-aware cache key as getTexturePreviewPath. - ensureModelBlocking: 120s timeout on the download loop (cancel + graceful fail). - CLI --tile-size: reject non-numeric input. - export-pbrify-onnx.py: pin the source to a fixed PBRify_Remix commit. .gitignore: ignore local src/python_scripts scratch. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Address CodeRabbit nit: the example now shows the mesh arg as optional and states maps are written next to the albedo, with binding+re-export only when a <mesh> is given. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Owner
Author
|
Addressed the review feedback in 0998529 (+ doc tweak f820169): Critical
Major
Minor
The height-only normal-derive finding is moot — that path was removed in the per-map |
|
fernandotonon
added a commit
that referenced
this pull request
Jun 21, 2026
Sync pinned doc refs via scripts/sync-doc-versions-from-cmake.sh. Covers the features merged since 3.7.0: - AI PBR map synthesis from albedo (ONNX) + multi-slot PBR texture UI (#404, #738) - QtMesh Cloud scan-report upload (#748) - In-app isometric sprite export UI (#724, #742) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
5 tasks
4 tasks
fernandotonon
added a commit
that referenced
this pull request
Jun 23, 2026
unit-tests-linux failed to link: AutoRig::* symbols undefined in libqtmesh_test_common.a (MCPServer::toolAutoRig and CLIPipeline::cmdRig reference them). The test target has its own TEST_SRC_FILES list separate from the app's src/CMakeLists.txt — add AutoRig.cpp + AutoRigController.cpp there, next to SkinWeights (same omission class as #738's PbrMapSynth gap). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
fernandotonon
added a commit
that referenced
this pull request
Jun 24, 2026
unit-tests-linux failed to link: AutoRig::* symbols undefined in libqtmesh_test_common.a (MCPServer::toolAutoRig and CLIPipeline::cmdRig reference them). The test target has its own TEST_SRC_FILES list separate from the app's src/CMakeLists.txt — add AutoRig.cpp + AutoRigController.cpp there, next to SkinWeights (same omission class as #738's PbrMapSynth gap). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
fernandotonon
added a commit
that referenced
this pull request
Jun 24, 2026
…embedding) (#754) * feat(#407): native auto-rig core + CLI rig subcommand Pinocchio (Baran & Popović 2007) is LGPL-2.1, which conflicts with the project's statically-linked permissive-distribution stance — so, like #401 (Instant Meshes) and #402 (libigl/TetGen), this is a native from-scratch implementation of the published *algorithm* (skeleton-template embedding), zero new deps. - AutoRig (src/AutoRig.h/.cpp): Ogre-free pure-data core — built-in templates (humanoid 19-bone / biped / quadruped / generic), fitTemplate() maps a template's normalised joint graph into the mesh AABB then recentres flagged joints toward per-height-slab centroids (spine→medial line, limb roots inside the silhouette). rigEntity() builds an Ogre::Skeleton (parent-relative bone positions, setBindingPose), binds via mesh->_notifySkeleton + entity ->_initialise(true) — the _initialise is REQUIRED or the exporters (both gate on entity->hasSkeleton()) silently drop the new rig. - AutoRigController (QML singleton, mirrors SkinWeightsController) for the GUI. - CLI: `qtmesh rig <file> [--skeleton T] [--skin] [--up-axis x|y|z] -o out` (cmdRig) — import, rig, optionally chain SkinWeights::computeAndApply, export. Registered in run() dispatch + AppLaunchHandler subcommand list. - AutoRig_test.cpp: pure-data unit tests (template well-formedness, AABB containment, vertical ordering, degenerate-input robustness, string/JSON). - Sentry breadcrumb ai.assist.auto_rig. Verified end-to-end: static OBJ -> 19-bone humanoid + skin -> glTF export with 1 skin / 17 joints; FBX export carries the skeleton too. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(#407): MCP auto_rig tool + GUI Auto-Rig + rig CLI tests + docs - MCP: `auto_rig` { template, skin?, up_axis?, output_path? } (MCPServer::toolAutoRig) — rigs the selected static mesh, optional skin chain + optional re-export. Registered + advertised. Breadcrumb ai.assist.auto_rig. - GUI: AutoRigDialog.qml (template + up-axis pickers, "also skin" checkbox) driven by AutoRigController; new "Rigging" CollapsibleSection in Animation Mode → Mode Tools, gated on AutoRigController.hasRiggableSelection (a static mesh — already-rigged meshes show "Skinning" instead). Lazy-loaded Loader + openAutoRigDialog(), registered in qml_resources.qrc. - Tests: CLIPipeline_cmdrig_coverage_test.cpp (arg-validation + file-missing branches need no GL; success path skips gracefully without Xvfb). - CLAUDE.md: CLI examples (skin + rig), recognized-subcommand list, and a full AutoRig architecture entry (incl. the LGPL→native rationale, the _initialise(true) export gotcha, and documented quality limits). App + UnitTests build clean on macOS arm64. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(#407): compile AutoRig.cpp into the test common lib unit-tests-linux failed to link: AutoRig::* symbols undefined in libqtmesh_test_common.a (MCPServer::toolAutoRig and CLIPipeline::cmdRig reference them). The test target has its own TEST_SRC_FILES list separate from the app's src/CMakeLists.txt — add AutoRig.cpp + AutoRigController.cpp there, next to SkinWeights (same omission class as #738's PbrMapSynth gap). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * ci: re-trigger CI for #407 (missed synchronize webhook) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(gui): blank QML panels + white models in installed builds (#755) * fix(gui): blank QML panels + white models in installed builds Two packaging/runtime bugs that only manifested in installed builds (Homebrew .app, .deb) — dev SDK runs masked both: 1. Blank white QML panels (Inspector / Context / Material docks). The Qt Quick *software* scene-graph backend was forced in the MainWindow ctor — AFTER QApplication, by which point Qt has already locked the default RHI. Moved QSG_RHI_BACKEND / setGraphicsApi(Software) to the top of main(), before QApplication (the only point where it takes effect). Replaced the now-dead call in mainwindow.cpp with a note. 2. White / untextured models in the macOS .app. Relative resource locations from resources.cfg were resolved against macBundlePath() (the .app bundle ROOT), but the media tree lives under Contents/MacOS/media (== applicationDirPath()). So <App>.app/media/... didn't exist, Ogre loaded no RTSS GLSL programs or textures, and every mesh rendered flat white. Resolve relative paths against applicationDirPath() first (matches Linux), with the bundle root kept as a fallback; only add a location if it exists. Verified: a macdeployqt'd bundle now loads media from Contents/MacOS/media and renders the mage.glb fully textured. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * chore: bump version to 3.9.1 (installed-build QML + macOS white-model fix) Bugfix release: QML software-backend set before QApplication, and macOS .app resource paths resolved against applicationDirPath() instead of the bundle root. Synced README + qtmesh action ref (verify-doc-versions gate). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(#407): address review — QML registration, upAxis, error paths Code review (Codex + CodeRabbit) on the auto-rig PR: - CRITICAL: register AutoRigController as a QML singleton in mainwindow.cpp (PropertiesPanel URI) like the sibling controllers + add its kill(). With qt_add_qml_module disabled, QML_SINGLETON alone doesn't expose it, so the Rigging section/dialog would ReferenceError. Verified no error at runtime now. - CRITICAL: the dialog's Up-axis picker was ignored — autoRigSelected() didn't take upAxis. Added a `const QString& upAxis` param (controller maps x/y/z → Options::upAxis) and pass dialog.upAxes[dialog.upAxisIndex] from QML. - AutoRig::appendPositions: guard a null vbuf->lock() (shrink `out` back, return false) instead of dereferencing. - AutoRig::rigEntity: on _initialise failure, detach the half-built skeleton (mesh->_notifySkeleton(null)) before removing it, so hasSkeleton() resets and a retry / exporter doesn't pick up a partial rig. - MCP toolAutoRig: validate output_path type; a requested skin that fails is now a hard error (no unskinned export reported as success); export wrapped in the try/catch (also catches std::exception); Sentry breadcrumb no longer logs the full output path. - PropertiesPanel openAutoRigDialog(): handle Loader.Error to allow retry. App + UnitTests build clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * ci: key macOS caches on the resolved Xcode version (fix libz.tbd mismatch) build-macos kept failing with "No rule to make target '.../Xcode_26.5.../libz.tbd'" even after the Pin-Xcode step: the producer (build-n-cache-ogre-macos) resolved "newest" to Xcode 26.3 on its runner image and cached OGRE with 26.3's absolute libz.tbd path, while the consumer (build-macos) resolved 26.5 and linked against the missing path. "newest" (sort -V | tail -1) is NOT deterministic across the per-job runner images. Fold the resolved Xcode app name into XCODE_TAG (exported by the Pin step) and append it to all macOS assimp/ogre cache keys + restore-keys. A consumer on a different Xcode now cache-misses and rebuilds OGRE/Assimp against its own SDK instead of linking a stale path. (Belongs on master too — same deploy.yml.) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * ci: bust stale macOS OGRE cache (xcode263) — fixes libz.tbd link error build-macos (incl. the 3.9.1 release deploy) failed: No rule to make target '.../Xcode_26.5/.../libz.tbd', needed by QtMeshEditor Diagnosis: the Pin-Xcode step reliably selects Xcode 26.3 on ALL macOS jobs (verified across runs), so compilation is consistent — but the restored OGRE cache was built earlier under Xcode 26.5 and its CMake export hardcodes 26.5's libz.tbd path. Restoring that into a 26.3 build breaks the link. Fix: bump MACOS_CACHE_VERSION xcode26b → xcode263 so OGRE/Assimp are rebuilt under the currently-pinned Xcode (26.3) and the stale 26.5 cache is discarded. Also reverted the earlier XCODE_TAG-in-cache-key experiment: build-macos only RESTORES the OGRE cache (no rebuild step), so a per-job Xcode-keyed miss would leave it with no OGRE at all ("Could not find OGRE"). With Xcode pinned consistently, a plain version bump is the correct, sufficient fix. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * ci: self-heal macOS OGRE cache across differing per-job Xcode images The real cause of the build-macos libz.tbd failure: the producer (build-n-cache-ogre-macos) and consumer (build-macos) run on DIFFERENT runner images whose "newest Xcode" differs — producer resolved Xcode 26.5 and cached OGRE with 26.5's absolute libz.tbd path baked into its CMake export; consumer resolved 26.3 and linked against the missing 26.5 path. Just pinning "newest" or bumping the cache version doesn't help because the two images disagree. Fix (self-healing): - Fold the resolved Xcode app name into XCODE_TAG and append it to all macOS assimp/ogre cache keys + restore-keys, so a job only restores a cache built under its OWN Xcode. - Give build-macos (consumer) the same "check out + build OGRE on cache miss" steps the producer has. When the consumer's Xcode differs from the producer's (cache miss), it rebuilds OGRE under its own SDK instead of failing on a stale libz.tbd path. This makes the macOS build robust regardless of which Xcode each runner image ships. (Bigger than the earlier one-line bump, but that couldn't fix a cross-image Xcode disagreement.) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * ci: keep assimp macOS cache Xcode-agnostic (only ogre is Xcode-keyed) Previous commit Xcode-keyed BOTH the assimp and ogre macOS caches. That broke build-macos on a runner whose Xcode differed from the producer's: assimp cache-missed (no assimp-build-on-miss exists) so find_package(assimp) failed with "Could not find a package configuration file provided by assimp". Assimp is a plain static lib that doesn't bake absolute SDK paths, so one assimp cache is valid across Xcode versions — revert XCODE_TAG on the 3 assimp keys, keeping it ONLY on the 2 ogre keys (ogre's CMake export DOES bake an absolute libz.tbd path, which is why ogre needs per-Xcode keying + the consumer's rebuild-on-miss). The shared assimp cache is then always present for the ogre rebuild to link against. Verified on the failing run: Qt + OGRE now resolve and link (no libz.tbd error); this removes the remaining assimp-not-found failure. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test: fix flaky MainWindowTest.ModeBarLoadsAndModeChange (show window first) This test failed intermittently on CI (Xvfb) with: Value of: window->m_modeBarShell->isHidden() Actual: true Expected: false The fixture constructs MainWindow but never show()s it. QToolBar::isHidden() reflects effective visibility, which is only realized once the parent window is mapped — so under Xvfb the shell reports hidden and the assertion is racy. It hit BOTH this branch and the unrelated CI-only PR #756 (which has no source changes), confirming it's a pre-existing flake, not a regression. Fix: show() the window and processEvents() before the visibility assertion. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * ci: pin SDKROOT so CMake ZLIB resolves under the selected Xcode (macOS) build-macos still failed with the Xcode_26.5 libz.tbd path even after pinning DEVELOPER_DIR=Xcode_26.3 and rebuilding OGRE: CMake's find_package(ZLIB) resolved to the SDK that `xcrun` defaults to (26.5 on these images) rather than the xcode-select'd one, so the OGRE SDK's CMake export baked a 26.5 libz.tbd path that the cache then carried forward. Fix: export SDKROOT (from `xcrun --sdk macosx --show-sdk-path` under the pinned Xcode) in the Pin step, so clang AND CMake resolve system libs under the SAME pinned SDK on every macOS job. Bump MACOS_CACHE_VERSION → sdkpin1 to discard the OGRE caches that still carry the 26.5 path. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * ci: bust stale macOS OGRE cache (xcode263) — unblock 3.9.1 macOS deploy (#756) * ci: bust stale macOS OGRE cache (xcode263) — unblock the macOS deploy The 3.9.1 release deploy failed on build-macos: No rule to make target '.../Xcode_26.5/.../libz.tbd', needed by QtMeshEditor The Pin-Xcode step selects Xcode 26.3 consistently on all macOS jobs, but the OGRE cache under key 'xcode26b' was built earlier under Xcode 26.5 and its CMake export hardcodes 26.5's libz.tbd path. Restoring it into a 26.3 build breaks the link. Bump MACOS_CACHE_VERSION xcode26b → xcode263 so OGRE/Assimp rebuild under the pinned 26.3 and the stale cache is discarded. (Windows + Linux .deb artifacts already published for 3.9.1; this lets the macOS artifact + Homebrew cask update complete on a deploy re-run.) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * ci: self-heal macOS OGRE cache across differing per-job Xcode images The real cause of the build-macos libz.tbd failure: the producer (build-n-cache-ogre-macos) and consumer (build-macos) run on DIFFERENT runner images whose "newest Xcode" differs — producer resolved Xcode 26.5 and cached OGRE with 26.5's absolute libz.tbd path baked into its CMake export; consumer resolved 26.3 and linked against the missing 26.5 path. Just pinning "newest" or bumping the cache version doesn't help because the two images disagree. Fix (self-healing): - Fold the resolved Xcode app name into XCODE_TAG and append it to all macOS assimp/ogre cache keys + restore-keys, so a job only restores a cache built under its OWN Xcode. - Give build-macos (consumer) the same "check out + build OGRE on cache miss" steps the producer has. When the consumer's Xcode differs from the producer's (cache miss), it rebuilds OGRE under its own SDK instead of failing on a stale libz.tbd path. This makes the macOS build robust regardless of which Xcode each runner image ships. (Bigger than the earlier one-line bump, but that couldn't fix a cross-image Xcode disagreement.) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * ci: keep assimp macOS cache Xcode-agnostic (only ogre is Xcode-keyed) Previous commit Xcode-keyed BOTH the assimp and ogre macOS caches. That broke build-macos on a runner whose Xcode differed from the producer's: assimp cache-missed (no assimp-build-on-miss exists) so find_package(assimp) failed with "Could not find a package configuration file provided by assimp". Assimp is a plain static lib that doesn't bake absolute SDK paths, so one assimp cache is valid across Xcode versions — revert XCODE_TAG on the 3 assimp keys, keeping it ONLY on the 2 ogre keys (ogre's CMake export DOES bake an absolute libz.tbd path, which is why ogre needs per-Xcode keying + the consumer's rebuild-on-miss). The shared assimp cache is then always present for the ogre rebuild to link against. Verified on the failing run: Qt + OGRE now resolve and link (no libz.tbd error); this removes the remaining assimp-not-found failure. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test: fix flaky MainWindowTest.ModeBarLoadsAndModeChange (show window first) This test failed intermittently on CI (Xvfb) with: Value of: window->m_modeBarShell->isHidden() Actual: true Expected: false The fixture constructs MainWindow but never show()s it. QToolBar::isHidden() reflects effective visibility, which is only realized once the parent window is mapped — so under Xvfb the shell reports hidden and the assertion is racy. It hit BOTH this branch and the unrelated CI-only PR #756 (which has no source changes), confirming it's a pre-existing flake, not a regression. Fix: show() the window and processEvents() before the visibility assertion. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * ci: pin SDKROOT so CMake ZLIB resolves under the selected Xcode (macOS) build-macos still failed with the Xcode_26.5 libz.tbd path even after pinning DEVELOPER_DIR=Xcode_26.3 and rebuilding OGRE: CMake's find_package(ZLIB) resolved to the SDK that `xcrun` defaults to (26.5 on these images) rather than the xcode-select'd one, so the OGRE SDK's CMake export baked a 26.5 libz.tbd path that the cache then carried forward. Fix: export SDKROOT (from `xcrun --sdk macosx --show-sdk-path` under the pinned Xcode) in the Pin step, so clang AND CMake resolve system libs under the SAME pinned SDK on every macOS job. Bump MACOS_CACHE_VERSION → sdkpin1 to discard the OGRE caches that still carry the 26.5 path. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * ci: pin EXACT Xcode 26.3 on all macOS jobs (stop per-image newest drift) (#758) Even with SDKROOT pinned, build-macos failed because the macOS jobs ran on runner images with different *newest* Xcodes: the ogre/assimp producers sometimes resolved Xcode 26.5 (newest on their image) and cached an SDK whose Codec_Assimp/ogre CMake export hardcodes 26.5's libz.tbd path, while the consumer (26.3) couldn't link it. "sort -V | tail -1" is non-deterministic across images. Pin a SPECIFIC Xcode (26.3) that's present on all current macos-latest images, falling back to newest only if absent — so producers and consumer always agree on the SDK. Bump MACOS_CACHE_VERSION → xc263pin to discard the assimp+ogre caches that still carry a 26.5 path. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(import): restore textured viewport and paint preview on File→Open (#757) * fix(import): restore textured viewport and paint preview on File→Open Rebind RTSS materials for newly imported entities (same path cloud downloads already used) and load texture-paint buffers from embedded/disk sources before GPU readback, which fails for many imported FBX textures on Linux. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(import): scope texture rebind per entity source file Use each mesh's qtme.source_path binding for sidecar texture lookup so multi-file File→Open imports cannot cross-bind common texture basenames. Co-authored-by: Cursor <cursoragent@cursor.com> * chore: bump version to 3.9.2 Sync README and website pinned refs via sync-doc-versions-from-cmake.sh. Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Cursor <cursoragent@cursor.com> * feat(#407): native auto-rig core + CLI rig subcommand Pinocchio (Baran & Popović 2007) is LGPL-2.1, which conflicts with the project's statically-linked permissive-distribution stance — so, like #401 (Instant Meshes) and #402 (libigl/TetGen), this is a native from-scratch implementation of the published *algorithm* (skeleton-template embedding), zero new deps. - AutoRig (src/AutoRig.h/.cpp): Ogre-free pure-data core — built-in templates (humanoid 19-bone / biped / quadruped / generic), fitTemplate() maps a template's normalised joint graph into the mesh AABB then recentres flagged joints toward per-height-slab centroids (spine→medial line, limb roots inside the silhouette). rigEntity() builds an Ogre::Skeleton (parent-relative bone positions, setBindingPose), binds via mesh->_notifySkeleton + entity ->_initialise(true) — the _initialise is REQUIRED or the exporters (both gate on entity->hasSkeleton()) silently drop the new rig. - AutoRigController (QML singleton, mirrors SkinWeightsController) for the GUI. - CLI: `qtmesh rig <file> [--skeleton T] [--skin] [--up-axis x|y|z] -o out` (cmdRig) — import, rig, optionally chain SkinWeights::computeAndApply, export. Registered in run() dispatch + AppLaunchHandler subcommand list. - AutoRig_test.cpp: pure-data unit tests (template well-formedness, AABB containment, vertical ordering, degenerate-input robustness, string/JSON). - Sentry breadcrumb ai.assist.auto_rig. Verified end-to-end: static OBJ -> 19-bone humanoid + skin -> glTF export with 1 skin / 17 joints; FBX export carries the skeleton too. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(#407): MCP auto_rig tool + GUI Auto-Rig + rig CLI tests + docs - MCP: `auto_rig` { template, skin?, up_axis?, output_path? } (MCPServer::toolAutoRig) — rigs the selected static mesh, optional skin chain + optional re-export. Registered + advertised. Breadcrumb ai.assist.auto_rig. - GUI: AutoRigDialog.qml (template + up-axis pickers, "also skin" checkbox) driven by AutoRigController; new "Rigging" CollapsibleSection in Animation Mode → Mode Tools, gated on AutoRigController.hasRiggableSelection (a static mesh — already-rigged meshes show "Skinning" instead). Lazy-loaded Loader + openAutoRigDialog(), registered in qml_resources.qrc. - Tests: CLIPipeline_cmdrig_coverage_test.cpp (arg-validation + file-missing branches need no GL; success path skips gracefully without Xvfb). - CLAUDE.md: CLI examples (skin + rig), recognized-subcommand list, and a full AutoRig architecture entry (incl. the LGPL→native rationale, the _initialise(true) export gotcha, and documented quality limits). App + UnitTests build clean on macOS arm64. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(#407): compile AutoRig.cpp into the test common lib unit-tests-linux failed to link: AutoRig::* symbols undefined in libqtmesh_test_common.a (MCPServer::toolAutoRig and CLIPipeline::cmdRig reference them). The test target has its own TEST_SRC_FILES list separate from the app's src/CMakeLists.txt — add AutoRig.cpp + AutoRigController.cpp there, next to SkinWeights (same omission class as #738's PbrMapSynth gap). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(#407): address review — QML registration, upAxis, error paths Code review (Codex + CodeRabbit) on the auto-rig PR: - CRITICAL: register AutoRigController as a QML singleton in mainwindow.cpp (PropertiesPanel URI) like the sibling controllers + add its kill(). With qt_add_qml_module disabled, QML_SINGLETON alone doesn't expose it, so the Rigging section/dialog would ReferenceError. Verified no error at runtime now. - CRITICAL: the dialog's Up-axis picker was ignored — autoRigSelected() didn't take upAxis. Added a `const QString& upAxis` param (controller maps x/y/z → Options::upAxis) and pass dialog.upAxes[dialog.upAxisIndex] from QML. - AutoRig::appendPositions: guard a null vbuf->lock() (shrink `out` back, return false) instead of dereferencing. - AutoRig::rigEntity: on _initialise failure, detach the half-built skeleton (mesh->_notifySkeleton(null)) before removing it, so hasSkeleton() resets and a retry / exporter doesn't pick up a partial rig. - MCP toolAutoRig: validate output_path type; a requested skin that fails is now a hard error (no unskinned export reported as success); export wrapped in the try/catch (also catches std::exception); Sentry breadcrumb no longer logs the full output path. - PropertiesPanel openAutoRigDialog(): handle Loader.Error to allow retry. App + UnitTests build clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(#407): Mixamo-style marker placement + undoable rig/skin Marker-guided auto-rig refinement: the user clicks 10 humanoid markers on the mesh in the viewport (chin, L/R shoulder, L/R wrist, L/R hip, L/R knee, pelvis) and each anchors its joint while the limb/spine chains interpolate between the anchors — so the rig follows real (incl. cartoon) proportions instead of the fixed proportional template. - AutoRig::fitTemplateWithMarkers + layChain (generic anchor→tip chain): arms lay shoulder→arm→forearm→hand toward the wrist; legs lay hip socket→knee→foot; spine distributes Spine/Chest/Neck evenly between the marked pelvis and chin; hips carry unmarked thigh roots, explicit hip markers override. Every marker optional (empty set ≡ fitTemplate). - AutoRigController marker session (begin/skip/undo/cancel/commit) with ray-picked PT_SPHERE overlays; clicks routed via TransformOperator before the knife/select paths. AutoRigDialog made non-modal so viewport clicks reach the scene; onClosing cancels any active session. - Undo/redo: AutoRig::unrigEntity + AutoRigCommand wrap rig (+ optional skin) in one undoable unit; both GUI paths push through UndoManager. Single Ctrl+Z reverts rig and skin together. - Skeleton section extracted in the Inspector (skeleton/weights toggles no longer gated on animations). - Tests: marker order/labels, empty≡fitTemplate, arm/leg chain layout, shoulder/hip anchoring, spine interpolation, command error/undo branches. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(#407): move auto-rig UI inline into Inspector + fix Skip - Replace the modal AutoRigDialog with an inline Rigging section in the Inspector (riggingToolsComponent). Skeleton-type picker is a primary control; up-axis stays under "Advanced options". Smart show/hide: idle shows entry points + options, marker mode swaps to the guidance label + Skip/Undo/Cancel/Rig-from-markers controls. Section cancels any active marker session when it disappears (replaces the dialog's onClosing). - Markers only offered for the humanoid template (they're humanoid-specific). - Fix Skip: marker progress is now a CURSOR into the order list. Skip advances the cursor past a slot without storing a marker (joint keeps the template fit); the old code pushed an unset placeholder that didn't count as resolved, so the cursor stuck and Skip did nothing. Place advances + stores; Undo steps back, dropping the marker if that slot was placed. New markerPlacedCount drives "Rig from markers"; markerCount = resolved slots (placed + skipped) for the N/total readout. - Remove qml/AutoRigDialog.qml + its qrc entry. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(#407): undoable rig crash — strip blend vertex elements on unrig Rigging with skin runs Ogre's _compileBoneAssignments, which adds BLEND_INDICES/BLEND_WEIGHTS vertex elements. On undo, clearing the bone-assignment list is NOT enough — Ogre only removes those elements inside compileBoneAssignments, which it skips when the list is empty, so the declaration kept advertising blend elements while the entity had no skeleton → null SkeletonInstance deref on the next _initialise/render. AutoRig::unrigEntity now explicitly strips the blend elements (unbind the buffer + removeElement) on shared + per-submesh vertex data, leaving a genuinely static mesh. AutoRigCommand also calls AutoRigController::notifyRiggingChanged on redo/undo — on undo BEFORE detaching, so any active skeleton-debug overlay tears down while the skeleton still exists (else it dangles), and the Inspector re-evaluates the Rigging/Skeleton sections. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(#407): coherent marker inference + mesh-bounds clamping Replace per-marker patching with a resolve-then-lay model: fitTemplate gives a proportional baseline + template segment vectors, then every key joint (Head, L/R Shoulder, L/R Hand, Hips, L/R UpLeg, L/R Knee) is resolved marked → inferred-from-marked-neighbours → template, and the dependent chains (spine/arms/legs) are laid from those anchors. A partial marker set now yields an anatomically-sane skeleton instead of stranding unmarked joints at the template (no more shoulder-above-head). Inference: Hips ← up-leg midpoint + template rise; Head ← template offset above Hips; UpLeg ← mirror the other / pelvis + socket offset; Shoulder ← along the live Hips→Head line at the template height fraction + lateral offset (chin+hips imply sane shoulders), else mirror; Hand ← shoulder + template arm vector (marked shoulder + skipped wrist still lays a full arm); mirroring reflects across the auto-detected sagittal plane. Mesh-bounds clamp: inferred legs no longer punch through the model. Knee skipped → foot drops to the mesh floor (AABB mn[up]) below the up-leg, knee halfway between; marked knee → foot extrapolated but floor-clamped; knee always kept between up-leg and foot in the up axis. Empty marker set still early-returns fitTemplate unchanged. Tests added for each inference rule + the floor clamp. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(#407): update stale 6-marker test to 10; bust stale macOS assimp cache - AutoRigMarkers.OrderAndLabelsAreStable still asserted 6 markers (and order[5]==Hips) from the first marker commit; the set grew to 10 (added L/R shoulder + L/R hip). unit-tests-linux caught it. Assert 10 and use front()/back() so the count is the single source of truth. - Bump MACOS_CACHE_VERSION sdkpin1→sdkpin2: the assimp macOS cache was built under Xcode 26.5 and baked .../MacOSX26.5.sdk/.../libz.tbd into Codec_Assimp, so OGRE's cache-miss rebuild under the pinned 26.3 failed with "No rule to make target .../libz.tbd". Busting the cache rebuilds assimp under the pinned SDK. (Pre-existing CI infra issue, not from this feature.) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: Cursor <cursoragent@cursor.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements #404 — predict normal + height maps from a single albedo/diffuse texture via an ONNX UNet, plus a roughness heuristic from albedo luminance. Lands ONNX Runtime as the project's first ONNX consumer.
Decisions (confirmed with maintainer)
ModelDownloaderfrom a configurable URL (QSettings ai/pbrModelUrl/QTMESH_PBR_MODEL_URL), empty by default until a permissive UNet host is chosen. The whole feature degrades gracefully offline.What's included
cmake/OnnxRuntime.cmake(ENABLE_ONNX, OFF by default) — downloads prebuilt ONNX Runtime 1.20.1 per-platform (verified SHA256), imported targetqtmesh_onnx. macOS universal2 (no per-arch trap; CoreML EP + CPU fallback). Runtime lib copied next to the binary/tests.PbrMapSynth(Ogre-free core, unit-tested) — NCHW packing, overlapping-tile inference with feathered seam blend, normal/height decode (strength + OpenGL/DirectXinvertG), roughness heuristic. Discovers model I/O shapes at runtime; derives normal-from-height via the existing SobelNormalMapGeneratorwhen the model emits only height.AIAssistManager(QML_SINGLETON) — model resolve/download/cache, synchronoussynthesizePbrMaps, slice-E slot binding, progress signals.generate_pbr_maps, CLIqtmesh material --texture <albedo> --generate-pbr [<mesh>] [-o] [--tile-size N] [--no-normal|--no-roughness|--no-height]. All bind normal/roughness into the canonical slice-E slots viaRTShaderHelper::wirePbrSlotsForFFP.ai.assist.pbr_synth.#404 acceptance criteria
AIAssistManager)ai.assist.pbr_synthTests
PbrMapSynth_test.cpp— GL/ONNX-free: NCHW round-trip, flat-normal→blue, invertG flip, height scaling, roughness heuristic. Verified locally via a standalone harness (gtest main needs GL on macOS).CLIPipeline_cmdmaterial_coverage_test.cpp—--generate-pbrerror/success contracts (each branches onENABLE_ONNX).-DENABLE_ONNX=ONon Linux.Known follow-ups (documented in CLAUDE.md)
ENABLE_ONNXstays OFF (MSVC-built archive won't link under MinGW); feature degrades to the "rebuild with -DENABLE_ONNX" message..dylib/.sois included in the packaged release bundles.🤖 Generated with Claude Code
Summary by CodeRabbit
material --generate-pbr --texture, including selective normal/roughness/height outputs, tiling, material slot binding, and cache reuse.generate_pbr_mapsfor programmatic PBR synthesis.qtmesh material --texture ... --generate-pbrexamples and ONNX/model download/cache behavior.