Skip to content

Live Paint: the Live Paint tools and livePaint.fill skip hidden faces - #3460

Merged
bflatastic merged 1 commit into
storytold:mainfrom
dotCooCoo:fix/live-paint-tools-skip-hidden-faces
Oct 21, 2026
Merged

bflatastic merged 1 commit into
storytold:mainfrom
dotCooCoo:fix/live-paint-tools-skip-hidden-faces

Conversation

@dotCooCoo

@dotCooCoo dotCooCoo commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

What and why

This change follows up #3305 (hidden edges). Select a face of a Live Paint group with the Live Paint Selection tool
and choose Object › Hide. On main the hidden face is still highlighted under the pointer, Live Paint Selection
selects it again, and the Live Paint Bucket fills it. Where it lies over another Live Paint group's face, neither tool
can highlight or pick the face behind it.

live_paint_at and lp_part_at in crates/tools/src/builder.rs now take the front-most face that is not hidden
(visible_face_at). A point on a hidden face falls through to the visible face of a group behind it. Where no visible
face lies under it, an edge pick takes the nearest visible edge as it does outside the faces. livePaint.fill also
skips hidden faces. With group, a point on a hidden face returns "no Live Paint face at that point". With ids, it
fills the face there of another Live Paint group among the objects, or else returns that error and does not merge the
group. docs/mcp.md describes the change.

How it was tested

  • builder::tests::live_paint_tools_skip_hidden_faces: of two groups whose faces overlap, the front one's face is
    hidden. Both tools highlight and pick the back face where they overlap and pick no face over the hidden one alone,
    and Alt-click inside it selects the back group's nearest side.
  • tests_build::live_paint_fill_skips_hidden_faces: livePaint.fill on a hidden face fails with group and with
    ids without filling or rebuilding the group, and with ids naming a second group it fills that group's face.

Each test fails with main's code.

On Windows 11 (x86-64) with rustc 1.99.0, on top of main at cb0a7d9:

  • CRAFT_FONTS_DIR=<craft-fonts checkout> cargo xtask ci passes its 10 steps: 8,791 tests passed, 0 failed, 13 ignored.

git merge-tree reports no conflicts with main at ff02191. #3275, #3255, #3238, #2526, #1849, #1838 and #1281
also change crates/tools/src/builder.rs, crates/engine/src/cmd/buildcmds.rs or crates/engine/src/tests_build.rs,
and this branch adds no conflict with any of them.

Checklist

  • cargo xtask ci passes (fmt, clippy, tests, assets, brands, layers, wasm).
  • No panics in shipped code: no unwrap(), expect(), panic!, unreachable!, todo! or
    unimplemented! outside tests, and no unchecked indexing on data from files, parameters or
    messages. Errors are returned as Result and shown to the user
    (why and how).
  • Clean-room: nothing copied from Adobe products or GPL/AGPL code (AGENTS.md).
  • Every new asset has a row in ASSETS.md.

Summary by CodeRabbit

  • Bug Fixes

    • Live Paint filling and hit testing now skip hidden faces.
    • Attempts to fill hidden faces return an error without modifying or merging groups.
    • Visible faces and edges remain selectable, including when overlapping hidden regions.
  • Documentation

    • Updated Live Paint fill documentation to clarify hidden-face behavior.
  • Tests

    • Added coverage for hidden faces, overlapping groups, hidden-only regions, and edge selection.

@coderabbitai

coderabbitai Bot commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Note

Currently processing new changes in this PR. This may take a few minutes, please wait...

⚙️ Run configuration
  • Configuration used: Repository: storytold/vectorcraft/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 74d785b5-7f40-4a9c-826e-8994d954e968
📥 Commits

Reviewing files that changed from the base of the PR and between 1728bc8 and 4bd5502.

📒 Files selected for processing: 2
  • crates/engine/src/cmd/buildcmds.rs
  • docs/mcp.md
+-------------------------------------+
| I don't chase bugs; they surrender. |
+-------------------------------------+
  \
   \     /)/)
        ( '')
        /  |
      o(___)

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: storytold/vectorcraft/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: e186d1e6-f4ce-4949-8da3-3cca2c6541e1
📥 Commits

Reviewing files that changed from the base of the PR and between 44104b6 and 1728bc8.

