From c9d100c99a0479d0868dca74a6d460e8cc5b370d Mon Sep 17 00:00:00 2001 From: TheMeinerLP Date: Sat, 10 Oct 2026 19:12:59 +0200 Subject: [PATCH] fix(brush): paint strokes reach the last clicked point 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 --- docs/images/brushes/paint-brush.svg | 282 +++++++----------- .../objects/brush/PaintBrush.java | 9 +- .../objects/brush/PaintBrushCurveTest.java | 24 ++ .../brush/PaintBrushEquivalenceTest.java | 11 +- 4 files changed, 149 insertions(+), 177 deletions(-) diff --git a/docs/images/brushes/paint-brush.svg b/docs/images/brushes/paint-brush.svg index 715ce4ba..65181aae 100644 --- a/docs/images/brushes/paint-brush.svg +++ b/docs/images/brushes/paint-brush.svg @@ -1,4 +1,4 @@ - + @@ -529,10 +529,10 @@ - + Paint Brush Top view - Side cross-section + Side cross-section @@ -563,8 +563,6 @@ - - @@ -595,8 +593,6 @@ - - @@ -608,7 +604,8 @@ - + + @@ -628,20 +625,19 @@ - - - + + - - + - + + @@ -652,7 +648,8 @@ - + + @@ -662,34 +659,36 @@ - - - + + - - + - + + - + + - - + - + + - + + - + + @@ -697,36 +696,35 @@ - - - + + - + + - - + - - + - + + - - + - + + @@ -734,8 +732,6 @@ - - @@ -748,8 +744,7 @@ - - + @@ -758,12 +753,13 @@ - + + - - + - + + @@ -771,33 +767,30 @@ - - - + + - - + - - + - + + - - + @@ -807,8 +800,6 @@ - - @@ -817,23 +808,21 @@ - - + - + + - - + - - + @@ -843,8 +832,6 @@ - - @@ -864,8 +851,7 @@ - - + @@ -877,8 +863,6 @@ - - @@ -897,8 +881,7 @@ - - + @@ -910,8 +893,6 @@ - - @@ -942,40 +923,8 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + + @@ -1004,10 +953,8 @@ - - - - + + @@ -1036,35 +983,32 @@ - - - - + + - + + - - + - + + - + + - - + - - + - - + @@ -1072,10 +1016,8 @@ - - - - + + @@ -1104,10 +1046,8 @@ - - - - + + @@ -1136,40 +1076,36 @@ - - - - - - - - - 1 - - 2 - - 3 - - 4 - - 5 - - 6 - - Grass Block - - Dirt - - Gold Block - - air - - - changed by the brush - - unchanged (faded) - - clicked block - - numbered: click order + + + + + 1 + + 2 + + 3 + + 4 + + 5 + + 6 + + Grass Block + + Dirt + + Gold Block + + air + + + changed by the brush + + unchanged (faded) + + clicked block + + numbered: click order diff --git a/src/main/java/net/onelitefeather/bettergopaint/objects/brush/PaintBrush.java b/src/main/java/net/onelitefeather/bettergopaint/objects/brush/PaintBrush.java index 4bb2e86c..e28dc944 100644 --- a/src/main/java/net/onelitefeather/bettergopaint/objects/brush/PaintBrush.java +++ b/src/main/java/net/onelitefeather/bettergopaint/objects/brush/PaintBrush.java @@ -169,15 +169,22 @@ public void apply(@NotNull Extent extent, @NotNull StrokeContext context) { /** * Samples the curve through the points once per stroke. The curve is the same for every candidate block, * only translated by the block position, so the samples are kept as offsets from the first point. + * The relative curve has one control point per clicked point: the first point is the origin, the others are + * their offsets from it. A curve with n control points has n - 1 segments, so sampling the parameter over + * 0 to n - 1 reaches the last clicked point. * * @param points the points of the stroke in click order * @param first the first point, the origin of the relative curve * @return the sample offsets from the first point, in curve order */ private List sampleOffsets(@NotNull List points, @NotNull BlockVector3 first) { + if (points.size() == 1) { + // A single point has no segment to sample. The stroke is the point itself. + return List.of(Vector3.ZERO); + } LinkedList relativeCurve = new LinkedList<>(); relativeCurve.add(Vector3.ZERO); - for (BlockVector3 point : points) { + for (BlockVector3 point : points.subList(1, points.size())) { relativeCurve.add(point.subtract(first).toVector3()); } BezierSpline spline = splines.apply(relativeCurve); diff --git a/src/test/java/net/onelitefeather/bettergopaint/objects/brush/PaintBrushCurveTest.java b/src/test/java/net/onelitefeather/bettergopaint/objects/brush/PaintBrushCurveTest.java index 55297a33..9a5507c3 100644 --- a/src/test/java/net/onelitefeather/bettergopaint/objects/brush/PaintBrushCurveTest.java +++ b/src/test/java/net/onelitefeather/bettergopaint/objects/brush/PaintBrushCurveTest.java @@ -68,4 +68,28 @@ void positionsAreWrittenOnce() { assertThat(extent.written()).isNotEmpty() .allSatisfy(position -> assertThat(extent.writeCount(position)).as("writes of %s", position).isEqualTo(1)); } + + @Test + @DisplayName("a paint stroke reaches the last clicked point") + void strokeReachesLastPoint() { + MemoryExtent extent = terrain.flat(); + BlockVector3 last = BlockVector3.at(6, Terrain.SURFACE, 6); + + brush.apply(extent, stroke().blocks(terrain.gold).size(8).points(target, second, last).build()); + + assertThat(extent.typeAt(last)).as("block at the last clicked point").isSameAs(terrain.gold); + } + + @Test + @DisplayName("a stroke of one point paints around that point") + void singlePointStrokePaintsAroundIt() { + MemoryExtent extent = terrain.flat(); + + brush.apply(extent, stroke().blocks(terrain.gold).size(8).points(target).build()); + + assertThat(extent.typeAt(target)).as("block at the single point").isSameAs(terrain.gold); + assertThat(extent.written()).as("positions written around the point") + .isNotEmpty() + .allSatisfy(position -> assertThat(position.distance(target)).as("distance of %s", position).isLessThanOrEqualTo(8.0)); + } } diff --git a/src/test/java/net/onelitefeather/bettergopaint/objects/brush/PaintBrushEquivalenceTest.java b/src/test/java/net/onelitefeather/bettergopaint/objects/brush/PaintBrushEquivalenceTest.java index 28b897ed..df2887c4 100644 --- a/src/test/java/net/onelitefeather/bettergopaint/objects/brush/PaintBrushEquivalenceTest.java +++ b/src/test/java/net/onelitefeather/bettergopaint/objects/brush/PaintBrushEquivalenceTest.java @@ -43,8 +43,9 @@ /** * Pins the paint brush to the result of the original algorithm, which built one spline per candidate block. - * The reference below is that algorithm, copied as it was, so the optimised brush must select the same - * positions and write the same blocks for a fixed seed. + * The reference below is that algorithm with the curve corrected to one control point per clicked point (see the + * comment in {@code perBlockCurves}), so the optimised brush must select the same positions and write the same + * blocks for a fixed seed. */ class PaintBrushEquivalenceTest { @@ -74,9 +75,13 @@ private void perBlockCurves(MemoryExtent extent, StrokeContext context) { return; } + // One control point per clicked point. The first one is the candidate block itself; adding the first + // clicked point again as a control point would make the curve one segment too long, and the parameter + // range below would then stop one segment short of the last clicked point. This reference was corrected + // for that bug together with the brush. The original per-block algorithm had the same duplicate. LinkedList newCurve = new LinkedList<>(); newCurve.add(block.toVector3()); - for (BlockVector3 point : points) { + for (BlockVector3 point : points.subList(1, points.size())) { newCurve.add(block.add(point.subtract(first)).toVector3()); } BezierSpline spline = new BezierSpline(newCurve);