feat(texture): multi-view AI texture bake (front+back projection onto UV0)#729
Merged
Conversation
…ojection baker First slice of the multi-view AI texture bake (see src/MULTIVIEW_TEXTURE_BAKE_DESIGN.md). Establishes the reprojection pipeline: generate per-view images, then project them onto the mesh's FIXED UV0 atlas (not "recompute UVs from the image"). - MeshDepthRenderer: add renderDepthMapView(entity, size, View) returning the depth image PLUS the exact view/projection matrices + camera dir used, so the baker re-projects through the identical camera. Built-in front/back/left/ right/top/bottom views; camera now positions along view.dir with an up hint (top/bottom use Z-up). renderDepthMap() stays as the front-view delegate so issue #403 is unchanged. - MultiViewTextureBaker (new, pure-data): for each triangle, per view compute a facing weight (max(0,-n·viewDir)^power, back-face/grazing culled), project the verts to viewport UV, rasterize in UV0 space, sample the view image at the barycentric-interpolated screen UV, accumulate color*weight; resolve by weight and seam-dilate (reuses VertexColorBaker::dilate + TexturePaintBuffer). - Unit tests: synthetic ±Z quad + solid front/back images assert front paints red, back-only is culled, front+back blends by facing, dilation fills margins, empty-input guards. No Ogre scene / GL / QApplication needed. Validated the math locally via a standalone harness (front-facing quad → full 64x64 atlas, center = red, back view correctly culled). Slice 2 will wire fromEntity() + the front+back orchestration in MaterialEditorQML. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…V0 bake
Wires the slice-1 reprojection baker to the live SD path and the UI.
- MultiViewTextureBaker::fromEntity(): extract world-space triangles + UV0 from
an Ogre::Entity (via EditableMesh::loadFromEntity), transforming positions by
the parent node's derived world matrix and normals by its inverse-transpose.
- MaterialEditorQML::generateMeshTextureMultiView(prompt, w, h, strength, views):
a state machine (MultiViewBakeState pimpl) that renders each view's depth +
camera matrices, generates one image per view sequentially through the SD
worker, and on the final completion runs MultiViewTextureBaker::bake over the
accumulated images and applies the baked atlas to the entity's diffuse via the
existing applyTextureToEntityDiffuse path. onSDGenerationCompleted routes
multi-view completions; onSDGenerationError aborts the in-flight bake. Defaults
to {front, back}. ENABLE_STABLE_DIFFUSION-guarded.
- ~MaterialEditorQML() defaulted in the .cpp so the unique_ptr pimpl sees the
complete MultiViewBakeState type.
- QML: "Bake front + back (projects onto UV map)" sub-toggle under the existing
"use selected mesh" checkbox; runTextureGeneration routes to the multi-view
path when both are checked.
App builds + launches clean (no QML errors). The pure-data baker is covered by
the slice-1 unit tests; the SD-driven orchestration is #ifdef-guarded and not
unit-tested (no model in CI), matching the #403 pattern.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The keychain prompt was back: migrateLegacySettingsIfNeeded() falls through to readLegacySecretStore() (SecItemCopyMatching / CredRead) whenever QSettings has no token — which is every launch while signed out. It's called repeatedly at startup (CloudAccountMenuButton::refresh and others), so macOS prompted again on each run, defeating the whole point of moving the session to QSettings. Gate the legacy probe behind a persisted Cloud/legacyMigrationDone flag: probe the OS secret store AT MOST ONCE ever (to carry a pre-QSettings session forward), set the flag, and never touch the keychain again — signed in or out. Sign-out (clearSession) does not reset the flag, so logging out can't re-trigger a probe. Test: LegacyMigrationProbesOnlyOnce asserts the flag is set after the first call and a legacy file written afterward is ignored. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
MeshDepthRenderer creates a persistent offscreen camera node named "QtMeshDepthCameraNode" (it's cached + reused across depth renders). It was showing up in the Inspector scene tree because isForbiddenNodeName only filtered unnamed/grid/gizmo nodes. Add it to the forbidden-node list (same canonical filter SceneTreeModel uses). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughIntroduces a multi-view depth-conditioned AI texture baking pipeline: ChangesMulti-View Texture Baking Pipeline
Cloud Credential Store One-Time Migration
Sequence Diagram(s)sequenceDiagram
participant QML as TexturePropertiesPanel
participant MaterialEditorQML
participant MeshDepthRenderer
participant SDManager
participant MultiViewTextureBaker
QML->>MaterialEditorQML: generateMeshTextureMultiView(prompt, w, h, controlStrength, ["front","back"])
MaterialEditorQML->>MaterialEditorQML: validate SD/model/mesh<br/>init MultiViewBakeState
loop for each view (front, then back)
MaterialEditorQML->>MeshDepthRenderer: renderDepthMapView(entity, size, view)
MeshDepthRenderer-->>MaterialEditorQML: RenderResult(depth, viewMatrix, projMatrix)
MaterialEditorQML->>SDManager: generateMeshTexture(prompt, depthImagePath)
SDManager-->>MaterialEditorQML: onSDGenerationCompleted(image)
MaterialEditorQML->>MaterialEditorQML: load image into view slot<br/>advance index or finish
end
MaterialEditorQML->>MultiViewTextureBaker: fromEntity(entity)
MultiViewTextureBaker-->>MaterialEditorQML: std::vector<Triangle>
MaterialEditorQML->>MultiViewTextureBaker: bake(triangles, views, buffer, opts)
MultiViewTextureBaker-->>MaterialEditorQML: Report
MaterialEditorQML->>MaterialEditorQML: write PNG to generated_textures/<br/>register Ogre resource<br/>apply to entity diffuse
MaterialEditorQML->>QML: emit textureNameChanged, sdTextureGenerated
Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 minutes 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 docstrings
🧪 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: 593e27bb73
ℹ️ 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".
| Ogre::Vector3 dir = view.dir; | ||
| if (dir.isZeroLength()) dir = Ogre::Vector3(0, 0, -1); | ||
| dir.normalise(); | ||
| const Ogre::Vector3 camPos = center - dir * dist; |
There was a problem hiding this comment.
For the existing mesh-conditioned path, renderDepthMap() now delegates to front(), but with front().dir == (0,0,-1) this new placement computes center - dir * dist, which puts the camera on +Z. The previous implementation explicitly used center + (0,0,-dist) because +Z captures the model's back for Mixamo-style meshes, so single-view generation now conditions on the back side and multi-view labels front/back swapped for the default workflow.
Useful? React with 👍 / 👎.
| QStringLiteral("entity=%1 views=%2 size=%3") | ||
| .arg(st->entityName).arg(viewNames.join(',')).arg(std::max(st->width, st->height))); | ||
|
|
||
| m_multiViewBake = std::move(st); |
There was a problem hiding this comment.
Once this state is installed, clicking the existing Stop button only calls SDManager::stopGeneration(), and onSDGenerationStopped() clears m_sdMeshTextureEntity but never resets m_multiViewBake. In that stopped scenario the next multi-view request is rejected as already in progress, and a later plain SD completion can be consumed by the stale multi-view branch.
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: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/MaterialEditorQML.cpp (1)
4508-4514:⚠️ Potential issue | 🟠 Major | ⚡ Quick winClear the multi-view bake state when SD generation is stopped.
onSDGenerationStopped()clears only the single-view mesh target. If the user stops during a multi-view bake,m_multiViewBakeremains set, blocking future bakes and letting a later unrelated SD completion enter the multi-view branch.🧹 Proposed stop-state cleanup
void MaterialEditorQML::onSDGenerationStopped() { m_sdMeshTextureEntity.clear(); + m_multiViewBake.reset(); m_sdGenerationProgress = 0.0f; emit sdGenerationProgressChanged(); emit sdIsGeneratingChanged();🤖 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/MaterialEditorQML.cpp` around lines 4508 - 4514, In the onSDGenerationStopped() function in MaterialEditorQML.cpp, the multi-view bake state flag m_multiViewBake is not being cleared when SD generation is stopped. This causes the flag to remain set if the user stops during a multi-view bake, blocking future bakes and potentially allowing unrelated SD completions to incorrectly enter the multi-view branch. Add a statement to clear or reset m_multiViewBake (likely to false or an equivalent cleared state) alongside the existing cleanup that clears m_sdMeshTextureEntity and resets m_sdGenerationProgress.
🤖 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/CloudCredentialStore_test.cpp`:
- Around line 154-180: The test LegacyMigrationProbesOnlyOnce is
environment-dependent because it calls migrateLegacySettingsIfNeeded() on line
159 without first ensuring the legacy cloud_session.dat file doesn't exist. If a
stale file remains from a previous test run, the first migration may behave
unexpectedly. Before the first migrateLegacySettingsIfNeeded() call, add code to
remove the legacy file at the path constructed from
QStandardPaths::writableLocation and the cloud_session.dat filename (similar to
how it is constructed later in the test) to guarantee clean preconditions.
In `@src/CloudCredentialStore.cpp`:
- Around line 195-223: Add Sentry breadcrumbs to the CloudCredentialStore legacy
migration operation to improve field diagnosability. Use
SentryReporter::addBreadcrumb() at key points in the migration flow: when
checking if migration is already done, before reading from the legacy secret
store in readLegacySecretStore(), when successfully writing migrated credentials
via writeToSettings(), and after marking the migration complete by setting
cloudLegacyMigrationDone(). Use the category 'file.import' for the legacy store
read operation and appropriate descriptive messages for each breadcrumb to track
the migration state transitions.
In `@src/MaterialEditorQML.cpp`:
- Around line 4085-4087: The current breadcrumb logging uses only the
ai.assist.mesh_texture_multiview category for both the multi-view generation
trigger and file write operations. According to coding guidelines, you need to
add separate breadcrumbs with appropriate categories: use ui.action category for
the multi-view generation trigger (user-facing action) and add a new file.export
category breadcrumb for the baked PNG write operation (file I/O). Update the
SentryReporter::addBreadcrumb calls to use these distinct categories based on
the type of operation being logged, and ensure both operations are properly
tracked with their respective category types.
- Around line 4183-4189: The entity name from s->entityName is being used
directly in the filename construction for multiview_bake_*.png without
sanitization, which can cause save failures due to special characters or path
separators. Sanitize s->entityName by removing or replacing filesystem-unsafe
characters (such as path separators, control characters, and reserved filesystem
names) before passing it to the QStringLiteral().arg() call that builds outName.
- Around line 4378-4384: The multi-view bake completion logic in the if block
checking img.isNull() only handles the success case where the image loads
successfully. When img.isNull() returns true, indicating the output image cannot
be loaded from outputPath, the code silently continues without resetting state,
leaving the baked view without pixels while still advancing s.current. This
causes the final bake to potentially fail late or process invalid input. Add an
else branch that resets m_multiViewBake to null (or clears the
MultiViewBakeState) when img.isNull() is true, ensuring the multi-view baking
process is immediately aborted on any unreadable output path.
In `@src/MeshDepthRenderer.h`:
- Around line 38-56: The View struct and its static methods (front, back, left,
right, top, bottom) have direction vectors that are semantically backwards. The
code in MeshDepthRenderer.cpp uses camPos = center - dir * dist, so the current
directions produce camera positions that capture the opposite side of the mesh
from what's intended. Flip the sign of the direction vectors in the View struct
default initialization and in all six static view methods (front, back, left,
right, top, bottom) to correct the semantics. Additionally, update the
explanatory comment in MeshDepthRenderer.cpp around line 168 to clarify the
corrected camera-side semantics after this fix.
In `@src/MULTIVIEW_TEXTURE_BAKE_DESIGN.md`:
- Line 31: The markdown file has two linting violations: the fenced code block
starting at line 31 is missing a language specifier (MD040), and the table in
the "Risks & mitigations" section lacks blank lines before and after it (MD058).
Add "text" as the language identifier to the opening fence (change the opening
``` to ```text), and add blank lines immediately before and after the table to
satisfy the blanks-around-tables rule. Apply the same fixes at line 119 as
indicated in the consolidated sites.
In `@src/MultiViewTextureBaker.cpp`:
- Around line 261-265: The code at lines 261-265 in MultiViewTextureBaker.cpp
sets an error message in errorOut when UV0 is unusable but continues execution
and returns triangles anyway. Since the caller in MaterialEditorQML.cpp only
checks if the returned triangles are empty to determine failure, this error
condition is silently ignored. Modify the error handling block to return an
empty triangles vector immediately after setting errorOut, so that the caller's
tris.empty() check will properly catch the UV0 usability issue and abort
processing.
---
Outside diff comments:
In `@src/MaterialEditorQML.cpp`:
- Around line 4508-4514: In the onSDGenerationStopped() function in
MaterialEditorQML.cpp, the multi-view bake state flag m_multiViewBake is not
being cleared when SD generation is stopped. This causes the flag to remain set
if the user stops during a multi-view bake, blocking future bakes and
potentially allowing unrelated SD completions to incorrectly enter the
multi-view branch. Add a statement to clear or reset m_multiViewBake (likely to
false or an equivalent cleared state) alongside the existing cleanup that clears
m_sdMeshTextureEntity and resets m_sdGenerationProgress.
🪄 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: 4df38bb8-e76e-4641-9f7c-6558f7d6e358
📒 Files selected for processing (14)
qml/TexturePropertiesPanel.qmlsrc/AppSettingsKeys.hsrc/CMakeLists.txtsrc/CloudCredentialStore.cppsrc/CloudCredentialStore_test.cppsrc/MULTIVIEW_TEXTURE_BAKE_DESIGN.mdsrc/Manager.cppsrc/MaterialEditorQML.cppsrc/MaterialEditorQML.hsrc/MeshDepthRenderer.cppsrc/MeshDepthRenderer.hsrc/MultiViewTextureBaker.cppsrc/MultiViewTextureBaker.hsrc/MultiViewTextureBaker_test.cpp
Comment on lines
+195
to
223
| // The OS secret store must be probed AT MOST ONCE, ever. Each probe is a | ||
| // SecItemCopyMatching (macOS Keychain) / CredRead (Windows) that can raise a | ||
| // confirmation prompt — re-probing on every launch (which happens when the | ||
| // user is signed out, since QSettings then has no token) is exactly the | ||
| // repeated-keychain-prompt bug we set out to kill. Persist a "done" flag the | ||
| // first time through and never touch the keychain again, signed in or not. | ||
| { | ||
| QSettings settings; | ||
| if (settings.value(AppSettingsKeys::cloudLegacyMigrationDone(), false).toBool()) | ||
| return; | ||
| } | ||
|
|
||
| // If a session already lives in QSettings there is nothing to migrate, but | ||
| // still mark the probe done so we don't keychain-probe on a later sign-out. | ||
| if (!readFromSettings().hasToken()) { | ||
| // One-time upgrade: a user who signed in on a pre-QSettings build has | ||
| // their token in the OS secret store / legacy fallback file. Carry it | ||
| // into QSettings so they stay signed in across the backend change. | ||
| const CloudSession legacy = readLegacySecretStore(); | ||
| if (legacy.hasToken() && writeToSettings(legacy)) { | ||
| g_sessionCache = legacy; | ||
| g_cacheValid = true; | ||
| } | ||
| } | ||
|
|
||
| QSettings settings; | ||
| settings.setValue(AppSettingsKeys::cloudLegacyMigrationDone(), true); | ||
| settings.sync(); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Add breadcrumbs for the one-time migration path.
Line 195–223 performs a significant operation (legacy probe/migration + persistence) but emits no Sentry breadcrumbs, which weakens field diagnosability of prompt-related regressions.
Proposed patch
+#include "SentryReporter.h"
@@
void CloudCredentialStore::migrateLegacySettingsIfNeeded()
{
+ SentryReporter::addBreadcrumb(QStringLiteral("file.import"),
+ QStringLiteral("cloud_legacy_migration_enter"));
// The OS secret store must be probed AT MOST ONCE, ever. Each probe is a
@@
{
QSettings settings;
- if (settings.value(AppSettingsKeys::cloudLegacyMigrationDone(), false).toBool())
+ if (settings.value(AppSettingsKeys::cloudLegacyMigrationDone(), false).toBool()) {
+ SentryReporter::addBreadcrumb(QStringLiteral("file.import"),
+ QStringLiteral("cloud_legacy_migration_skip_done"));
return;
+ }
}
@@
const CloudSession legacy = readLegacySecretStore();
if (legacy.hasToken() && writeToSettings(legacy)) {
g_sessionCache = legacy;
g_cacheValid = true;
+ SentryReporter::addBreadcrumb(QStringLiteral("file.import"),
+ QStringLiteral("cloud_legacy_migration_success"));
+ } else if (legacy.hasToken()) {
+ SentryReporter::addBreadcrumb(QStringLiteral("file.import"),
+ QStringLiteral("cloud_legacy_migration_write_failed"));
+ } else {
+ SentryReporter::addBreadcrumb(QStringLiteral("file.import"),
+ QStringLiteral("cloud_legacy_migration_no_legacy_data"));
}
}
@@
settings.setValue(AppSettingsKeys::cloudLegacyMigrationDone(), true);
settings.sync();
+ SentryReporter::addBreadcrumb(QStringLiteral("file.export"),
+ QStringLiteral("cloud_legacy_migration_mark_done"));
}As per coding guidelines, "Add Sentry breadcrumbs for all user-facing actions and significant operations using SentryReporter::addBreadcrumb() with categories: 'ui.action' for toolbar/menu clicks, 'ai.tool_call' for MCP tools, 'file.import'/'file.export' for I/O".
🤖 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/CloudCredentialStore.cpp` around lines 195 - 223, Add Sentry breadcrumbs
to the CloudCredentialStore legacy migration operation to improve field
diagnosability. Use SentryReporter::addBreadcrumb() at key points in the
migration flow: when checking if migration is already done, before reading from
the legacy secret store in readLegacySecretStore(), when successfully writing
migrated credentials via writeToSettings(), and after marking the migration
complete by setting cloudLegacyMigrationDone(). Use the category 'file.import'
for the legacy store read operation and appropriate descriptive messages for
each breadcrumb to track the migration state transitions.
Source: Coding guidelines
|
|
||
| ## Pipeline | ||
|
|
||
| ``` |
There was a problem hiding this comment.
Fix markdownlint warnings for the fenced block and table spacing.
This file currently triggers MD040 and MD058 (fenced-code-language, blanks-around-tables).
🧹 Proposed fix
-```
+```text
selected Entity
│
├─ 0. UV gate: UvUnwrap::infoForEntity → if !hasUv0 || uv0Coverage < THRESH
@@
-## Risks & mitigations
+## Risks & mitigations
+
| Risk | Mitigation |
|---|---|
| Occlusion / back-facing bleed | facing weight `max(0,−n·viewDir)` + per-view depth occlusion test |
| Front/back style mismatch | same seed + prompt; (later) img2img-condition back on front |
| Side seam with only 2 views | feathered facing blend + dilate; document that 4 views removes it |
| Bad/missing UV0 | auto-unwrap gate (UvUnwrap) before bake |
| Live skinned-mesh buffer mutation | bake reads geometry read-only; unwrap uses the GUI-safe `unwrapEntityToFile` snapshot/restore path |
| RTSS apply unreliability (`#403`) | bake targets real UV0 atlas → `applyTextureToEntityDiffuse` binds a normal diffuse_map |
+
## Test plan (headless, CI Linux+Xvfb; ASSERT_TRUE(tryInitOgre), no GTEST_SKIP)Also applies to: 119-119
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 31-31: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 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/MULTIVIEW_TEXTURE_BAKE_DESIGN.md` at line 31, The markdown file has two
linting violations: the fenced code block starting at line 31 is missing a
language specifier (MD040), and the table in the "Risks & mitigations" section
lacks blank lines before and after it (MD058). Add "text" as the language
identifier to the opening fence (change the opening ``` to ```text), and add
blank lines immediately before and after the table to satisfy the
blanks-around-tables rule. Apply the same fixes at line 119 as indicated in the
consolidated sites.
Source: Linters/SAST tools
…ustness - MeshDepthRenderer: FLIP the built-in view directions (Codex P1 / CodeRabbit major). With camPos = center - dir*dist, front must be dir=(0,0,1) to place the camera on -Z (the front for -Z-facing imports); the previous (0,0,-1) put it on +Z and captured the BACK, regressing the #403 single-view path and swapping front/back in multi-view. All six views flipped + comments corrected. This also fixes the camDirection fed to the baker's facing cull. - MultiViewTextureBaker::fromEntity: return EMPTY when the mesh has no usable UV0 (was returning triangles + only setting errorOut, so the caller baked garbage). - MaterialEditorQML multi-view: abort the run if a per-view image fails to load (was advancing with a null image → late bake failure); clear m_multiViewBake in onSDGenerationStopped so Stop can't strand it; sanitize the entity name into a filesystem-safe bake filename; emit ui.action + file.export breadcrumbs. - Design doc: markdownlint (fenced-code language + table blank line). Skipped: adding Sentry breadcrumbs inside CloudCredentialStore (CodeRabbit) — the store is intentionally dependency-free (QSettings/QFile/Security only); coupling it to SentryReporter for a low-value breadcrumb isn't worth it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Owner
Author
|
Addressed the review feedback in 13a5f63:
Skipped: adding Sentry breadcrumbs inside App + UnitTests build clean. |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/MaterialEditorQML.cpp (2)
4050-4064:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReject multi-view starts while any SD job is already running.
m_multiViewBakemakesonSDGenerationCompleted()capture the next SD completion, so starting this while another SD generation is in flight can pair an unrelated output with the first bake view. Add ansdManager->isGenerating()guard before creating the multi-view state.🐛 Proposed guard
SDManager *sdManager = SDManager::instance(); if (!sdManager->isModelLoaded()) { emit sdGenerationError("No SD model loaded. Please download and load a model from AI Settings."); return; } + if (sdManager->isGenerating()) { + emit sdGenerationError("Stable Diffusion is already generating. Stop or wait for the current job before starting a multi-view bake."); + return; + } auto* sel = SelectionSet::getSingleton();🤖 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/MaterialEditorQML.cpp` around lines 4050 - 4064, The multi-view bake start logic in the validation block needs an additional guard to prevent starting a multi-view bake while an SD generation is already in flight. Add a check using sdManager->isGenerating() after the existing sdManager->isModelLoaded() check and before the m_multiViewBake check, emitting an appropriate sdGenerationError if a generation is already running, to prevent the onSDGenerationCompleted() callback from incorrectly pairing unrelated SD outputs with the first bake view.
4065-4074:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFail fast when the selected mesh cannot bake to UV0.
finishMultiViewBake()rejects emptyfromEntity()results only after all view images have been generated. Preflight the same UV0/triangle extraction here so meshes without usable UV0 fail before spending multiple SD runs.⚡ Proposed preflight
} Ogre::Entity* entity = entities.first(); + + QString geoErr; + if (MultiViewTextureBaker::fromEntity(entity, &geoErr).empty()) { + emit sdGenerationError(QStringLiteral("Bake failed: %1").arg(geoErr)); + return; + } // LCOV_EXCL_START — requires a loaded SD model + a mesh auto st = std::make_unique<MultiViewBakeState>();🤖 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/MaterialEditorQML.cpp` around lines 4065 - 4074, Add a preflight validation check for UV0/triangle extraction immediately after obtaining the entity from `entities.first()` and before populating the `MultiViewBakeState` object. This check should verify that the mesh has usable UV0 coordinates using the same validation logic that `finishMultiViewBake()` performs later. If the UV0 validation fails, return early to avoid wasting computational resources on multiple Stable Diffusion runs. The check should happen before the `std::make_unique<MultiViewBakeState>()` call to fail fast for meshes without usable UV0.
🤖 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/MaterialEditorQML.cpp`:
- Around line 4203-4204: The SentryReporter::addBreadcrumb call at the specified
location logs the full outPath which contains absolute local paths that may
include sensitive information like usernames or project paths. Modify the
breadcrumb to log only the baked filename instead of the complete outPath by
extracting just the filename component, while keeping the "file.export"
breadcrumb category and descriptive message format.
---
Outside diff comments:
In `@src/MaterialEditorQML.cpp`:
- Around line 4050-4064: The multi-view bake start logic in the validation block
needs an additional guard to prevent starting a multi-view bake while an SD
generation is already in flight. Add a check using sdManager->isGenerating()
after the existing sdManager->isModelLoaded() check and before the
m_multiViewBake check, emitting an appropriate sdGenerationError if a generation
is already running, to prevent the onSDGenerationCompleted() callback from
incorrectly pairing unrelated SD outputs with the first bake view.
- Around line 4065-4074: Add a preflight validation check for UV0/triangle
extraction immediately after obtaining the entity from `entities.first()` and
before populating the `MultiViewBakeState` object. This check should verify that
the mesh has usable UV0 coordinates using the same validation logic that
`finishMultiViewBake()` performs later. If the UV0 validation fails, return
early to avoid wasting computational resources on multiple Stable Diffusion
runs. The check should happen before the
`std::make_unique<MultiViewBakeState>()` call to fail fast for meshes without
usable UV0.
🪄 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: eef531a9-83cb-4279-9779-9dbdafb1dde7
📒 Files selected for processing (6)
src/CloudCredentialStore_test.cppsrc/MULTIVIEW_TEXTURE_BAKE_DESIGN.mdsrc/MaterialEditorQML.cppsrc/MeshDepthRenderer.cppsrc/MeshDepthRenderer.hsrc/MultiViewTextureBaker.cpp
✅ Files skipped from review due to trivial changes (1)
- src/MULTIVIEW_TEXTURE_BAKE_DESIGN.md
🚧 Files skipped from review as they are similar to previous changes (3)
- src/CloudCredentialStore_test.cpp
- src/MeshDepthRenderer.cpp
- src/MultiViewTextureBaker.cpp
…crumb (#729) CodeRabbit: the file.export breadcrumb logged the full app-data outPath, which embeds the local username/path — PII in telemetry. Log outName (basename) only. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Owner
Author
|
Addressed the absolute-path breadcrumb finding in abdc81e: the |
|
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.
Generates a diffuse texture by ControlNet-generating images from multiple camera views (front + back to start) and projection-baking them onto the mesh's existing UV0 atlas — replacing the prior planar bind that was unreliable on RTSS/PBR materials (#403 known limitation). See
src/MULTIVIEW_TEXTURE_BAKE_DESIGN.md.Key idea: the UV layout stays fixed; each view's generated image is re-projected through the exact camera that produced its depth map and rasterized into UV0 (this is a bake, not "recompute UVs from the image").
Slice 1 — reprojection foundation
MeshDepthRenderer::renderDepthMapView(entity, size, View)returns the depth image plus the exact view/projection matrices + camera dir used. Built-in front/back/left/right/top/bottom views.renderDepthMap()stays as the front-view delegate so AI: Mesh-aware texture generation (depth-conditioned sd.cpp) #403 is unchanged.MultiViewTextureBaker(pure-data): per triangle × view, facing-weight cull (max(0,-n·viewDir)^p), project to viewport UV, rasterize in UV0 space, sample the view image at the barycentric-interpolated screen UV, accumulatecolor×weight; resolve by weight + seam-dilate (reusesVertexColorBaker::dilate+TexturePaintBuffer).Slice 2 — front+back orchestration + UV0 bake
MultiViewTextureBaker::fromEntity()extracts world-space triangles + UV0 from a live entity (world matrix on positions, inverse-transpose on normals).MaterialEditorQML::generateMeshTextureMultiView(prompt,w,h,strength,views): a state machine that renders each view's depth+matrices, generates one image per view sequentially through the SD worker, and on the final completion bakes the accumulated images onto UV0 and applies the atlas to the entity's diffuse.onSDGenerationErroraborts cleanly. Defaults to{front, back}.ENABLE_STABLE_DIFFUSION-guarded.Incidental fixes found while building this
migrateLegacySettingsIfNeeded()re-probed the OS secret store on every signed-out launch (SecItemCopyMatching → macOS prompt). Now gated behind a persistedCloud/legacyMigrationDoneflag → probes at most once, ever. Regression test added.QtMeshDepthCameraNodefrom the Inspector (added toManager::isForbiddenNodeName).Testing
#ifdef-guarded (no model in CI), matching AI: Mesh-aware texture generation (depth-conditioned sd.cpp) #403.🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Other