Skip to content

Revive drawing capabilities (new and improved!)#261

Open
cwervo wants to merge 10 commits into
mainfrom
ac/drawing-may-2026
Open

Revive drawing capabilities (new and improved!)#261
cwervo wants to merge 10 commits into
mainfrom
ac/drawing-may-2026

Conversation

@cwervo

@cwervo cwervo commented May 7, 2026

Copy link
Copy Markdown
Collaborator

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):

IMG_5693 Small IMG_5694 Small IMG_5695 Small IMG_5696 Small IMG_5697 Small IMG_5698 Small IMG_5699 Small IMG_5700 Small IMG_5701 Small

@cwervo cwervo requested review from osnr and ppkn and removed request for osnr May 7, 2026 00:48
Comment thread builtin-programs/draw/shapes.folk Outdated
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}]

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@ppkn

ppkn commented May 7, 2026

Copy link
Copy Markdown
Collaborator

I plan to test this out on my system today.

@ppkn ppkn left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@cwervo cwervo force-pushed the ac/drawing-may-2026 branch 2 times, most recently from 23d2434 to e5e18de Compare May 20, 2026 23:58
@cwervo cwervo force-pushed the ac/drawing-may-2026 branch 14 times, most recently from 7056a3e to 2282866 Compare May 29, 2026 22:33
@cwervo

cwervo commented May 29, 2026

Copy link
Copy Markdown
Collaborator Author

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

osnr commented May 29, 2026 via email

Copy link
Copy Markdown
Collaborator

@cwervo

cwervo commented May 29, 2026

Copy link
Copy Markdown
Collaborator Author

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

Comment thread builtin-programs/draw/apriltags.folk Outdated
layer $layer
}

Hold! -on builtin-programs/draw/apriltags.folk -key draw-apriltags-demo-code \

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(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/ &\

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This stuff is really ugly:

Image

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.

Comment thread builtin-programs/gpu/textures.folk Outdated
// mostly for debugging:
char* description;

// Image-wish cache metadata. Cached textures stay resident after the

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What bug is fixed by this?

Comment thread builtin-programs/demos.folk Outdated
}

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

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@cwervo

cwervo commented May 30, 2026

Copy link
Copy Markdown
Collaborator Author

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 cwervo force-pushed the ac/drawing-may-2026 branch from 8cb109e to ab19c26 Compare June 1, 2026 19:45
@cwervo cwervo requested review from osnr and smj-edison June 2, 2026 04:28
Comment thread Makefile

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will remove this from this PR tomorrow, accidentally mixed unrelated work back into this branch, apologies!

Comment thread builtin-programs/draw/arc.folk Outdated
return vec4(0.0);
}]]

When the color map is /colorMap/ &\

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Weird indentation here

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

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We probably still need this FIXME comment?

Comment thread builtin-programs/draw/circle.folk Outdated
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/ {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is this a collect and not just a When?


Wish to draw a polygon onto $page with points $points {*}$fillOpts
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the rest of this file is weird / repetitive

Comment thread builtin-programs/draw/text.folk Outdated

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

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this do anything?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

^

@cwervo cwervo Jun 15, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Vestigial, yep, removed

@cwervo

cwervo commented Jun 2, 2026

Copy link
Copy Markdown
Collaborator Author

@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.

@cwervo cwervo requested a review from osnr June 11, 2026 16:18
Comment thread lib/math.tcl Outdated
set ::PI 3.142
set ::TAU 6.283

proc drawTruthy {value} {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should probably get rid of this function and just fix the shader value coercion to convert all of these to 1?

Comment thread builtin-programs/decorations/label.folk Outdated
set maxLength $lineLength
}
}
if {$maxLength == 0} {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't we just return early here? also removed the comment for the other set scale

Comment thread builtin-programs/draw/shapes.folk Outdated
Wish $p draws a $shape with {*}$options
}

When /someone/ wishes /p/ draws a rect with width /width/ height /height/ {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mentioned in prev comment but we shouldn't need most of the variants in this file

@cwervo cwervo requested a review from osnr June 15, 2026 19:16
@cwervo cwervo force-pushed the ac/drawing-may-2026 branch 2 times, most recently from bc81cb0 to 6a50807 Compare June 15, 2026 19:30
Comment thread builtin-programs/gpu/draw.folk Outdated
tailcall "$drawLib $drawLibCmd" {*}$args
}
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this code do anything / is it actually called? It should already be present in pipelines.folk

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope, not called anywhere, removed! Annoyingly, I think I kept recovering old branch state and accidentally committing it.

@cwervo cwervo force-pushed the ac/drawing-may-2026 branch from 6a50807 to 9c1edde Compare June 15, 2026 22:41
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.

4 participants