feat(#805): MV-Adapter slice 1 — 4-view AI texture bake for image→3D meshes - #827
Conversation
…eft/right) First slice of the MV-Adapter epic: adopt its multi-view CAMERA LAYOUT on the EXISTING sd.cpp depth-ControlNet stack (no MV-Adapter weights, no new dependency). The AI texture pass now generates 4 depth-conditioned views (front/back/left/right — MV-Adapter's 6-view orthographic layout minus the two poles) instead of 2, and MultiViewTextureBaker projects+blends all of them onto UV0. Full horizontal coverage + seam overlap for the baker's facing-weighted cross-view blend — a clear step up from front+back (which left the sides stretched/blurred) without the ~3x cost of all 6 views. MeshDepthRenderer already exposes all 6 views and MultiViewTextureBaker is already N-view (facing = -normal.camDir, blend by facing^0.5, color-match to view 0), so this is a layout/wiring change only. Per-view SD sampling still drifts (imperfect consistency) — that's exactly what the later MV-Adapter decoupled-attention slice replaces, behind this same N-view camera + bake plumbing. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…t after) The AI-texture unwrap ran in finishMultiViewBake — AFTER all SD views complete. With 4 views (~several minutes of SD), the stale 'Auto- unwrapping UVs before bake…' status made it look frozen for an hour when it was really grinding the SD passes (confirmed: main thread in the render loop at 91% CPU, no xatlas frames — not hung). Two problems, one fix: extract the degenerate-clean + decimate-to-16k + xatlas unwrap into prepareMeshForTexturing() and run it ONCE up front in generateMeshTextureMultiView, before the depth renders + SD. Now: - the ~2s unwrap happens immediately (no stale 'unwrapping' status sitting through the long SD phase); - every depth view AND the final bake use the SAME decimated+unwrapped mesh (previously the views were rendered from the full mesh and the bake ran on the decimated one — a mismatch); - effectively 'parallel' from the user's view — the unwrap cost is negligible next to the SD passes and no longer trails them. finishMultiViewBake keeps a safety-net prepare call. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…VLM caption
Multi-view AI texture bake succeeded end-to-end but the result never reached
the screen, and the SmolVLM caption was wrong. Four fixes:
Texture never rendered on TripoSG (geometry-only) meshes:
- applyTextureToEntityDiffuse only handled RENAMING an existing diffuse TUS.
The shared "MeshGen/NeutralClay" material has NO texture unit, so the baked
diffuse had nowhere to bind and the mesh stayed clay. Now create a named
"diffuse_map" TUS (and reset the clay tint to white so colours show true).
- NeutralClay is shared across all generated meshes; clone it per-entity before
texturing so one bake doesn't tint every generated mesh in the scene.
- The viewport renders via RTSS (ShaderGeneratorDefaultScheme), whose technique
was generated before the TUS existed and had 0 texture units.
removeAllShaderBasedTechniques doesn't drop a cloned material's untracked RTSS
technique, so it survived stale (confirmed empty 1.5s later). Remove that
scheme's technique directly off the material + recompile so RTSS regenerates
from FFP tech 0 (which has the diffuse_map). This is why a manual material
Apply "fixed" it — that path fully rebuilds the technique.
Status stuck at "view 4/4 — step 12/12":
- sdTextureGenerated was emitted per view; the panel's onSdTextureGenerated
marks the bake done on the first emit (view 0), then later per-view SD notices
only updated the raw status text, freezing it at the last notice. Emit
sdTextureGenerated only once, from finishMultiViewBake after the real apply.
SmolVLM captioned a person as "a fluffy brown and white rabbit":
- The prompt contained a worked EXAMPLE ("a fluffy brown and white rabbit, …")
which the tiny 500M model parroted verbatim instead of looking at the image.
Removed the example; rewrote the prompt to name the subject first, then its
surface detail (skin/hair/eyes/makeup/clothing for people), and ignore the
background.
- Fixed the chat template to match the GGUF's embedded Jinja exactly
(image-first "User:" with no trailing space).
- Serialise caption() with a mutex — concurrent detached caption threads (from
quickly switching images) share llama.cpp global state and produced garbage.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 43 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?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 reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. 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, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. 📝 WalkthroughWalkthroughThis PR expands the AI texture generation flow to request four camera views instead of two, moves mesh preprocessing (cleanup, decimation, UV unwrapping) earlier via a new ChangesMulti-view Texture Generation and Application
Estimated code review effort: 4 (Complex) | ~60 minutes Image Captioner Thread-Safety and Prompt Update
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6f38a48871
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // A flat clay diffuse colour would MODULATE the new texture to | ||
| // a muddy tint; reset to white so the baked colours show true. | ||
| pass->setDiffuse(Ogre::ColourValue::White); |
There was a problem hiding this comment.
When the AI texture option is used with the TripoSR path, the build skips the field bake and creates a MeshGen/VertexColor material; this new branch then clones that material and adds a diffuse_map, but only sets the diffuse colour to white. The cloned material still has vertex-colour tracking enabled from MeshGenBuilder, so the newly baked AI texture is multiplied/tinted by the old decoder vertex colours instead of replacing them. Clear vertex colour tracking when repurposing these MeshGen/* no-texture materials for the generated diffuse map.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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/ImageTo3D/ImageCaptioner.cpp`:
- Around line 121-129: The QML SD flow is still calling
ImageCaptioner::caption() and ensureModelBlocking() synchronously from
MaterialEditorQML, so the new mutex can block the UI for the full
load/inference. Move this captioning work off the UI thread by mirroring the
detached-worker pattern used by MeshGenController, and keep the existing
per-path stale-result handling via setCaptionResult so only the async completion
updates state.
🪄 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: 13e558a2-2ef5-4d14-9fa5-25c108a3629e
📒 Files selected for processing (5)
qml/PropertiesPanel.qmlsrc/ImageTo3D/ImageCaptioner.cppsrc/ImageTo3D/ImageCaptioner.hsrc/MaterialEditorQML.cppsrc/MaterialEditorQML.h
| // Captioning runs on a detached worker thread (MeshGenController), and the | ||
| // user can pick a new image while a previous caption is still in flight — | ||
| // spawning a second concurrent call. llama.cpp model load + inference is NOT | ||
| // safe to run from two threads at once (global ggml backend state); doing so | ||
| // produced garbage captions (e.g. a person still described from the earlier | ||
| // image). Serialise: only one caption() runs at a time. The caller's | ||
| // per-path guard (setCaptionResult) drops any now-stale result. | ||
| static std::mutex s_captionMutex; | ||
| std::lock_guard<std::mutex> lock(s_captionMutex); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find the enclosing function of the caption() call in MaterialEditorQML and check its thread context
rg -nP -C40 'ImageCaptioner::caption\s*\(' src/MaterialEditorQML.cpp | rg -nP -C3 '(std::thread|QtConcurrent|QThread|invokeMethod|QueuedConnection|::run|detach)'Repository: fernandotonon/QtMeshEditor
Length of output: 164
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate ImageCaptioner::caption usages and inspect surrounding context.
rg -n -C 30 'ImageCaptioner::caption\s*\(' src
# Find the specific MaterialEditorQML.cpp location if it exists under a different path.
fd -a 'MaterialEditorQML.cpp' src
# Search for the captioning flow and any thread dispatch around it.
rg -n -C 20 'startCaptioning|setCaptionResult|caption\(' src/MaterialEditorQML.cpp src/ImageTo3D/ImageCaptioner.cpp srcRepository: fernandotonon/QtMeshEditor
Length of output: 48898
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the surrounding class/interface to determine whether the SD flow is a QML/UI entry point.
rg -n -C 20 'class MaterialEditorQML|Q_INVOKABLE|generateSelected|sdGenerationNotice|sdGenerationError' src/MaterialEditorQML.h src/MaterialEditorQML.cpp
# Show the function signature around the caption call for direct context.
sed -n '4628,4685p' src/MaterialEditorQML.cppRepository: fernandotonon/QtMeshEditor
Length of output: 50382
Move captioning off the UI thread MaterialEditorQML.cpp:4664-4671 calls ensureModelBlocking() and ImageCaptioner::caption() inline in the QML SD flow, so the mutex can stall the editor for the full model load/inference. Mirror the detached-worker path used by MeshGenController.
🤖 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/ImageTo3D/ImageCaptioner.cpp` around lines 121 - 129, The QML SD flow is
still calling ImageCaptioner::caption() and ensureModelBlocking() synchronously
from MaterialEditorQML, so the new mutex can block the UI for the full
load/inference. Move this captioning work off the UI thread by mirroring the
detached-worker pattern used by MeshGenController, and keep the existing
per-path stale-result handling via setCaptionResult so only the async completion
updates state.
…rials Codex review: the TripoSR + AI-texture path repurposes "MeshGen/VertexColor", which has per-vertex colour tracking enabled (set on the decoder colours in MeshGenBuilder). When the bake adds a diffuse_map and sets diffuse to white, the baked AI texture was still multiplied by the old decoder vertex colours instead of replacing them. Disable vertex-colour tracking (TVC_NONE) when creating the diffuse_map TUS on any MeshGen/* no-texture material. No-op for NeutralClay, which never enabled tracking. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Addressed the reviews: Codex P2 (vertex-colour tracking on MeshGen materials) — valid, fixed in bf5d1cd. When repurposing a CodeRabbit (move captioning off the UI thread in |
|
1 participant
MV-Adapter slice 1 (#805): 4-view AI texture bake + image→3D texture fixes
Builds on the TripoSG image→3D backend (#794, shipped in 3.18.0) to give the geometry-only meshes a real, image-derived texture — and fixes the chain of bugs that kept that texture from actually reaching the screen.
What's new
Bug fixes (this round — reported during hands-on testing)
MeshGen/NeutralClaymaterial has no texture unit, so the successfully-baked diffuse had nowhere to bind and the mesh stayed clay. Now: create a nameddiffuse_mapTUS (white base tint), clone the shared clay material per-entity so one bake doesn't tint every generated mesh, and — the crucial one — remove the stale RTSS technique directly off the material so it regenerates from the FFP pass (a cloned material's RTSS technique isn't tracked byremoveAllShaderBasedTechniques, which is why only a manual material Apply "fixed" it).sdTextureGeneratedwas emitted per view, prematurely marking the bake done on view 0; now emitted once after the real apply.caption()(concurrent caption threads corrupted llama.cpp global state).Models
fernandotonon/QtMeshEditor-modelsHF repo undercaption/, downloads on first use. Verified present (HTTP 302).Testing
🤖 Generated with Claude Code
Summary by CodeRabbit