Revive drawing capabilities (new and improved!)#261
Conversation
| 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}] |
There was a problem hiding this comment.
I'm slightly surprised that there's no PI (or TAU) constants. Only slightly surprised though.
There was a problem hiding this comment.
Oh yeah, I should definitely just expose PI and Tau globals
|
I plan to test this out on my system today. |
ppkn
left a comment
There was a problem hiding this comment.
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 white23d2434 to
e5e18de
Compare
7056a3e to
2282866
Compare
|
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) |
|
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: ***@***.***>
|
|
@osnr yes, of course! Happy to wait for your review — thanks! |
| layer $layer | ||
| } | ||
|
|
||
| Hold! -on builtin-programs/draw/apriltags.folk -key draw-apriltags-demo-code \ |
There was a problem hiding this comment.
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?)
There was a problem hiding this comment.
(and this comment applies to all the demo code in the PR, none of them should use Hold!)
| width $outlineWidth color $color layer $layer | ||
| } | ||
|
|
||
| When /thing/ has resolved geometry /geom/ &\ |
There was a problem hiding this comment.
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.
| // mostly for debugging: | ||
| char* description; | ||
|
|
||
| // Image-wish cache metadata. Cached textures stay resident after the |
| } | ||
|
|
||
| When /someone/ wishes /program/ runs demo code from /filename/ { | ||
| Hold! -key "active-demo-$program" Claim $program running demo code from $filename |
There was a problem hiding this comment.
shouldn't need to use Hold! ; also not a proper English sentence
|
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! |
8cb109e to
ab19c26
Compare
There was a problem hiding this comment.
Will remove this from this PR tomorrow, accidentally mixed unrelated work back into this branch, apologies!
| return vec4(0.0); | ||
| }]] | ||
|
|
||
| When the color map is /colorMap/ &\ |
| set options [dict merge $options [dict get $result options]] | ||
| } | ||
| set options [dict create x $x y $y scale $defaultScale] | ||
| # FIXME: support per-label options; right now, this just |
There was a problem hiding this comment.
We probably still need this FIXME comment?
| When the color map is /colorMap/ &\ | ||
| /p/ has canvas /id/ with /...wiOptions/ &\ | ||
| /p/ has canvas projection /surfaceToClip/ &\ | ||
| the collected results for [list /someone/ wishes to draw a circle onto /canvas/ with /...options/] are /results/ { |
There was a problem hiding this comment.
why is this a collect and not just a When?
|
|
||
| Wish to draw a polygon onto $page with points $points {*}$fillOpts | ||
| } | ||
|
|
There was a problem hiding this comment.
the rest of this file is weird / repetitive
|
|
||
| set im [{*}$loadImage "[pwd]/vendor/fonts/$name.png"] | ||
| Wish the GPU loads image $im as texture | ||
| Wish the GPU loads image $im as texture with pinned true |
There was a problem hiding this comment.
Vestigial, yep, removed
|
@osnr - a bunch of changes made it through from erroneously merging a previous version of this branch (the source of many of these weird formatting / confusing no-op edits / unnecessary collects). Will go through again this afternoon and fix again, sorry for the mess, I should have waited to re-request review until today. |
| set ::PI 3.142 | ||
| set ::TAU 6.283 | ||
|
|
||
| proc drawTruthy {value} { |
There was a problem hiding this comment.
I think we should probably get rid of this function and just fix the shader value coercion to convert all of these to 1?
| set maxLength $lineLength | ||
| } | ||
| } | ||
| if {$maxLength == 0} { |
There was a problem hiding this comment.
shouldn't we just return early here? also removed the comment for the other set scale
| Wish $p draws a $shape with {*}$options | ||
| } | ||
|
|
||
| When /someone/ wishes /p/ draws a rect with width /width/ height /height/ { |
There was a problem hiding this comment.
mentioned in prev comment but we shouldn't need most of the variants in this file
bc81cb0 to
6a50807
Compare
| tailcall "$drawLib $drawLibCmd" {*}$args | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Does this code do anything / is it actually called? It should already be present in pipelines.folk
There was a problem hiding this comment.
Nope, not called anywhere, removed! Annoyingly, I think I kept recovering old branch state and accidentally committing it.
6a50807 to
9c1edde
Compare
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 remotewhen 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):