Skip to content

Add pen-width taper (forward(_:widthTo:) / circle(radius:extent:widthTo:)) - #50

Open
wildthink wants to merge 1 commit into
temoki:mainfrom
wildthink:pen-width-taper
Open

wildthink wants to merge 1 commit into
temoki:mainfrom
wildthink:pen-width-taper

Conversation

@wildthink

Copy link
Copy Markdown

Ramps the pen width across a move, so a stroke can thicken or thin as the tortoise lays it down:

🐢.penWidth = 1
🐢.forward(200, widthTo: 12)
🐢.circle(radius: 70, extent: 270, widthTo: 10)

Sugar, not a primitive. The move is subdivided into short constant-width segments emitting ordinary .penWidth + .forward/.arc pairs. No new TortoiseCommand case, no wire-format change, and no renderer change — a variable-width Stroke would have forced both renderers to fill outline polygons instead of stroking lines, which is the expensive half of the feature and a much larger change to review.

Programs that don't use it pay nothing. No branch was added to CommandPlayer or either renderer. Beyond that:

  • The automatic step count keys off the width delta rather than the distance (taperWidthQuantum, maxTaperSteps), so a gentle ramp over a long move stays cheap — forward(500, widthTo: penWidth + 1) costs four sub-segments, not five hundred.
  • A taper whose end width equals its start width emits exactly one command, so it stays eligible for the same-width stroke batching in CanvasRenderer.drawElements (TortoiseUI: 確定要素の描画を同色・同幅ストロークでバッチする(コミット時 O(n) の定数を 6 倍改善) #37).
  • Widths are sampled at each sub-segment's midpoint, which centers the error; a trailing .penWidth(endWidth) then lands the pen exactly on the requested width.

Adds 13 tests, a taperedStrokes drawing scenario with both goldens, and a TaperedPetals gallery example with its regenerated SVG.

One caveat worth flagging: I could not verify the recorded canvas golden locally. DrawingScenarioCanvasTests crashes on macOS 27 inside swift-snapshot-testing's perceptualPrecision path (-[NSConcreteValue CGRectValue]: unrecognized selector via CIAreaAverage). That crash reproduces on clean main, so it is unrelated to this change — but it means CI is the first real check of scenario.taperedStrokes.png. Happy to re-record if it fails.

swift build, swift-format lint --recursive --strict Sources Tests, and the rest of the suite all pass locally.

🤖 Generated with Claude Code

Ramp the pen width across a move:

    🐢.penWidth = 1
    🐢.forward(200, widthTo: 12)
    🐢.circle(radius: 70, extent: 270, widthTo: 10)

Implemented as sugar over the existing command stream — the move is
subdivided into short constant-width segments emitting ordinary
.penWidth + .forward/.arc pairs. No new TortoiseCommand case, no
wire-format change, and no renderer change: a variable-width Stroke
would have forced both renderers to fill outline polygons instead of
stroking lines, which is the expensive half of the feature.

Three properties keep the cost off programs that don't use it:

- The automatic step count keys off the width delta, not the distance
  (taperWidthQuantum, maxTaperSteps), so a gentle ramp over a long move
  stays cheap.
- A taper whose end width equals its start width emits exactly one
  command, so it stays eligible for the same-width stroke batching in
  CanvasRenderer.drawElements.
- Widths are sampled at each sub-segment's midpoint, which centers the
  error; a trailing .penWidth(endWidth) then lands the pen exactly on
  the requested width.

Adds a taperedStrokes drawing scenario (both goldens recorded) and a
TaperedPetals gallery example with its regenerated SVG.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@temoki

temoki commented Sep 19, 2026 •

Copy link
Copy Markdown
Owner

@wildthink

Thank you for this. I can't merge it in this shape, though, and the reason is the animation model rather than the code.

Watching the tortoise draw is this library's main feature, and that rests on one command being one frame being one fixed-duration step (stepDuration is 0.5 / max(1, speed), independent of distance). So forward(280, widthTo: 12) from width 1, for example, becomes 89 commands, which ends up taking 8.9 seconds at the default speed. The SVG output is split up too — 44 separate <line> elements for that one stroke.

I'm also concerned about the seams: with alpha < 1, the overlapping round caps darken where the sub-segments meet.

@wildthink

Copy link
Copy Markdown
Author

@temoki You're right, and I went and measured it rather than argue — your numbers are exact.

For penWidth = 1; forward(280, widthTo: 12):

measured
commands emitted by the taper 89
<line> elements in the SVG 44
animation time at default speed 5 8.9 s

CommandPlayer.play appends a PlaybackFrame per command unconditionally, state-only .penWidth included, and stepDuration is distance-independent — so command count is wall-clock time. I had been reasoning about the cost in commands and never converted it to seconds, which is the mistake.

The seams reproduce too. A translucent taper of width 1→8 over 100px emits 28 segments, so 27 double-blended cap overlaps — and since translucent strokes are deliberately excluded from the batching in CanvasRenderer.drawElements, it looks the same in both renderers. Not something I can hide on the canvas side.

I now think your three points are one point, and it's the one that sinks the design: in this library the unit of time is the command, so sugar that expands to N commands necessarily buys pen-width fidelity with an N× longer animation. No choice of taperWidthQuantum escapes that — it only moves where the trade sits. Trading animation quality for pen-width quality is the wrong trade in a library whose main feature is watching the tortoise draw. "Sugar, not a primitive" was the headline of the PR and it's exactly the part that's wrong.

So: would you be open to the primitive version instead? Sketch, before I build anything:

  • New command cases .forward(distance, widthTo:) and .arc(radius:extent:widthTo:). Additive, so old streams still decode and the 2.x wire format stays intact.
  • Stroke and ArcStroke grow an endWidth alongside width.
  • TortoiseSVG emits one outline <path> per tapered stroke instead of a <line>; CanvasRenderer fills that path instead of stroking it.

That gets all three of your objections at once, by construction rather than by tuning: one command → one frame → one animation step at the same duration as any other move; one SVG element; and no interior caps, so no seams at any alpha. A tapered arc's outline still needs a polyline approximation, but it lives inside the renderers where it costs draw calls, never commands — so it can't touch the animation clock or the serialized stream.

Two invariants I'd want to keep, and I think this does: TortoiseState.interpolated(toward:progress:) stays position-and-heading-only, since the width ramp is carried by the Stroke rather than by the state; and untapered strokes keep endWidth == width, so same-width batching is unaffected for every program that doesn't use the feature.

The honest cost is that this is the outline-filling work I talked myself out of in the PR description. You were right that it's the real feature and I took the cheap half. It touches both renderers, DrawingBounds, and the sub-frame in-progress stroke path, so I'd rather hear whether you want it at all — and whether a tapered pen is a direction you want the library to go — before I write it.

Separately: the canvas-golden caveat in the description is resolved. Rebasing onto 2.2.0 picks up your byte-wise comparison from #48, and DrawingScenarioCanvasTests now runs and passes locally, taper scenario included. So #49 is no longer blocking review here either way.

Happy to close this PR and open a fresh one for the primitive if that's cleaner.

@temoki

temoki commented Sep 21, 2026

Copy link
Copy Markdown
Owner

@wildthink

Thank you for going and measuring rather than arguing. The numbers and the diagnosis are both right. The primitive version is the correct shape, too.

I'm still not going to take it, though. The reason isn't the implementation — it's who this library is for.

I want the drawing commands to stay primitives, in the Logo sense of the word. I want children to think by combining them. Hand someone a finished effect as a single command and there's nothing left for them to build.

A taper is on the side of things you can reach by composition. Your first implementation is the proof of that — penWidth and a run of short forwards is all it takes. It's something a child can arrive at on their own, and that's exactly why I don't think the library should own it.

I'll close this PR, then. Nothing wrong with the code — it's the feature I'm turning down.

Thanks also for the #49 note. That rebasing onto 2.2.0 makes it pass locally is useful to me quite apart from this PR.

This branch has not been deployed

No deployments
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.

3 participants