feat: paint tools (Epic #313) — vertex + texture painting in Material Mode - #529
Merged
Conversation
Addresses Epic #313 — closes #317 (texture paint MVP), #318 (texture paint I/O + dirty rect), #319 (bake vertex colors to texture), #321 (docs). - TexturePaintBuffer: pure-data RGBA8 pixel buffer with dirty-rect tracking, brush stamp (radius/strength/falloff matching vertex paint), save/load via QImage. Unit tested. - VertexColorBaker: per-triangle UV-space rasterizer with barycentric color interpolation + edge-dilation pass for seam masking. Unit tested with degenerate / flipped-winding / dilation coverage. - TexturePaintController: QML_SINGLETON owning the active paint session (buffer + live Ogre texture). Hit-tests via the existing EditModeController raycast, recovers barycentric UV via Möller–Trumbore, paints into the CPU buffer, then uploads only the dirty rect to the GPU each stroke. Stroke undo/redo via a snapshot command pushed to UndoManager. - Wires into TransformOperator mouse pipeline alongside vertex paint. - New CLI: `qtmesh bake-vertex-colors <file> -o out.png [--resolution N] [--dilation N] [--json]`. - QML "Texture Paint" section in Inspector: enable mode, create/save/ load texture, bake button, color picker + radius/strength/falloff sliders. - README workflow doc + features bullet; website feature card. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…preview Major refactor that unblocks texture paint in Material Mode and adds several requested workflow features. **Fixes the mode-bounce bug.** Texture paint no longer calls EditModeController::enterEditMode() — that flipped the workspace to Edit Mode and silently disabled the brush via the Material-Mode visibility hook. TexturePaintController now owns its own EditableMesh built from the active entity (via EditableMesh::loadFromEntity), decoupled from any Edit-Mode UI state. **Texture slot picker** in the Material-Mode Texture Paint panel. Enumerates every diffuse-like TUS (`albedo`, `diffuse_map`, or unnamed first-TUS fallback) on every submesh of the selected entity. A ThemedComboBox lets the user choose which slot to paint. Selection changes are observed via SelectionSet::selectionChanged. **Brush tool modes**: Paint, Erase, Fill, Pick (color picker), Smudge. Five-button row at the top of the panel; the current tool is sticky. Fill and Pick fire once per stroke (single-stamp ops). Smudge tracks previous-stamp UV and blends pixels in the brush direction. **Live preview** of the active paint buffer as a 256×256 data-URI image. Regenerated on every dirty-rect flush so strokes show in the panel in real time. Clicking and dragging on the preview paints directly in UV space via TexturePaintController::beginStrokeUV / updateStrokeUV — same brush, same Ogre upload path. **Brush ring overlay on the mesh.** A red ring drawn at the cursor hit point in 3D, plus the same ring driven by 2D-panel hover via a reverse UV → 3D position lookup (findMeshPointForUV). Hovering the texture preview shows where on the model the user is pointing. **Brush radius scaled to mesh size.** The shared toolbar brush radius is in local mesh units; texture paint divides it by the mesh bounding-box extent before applying, so a 0.25 brush is reasonable on both a unit cube and a 100×100 character. **Imported PBR materials**: rebinding logic from the previous commit (walk every TUS named `albedo` / `diffuse_map`) is now slot-aware — the chosen slot wins, and only its texture is rebound. Also extracted floodFill onto TexturePaintBuffer as a pure-data helper so it's testable independently. Added Google Tests for erase behavior, hard-edge brush stamp, and flood-fill region/no-op cases. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- TexturePaintController_test.cpp: reverse-UV lookup against an in-memory unit triangle (uv (0,0) → vertex 0, etc.) and the brush-tool change-signal contract. - TexturePaintBuffer_test.cpp: flood-fill respects 4/255 per-channel tolerance so a near-white pixel is bridged. Also draw the brush ring on the mesh during active strokes (both screen-space and UV-space drivers), so the user sees the paint location continuously, not just on hover. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Debounce previewDataUri regeneration to 60ms intervals. A 1024×1024 PNG + base64 encode is ~150ms of CPU work; doing that on every stroke move made dragging hitchy. Drift between buffer and preview during the debounce window is invisible. - Slot enumeration now includes every non-empty TUS, not just diffuse-like ones. Labelled "sub N — slot" so the user can pick which texture to paint when a material has multiple bindings. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
When paint is enabled and the user changes selection, transparently ensure a session is set up for the new entity. Without this they had to manually click "Create / Attach Texture" each time. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Material Mode is now the home for texture paint (decoupled from Edit Mode). - Documents the new slot picker, brush tools, 2D preview panel, and bidirectional hover. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
closeSession() detaches the brush-ring scene node and drops the manual object before Ogre tears down. The default destructor would leak both during process shutdown. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
256/512/1024/2048/4096 dropdown. The Create / Attach Texture and Bake Vertex Colors buttons both read from it, so the user can pick texture quality before either operation. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Toggle (UV checkbox next to the slot picker) draws every UV-mapped triangle's outline at texture resolution as a transparent PNG layered over the paint preview. Lets the user see exactly where each submesh maps before painting. The overlay is generated lazily on first toggle, then refreshed on session create (resolution change rebuilds it). Cost: ~tris × 3 QPainter drawLine calls at session-create time; ~zero cost during painting since the overlay doesn't change with strokes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Without this, picking a new slot dropped back to 1024 even after the user explicitly created the session at 2048+. We now snapshot the current buffer width and pass it through to the post-switch ensurePaintableTexture call. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Otherwise the stale wireframe sits in the QML preview even after the buffer is empty, confusing the user about whether a session is active. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The ring multiplied the toolbar radius by the mesh bounding-box extent, which made it grow proportionally to mesh size — a 0.25 local-unit brush on a 100-unit character drew a 10-unit ring on the surface. The toolbar slider is already in local units; just plot the ring at that radius directly. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The controller's destructor calls closeSession() which touches the Ogre SceneManager. Without an explicit kill() before Manager::kill() in MainWindow::~MainWindow, the static singleton destructor runs at process exit — after Ogre is gone — and segfaults (exit code 139). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ebind Three big behavior changes to fix: - "Model goes textureless on slot select" — was caused by a fallback to all-white when convertToImage failed, plus a too-broad rebind that clobbered every diffuse-slot TUS regardless of which submesh the user picked. Now only TUSes pointing at the user's original slot texture get rebound. - "Crashes on second slot switch" — m_boundSlots stored raw material pointers that could dangle across close→reopen cycles. Switched to storing material name + looking up via MaterialManager on restore. - "Painting not visible" — added explicit isLoaded()/load() before convertToImage so a deferred-load texture actually has pixels available to copy into the paint buffer. Switched to TU_DYNAMIC_WRITE_ONLY_DISCARDABLE for the GL upload path. Adds Sentry breadcrumbs for: rebound TUS count, source texture name, and blit failures so we can diagnose any remaining cases. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…eadback
Major UX change to fix "model goes textureless on paint enable":
- setTexturePaintEnabled() no longer auto-creates a session. Toggling
the brush is now non-destructive — the model's render is unchanged
until the user actually paints (which lazily creates a session in
beginStroke). Disabling paint also closes the session, restoring
the original TUS bindings.
- Texture readback now tries three strategies in order:
1. TextureManager::getByName → convertToImage (works for most
in-memory textures Ogre has uploaded).
2. Texture::getBuffer()->blitToMemory (works for GPU-resident
textures whose source Image was discarded post-upload — the
common case for imported meshes).
3. Ogre::Image::load(name, group) — works when the texture name
is also a filename in a registered resource location.
- If all three fail, we now start from a blank buffer with a
breadcrumb explaining why (rather than the previous behavior of
silently binding a white texture which made the user think the
model went textureless).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two distinct user-reported breakages, fixed together: 1. Vertex paint button stopped working — visibility hook hid the brush outside Material Mode, so Edit Mode had no toolbar affordance for vertex paint. Now the brush appears in BOTH Material Mode AND Edit Mode and dispatches to the right kind of paint based on current mode (texture vs vertex). 2. Enabling texture paint wiped the model's diffuse — the toolbar toggle path called refreshSlots() → ensurePaintableTexture() → immediate rebind, even though we'd "removed" the auto-create from setTexturePaintEnabled itself. The auto-create was hiding inside refreshSlots. Now refreshSlots is pure metadata (no side effects) AND createOgreTextureFromBuffer defers the material rebind until the first dirty-rect flush in flushDirtyToOgre. Toggling the brush button is now non-destructive; the model only changes when the user actually paints something visible. Explicit user actions (Bake Vertex Colors → Texture, Load Texture) still rebind immediately since the user wants to see the result. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Hover and brush-ring overlay queries need the EditableMesh to run a UV hit-test. Before this change the mesh was only built when a paint session was created — which we now defer to the first stroke — so hover queries silently failed and the user saw "no brush circle on hover". EditableMesh is now built immediately when paint is enabled and an entity is selected (or when selection changes), independently of whether a GPU session exists. It's a pure CPU mirror of mesh data and has no rendering side effects. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Three symptoms with one root cause: the OgreWidget had mouse tracking disabled outside Edit-Mode vertex paint. Without tracking, mouse-move events only fired when LMB was held, so: - No brush-ring overlay on hover (updateMeshHover never reached). - No pointer feedback (cursor stayed default arrow). - "First click does nothing" (LMB press routes to box-select if the cursor wasn't being tracked over the mesh). Enable mouse tracking AND set a crosshair cursor whenever any paint mode is on (vertex OR texture), wired to fire via the same onSelectionChanged hook that already updates gizmos. Also added a TexturePaintController::texturePaintChanged → onSelectionChanged connection so the tracking flag updates as soon as the user toggles the brush. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…hover-side mesh build Three user-reported issues addressed: 1. "Brush stops working after disabling texture paint" — when the workspace mode changes, refreshPaintBrushVisibility silently sets both paint controllers off but left the button visually checked. Next click toggled "on→off" rather than enabling the new mode's paint. Now reset the button's checked flag along with the controllers. 2. "Model goes textureless on first paint" — the 3-strategy texture readback I added was still failing for some textures, leaving the buffer white. Added a 4th strategy (QImage from raw filesystem path) and a 2nd-strategy improvement that reads in the texture's native format and converts via Ogre::PixelUtil — some Metal/GL drivers reject the format-mismatch path used by blitToMemory(RGBA). Also added a fmt+size breadcrumb so we can diagnose remaining failure cases. 3. "Brush ring doesn't appear on hover" — the EditableMesh wasn't built on first hover when the user enabled paint before selecting the mesh. updateMeshHover now builds it lazily on first call so the brush ring shows as soon as the cursor touches the surface. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Yellow body + black brush patch in the latest screenshot was Ogre's "missing texture" fallback (yellow) plus our manual paint texture showing transparent black around the painted strokes. Two problems in one symptom: my readback strategies couldn't read the original texture (so the buffer was blank), and the rebind made the model sample our blank+strokes texture instead of the original. New approach: when flushing dirty rects, try blitting them **directly into the original texture's GPU buffer**. No rebind, no readback — just modify the existing pixels in place. Falls back to the manual-texture rebind path only if blit-to-original throws (e.g. the texture is read-only). This sidesteps both readback failures AND rebind ordering issues — the existing material binding keeps working, and we just nudge the sampled pixels. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ntity rebind refresh Per user request: the paint brush is now exclusive to Material Mode (removed from Edit Mode), and supports BOTH vertex and texture painting via a new "Target" picker in the Texture Paint panel. - PaintTarget enum (TargetTexture / TargetVertex) on TexturePaintController. - beginStroke / updateStroke dispatch by target. Vertex paint re-uses EditModeController::applyVertexColorBrush (static) against the controller's own EditableMesh and commits via m_paintMesh->commitVertexColorsToEntity — no Edit Mode required. - Toolbar brush button is Material-Mode-only and writes the single TexturePaintController state. EditModeController vertex paint flag is no longer touched by the toolbar. Also addresses the "model loses texture on paint" issue from the last round: after rebindEntityDiffuseToPaintTexture, call SubEntity::setMaterialName(name) on each subentity to force the runtime material override pointer to refresh. The previous in-place blit path (write to original GPU texture) is still the preferred fast path; this is the rebind-fallback safety net. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…rtex Replaced the enable checkbox + separate target row with a single 3-button switch: Off / Vertex / Texture. Clicking Vertex or Texture both enables paint and sets the target in one action. Clicking Off disables paint. Default target is now Vertex. Also wired paintTargetChanged through the QML Connections block so the switch buttons highlight reactively, and reworded the description text to cover both paint kinds. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two changes addressing the user-reported crash during texture paint: 1. Re-resolve m_originalTexture from TextureManager by name on every flush. RTSS material reload (which can happen between strokes for various reasons) can invalidate the cached TexturePtr, so calling getBuffer() on a stale handle segfaults. Looking up by name each time keeps the handle fresh. 2. Removed the SubEntity::setMaterialName refresh call I added in the previous round — it was likely causing material rebind during render, invalidating pointers we held. 3. Safer destructor: if Ogre::Root is already gone, skip closeSession() and null out raw handles directly. Avoids touching destroyed Ogre singletons at process exit. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The crash during texture paint was most likely due to two issues: 1. bulkPixelConversion to a compressed (DXT/BC) format from RGBA8 can crash on Metal because there's no built-in CPU compressor. Now we only run the in-place blit when the original texture's format is a known plain uncompressed RGBA/BGR variant. For compressed textures we fall through to the manual-texture + rebind path. 2. rebindEntityDiffuseToPaintTexture ran mat->compile() + mat->reload() on the same call stack as the mouse-move event handler. Material reload can destroy/recreate render passes mid- render. Now deferred via QTimer::singleShot(0) so the rebind happens on the next event-loop tick, off the mouse-handler stack. Also removed leftover duplicate "successful blit" code that was running both in the format-aware and non-aware paths. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Reverted both the deferred-rebind and the in-place-blit optimizations — they introduced races/crashes during active strokes. Restored the eager rebind: createOgreTextureFromBuffer at session create binds the manual paint texture to the model's diffuse TUSes before any stroke fires. No mid-stroke material reloads, no re-entrant render hazards, no compressed-format conversions. The user reported "material painting working well" prior to those optimizations — this returns to that baseline. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two changes targeting "texture not displaying" + the crash: - Rebind uses tusN->setTexture(TexturePtr) instead of setTextureName(name). Direct binding avoids resource-group name resolution ambiguity that can fail on macOS Metal and leave the TUS sampling Ogre's missing-texture fallback (yellow). - Restored the in-place blit fast path so painting modifies the original texture in place when possible (best case: no rebind needed at all). The compressed-format skip from the previous commit is preserved so we don't crash trying to bulkPixelConversion to DXT/BC. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…n Metal) After rebind to the manual paint texture, strokes were blitting only the dirty rect via blitFromMemory(box). On macOS Metal that sub- rect path sometimes produces no visible update — the upload appears to land in the GPU texture but the renderer keeps sampling the previous content. Switched to uploading the full buffer per flush. Heavier per stroke (~4 MB at 1024² vs ~hundreds of KB for a typical brush stamp) but the model updates reliably. The preview debounce already shields CPU cost on the QML side. Also added a guard so the in-place blit path is skipped once the rebind has fired — without this, strokes were writing to the original texture (which is no longer bound) and never to the new paint texture (which is bound). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…e panel Two UX issues addressed: 1. **Lag while painting in the 2D preview.** The full-buffer blitFromMemory (~4 MB at 1024²) was running on every QML mouse-move at 100+ Hz. Debounce: schedule one coalesced GPU upload per ~16 ms (one render tick) via QTimer. Dirty rect keeps accumulating between flushes so no pixels are lost. endStroke() forces an immediate flush so the final stroke is visible without waiting for the timer. 2. **Stroke gets stuck if mouse released outside the panel.** The MouseArea's onReleased only fires when the release happens inside the area, so `dragging` stayed true and subsequent hover events kept painting. Now: use the live `pressed` property as truth — if pressed becomes false but dragging is still true, finalize the stroke immediately. Also added onCanceled and onExited cleanup paths. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ndant panel brush controls User reported lag during painting on either surface (2D preview or 3D mesh). Two cross-sync calls were the culprit: - 3D-mesh stroke called findMeshPointForUV(uv) to redraw the brush ring on the mesh AFTER hit-testing. Two mesh walks per move. - 2D-panel stroke called findMeshPointForUV(uv) to draw the brush ring on the mesh. One full mesh walk per move on top of the paintBrush + GPU flush. Both rings are hover feedback — during an active stroke the user is already seeing live paint strokes, so the ring is redundant. Now skipped during strokes (hover-only). 3D and 2D updates still happen independently; the user's preference is "update as possible, drop sync if needed, never lag the paint." Also removed the redundant "Paint Brush" CollapsibleSection from the right panel — brush color/radius/strength/falloff live solely on the toolbar brush popup now. The component definition is left as a dormant fallback to avoid churning a large unrelated diff. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The 2D preview panel is fixed at 256×256, but we were PNG-encoding the full-resolution buffer (1024² or up to 4096²) on every refresh. Encoding a 4 MB image to base64 burns ~50–100 ms of main-thread time and was contributing to paint lag. Now scale the source buffer down to 256×256 (Qt::FastTransformation — cheapest, just nearest-neighbour) before encoding. ~16× less work per refresh at the default 1024². Visual fidelity loss is invisible since the preview is rendered into a 256×256 viewport anyway. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
User reported "stop following the mouse right after I start moving" in the 2D preview painting flow. The onPositionChanged handler was guarding with `pressed && (m.buttons & Qt.LeftButton)`. Some macOS Qt6 mouse-move events ship with m.buttons==0 mid-drag, which made the guard fail and the stroke flip into hover mode. Now we rely on the `dragging` flag alone — set by onPressed, cleared by onReleased / onCanceled. Drag-off-and-back-on the panel works too (onReleased fires even when released outside since the press grabs the mouse). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… was eating the drag User reported "only short strokes" in the 2D preview painting. Root cause: the entire Inspector panel lives inside a ScrollView (line 78), and ScrollView's internal Flickable steals mouse-drag gestures after a few pixels — interpreting them as scroll. The brush MouseArea would receive onCanceled, the dragging flag cleared, and the stroke silently ended. `preventStealing: true` on the paint MouseArea keeps the press grab locked here regardless of parent flickable gestures. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
User reported "crashed while painting in the model" — the mouse press path was calling createOgreTextureFromBuffer which immediately ran mat->compile() and mat->reload() to make the new texture binding stick. That compile/reload races with Ogre's render thread and segfaulted mid-paint when fast strokes hit the eager rebind path. Now: session create allocates the GPU texture but skips the material rebind. The rebind happens on the next event-loop tick via the existing deferred-rebind code in doFlushDirtyToOgre (which uses QTimer::singleShot(0) to defer off the mouse stack). Also added a transient copy of the buffer before blitFromMemory so the source pixels don't get mutated by the next stroke if Metal's upload is asynchronous. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Both 3D-mesh stroke and 2D-panel stroke now redraw the brush ring on the model during active painting (was hover-only). Costs one extra mesh walk per move (~thousands of triangles at 100+ Hz) but the user requested a perf test of the cross-surface sync. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…rsist paint on stroke end Three user-reported issues: 1. Texture painting now requires one click to start. setTexturePaintEnabled now pre-creates the paint session (for TargetTexture only) when toggled on, so the preview thumbnail populates immediately and the first stroke doesn't have to do session setup work. 2. Loading a texture didn't update the panel thumbnail. loadPaintBuffer now calls refreshPreviewUri() after writing the buffer + creating the Ogre texture. 3. Painted pixels didn't persist on export. Added bakeToOriginalFile() which writes m_buffer back to the original texture's on-disk file (resolves the path by searching every registered FileSystem resource location). Called automatically at endStroke for TargetTexture, so each stroke persists. Silently no-ops for embedded textures (no source file). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…export The disk-bake path (bakeToOriginalFile) only works for textures that originated from a registered file-system location. FBX- embedded textures (like Boss_diffuse.png inside Rumba Dancing.fbx) have no disk source — so bakeToOriginalFile silently no-op'd and exports kept the un-painted bytes. Now, on every stroke end for TargetTexture, we also encode the current buffer as PNG and store it in EmbeddedTextureCache under the original texture name. FBX export's fbxResourceBytes::read queries the cache first, so the painted PNG ends up in the exported file's Video.Content section. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
TexturePaintController_test.cpp uses findMeshPointForUV to verify the reverse-UV → 3D math against an in-memory unit triangle, but the helper was declared private (used to only be a hover implementation detail). Moved the declaration to the public section so the test compiles. Behavior unchanged. Caught during pre-PR full-test-target build — CI Linux runner would have failed the same way. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (11)
📝 WalkthroughWalkthroughThis PR implements a complete paint tools feature, adding texture painting and vertex-color baking to the mesh editor. The changes introduce a CPU-backed RGBA pixel buffer with brush/fill operations, a controller managing paint sessions with Ogre texture synchronization, QML UI components, transform operator integration for mouse input, a CLI baking command, and full documentation. ChangesPaint Tools Feature Implementation
Sequence Diagram(s)sequenceDiagram
participant User as User
participant QML as QML UI
participant Controller as TexturePaintController
participant Buffer as TexturePaintBuffer
participant Ogre as Ogre Texture
participant Mesh as EditableMesh
User->>QML: Click "Ensure Paintable Texture"
QML->>Controller: ensurePaintableTexture(resolution)
Controller->>Mesh: create from selected entity
Controller->>Buffer: initialize width×height RGBA
Controller->>Ogre: createOgreTextureFromBuffer()
Controller->>Ogre: rebindEntityDiffuseToPaintTexture()
User->>QML: Stroke on 2D preview
QML->>Controller: beginStrokeUV(u, v)
Controller->>Buffer: paintBrush(uv, radius, color, strength, falloff)
Buffer-->>Buffer: update dirtyRect
User->>QML: Release stroke
QML->>Controller: endStrokeUV()
Controller->>Controller: scheduleGpuUpload (debounced)
Controller->>Ogre: doFlushDirtyToOgre()
Ogre-->>User: live texture update in 3D viewport
User->>QML: Click "Bake Vertex Colors → Texture"
QML->>Controller: bakeVertexColorsToTexture(resolution, dilation)
Controller->>Mesh: iterate vertices
Controller->>Buffer: VertexColorBaker::bake()
Buffer->>Buffer: rasterizeTriangle + dilate
Controller->>Ogre: update paint texture with baked result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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 |
![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: 796b601d3f
ℹ️ 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".
Comment on lines
+952
to
+953
| if (!hasActiveSession()) { | ||
| if (!ensurePaintableTexture(m_buffer.width() > 0 ? m_buffer.width() : 1024)) { |
There was a problem hiding this comment.
This branch only checks hasActiveSession() before starting a texture stroke, but hasActiveSession() does not verify that the session belongs to the currently selected entity. After selecting a different mesh, refreshSlots() updates m_paintMesh to the new entity, so strokes can hit-test on mesh B while still writing into mesh A’s old texture/session state, which can paint the wrong asset or overwrite the previous selection’s texture unexpectedly.
Useful? React with 👍 / 👎.
Comment on lines
+83
to
+84
| outX = static_cast<int>(std::floor(uv.x * static_cast<float>(m_width))); | ||
| outY = static_cast<int>(std::floor(uv.y * static_cast<float>(m_height))); |
There was a problem hiding this comment.
uvToPixel uses floor(uv * size) without clamping, so uv == 1.0 maps to x == width / y == height (out of bounds). The preview panel clamps mouse UVs to [0,1], so clicks/drags on the right or bottom edge can no-op for tools that rely on this conversion (e.g. fill/picker/smudge sampling), making border texels unreachable.
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: 7
🧹 Nitpick comments (5)
src/TexturePaintController.h (1)
102-104: 💤 Low valueSingleton accessor naming diverges from the project convention.
The other singletons in this codebase (
Manager,SelectionSet,TransformOperator,UndoManager,EditModeController, …) exposegetSingleton()/getSingletonPtr()and akill()teardown. This class usesinstance()plus a separateqmlInstance()factory. Mixing the two patterns makes it harder for readers to know which accessor to use for a given singleton, and theinstance()accessor isn't grep-friendly with the existing call sites.If
qmlInstanceneeds to stay separate for QML registration, consider at least renaminginstance()→getSingleton()/getSingletonPtr()so the rest of the codebase can pattern-match.As per coding guidelines: "Singleton classes (Manager, SelectionSet, TransformOperator) must run on the main thread. Access via ClassName::getSingleton() or ClassName::getSingletonPtr(). Destroy with ClassName::kill()."
🤖 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/TexturePaintController.h` around lines 102 - 104, Rename the singleton accessor to follow project convention: replace TexturePaintController::instance() with TexturePaintController::getSingleton() (or getSingletonPtr() if returning pointer) and keep TexturePaintController::kill() as-is; leave qmlInstance(QQmlEngine*, QJSEngine*) as a separate QML factory but ensure it calls/gets the singleton via the new getSingleton/getSingletonPtr accessor so callers can grep for the standard name and the class conforms to the project's singleton pattern.src/TexturePaintController.cpp (1)
1242-1251: 💤 Low value
QApplication::processEvents()before each native dialog is risky.Calling
processEvents()on the main thread immediately before showing a modal dialog is the classic source of Qt reentrancy bugs: any queued mouse-move / mouse-release event for the same controller can fire mid-call, potentially completing or restarting a stroke right when the user clicks "Save" or "Pick color". The texture-paint controller already has a stroke state machine (m_strokeActive,m_strokeJustBegan, debounce timers) and the wrong interleaving here can leave it in an inconsistent state.The original justification for
processEvents()in Qt code is usually "flush a paint event before showing a dialog so the UI doesn't look frozen" — that's almost always unnecessary with native dialogs since they spin their own event loop.Consider just removing the three
QApplication::processEvents()calls. If a specific repaint is needed, preferQWidget::repaint()on the relevant widget.Also applies to: 1266-1269, 1339-1348
🤖 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/TexturePaintController.cpp` around lines 1242 - 1251, Remove the unsafe QApplication::processEvents() calls in TexturePaintController before showing native dialogs (e.g., the call preceding QFileDialog::getSaveFileName and the other two occurrences) so you don't risk reentrancy into the stroke state machine (m_strokeActive, m_strokeJustBegan, debounce timers); if a repaint is actually required before opening the dialog, call repaint() on the specific widget (e.g., the painting canvas or parent QWidget) instead of processEvents(), and ensure no code mutates stroke state around the dialog show calls.src/VertexColorBaker.cpp (1)
86-128: 💤 Low valueStylistic:
dx = 2; dy = 2;to break nested loops is hard to read.The intent ("found a filled neighbor — stop scanning") is correct and the math works (post-increment lands both at 3, exiting both
<= 1checks), but a future maintainer adding another statement below this block will silently break the early-exit. A small lambda / helper or agoto found;pattern reads better and is robust to surrounding edits.♻️ Alternative
- for (int dy = -1; dy <= 1; ++dy) { - for (int dx = -1; dx <= 1; ++dx) { - if (dx == 0 && dy == 0) continue; - ... - nextCov[idx] = 1; - ++flippedThisPass; - dx = 2; // break inner loops - dy = 2; - } - } + bool filled = false; + for (int dy = -1; dy <= 1 && !filled; ++dy) { + for (int dx = -1; dx <= 1 && !filled; ++dx) { + if (dx == 0 && dy == 0) continue; + ... + nextCov[idx] = 1; + ++flippedThisPass; + filled = true; + } + }🤖 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/VertexColorBaker.cpp` around lines 86 - 128, The nested-loop early-exit in VertexColorBaker.cpp currently uses "dx = 2; dy = 2" to break out when a filled neighbor is found (inside the loops iterating x,y,dx,dy and operating on nextPixels/nextCov/coverage/pixels), which is fragile and hard to read; replace that trick with a clear control flow: either introduce a small lambda/helper (e.g., findFilledNeighbor) or use a labeled break/goto (e.g., label "found") to immediately jump out of the dx/dy loops when you copy the 4 RGBA bytes into nextPixels and set nextCov[idx] and increment flippedThisPass, keeping the rest of the logic (updating pixels = nextPixels; coverage = nextCov; totalFlipped and buffer.markDirty) unchanged so behavior is identical but readability and robustness are improved.src/TexturePaintBuffer.cpp (1)
181-200: ⚡ Quick winDead bounding-box tracking in
floodFill.
tx0,tx1,ty0,ty1are initialized on line 181 and updated on lines 192–193, but the function returnsaffectedwithout using them. The dirty-rect bookkeeping is already handled per-pixel bysetPixel()on line 190, so this local AABB is unused.Either drop the variables, or replace the per-pixel
setPixelcalls (each of which re-runsexpandDirtyand bounds-checks) with raw byte writes plus a singleexpandDirty(tx0, ty0, tx1, ty1)at the end — the latter is also faster on large fills.♻️ Suggested cleanup (minimum: drop dead writes)
- std::vector<uint8_t> visited(static_cast<size_t>(m_width) * m_height, 0); - int affected = 0; - int tx0 = m_width, tx1 = 0, ty0 = m_height, ty1 = 0; - while (!stack.empty()) { + std::vector<uint8_t> visited(static_cast<size_t>(m_width) * m_height, 0); + int affected = 0; + while (!stack.empty()) { auto [x, y] = stack.back(); stack.pop_back(); if (x < 0 || y < 0 || x >= m_width || y >= m_height) continue; const size_t idx = static_cast<size_t>(y) * m_width + x; if (visited[idx]) continue; if (!sameColor(pixel(x, y))) continue; visited[idx] = 1; setPixel(x, y, fill); ++affected; - tx0 = std::min(tx0, x); ty0 = std::min(ty0, y); - tx1 = std::max(tx1, x + 1); ty1 = std::max(ty1, y + 1); stack.push_back({x + 1, y}); stack.push_back({x - 1, y}); stack.push_back({x, y + 1}); stack.push_back({x, y - 1}); } return affected;🤖 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/TexturePaintBuffer.cpp` around lines 181 - 200, The local bounding-box vars tx0/tx1/ty0/ty1 in floodFill are computed but never used; remove them to eliminate dead state and keep the existing per-pixel setPixel calls, or for a performance win replace per-pixel setPixel calls inside floodFill with direct pixel writes to the buffer and at the end call expandDirty(tx0, ty0, tx1, ty1) once; locate floodFill, remove tx0/tx1/ty0/ty1 and their updates if dropping them, or if optimizing, ensure you replicate setPixel's write semantics (raw buffer write + visited marking) and then call expandDirty with the accumulated AABB before returning affected.qml/PropertiesPanel.qml (1)
952-1073: 💤 Low valueOptional: prune the dormant
paintBrushComponentin a follow-up.The 100+-line dormant block is intentionally retained per the header comment, but if/when this area sees its next non-trivial diff, deleting it (and relying on git history if the brush popup needs revival) would remove a maintenance footgun — future contributors will read the bindings to
TexturePaintController.texturePaintColor/Radius/Strength/Falloffand assume they're live wiring.🤖 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/PropertiesPanel.qml` around lines 952 - 1073, The dormant paintBrushComponent block (Component { id: paintBrushComponent ... }) should be removed to avoid misleading bindings to TexturePaintController.texturePaintColor/texturePaintRadius/texturePaintStrength/texturePaintFalloff; delete the entire Component definition (or move it to a clearly labeled legacy file) and ensure no other UI references expect paintBrushComponent to exist, leaving the toolbar popup as the single source of brush settings.
🤖 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/mainwindow.cpp`:
- Around line 1406-1409: When mode switches force-disable paint states (calls to
EditModeController::instance()->setVertexPaintEnabled(false) and
TexturePaintController::instance()->setTexturePaintEnabled(false) and unchecking
vertexPaintButton), add a telemetry breadcrumb via SentryReporter::addBreadcrumb
with a clear category like "paint.mode-change" and a message describing which
paint modes were disabled (e.g., "vertex paint disabled due to mode change",
"texture paint disabled due to mode change"); place the calls immediately after
the corresponding set*Enabled(false) and button->setChecked(false) lines so the
forced state changes are logged for debugging/telemetry.
In `@src/TexturePaintBuffer.h`:
- Around line 12-126: The header docs incorrectly state V is flipped; update all
docstrings in TexturePaintBuffer to reflect that UV origin is top-left and V is
not inverted (UV (0,0) → pixel (0,0), uv.y increases downward), e.g. change the
class comment, the paintBrush param note, and the uvToPixel comment to say "UV
origin = top-left (no V flip)" or similar so they match the uvToPixel
implementation and existing cpp comment and tests; ensure references to "V is
flipped" are removed or replaced with "V is top-left / not flipped" and keep the
note that callers must clamp to bounds.
In `@src/TexturePaintController_test.cpp`:
- Line 20: Replace the silent skip in the test fixture setup with hard
assertions so CI fails if prerequisites are missing: in the fixture SetUp (where
tryInitOgre() is called) change the conditional GTEST_SKIP() usage to
ASSERT_TRUE(tryInitOgre()) and likewise ensure any check for mesh availability
uses ASSERT_TRUE(canLoadMeshFiles()) so missing/broken Ogre environment causes
test failure rather than silent skipping; locate these calls around the
TexturePaintController test fixture setup (tryInitOgre, canLoadMeshFiles) and
update them accordingly.
In `@src/TexturePaintController.cpp`:
- Around line 1178-1197: The code currently PNG-encodes the full m_buffer twice
on the main thread for texture targets (bakeToOriginalFile() writes disk PNG,
then the try-block builds a QImage and encodes again before calling
EmbeddedTextureCache::store), causing heavy hitches; change bakeToOriginalFile()
to return the encoded QByteArray (or the disk path) so you can reuse that
encoded data for EmbeddedTextureCache::store and avoid the second encode, or
skip calling EmbeddedTextureCache::store when bakeToOriginalFile() succeeded and
produced an on-disk file (letting the FBX exporter read that file);
additionally, move the encoding/cache write work off the GUI thread (e.g., use
QtConcurrent::run) so TexturePaintController::bakeToOriginalFile / the
QImage->PNG encode do not run on the main thread.
- Around line 822-830: The lambda scheduled by QTimer::singleShot captures a raw
Ogre::Entity* (ent) and can call rebindEntityDiffuseToPaintTexture(ent) on a
freed pointer; fix by (1) registering TexturePaintController to
Manager::sceneClearing() and clearing m_paintMeshEntity there (so the pointer is
nulled before scene teardown) and (2) additionally validate the entity still
exists in SelectionSet (call SelectionSet::contains(ent)) inside the lambda
before calling rebindEntityDiffuseToPaintTexture(ent) and only proceed if both
m_paintMeshEntity == ent and SelectionSet::contains(ent) are true; keep
m_rebindScheduled semantics intact.
In `@src/TransformOperator.cpp`:
- Around line 632-639: The crosshair cursor/hover update should only occur when
the Select tool is routing paint strokes: restrict the paint-on check to also
verify the current tool/state is TS_SELECT (same guard used by beginStroke()),
so modify the logic around m_pActiveWidget->setCursor(...) to include that
TS_SELECT check (and apply the same change to the duplicate block around the
code at the later occurrence near the 1303–1313 region); use the existing
TransformOperator/BeginStroke routing state (e.g., beginStroke()/TS_SELECT) to
gate paintOn so hover work and cursor changes only run when Select is active.
In `@src/VertexColorBaker.cpp`:
- Around line 167-182: The coverage mask is being rebuilt by comparing pixels to
options.background which can drop legitimate rasterized pixels; modify
rasterizeTriangle to accept an optional coverage out-param (e.g., a reference to
the coverage vector) and set coverage[pixelIndex] = 1 at the same time it writes
the pixel, then remove the full-image scan in bake() that compares px[] to
bgR/bgG/bgB/bgA and instead rely on the rasterizer-populated coverage before
calling dilate(buffer, coverage, options.dilationPixels); update references to
rasterizeTriangle and the coverage vector accordingly.
---
Nitpick comments:
In `@qml/PropertiesPanel.qml`:
- Around line 952-1073: The dormant paintBrushComponent block (Component { id:
paintBrushComponent ... }) should be removed to avoid misleading bindings to
TexturePaintController.texturePaintColor/texturePaintRadius/texturePaintStrength/texturePaintFalloff;
delete the entire Component definition (or move it to a clearly labeled legacy
file) and ensure no other UI references expect paintBrushComponent to exist,
leaving the toolbar popup as the single source of brush settings.
In `@src/TexturePaintBuffer.cpp`:
- Around line 181-200: The local bounding-box vars tx0/tx1/ty0/ty1 in floodFill
are computed but never used; remove them to eliminate dead state and keep the
existing per-pixel setPixel calls, or for a performance win replace per-pixel
setPixel calls inside floodFill with direct pixel writes to the buffer and at
the end call expandDirty(tx0, ty0, tx1, ty1) once; locate floodFill, remove
tx0/tx1/ty0/ty1 and their updates if dropping them, or if optimizing, ensure you
replicate setPixel's write semantics (raw buffer write + visited marking) and
then call expandDirty with the accumulated AABB before returning affected.
In `@src/TexturePaintController.cpp`:
- Around line 1242-1251: Remove the unsafe QApplication::processEvents() calls
in TexturePaintController before showing native dialogs (e.g., the call
preceding QFileDialog::getSaveFileName and the other two occurrences) so you
don't risk reentrancy into the stroke state machine (m_strokeActive,
m_strokeJustBegan, debounce timers); if a repaint is actually required before
opening the dialog, call repaint() on the specific widget (e.g., the painting
canvas or parent QWidget) instead of processEvents(), and ensure no code mutates
stroke state around the dialog show calls.
In `@src/TexturePaintController.h`:
- Around line 102-104: Rename the singleton accessor to follow project
convention: replace TexturePaintController::instance() with
TexturePaintController::getSingleton() (or getSingletonPtr() if returning
pointer) and keep TexturePaintController::kill() as-is; leave
qmlInstance(QQmlEngine*, QJSEngine*) as a separate QML factory but ensure it
calls/gets the singleton via the new getSingleton/getSingletonPtr accessor so
callers can grep for the standard name and the class conforms to the project's
singleton pattern.
In `@src/VertexColorBaker.cpp`:
- Around line 86-128: The nested-loop early-exit in VertexColorBaker.cpp
currently uses "dx = 2; dy = 2" to break out when a filled neighbor is found
(inside the loops iterating x,y,dx,dy and operating on
nextPixels/nextCov/coverage/pixels), which is fragile and hard to read; replace
that trick with a clear control flow: either introduce a small lambda/helper
(e.g., findFilledNeighbor) or use a labeled break/goto (e.g., label "found") to
immediately jump out of the dx/dy loops when you copy the 4 RGBA bytes into
nextPixels and set nextCov[idx] and increment flippedThisPass, keeping the rest
of the logic (updating pixels = nextPixels; coverage = nextCov; totalFlipped and
buffer.markDirty) unchanged so behavior is identical but readability and
robustness are improved.
🪄 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: ef56b74a-c61b-4026-9fec-0d18ee5f1224
📒 Files selected for processing (20)
README.mdqml/PropertiesPanel.qmlsrc/CLIPipeline.cppsrc/CLIPipeline.hsrc/CMakeLists.txtsrc/TexturePaintBuffer.cppsrc/TexturePaintBuffer.hsrc/TexturePaintBuffer_test.cppsrc/TexturePaintController.cppsrc/TexturePaintController.hsrc/TexturePaintController_test.cppsrc/TransformOperator.cppsrc/TransformOperator.hsrc/VertexColorBaker.cppsrc/VertexColorBaker.hsrc/VertexColorBaker_test.cppsrc/main.cppsrc/mainwindow.cpptests/CMakeLists.txtwebsite/src/data/content.js
Comment on lines
+632
to
+639
| // Crosshair cursor while any paint mode is on so the user | ||
| // gets clear feedback that clicks will paint, not select. | ||
| const bool paintOn = | ||
| (EditModeController::instance()->isEditModeActive() | ||
| && EditModeController::instance()->vertexPaintEnabled()) | ||
| || TexturePaintController::instance()->texturePaintEnabled(); | ||
| m_pActiveWidget->setCursor(paintOn ? Qt::CrossCursor : Qt::ArrowCursor); | ||
| } |
There was a problem hiding this comment.
Gate paint cursor/hover updates to Select tool to match stroke routing.
beginStroke() is restricted to TS_SELECT (Line 1028), but crosshair cursor and hover updates currently run even in other transform states when paint is enabled. That creates misleading input feedback and unnecessary hover work while translating/rotating/scaling.
Suggested patch
- const bool paintOn =
+ const bool paintOn =
+ (mTransformState == TS_SELECT) &&
(EditModeController::instance()->isEditModeActive()
&& EditModeController::instance()->vertexPaintEnabled())
|| TexturePaintController::instance()->texturePaintEnabled();
m_pActiveWidget->setCursor(paintOn ? Qt::CrossCursor : Qt::ArrowCursor);
@@
- if (mTexturePaintDragActive && (e->buttons() & Qt::LeftButton) && m_pActiveWidget)
+ if (mTransformState == TS_SELECT
+ && mTexturePaintDragActive
+ && (e->buttons() & Qt::LeftButton)
+ && m_pActiveWidget)
{
texPaint->updateStroke(m_pActiveWidget, e->pos());
- } else if (texPaint->texturePaintEnabled() && m_pActiveWidget) {
+ } else if (mTransformState == TS_SELECT
+ && texPaint->texturePaintEnabled()
+ && m_pActiveWidget) {
texPaint->updateMeshHover(m_pActiveWidget, e->pos());
}Also applies to: 1303-1313
🤖 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/TransformOperator.cpp` around lines 632 - 639, The crosshair cursor/hover
update should only occur when the Select tool is routing paint strokes: restrict
the paint-on check to also verify the current tool/state is TS_SELECT (same
guard used by beginStroke()), so modify the logic around
m_pActiveWidget->setCursor(...) to include that TS_SELECT check (and apply the
same change to the duplicate block around the code at the later occurrence near
the 1303–1313 region); use the existing TransformOperator/BeginStroke routing
state (e.g., beginStroke()/TS_SELECT) to gate paintOn so hover work and cursor
changes only run when Select is active.
…ances CodeRabbit inline findings on PR #529: 1) mainwindow.cpp: mode-switch paint reset is now also breadcrumbed for diagnostics — when the user leaves Material Mode while painting we already shut both controllers off; now the trace shows it happened. 2) TexturePaintBuffer.h: fix the uvToPixel docstring that still claimed V was flipped. Origin is top-left, both axes direct — matches the class header comment and the implementation since Codex P2. 3) TexturePaintController_test.cpp: replace GTEST_SKIP with ASSERT_TRUE(tryInitOgre()) per the TestHelpers.h contract — silent skips were hiding a real CI failure. 4) TexturePaintController.cpp deferred-rebind lambda: validate the captured Ogre::Entity* via SelectionSet::contains() before dereferencing. By the time the singleShot fires the entity could have been destroyed (mesh reimport, selection change closing the session) and the previous m_paintMeshEntity==ent guard wasn't enough — that pointer comparison can succeed against a freed address. 5) TexturePaintController.cpp stroke-end persistence: stop encoding PNG bytes twice. bakeToOriginalFile already returns the on-disk path when it wrote successfully; only fall back to the EmbeddedTextureCache PNG encode when the source was embedded (empty return) since that's the only case the FBX exporter needs it. Saves ~150ms per stroke on disk-backed textures. 6) TransformOperator.cpp: gate the paint-crosshair cursor override to TS_SELECT. In Translate/Rotate/Scale the gizmo is the active interaction and the cursor should be the default — a stale paint flag was forcing crosshair over gizmo handles. 7) VertexColorBaker: pass an explicit coverage out-param through rasterizeTriangle instead of inferring "this pixel was painted" from "differs from background". The old heuristic silently dropped triangles whose interpolated vertex color equalled the background (e.g. all-white verts on a white background → empty dilation mask → no seam dilation). Plus a fix for an existing test-suite failure on Linux CI: TexturePaintBufferTest tolerances were too tight. uvToPixel returns floor(uv * size), so for UV (0.5, 0.5) on a 32x32 buffer it returns (16, 16) — the lower-left of the four pixels straddling the geometric center. The brush peak sits ~0.5 px from that sample center, so a hard-falloff stroke leaves ~5% of the original white behind. Loosened EXPECT_NEAR tolerance from 0.02 -> 0.06 (0.10 for PNG round-trip which also has byte-rounding) and added a comment explaining the offset so the tolerance doesn't look arbitrary. Smoke-tested on macOS: app launches, paints, and persists exports the same as before. Local UnitTests can't run on macOS (plugin path) but CI Linux Xvfb will verify. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
These fixes existed on a prior rebased branch tip that was lost during the push reconciliation; reapplying them on top of the current PR head so the published branch has the full set: - TexturePaintBuffer.cpp / _test: clamp uvToPixel to [0, size-1] so uv=(1.0, 1.0) maps to the last in-bounds texel rather than producing the out-of-range (width, height). Fix the matching round-trip test to assert (63, 31) on a 64x32 buffer instead of the previous (64, 32). Without the clamp, fill seed/picker/smudge sampling on right- and bottom-edge UVs silently no-op'd. (Codex P2) - TexturePaintController.cpp: beginStroke for texture target now detects a stale session (m_sessionEntity != activeEntity) and tears it down before reseeding. Without this, switching selection between two painted entities kept the old buffer/texture bindings and the next stroke wrote to the previous selection's session state. (Codex P1) - .github/workflows/deploy.yml: the per-suite test runner now counts the number of testcases gtest reports vs the number it actually ran and fails the job if they don't match. This catches "test compiled, was discovered, but never executed" regressions (constructor crash during static init, --gtest_filter typo, segfault before first RUN_TEST line) that previously slipped past the no-skipped check. - CLIPipeline_test.cpp: add coverage for cmdAtlas — input validation (missing inputs / output / empty entries), runtime errors on missing source image, and a clearManagerScene helper for tests that need Manager state reset between fixtures. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ession ensurePaintableTexture was calling ensureEditableMesh BEFORE closeSession, but closeSession resets m_paintMesh to nullptr — so on every fresh session creation the function returned true with m_paintMesh empty. findMeshPointForUV then walked an empty submeshes list and returned false, breaking every UV→3D hit-test (paint mode worked anyway because the GUI path goes through a different ensure-then-stroke flow that hits ensureEditableMesh a second time via beginStroke). The Linux CI test TexturePaintControllerTest.FindMeshPointForUVHits- CorrectTriangle exercised exactly this gap because it calls ensurePaintableTexture(64) and then immediately calls findMeshPointForUV without going through beginStroke — and uncovered the bug. Fix: reorder so closeSession runs first (tearing down the prior session cleanly), then ensureEditableMesh builds a fresh CPU mirror, then the rest of the session-setup machinery runs. Also handle the already-active-session fast path explicitly at the top so it can revalidate m_paintMesh without going through the full setup. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
4 tasks
This was referenced Jul 20, 2026
5 tasks
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.
Summary
Closes the texture-paint MVP (#317), texture I/O + dirty rect (#318), bake vertex colors to texture (#319), and docs (#321) sub-issues from epic #313 — Paint Tools. Vertex-color export (#320) was already closed by an earlier change.
Adds a paint workflow in Material Mode that supports both vertex-color painting (polypaint) and direct BaseColor texture painting, with a 2D preview panel that you can paint into as well.
What's in the box
New controller
TexturePaintController—QML_SINGLETONthat owns the paint session for the selected entity. Builds its ownEditableMesh(independent of Edit Mode), holds an Ogre paint texture, and dispatches stroke events to vertex paint or texture paint depending on the user-picked target.Brush input
Paint targets
EditModeController::applyVertexColorBrushagainstm_paintMeshand commits viaEditableMesh::commitVertexColorsToEntity. No Edit Mode required.TexturePaintBuffer(pure-data RGBA8 with dirty-rect tracking) and uploads to a manualTU_DYNAMIC_WRITE_ONLYOgre texture. The texture is rebound onto every TUS pointing at the same source name (catchesalbedo+diffuse_mapaliasing on imported PBR materials).Brush tools
2D preview panel
uvOverlayDataUri).Slot picker + texture I/O
VertexColorBaker::bakewith configurable resolution + dilation).Persistence
findResourceLocationwalk) and (b)EmbeddedTextureCachekeyed by the original texture name. The FBX exporter'sfbxResourceBytes::readpicks up the cached bytes first, so painted Mixamo-style embedded textures persist across export.CLI
qtmesh bake-vertex-colors <file> -o out.png [--resolution N] [--dilation N] [--json]— surfacesVertexColorBakerfor headless asset pipelines.Tests
TexturePaintBuffer_test.cpp— brush stamp, dirty-rect tracking, save/load round-trip, erase, flood-fill region + tolerance + no-op cases.VertexColorBaker_test.cpp— barycentric interp, degenerate / flipped-winding triangles, dilation expansion, default-options overload.TexturePaintController_test.cpp— reverse UV → 3D lookup against an in-memory unit triangle, brush-tool change-signal contract.Performance work (cost notes)
blitFromMemoryproduced no visible update on macOS Metal.preventStealing: trueon the panelMouseAreaso the Inspector'sScrollViewFlickable doesn't eat the drag and cap strokes at a few pixels.Test plan
qtmesh bake-vertex-colorsCLI smokecmake --build build_local --target UnitTests(CI runs the actual tests on Linux/Xvfb)🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation