Skip to content

fix(brush): paint strokes reach the last clicked point - #57

Merged
TheMeinerLP merged 1 commit into
mainfrom
fix/paint-stroke-last-segment
Oct 10, 2026
Merged

TheMeinerLP merged 1 commit into
mainfrom
fix/paint-stroke-last-segment

Conversation

@TheMeinerLP

Copy link
Copy Markdown
Collaborator

Cause

Since the 2.0 rewrite, PaintBrush.sampleOffsets builds the relative curve with the origin as a control point and then adds every clicked point, including the first one, which is the origin again. The curve therefore has n + 1 control points and n segments, while the parameter is sampled over 0 .. n - 1. The last segment, from the second-to-last to the last clicked point, is never sampled, so the stroke stops one segment short.

The upstream goPaint builds exactly one control point per clicked point (the candidate block first, then block + (p_i - p_0) for i >= 1).

Fix

  • The relative curve has one control point per clicked point: ZERO for the first point, p_i - first for the others. The parameter range 0 .. n - 1 is unchanged, and maxCount is computed on the corrected spline.
  • A single-point stroke has no segment, so sampleOffsets returns the origin only. It paints around that point and does not call BezierSpline.getPoint on a spline without segments.
  • The docs illustration docs/images/brushes/paint-brush.svg is regenerated. Before the fix the painted stroke ends at the segment 5 to 6; after it, the stroke runs through point 6, and the painted block next to point 6 is gold. Sampling is about 0.4 blocks apart and floors to the block grid, so the block under the last click is not always painted itself.

Equivalence test

PaintBrushEquivalenceTest pins the optimised brush to a copy of the original per-block algorithm. That copy had the same duplicated start control point, so brush and reference agreed on the wrong curve and the test passed. The reference now uses the corrected curve, and the comment in perBlockCurves says so. The negative-offset and mixed-offset cases are kept.

Tests

  • PaintBrushCurveTest.strokeReachesLastPoint (new): flat terrain, three points, the block at the last clicked point must be gold. It failed on the previous code (stone at the last point) and passes now.
  • PaintBrushCurveTest.singlePointStrokePaintsAroundIt (new): a one-point stroke paints the point and stays within the brush radius.
  • PaintBrushEquivalenceTest: reference corrected, all four cases unchanged.

Verification

  • ./gradlew check green: 43 suites, 203 tests, plus checkDocsImages, checkDocsLinks, docsGenTest and spotless.
  • docs/how-to/paint-a-path-with-the-paint-brush.md and docs/reference/brushes.md already describe the intended behaviour ("paints the curve through all points"), so no text change.

https://claude.ai/code/session_01JavEedb4MyR9NehY3HusTn

The relative curve of a paint stroke started with the origin and then
repeated the first clicked point, which is also the origin, so it had
n + 1 control points and n segments. The parameter was sampled over
0 to n - 1 and never reached the last segment. The curve now has one
control point per clicked point, and a single-point stroke paints
around that point.

The equivalence test reference gets the same curve. The brush and the
reference were wrong the same way, so the equivalence test passed.

The docs illustration of the Paint Brush is regenerated.

Claude-Session: https://claude.ai/code/session_01JavEedb4MyR9NehY3HusTn
@TheMeinerLP
TheMeinerLP requested a review from a team as a code owner October 10, 2026 17:13
@github-actions

Copy link
Copy Markdown

Unit Test Results

 43 files   43 suites   8s ⏱️
202 tests 202 ✅ 0 💤 0 ❌
203 runs  203 ✅ 0 💤 0 ❌

Results for commit c9d100c.

@TheMeinerLP
TheMeinerLP merged commit 2937bdc into main Oct 10, 2026
14 checks passed
@TheMeinerLP
TheMeinerLP deleted the fix/paint-stroke-last-segment branch October 10, 2026 17:16
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.

1 participant