feat(materials): pack-textures live preview, drop, presets (Phase 5 slice G2+G3) - #479
Conversation
…ice G2+G3)
Two epic acceptance items missing from slice G:
- "Preview packed result before applying"
- "Drag texture → channel"
Plus quality-of-life additions: one-click preset buttons and per-row
reset.
C++ (src/MaterialEditorQML.{h,cpp}):
- Q_INVOKABLE previewPackedTextureChannels(...) — same parameters as
packTextureChannels minus the output path, plus a previewSize. Calls
TextureChannelPacker::pack() in-memory and returns
"data:image/png;base64,..." so QML Image can show it directly with
no temp-file dance. previewSize is clamped to [32, 512].
QML (qml/TextureChannelPackerDialog.qml):
- 2-column layout: channel rows on the left, 256×256 live preview
Rectangle/Image on the right.
- Live refresh: refreshPreview() re-renders on every input change
via property-change handlers (paths, constants, inverts,
includeAlpha).
- DropArea on each channel row's source field — drop a file from
Finder/Explorer/Asset Browser onto the row to set the channel path.
normaliseDroppedPath() strips the file:// scheme.
- Preset row: "Unity ORM", "Unreal MR", "Spec → Gloss" buttons that
filename-heuristically match existing source paths to the right
channels (find "ao"/"occlusion", "rough", "metal" substrings) and
set the appropriate invert / includeAlpha flags.
- Per-row trash-can reset button (28px column) clears the path,
resets the constant to its row's neutral default (0% for R/G/B,
100% for A), and unticks invert. Disabled when the row is at
defaults.
Tests (src/MaterialEditorQML_test.cpp):
- AllConstantsReturnsDataUrl — base64 PNG round-trips to a valid
64×64 QImage with the expected channel values.
- MissingFileReturnsEmpty — broken source path → empty string.
- SizeIsClampedToBounds — request previewSize=8 → 32×32 output.
Documentation: CLAUDE.md TextureChannelPacker entry expanded.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR adds a live preview thumbnail, drag-and-drop file inputs, one-click texture mapping presets, and per-channel reset controls to the TextureChannelPackerDialog. A new C++ API generates packed texture previews as base64 PNG data URLs, which the QML dialog displays and refreshes automatically as the user adjusts channel inputs. ChangesTexture Channel Packer Live Preview & Dialog Enhancement
Sequence Diagram(s)sequenceDiagram
participant User
participant QMLDialog as QML Dialog
participant MaterialEditorQML as C++ Backend
User->>QMLDialog: Edit channel path or constant
QMLDialog->>QMLDialog: Trigger refreshPreview()
QMLDialog->>MaterialEditorQML: Call previewPackedTextureChannels
MaterialEditorQML->>MaterialEditorQML: Pack channels
MaterialEditorQML->>MaterialEditorQML: Encode as PNG base64
MaterialEditorQML-->>QMLDialog: Return PNG URL
QMLDialog->>QMLDialog: Update previewDataUrl
QMLDialog->>User: Live preview refreshes
User->>QMLDialog: Click preset button
QMLDialog->>QMLDialog: Apply preset mapping
QMLDialog->>QMLDialog: Trigger refreshPreview
QMLDialog->>MaterialEditorQML: Call previewPackedTextureChannels
MaterialEditorQML-->>QMLDialog: Return PNG URL
QMLDialog->>User: Preview updates
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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: 18d7f21a57
ℹ️ 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".
| // packer's QImageReader sees a plain filesystem path. | ||
| function normaliseDroppedPath(url) { | ||
| const s = url.toString() | ||
| return s.startsWith("file://") ? s.substring(7) : s |
There was a problem hiding this comment.
The drag-and-drop path normalization is currently string-based (file:// prefix stripping), which produces invalid filesystem paths for common URL forms (for example file:///C:/... on Windows becomes /C:/..., and UNC paths lose their authority component). In these cases the dropped file path cannot be read by the packer, so live preview and packing fail only for drag-and-drop inputs while the Browse flow still works. Converting the dropped url via URL APIs (e.g., local-file conversion) avoids these platform-specific path regressions.
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: 3
🧹 Nitpick comments (3)
src/MaterialEditorQML_test.cpp (1)
2674-2688: ⚡ Quick winAdd max-bound clamp coverage to complete the contract.
This test checks the lower clamp path only. Please add an assertion for an oversized request (for example,
previewSize=2048) to verify clamping to512.🤖 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_test.cpp` around lines 2674 - 2688, The test only verifies the lower clamp; add a second call to previewPackedTextureChannels in TEST_F(MaterialEditorQMLTest, PreviewPackedTextureChannels_SizeIsClampedToBounds) using previewSize=2048 and assert the returned data URL starts with "data:image/png;base64,", decode to payload, load into QImage and EXPECT_EQ(img.width(), 512) to confirm the upper clamp; use the same parameter ordering and decoding pattern as the existing lower-clamp case for previewPackedTextureChannels.src/MaterialEditorQML.h (1)
393-395: ⚡ Quick winAlign the API comment with actual preview geometry behavior.
The comment says natural aspect ratio is preserved, but the implementation currently forces a square output. Please make the contract explicit in this header (or adjust implementation) to avoid caller confusion.
🤖 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.h` around lines 393 - 395, The doc comment for previewSize is inconsistent with behavior: the implementation forces a square preview instead of preserving the source aspect ratio. Either update the comment on previewSize to state it is the resulting square side length (aspect ratio is NOT preserved), or change the implementation that computes preview geometry to preserve aspect ratio; locate references to previewSize in MaterialEditorQML (and the preview/packer code that computes output dimensions) and: if choosing comment change, replace the text to explicitly say "previewSize is the resulting square side length in pixels; output is forced square and aspect ratio is not preserved"; if choosing implementation change, modify the packer/preview geometry logic to compute width and height by scaling the source's natural aspect ratio so the larger edge equals previewSize and update any callers that assume square outputs.qml/TextureChannelPackerDialog.qml (1)
93-99: ⚡ Quick winExtract the repeated filename-heuristic finder into one helper.
The same
lower/all/findblock is duplicated three times across presets. A shared helper keeps behavior consistent and reduces drift when heuristic keywords evolve.Also applies to: 120-126, 147-151
🤖 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 `@qml/TextureChannelPackerDialog.qml` around lines 93 - 99, Extract the repeated filename-heuristic logic into a single helper function and replace each duplicated block with calls to it: create a helper (e.g., function findByFilename(needle, ...paths) or a top-level util object) that internally defines the lowercase conversion and iterates over the provided array to return the first matching path (mirroring the current behavior of lower, all, and find). Replace the three duplicated occurrences (the blocks around symbols lower/all/find at lines 93-99, 120-126, and 147-151) to call this new helper with the appropriate arguments (oldAo, oldRough, oldMetal, alphaPath) so behavior stays identical and maintenance is centralized. Ensure the helper returns an empty string when no match is found to preserve existing callers' expectations.
🤖 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/TextureChannelPackerDialog.qml`:
- Around line 59-79: The preview is being regenerated on every property change
(all the onXChanged handlers calling refreshPreview), causing many back-to-back
repacks; change refreshPreview to be debounced/queued so multiple rapid property
changes only trigger a single rebuild: implement a single queuedRefresh flag or
timer inside the same scope as refreshPreview and have all on...Changed handlers
call scheduleRefresh() instead of refreshPreview() directly; when scheduled,
call MaterialEditorQML.previewPackedTextureChannels with the current redPath,
greenPath, bluePath, alphaPath, redConstant, greenConstant, blueConstant,
alphaConstant, invertRed, invertGreen, invertBlue, invertAlpha, includeAlpha,
256 and clear the queue. Ensure the same debouncing is applied for the other
block referenced (lines 86-166) so presets that mutate many properties cause one
preview update.
- Around line 52-57: normaliseDroppedPath currently returns a raw url string
which leaves percent-encoded segments (e.g. %20) and can produce Windows paths
with a leading slash (e.g. /C:/...), breaking QImageReader; update the
normaliseDroppedPath(url) implementation to decode the URL (use a URL- or
QUrl-based decode method) and strip a leading slash only when followed by a
Windows drive letter pattern (e.g. ^/[A-Za-z]:/) so "file:///" and "%20" are
handled correctly; keep the single function change (used by all four channel
rows) so all callers benefit.
In `@src/MaterialEditorQML.cpp`:
- Around line 2837-2885: Add Sentry breadcrumb tracking to the
previewPackedTextureChannels function (similar to packTextureChannels) by
calling SentryReporter::addBreadcrumb("preview", "<short message>") when a
live-preview generation is initiated and optionally on success; to avoid noise
during rapid input changes, throttle these breadcrumbs (e.g. use a static
timestamp or lastBreadcrumbMs check inside previewPackedTextureChannels to only
add a breadcrumb if N ms have elapsed since the last one). Ensure the breadcrumb
message includes identifying info (e.g. include includeAlpha and previewSize or
a short hash of the spec) and place the call near the start of
previewPackedTextureChannels before packing, referencing
TextureChannelPacker::PackingSpec and TextureChannelPacker::pack for context.
---
Nitpick comments:
In `@qml/TextureChannelPackerDialog.qml`:
- Around line 93-99: Extract the repeated filename-heuristic logic into a single
helper function and replace each duplicated block with calls to it: create a
helper (e.g., function findByFilename(needle, ...paths) or a top-level util
object) that internally defines the lowercase conversion and iterates over the
provided array to return the first matching path (mirroring the current behavior
of lower, all, and find). Replace the three duplicated occurrences (the blocks
around symbols lower/all/find at lines 93-99, 120-126, and 147-151) to call this
new helper with the appropriate arguments (oldAo, oldRough, oldMetal, alphaPath)
so behavior stays identical and maintenance is centralized. Ensure the helper
returns an empty string when no match is found to preserve existing callers'
expectations.
In `@src/MaterialEditorQML_test.cpp`:
- Around line 2674-2688: The test only verifies the lower clamp; add a second
call to previewPackedTextureChannels in TEST_F(MaterialEditorQMLTest,
PreviewPackedTextureChannels_SizeIsClampedToBounds) using previewSize=2048 and
assert the returned data URL starts with "data:image/png;base64,", decode to
payload, load into QImage and EXPECT_EQ(img.width(), 512) to confirm the upper
clamp; use the same parameter ordering and decoding pattern as the existing
lower-clamp case for previewPackedTextureChannels.
In `@src/MaterialEditorQML.h`:
- Around line 393-395: The doc comment for previewSize is inconsistent with
behavior: the implementation forces a square preview instead of preserving the
source aspect ratio. Either update the comment on previewSize to state it is the
resulting square side length (aspect ratio is NOT preserved), or change the
implementation that computes preview geometry to preserve aspect ratio; locate
references to previewSize in MaterialEditorQML (and the preview/packer code that
computes output dimensions) and: if choosing comment change, replace the text to
explicitly say "previewSize is the resulting square side length in pixels;
output is forced square and aspect ratio is not preserved"; if choosing
implementation change, modify the packer/preview geometry logic to compute width
and height by scaling the source's natural aspect ratio so the larger edge
equals previewSize and update any callers that assume square outputs.
🪄 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: c5d8c2b3-4aa6-4190-867c-c1bb3cffd369
📒 Files selected for processing (5)
CLAUDE.mdqml/TextureChannelPackerDialog.qmlsrc/MaterialEditorQML.cppsrc/MaterialEditorQML.hsrc/MaterialEditorQML_test.cpp
| // Strip the `file://` scheme some drag/drop sources prepend so the | ||
| // packer's QImageReader sees a plain filesystem path. | ||
| function normaliseDroppedPath(url) { | ||
| const s = url.toString() | ||
| return s.startsWith("file://") ? s.substring(7) : s | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Qt Quick DropArea, what is the recommended way to convert drop.urlsfile URLs (includingfile:///C:/...and%20-encoded paths) into local filesystem paths across Windows/macOS/Linux?
💡 Result:
Recommended approach: treat each element of DropArea’s drop.urls (file:// URLs) as a QUrl and use QUrl::toLocalFile to convert it to a native filesystem path; this is the Qt-provided way to handle cross-platform local-file URLs including forms like file:///C:/... and percent-encoded paths such as %20. Evidence: - QUrl::toLocalFile is documented as converting “a URL to a local file path” and returning the local path formatted appropriately by Qt [1]. - QQmlFile provides QML-side helpers; it documents that urlToLocalFileOrQrc returns a path suitable for passing to QFile when the URL is a local file (file:/ or qrc:) [2]. How to use it: - C++: - QString localPath = QUrl(urlString).toLocalFile; - QML: - Use QQmlFile.urlToLocalFileOrQrc(urlString) (or route the URL string to a small C++ helper that calls QUrl::toLocalFile). This avoids manual parsing/stripping of file:/// and avoids manual percent-decoding; Qt’s URL conversion API is responsible for producing a filesystem-appropriate path across Windows/macOS/Linux [1][2].
Citations:
🏁 Script executed:
# Check if the file exists and get its structure
fd -t f TextureChannelPackerDialog.qmlRepository: fernandotonon/QtMeshEditor
Length of output: 105
🏁 Script executed:
# Read the file to see the function and context
cat -n qml/TextureChannelPackerDialog.qml | head -100Repository: fernandotonon/QtMeshEditor
Length of output: 4666
🏁 Script executed:
# Check lines around 52-57 specifically
cat -n qml/TextureChannelPackerDialog.qml | sed -n '45,70p'Repository: fernandotonon/QtMeshEditor
Length of output: 1154
🏁 Script executed:
# Check the other occurrences mentioned: 509-515, 563-569, 618-623, 675-682
cat -n qml/TextureChannelPackerDialog.qml | sed -n '505,690p'Repository: fernandotonon/QtMeshEditor
Length of output: 8733
🏁 Script executed:
# Search for URL handling patterns in QML files
rg -i "toLocalFile|urlToLocalFile|decodeURI" --type qmlRepository: fernandotonon/QtMeshEditor
Length of output: 52
🏁 Script executed:
# Check if QUrl is imported or used in QML files
rg "QUrl|url.*toString" --type qml | head -20Repository: fernandotonon/QtMeshEditor
Length of output: 138
🏁 Script executed:
# Search for how the project handles file paths from dialogs/drag-drop
rg "drop\.urls|openFileDialog" --type qml -A 3 | head -30Repository: fernandotonon/QtMeshEditor
Length of output: 2031
🏁 Script executed:
# Check if there are any existing utility functions for path handling
rg "function.*[Pp]ath|function.*[Uu]rl" --type qml | head -20Repository: fernandotonon/QtMeshEditor
Length of output: 328
🏁 Script executed:
# Check what MaterialEditorQML provides (look for C++ bindings)
fd -type f -name "*.cpp" -o -name "*.h" | xargs rg "openFileDialog|MaterialEditorQML" | head -15Repository: fernandotonon/QtMeshEditor
Length of output: 1529
🏁 Script executed:
# Look for Qt imports in any QML files to see what's available
rg "^import" --type qml | sort -uRepository: fernandotonon/QtMeshEditor
Length of output: 5274
🏁 Script executed:
# Check if QUrl or any URL conversion utilities are exposed to QML
rg -i "qurl|toLocalFile" --type cpp | head -20Repository: fernandotonon/QtMeshEditor
Length of output: 2018
🏁 Script executed:
# Check what MaterialEditorQML exposes as a module
fd -name "*.h" | xargs rg "MaterialEditorQML|Q_INVOKABLE|qmlRegister" | grep -i "path\|url\|file" | head -20Repository: fernandotonon/QtMeshEditor
Length of output: 2301
🏁 Script executed:
# Look for any helper functions in the MaterialEditorQML or related files
find src -name "*.h" -o -name "*.cpp" | head -5 | xargs ls -laRepository: fernandotonon/QtMeshEditor
Length of output: 440
🏁 Script executed:
# Search for how openFileDialog is implemented
rg "openFileDialog" -A 5 | head -25Repository: fernandotonon/QtMeshEditor
Length of output: 1364
🏁 Script executed:
# Check if there's a custom URL converter or path handler in the C++ code
rg "toLocalFile|normalise|Drop" src/ | head -20Repository: fernandotonon/QtMeshEditor
Length of output: 1682
🏁 Script executed:
# Find MaterialEditorQML module definition
find src -name "*.h" -o -name "*.cpp" | xargs rg "MaterialEditorQML|class.*QML" | grep -v test | head -20Repository: fernandotonon/QtMeshEditor
Length of output: 1943
🏁 Script executed:
# Look for the MaterialEditorQML class definition
rg "class MaterialEditorQML" -A 30 | head -50Repository: fernandotonon/QtMeshEditor
Length of output: 2981
🏁 Script executed:
# Check if Qt provides URL conversion APIs accessible from QML
rg "qmlRegisterType|qmlRegisterSingletonType" src/ | head -10Repository: fernandotonon/QtMeshEditor
Length of output: 1189
🏁 Script executed:
# Try to find if there's any existing URL/path conversion in the Material Editor
rg "QUrl|toLocalFile" src/MaterialEditorQML.h | head -20Repository: fernandotonon/QtMeshEditor
Length of output: 52
🏁 Script executed:
# Find and read MaterialEditorQML.h header
find src -name "MaterialEditorQML.h" -exec cat {} \;Repository: fernandotonon/QtMeshEditor
Length of output: 32644
🏁 Script executed:
# Look for the openFileDialog implementation in the MaterialEditorQML class
rg "openFileDialog" src/MaterialEditorQML -A 10Repository: fernandotonon/QtMeshEditor
Length of output: 132
🏁 Script executed:
# Check how drop.urls are used in the application
rg "drop\.urls" --type qml -B 2 -A 5Repository: fernandotonon/QtMeshEditor
Length of output: 2760
🏁 Script executed:
# Check if there are any examples of using QUrl methods in QML in this codebase
rg "drop\.urls\[" qml/ -A 5Repository: fernandotonon/QtMeshEditor
Length of output: 1740
🏁 Script executed:
# Search for any toLocalFile usage or Qt URL handling in the project
rg "\.toLocalFile\|QUrl\|qrc:" qml/Repository: fernandotonon/QtMeshEditor
Length of output: 52
🏁 Script executed:
# Check if QQmlFile is imported or available in any QML file
rg "QQmlFile|import.*QQml" qml/Repository: fernandotonon/QtMeshEditor
Length of output: 52
🏁 Script executed:
# Look for how the project handles similar URL/path conversions elsewhere
rg "toString\(\)|substring|decodeURI" qml/ | head -15Repository: fernandotonon/QtMeshEditor
Length of output: 824
🏁 Script executed:
# Verify the issue by checking if Windows paths would indeed be problematic
python3 << 'EOF'
# Simulate the current implementation
def current_normalise(url_str):
s = url_str
return s[7:] if s.startswith("file://") else s
# Test cases
test_cases = [
("file:///C:/Users/test/image.png", "Windows absolute path"),
("file:///home/user/image.png", "Linux absolute path"),
("file:///Users/test/image.png", "macOS absolute path"),
("file:///C:/Users/test/my%20image.png", "Windows with encoded space"),
("file:///home/user/my%20image.png", "Linux with encoded space"),
]
print("Current implementation results:")
for url, desc in test_cases:
result = current_normalise(url)
print(f" {desc}:")
print(f" Input: {url}")
print(f" Output: {result}")
print()
EOFRepository: fernandotonon/QtMeshEditor
Length of output: 639
Apply the proposed fix for cross-platform drag-and-drop file path handling.
The current normaliseDroppedPath() implementation has two issues: it leaves Windows paths with a leading slash (e.g., /C:/...), making them invalid for QImageReader, and it does not decode percent-encoded segments like %20. This breaks file resolution when dragging files from Windows Explorer or Finder. The proposed fix addresses both by decoding the URL and handling Windows drive letters correctly. Apply the same fix to all four channel rows (lines 509–515, 563–569, 618–623, 675–682 currently call the same function, so fixing the function covers all cases).
🤖 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 `@qml/TextureChannelPackerDialog.qml` around lines 52 - 57,
normaliseDroppedPath currently returns a raw url string which leaves
percent-encoded segments (e.g. %20) and can produce Windows paths with a leading
slash (e.g. /C:/...), breaking QImageReader; update the
normaliseDroppedPath(url) implementation to decode the URL (use a URL- or
QUrl-based decode method) and strip a leading slash only when followed by a
Windows drive letter pattern (e.g. ^/[A-Za-z]:/) so "file:///" and "%20" are
handled correctly; keep the single function change (used by all four channel
rows) so all callers benefit.
| function refreshPreview() { | ||
| previewDataUrl = MaterialEditorQML.previewPackedTextureChannels( | ||
| redPath, greenPath, bluePath, alphaPath, | ||
| redConstant, greenConstant, blueConstant, alphaConstant, | ||
| invertRed, invertGreen, invertBlue, invertAlpha, | ||
| includeAlpha, 256) | ||
| } | ||
|
|
||
| onRedPathChanged: refreshPreview() | ||
| onGreenPathChanged: refreshPreview() | ||
| onBluePathChanged: refreshPreview() | ||
| onAlphaPathChanged: refreshPreview() | ||
| onRedConstantChanged: refreshPreview() | ||
| onGreenConstantChanged: refreshPreview() | ||
| onBlueConstantChanged: refreshPreview() | ||
| onAlphaConstantChanged: refreshPreview() | ||
| onInvertRedChanged: refreshPreview() | ||
| onInvertGreenChanged: refreshPreview() | ||
| onInvertBlueChanged: refreshPreview() | ||
| onInvertAlphaChanged: refreshPreview() | ||
| onIncludeAlphaChanged: refreshPreview() |
There was a problem hiding this comment.
Debounce preview regeneration to prevent repack storms.
refreshPreview() is invoked on every property mutation, and each preset mutates many properties in sequence. That creates back-to-back repacks and image decodes, which can visibly stall the dialog.
⚡ Proposed fix (single queued refresh)
+ Timer {
+ id: previewRefreshTimer
+ interval: 0
+ repeat: false
+ onTriggered: refreshPreviewNow()
+ }
+
+ function schedulePreviewRefresh() {
+ previewRefreshTimer.restart()
+ }
+
- function refreshPreview() {
+ function refreshPreviewNow() {
previewDataUrl = MaterialEditorQML.previewPackedTextureChannels(
redPath, greenPath, bluePath, alphaPath,
redConstant, greenConstant, blueConstant, alphaConstant,
invertRed, invertGreen, invertBlue, invertAlpha,
includeAlpha, 256)
}
- onRedPathChanged: refreshPreview()
- onGreenPathChanged: refreshPreview()
- onBluePathChanged: refreshPreview()
- onAlphaPathChanged: refreshPreview()
- onRedConstantChanged: refreshPreview()
- onGreenConstantChanged: refreshPreview()
- onBlueConstantChanged: refreshPreview()
- onAlphaConstantChanged: refreshPreview()
- onInvertRedChanged: refreshPreview()
- onInvertGreenChanged: refreshPreview()
- onInvertBlueChanged: refreshPreview()
- onInvertAlphaChanged: refreshPreview()
- onIncludeAlphaChanged: refreshPreview()
+ onRedPathChanged: schedulePreviewRefresh()
+ onGreenPathChanged: schedulePreviewRefresh()
+ onBluePathChanged: schedulePreviewRefresh()
+ onAlphaPathChanged: schedulePreviewRefresh()
+ onRedConstantChanged: schedulePreviewRefresh()
+ onGreenConstantChanged: schedulePreviewRefresh()
+ onBlueConstantChanged: schedulePreviewRefresh()
+ onAlphaConstantChanged: schedulePreviewRefresh()
+ onInvertRedChanged: schedulePreviewRefresh()
+ onInvertGreenChanged: schedulePreviewRefresh()
+ onInvertBlueChanged: schedulePreviewRefresh()
+ onInvertAlphaChanged: schedulePreviewRefresh()
+ onIncludeAlphaChanged: schedulePreviewRefresh()Also applies to: 86-166
🤖 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 `@qml/TextureChannelPackerDialog.qml` around lines 59 - 79, The preview is
being regenerated on every property change (all the onXChanged handlers calling
refreshPreview), causing many back-to-back repacks; change refreshPreview to be
debounced/queued so multiple rapid property changes only trigger a single
rebuild: implement a single queuedRefresh flag or timer inside the same scope as
refreshPreview and have all on...Changed handlers call scheduleRefresh() instead
of refreshPreview() directly; when scheduled, call
MaterialEditorQML.previewPackedTextureChannels with the current redPath,
greenPath, bluePath, alphaPath, redConstant, greenConstant, blueConstant,
alphaConstant, invertRed, invertGreen, invertBlue, invertAlpha, includeAlpha,
256 and clear the queue. Ensure the same debouncing is applied for the other
block referenced (lines 86-166) so presets that mutate many properties cause one
preview update.
| QString MaterialEditorQML::previewPackedTextureChannels(const QString& redPath, | ||
| const QString& greenPath, | ||
| const QString& bluePath, | ||
| const QString& alphaPath, | ||
| double redConstant, | ||
| double greenConstant, | ||
| double blueConstant, | ||
| double alphaConstant, | ||
| bool invertRed, | ||
| bool invertGreen, | ||
| bool invertBlue, | ||
| bool invertAlpha, | ||
| bool includeAlpha, | ||
| int previewSize) | ||
| { | ||
| TextureChannelPacker::PackingSpec spec; | ||
| spec.red.path = redPath; | ||
| spec.red.constantValue = static_cast<float>(redConstant); | ||
| spec.red.invert = invertRed; | ||
| spec.green.path = greenPath; | ||
| spec.green.constantValue = static_cast<float>(greenConstant); | ||
| spec.green.invert = invertGreen; | ||
| spec.blue.path = bluePath; | ||
| spec.blue.constantValue = static_cast<float>(blueConstant); | ||
| spec.blue.invert = invertBlue; | ||
| spec.alpha.path = alphaPath; | ||
| spec.alpha.constantValue = static_cast<float>(alphaConstant); | ||
| spec.alpha.invert = invertAlpha; | ||
| spec.includeAlpha = includeAlpha; | ||
|
|
||
| // Cap preview size so it stays cheap on every input change. The | ||
| // packer scales smaller sources up to the largest source — by | ||
| // forcing the output dimensions here we both make this fast and | ||
| // guarantee a square thumbnail QML can show without flicker. | ||
| const int cappedSize = std::clamp(previewSize, 32, 512); | ||
| spec.outputWidth = cappedSize; | ||
| spec.outputHeight = cappedSize; | ||
|
|
||
| auto r = TextureChannelPacker::pack(spec); | ||
| if (!r.ok) return QString(); | ||
|
|
||
| // Encode as a base64 PNG so QML can display via "data:" URL without | ||
| // touching the filesystem. | ||
| QByteArray bytes; | ||
| QBuffer buf(&bytes); | ||
| buf.open(QIODevice::WriteOnly); | ||
| if (!r.image.save(&buf, "PNG")) return QString(); | ||
| return QStringLiteral("data:image/png;base64,") + bytes.toBase64(); | ||
| } |
There was a problem hiding this comment.
Add breadcrumb tracking for live preview generation.
previewPackedTextureChannels(...) is a user-facing operation and currently has no Sentry breadcrumb, unlike packTextureChannels(...). Please add breadcrumb tracking (potentially throttled to avoid noise during rapid input changes).
Proposed patch
QString MaterialEditorQML::previewPackedTextureChannels(const QString& redPath,
const QString& greenPath,
const QString& bluePath,
const QString& alphaPath,
double redConstant,
double greenConstant,
double blueConstant,
double alphaConstant,
bool invertRed,
bool invertGreen,
bool invertBlue,
bool invertAlpha,
bool includeAlpha,
int previewSize)
{
+ SentryReporter::addBreadcrumb("ui.action", "Preview packed texture channels");
+
TextureChannelPacker::PackingSpec spec;
spec.red.path = redPath;As per coding guidelines, "All user-facing actions and significant operations must be tracked with Sentry breadcrumbs using SentryReporter::addBreadcrumb(category, message)."
🤖 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 2837 - 2885, Add Sentry breadcrumb
tracking to the previewPackedTextureChannels function (similar to
packTextureChannels) by calling SentryReporter::addBreadcrumb("preview", "<short
message>") when a live-preview generation is initiated and optionally on
success; to avoid noise during rapid input changes, throttle these breadcrumbs
(e.g. use a static timestamp or lastBreadcrumbMs check inside
previewPackedTextureChannels to only add a breadcrumb if N ms have elapsed since
the last one). Ensure the breadcrumb message includes identifying info (e.g.
include includeAlpha and previewSize or a short hash of the spec) and place the
call near the start of previewPackedTextureChannels before packing, referencing
TextureChannelPacker::PackingSpec and TextureChannelPacker::pack for context.
|
1 participant
Summary
Wraps up the Phase 5 epic's two remaining "Texture Channel Packing" acceptance items, plus QoL improvements.
What ships
Live preview (slice G2)
256×256 thumbnail next to the channel rows, refreshes on every input change. Implemented as
MaterialEditorQML.previewPackedTextureChannels(...)returning adata:image/png;base64,…URL the QML Image element shows directly — no temp file.previewSizeis clamped to [32, 512] so updates stay cheap.Drag-and-drop (slice G3)
DropAreaon each channel row's source field. Drop a file from Finder/Explorer/the Asset Browser to set that channel's path.file://scheme is stripped automatically.Presets
Three one-click buttons that filename-heuristically wire existing source paths to the right channels:
ao/occlusion/rough/metalsubstrings, maps to R/G/B, alpha clearedmetal/rough, maps to R/G, alpha disabledroughsource, puts it on R, ticks invert-R, alpha disabledPer-row reset (🗑)
Each channel row has a trash-can button. Clears the path, resets the constant to that row's neutral default (0% for R/G/B, 100% for A), unticks invert. Disabled when the row is already at defaults.
Test plan
MaterialEditorQMLTest.PreviewPackedTextureChannels_AllConstantsReturnsDataUrl— base64 PNG decodes back to a 64×64 QImage with expected channel valuesMaterialEditorQMLTest.PreviewPackedTextureChannels_MissingFileReturnsEmpty— broken source path returns empty stringMaterialEditorQMLTest.PreviewPackedTextureChannels_SizeIsClampedToBounds— requestpreviewSize=8produces 32×32 output (lower bound)🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
Documentation