📒 Files selected for processing: 4
  • crates/engine/src/cmd/buildcmds.rs
  • crates/engine/src/tests_build.rs
  • crates/tools/src/builder.rs
  • docs/mcp.md

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

📝 Walkthrough

Live Paint hit testing and filling now skip hidden faces. A fill on a hidden face returns an error without merging objects. When another Live Paint group has a visible face at that point, filling can use that group instead.

Changes: Live Paint hidden-face handling

Layer / File(s) Summary
Visible-face hit testing
crates/tools/src/builder.rs
Face lookup can filter hidden faces. Live Paint group and in-group lookups use visible faces, while visible-edge lookup remains available. Tests cover overlapping groups, hidden-only regions, and nearby visible edges.
Fill visible faces
crates/engine/src/cmd/buildcmds.rs, crates/engine/src/tests_build.rs, docs/mcp.md
lp_fill searches for visible faces and errors on a hidden-face target before merging objects. It reuses a group with a visible face at the point, or creates a group when no face matches. Tests and documentation describe these cases.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~15 minutes

Change: Bug fix

Suggested reviewers: echelon

Merge Risk: ⚪ Minimal · up to 1728b

The change skips hidden Live Paint faces while retaining visible-face and edge behavior. No actionable merge risk is identified beyond normal checks.

Architecture risk: 🟡 Medium · up to 1728b

The change affects 2 systems.

Changed systems: crates, docs

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

  • observed — crates (service) was modified; 3 changed files map to changed impact.
  • observed — docs (service) was modified; 1 changed file maps to changed impact.

Before / after behavior

  • observed — Modified behavior in crates/engine/src/cmd/buildcmds.rs: Updated the livePaint.fill command description to document that hidden faces are skipped.
  • observed — Modified behavior in crates/engine/src/cmd/buildcmds.rs: lp_fill now searches for visible faces rather than any face. Points on hidden faces return an error before objects are merged; visible-face matches reuse their group, and unmatched points still create a Live Paint group. Final face lookup also requires visibility.
  • observed — Modified behavior in crates/engine/src/tests_build.rs: Added coverage for hidden-face selection during Live Paint filling, including unchanged group state on errors and fallback to another visible group at the same point.
  • observed — Modified behavior in crates/tools/src/builder.rs: Added shared face hit-testing through front_face, retained face_at for all faces, and added visible_face_at to search only visible faces.

Reliability and maintainability

  • inferred — Risk-relevant change factors for crates: blast_radius_1; blast_radius_2; direct_dependents_1; direct_dependents_2
Pre-merge checks: Passed 5
Check name Status Explanation
Title check Passed The title clearly and concisely describes the main change: Live Paint tools and livePaint.fill now skip hidden faces.
Description check Passed The description includes the required What and why, How it was tested, and Checklist sections. It explains the behavior changes, identifies added tests, reports CI results, and marks all checklist ite…
Docstring Coverage Passed Docstring coverage is 85.71% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 14 functions across 3 files. (1 skipped: 1 …
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Description check

Explanation

The description includes the required What and why, How it was tested, and Checklist sections. It explains the behavior changes, identifies added tests, reports CI results, and marks all checklist items complete.

Full details: Docstring Coverage

Explanation

Docstring coverage is 85.71% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 14 functions across 3 files. (1 skipped: 1 unsupported.)

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

A face hidden on its own (for example with Object > Hide after Live
Paint Selection picks it) while its Live Paint group stays visible was
still highlighted, selected by Live Paint Selection and filled by the
Live Paint Bucket, and it kept the face of a group behind it from being
picked.

live_paint_at and lp_part_at now take the front-most face that is not
hidden (visible_face_at). A point on a hidden face falls through to the
visible face of a group behind it; where there is none, an edge pick
takes the nearest visible edge as it does outside the faces.
livePaint.fill also skips hidden faces. With group, a point on a hidden
face is an error. With ids, it fills the face there of another Live
Paint group among the objects, or else fails and does not merge the
group.

Docs: docs/mcp.md.
@bflatastic
bflatastic force-pushed the fix/live-paint-tools-skip-hidden-faces branch from 1728bc8 to 4bd5502 Compare October 21, 2026 19:12
@bflatastic
bflatastic merged commit 19c7d61 into storytold:main Oct 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants