Revive drawing capabilities (new and improved!) by cwervo · Pull Request #261 · FolkComputer/folk

cwervo · 2026-05-07T00:48:38Z

This PR is a little large because it touches a bunch of drawing files. We get back a suite of drawing capabilities. This PR also has an unrelated change to running make remote when working out of a git worktree, happy to splice this into another PR.

Some examples of running the demo code via Wish $this runs demo code from builtin-programs/title.folk (which is a new wish added in this PR):

ppkn · 2026-05-07T01:46:05Z

+    lassign $center cx cy
+    set points [list]
+    for {set i 0} {$i < $sides} {incr i} {
+        set theta [expr {$radians + $i * 2.0 * 3.141592653589793 / $sides - 1.5707963267948966}]


I'm slightly surprised that there's no PI (or TAU) constants. Only slightly surprised though.

Oh yeah, I should definitely just expose PI and Tau globals

ppkn · 2026-05-07T15:34:37Z

I plan to test this out on my system today.

ppkn

This looks good. The drawSpaceLib api feels a little clunky always having to pass poselib and display. I wonder if there is a cleaner way we could do that in the future.

I wrote a small way to preview all of the drawing demos every 2 seconds so I could quickly preview them. It didn't work for shapes.folk though so I had to check that out separately.

When the clock time is /t/ {

    set demos {arc circle curve dashed-line fill image line shapes space text}
    set i $(int($t)/2 % [llength $demos])
    set demo [lindex $demos $i]
    Wish $this runs demo code from builtin-programs/draw/${demo}.folk
}

Wish $this is outlined white

…s overhaul - Replaces old CPU-based connection arrows with high-performance Vulkan shaders - Animates dynamic connections natively using injected GPU float time - Overhauls basic shapes, polygons, arcs, and curves to use GPU canvases - Fixes various drawing pipeline memory bounds corruption bugs - Reorganizes the builtin-programs/draw module - Adds forgiving error handlers to prevent terminal UI log spam from malformed syntax

cwervo · 2026-05-29T23:40:38Z

Finally going to merge this, it's looking very stable on folk-convivial:

folk_shapes_may_29_2026.mp4

(Will address bringing back connections.folk and resolving the issues in terminal/terminal-ui.folk in future PRs)

osnr · 2026-05-29T23:42:30Z

Can you hold off on merging until I can review? Way too many deep changes here for me to be comfortable landing it

…

On Fri, May 29, 2026, at 7:41 PM, Andrés Cuervo wrote: *cwervo* left a comment (FolkComputer/folk#261) <#261 (comment)> Finally going to merge this, it's looking very stable on folk-convivial: https://github.com/user-attachments/assets/0cbe158b-4208-4547-999d-ca80ee7ec8ac (Will address bringing back connections.folk and resolving the issues in terminal/terminal-ui.folk in future PRs) — Reply to this email directly, view it on GitHub <#261?email_source=notifications&email_token=AAAXUWKKRDTF5Z6CJLYYEPT45INY3A5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTINJYGA3DQMJXGY3KM4TFMFZW63VQOJSXM2LFO5PXEZLROVSXG5DFMSSWK5TFNZ2KYZTPN52GK4S7MNWGSY3L#issuecomment-4580681766>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAXUWNUICAOHPQXBHRYCU345INY3AVCNFSM6AAAAACYT3FNYWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHM2DKOBQGY4DCNZWGY>. Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS <https://github.com/notifications/mobile/ios/AAAXUWIRV363ICFY2SC4PXD45INY3A5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTINJYGA3DQMJXGY3KM4TFMFZW63VQOJSXM2LFO5PXEZLROVSXG5DFMSSWK5TFNZ2KUZTPN52GK4S7NFXXG> and Android <https://github.com/notifications/mobile/android/AAAXUWKTQBK5J7ZV4MSSUA345INY3A5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTINJYGA3DQMJXGY3KM4TFMFZW63VQOJSXM2LFO5PXEZLROVSXG5DFMSSWK5TFNZ2K4ZTPN52GK4S7MFXGI4TPNFSA>. Download it today! You are receiving this because your review was requested.Message ID: ***@***.***>

cwervo · 2026-05-29T23:48:44Z

@osnr yes, of course! Happy to wait for your review — thanks!

osnr · 2026-05-30T20:07:03Z

        layer $layer
 }
+
+Hold! -on builtin-programs/draw/apriltags.folk -key draw-apriltags-demo-code \


You should just do Claim $this has demo code ... here -- you definitely shouldn't be using Hold! or spelling out the file path.

(in fact, I don't think you should need to use Hold! at all in this entire PR, since it's all purely functional drawing code?)

(and this comment applies to all the demo code in the PR, none of them should use Hold!)

osnr · 2026-05-30T20:10:31Z

+        width $outlineWidth color $color layer $layer
+}
+
+When /thing/ has resolved geometry /geom/ &\


This stuff is really ugly:

I would rather standardize on one variant (even if we have to break compatibility) and/or add some syntax sugar (maybe so one handler can deal with both with and non-with variants?) than have all of this stuff all over the codebase -- it's very low-information-density and hard to read.

osnr · 2026-05-30T20:10:56Z

    // mostly for debugging:
    char* description;
+
+    // Image-wish cache metadata. Cached textures stay resident after the


What bug is fixed by this?

osnr · 2026-05-30T20:11:33Z

 }
+
+When /someone/ wishes /program/ runs demo code from /filename/ {
+    Hold! -key "active-demo-$program" Claim $program running demo code from $filename


shouldn't need to use Hold! ; also not a proper English sentence

osnr · 2026-05-30T20:12:18Z

                # network, or broadcasting out, and getting pages from
                # there.
-                exec curl --output "$saveDir/$obj.folk" \
+                exec curl -f --output "$saveDir/$obj.folk" \


good addition -- can you document what this fixes? were we not properly faulting?

osnr · 2026-05-30T20:13:06Z

@@ -1,4 +1,5 @@
 # Manage terminal UI for Folk.
+return; # TODO: FIXME


I would rather let this error out

osnr · 2026-05-30T20:13:15Z

@@ -1,4 +1,5 @@
 # Terminal
+return; # TODO: FIXME


I would rather let this error out

osnr · 2026-05-30T20:14:38Z

    // Will be NULL if not running in an Atomically
    // convergence-tracking subgraph.
    AtomicallyVersion* _Atomic atomicallyVersion;
+    _Atomic bool isAtomicallyRootMatch;


What bug does this fix? / why do we need this?

osnr · 2026-05-30T20:15:04Z

 		lldb -o "process handle -p true -s false SIGUSR1" -- ./folk; \
 	else \
-		DEBUGINFOD_URLS="" gdb -ex "handle SIGUSR1 nostop" -ex "handle SIGPIPE nostop" ./folk; \
+		gdb -ex "handle SIGUSR1 nostop" -ex "handle SIGPIPE nostop" ./folk; \


Why was this removed? The debuginfod suppression is there because it would hang in gdb on accessing it for me (can add a comment about that)

osnr · 2026-05-30T20:15:14Z

 	fi

 FOLK_REMOTE_NODE ?= folk-live
+FOLK_SYNC_IGNORES ?= $(shell git rev-parse --git-path ignores.tmp 2>/dev/null || printf '%s\n' .git/ignores.tmp)


what does this fix?

osnr · 2026-05-30T20:16:28Z

+fn drawLabelDefaultScale {text} {
+    set maxLength [drawLabelMaxLineLength $text]
+    if {$maxLength == 0} { return 0.02 }
+    ::math::min 0.02 [/ 0.45 $maxLength]


would be better style to use explicit return

osnr · 2026-05-30T20:17:25Z

+    ::math::min 0.02 [/ 0.45 $maxLength]
+}
+
+fn drawLabelDefaultOptions {text width height} {


In general, I don't really like all these utility functions, but I could be convinced otherwise. @ppkn @smj-edison what do you think?

Lots of repetition since you need to restate and pass in all the parameters, and it doesn't fit the style we use in existing drawing programs. I think a straight-line When body is easier to read than having like 4 functions in each file.

I'm not opposed to utility functions, as long as they're used multiple times and don't cause indirection fatigue. I think in this case it might be clearer inline, since they're not used that often.

osnr · 2026-05-30T20:18:08Z

+     /p/ has canvas projection /surfaceToClip/ {
+
+    set query [list /someone/ wishes to draw a circle onto $p with /...options/]
+    When the collected results for $query are /results/ {


Why do we need to collect here? Why not use a When?

osnr · 2026-05-30T20:19:53Z

+
+When /someone/ wishes /p/ draws a /shape/ with radius /radius/ {
+    Wish $p draws a $shape with radius $radius
+}


why do we have these radius/width/height variants? aren't they just subsumed by the options handler anyway?

osnr · 2026-05-30T20:20:43Z

+
+When /someone/ wishes /p/ draws a polygon with points /points/ /...options/ {
+    # Forgiving syntax for users who write 'draws a polygon with points $points color white'
+    # and use ( ) instead of { } for points.


Does this actually work? I feel like this wouldn't work unless you did "( ...points )" in quotes?

osnr · 2026-05-30T20:24:27Z

-                set paddingMeters 0.02
-                set offset [scale $paddingMeters [unitLengthVector $physicalDir]]
-                set physicalPos [add $physicalPos $offset]
+            When -atomically $thing has quad /q/ {


need to bring back the -atomicallyWithKey here to stop titles from blinking

osnr

Broadly, I think there are too many changes here that need individual review. There are a lot of changes that aren't clearly motivated and that touch really tricky/complicated parts of the code where I'd want to be convinced that we know why we're making the change and why it works.

It might be better to split into a remove-dead-modules PR, an image PR, a shapes PR, a textures PR, a titles PR, and a demos PR, or something like that. Then most of the PRs that just add a new drawing feature are easy to clear through (even if they blink, go magenta occasionally, etc) and we can focus on the trickier stuff separately.

cwervo · 2026-05-30T21:41:42Z

Fair — I’ll strip this PR down to just shapes then and make PR’s of the other batches of work with more proof/documentation. Thanks!

cwervo requested review from osnr and ppkn and removed request for osnr May 7, 2026 00:48

ppkn reviewed May 7, 2026

View reviewed changes

ppkn approved these changes May 8, 2026

View reviewed changes

cwervo force-pushed the ac/drawing-may-2026 branch 2 times, most recently from 23d2434 to e5e18de Compare May 20, 2026 23:58

cwervo force-pushed the ac/drawing-may-2026 branch 7 times, most recently from 7833d2c to 201c035 Compare May 29, 2026 01:37

Fix: Prevent UAF segfaults in Atomically statement cleanup

99f085d

cwervo force-pushed the ac/drawing-may-2026 branch 6 times, most recently from b48d29f to 7056a3e Compare May 29, 2026 22:24

cwervo force-pushed the ac/drawing-may-2026 branch from 7056a3e to 2282866 Compare May 29, 2026 22:33

cwervo added 4 commits May 29, 2026 19:08

Revert connections.folk to origin/main

5365688

Remove connection tests that break due to reverting connections.folk

5bcac76

Return on outdated builtins

88fb8b8

Fix arc and sprites demos (nit)

8cb109e

osnr reviewed May 30, 2026

View reviewed changes

osnr requested changes May 30, 2026

View reviewed changes

		@@ -1,4 +1,5 @@
		# Manage terminal UI for Folk.
		return; # TODO: FIXME

Uh oh!

Conversation

cwervo commented May 7, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ppkn commented May 7, 2026

Uh oh!

ppkn left a comment

Choose a reason for hiding this comment

Uh oh!

cwervo commented May 29, 2026

Uh oh!

osnr commented May 29, 2026 via email

Uh oh!

cwervo commented May 29, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

osnr left a comment

Choose a reason for hiding this comment

Uh oh!

cwervo commented May 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants