KCL functions both modify sketches in place and return them

[dcl]

This isn't a bug per se, but it is something that lends itself well to a great deal of unintuitive behavior. Consider the following KCL:

let exampleSketch = startSketchAt([0, 0])
  |> line([1, 0], %)
  |> line([0, 1], %)
  |> line([-1, 0], %)

let exampleSketch2 = line([0, -1], exampleSketch)

let example = extrude(1, exampleSketch2)

This creates a unit cube, as expected. So too does this:

let exampleSketch = startSketchAt([0, 0])
  |> line([1, 0], %)
  |> line([0, 1], %)
  |> line([-1, 0], %)

let exampleSketch2 = line([0, -1], exampleSketch)

let example = extrude(1, exampleSketch)

This seems to be because exampleSketch and exampleSketch2 end up referencing the same object. This leads to the somewhat odd situation where the pipeline operator doesn't actually do anything; we can also write the above as:

let exampleSketch = startSketchAt([0, 0])

line([1, 0], exampleSketch)
line([0, 1], exampleSketch)
line([-1, 0], exampleSketch)
line([0, -1], exampleSketch)

extrude(1, exampleSketch)

This generates the same unit cube. However, what we can't then do is reference exampleSketch again -- it seems extrude() doesn't so much modify the passed sketch as delete every reference to it from the local namespace. Returning to our first example:

let exampleSketch = startSketchAt([0, 0])
  |> line([1, 0], %)
  |> line([0, 1], %)
  |> line([-1, 0], %)

let exampleSketch2 = line([0, -1], exampleSketch)

let example = extrude(1, exampleSketch2)

let test = exampleSketch2 // this is an error, 'exampleSketch2' doesn't exist
let test = exampleSketch // this is also an error, 'exampleSketch' now also doesn't exist

Further, the error returned in this case is an API error, indicating this isn't getting caught in the frontend by the KCL interpreter, which makes me suspect that some or all of this behavior is unintended.

[Irev-Dev]

Just a related point, kcl kinda has fake immutability, We don't allow user to re-assign variables, but the stdlib functions do very often mutate the underlying sketchGroup (extrudeGroup etc). Which I think is at least in part, what's happening here.

[jtran]

I'd really like to understand this problem more. It's pretty fundamental.

Does anyone know, is the mutation happening in the sketchGroup in the KCL interpreter, or is this something on the engine side?

[adamchalmers]

Interpreter

[jtran]

I'm bumping into this when implementing artifact tracking.

Chalmers and I were chatting, and we came up with a potential way to deal with this. I'm going to talk about lines and sketches, but the same logic would need to apply to extrusions and volumes and any other type of geometry.

Eager Mutation Approach

• Mutate as we've been doing while we create a sketch for each line() call
• As we do this, each line() call essentially consumes the previous sketch
• When we try to use a previous sketch again through a variable, detect that the sketch has been consumed, and re-send commands to the engine to recreate the sketch again with a fresh UUID

Benefits

• In the simple case where we use a sketch exactly once, engine messages do not change, and when they're sent does not change
• When additional engine commands are needed, they're generated lazily only when needed

Drawbacks

• Every time we use a sketch, we need to check whether it's been consumed
• If it has been consumed, we need to recreate it from only the SketchGroup

Having written down this approach, I feel compelled to write down an alternative that has also been talked about.

Pure Geometry with a Show Command

• Functions like line() do not send engine commands, and they do not mutate anything
• Instead, they build up immutable data structures describing the geometry
• Users must use show(myGeometry) to actually send it to the engine and display it

For example, this KCL:

let sketch1 = startSketchOn('XZ')
  |> startProfileAt([0, 0], %)
  |> lineTo([10, 10], %)
  |> lineTo([-9, 0], %)
  |> close(%)
let extrude1 = extrude(20, sketch1)

show(extrude1)

Note that desugaring the pipeline would look like this:

let sketch1 = close(
  lineTo(
    [-9, 0],
    lineTo(
      [10, 10],
      startProfileAt(
        [0, 0],
        startSketchOn('XZ')))))
let extrude1 = extrude(20, sketch1)

show(extrude1)

Using this approach, the above would build up a data structure like this:

let sketch1 = Solid2d(
  Path::Line(
    [-9, 0],
    Path::Line(
      [10, 10],
      ProfileStart(
        [0, 0],
        Plane('XZ')))))
let extrude1 = Extrude(20, sketch1)

Note how the data structure directly mirrors the KCL stdlib functions.

Put another way, the pseudocode implementation of lineTo() stdlib function would look like this:

enum Path =
    ProfileStart([number, number], Plane)
  | Line([number, number], Path)
  | Arc( /* omitted */, Path)

function lineTo(point: [number, number], path: Path): Path {
  return Path::Line(point, path)
}

Nothing gets sent to the engine. All it does is bundle up its parameters into a persistent data structure.

Only when we execute show(geometry) do we translate the above data structure to engine commands and send them. This is simple because the entire geometry is there. We just traverse it.

This might even open up opportunities for optimizations (batching?) because we could look for patterns in the geometry and treat them differently.

Benefits

• There's no mutation or consumption tracking
• There's no lazy recreation of geometry
• Lots of KCL stdlib functions get simpler

Drawbacks

• This is a major breaking change because people need to add show(geometry) to their KCL
• show is arguably boilerplate that users need to remember to do. If they forget, the failure mode is bad: nothing appears. We could add a lint that detects unused/not-shown geometry, but that's more work.
• Lots of KCL stdlib functions need to change

[adamchalmers]

Plan 2 matches up nicely with my old vision of KCL (the fantasy docs), where the whole thing is just defining geometry and giving it names, then at the end, you choose which function to actually render.

[adamchalmers]

This change is unrelated, but, if we're already going to ship a big breaking change, maybe we should consider this too:

Change the path API to be an array of path segments, rather than a nested list of path segments. Something like

let squareSketch = sketchOn('XY', [
  line(4, 0), 
  line(0, 4), 
  line(-4, 0), 
  line(0, -4),
])

I think making the path segments an array, rather than using nested function calls to modify a sketchgroup, is much simpler. It's also IMO more expressive: users can build their sketches up via array processing (e.g. push and map) rather than repeated function calls, which need a loop or reduce.

In this approach, sketchOn would close the path and return a solid2d. This would work similarly for sketching on a face, at a starting point, etc etc.

This is entirely compatible with the show change.

[franknoirot]

In this approach, sketchOn would close the path and return a solid2d. This would work similarly for sketching on a face, at a starting point, etc etc.

I like this array syntax @adamchalmers. How do you express non-closed paths in this approach? Would it be worth having sketchOn rather be closedPath, with an openPath function that doesn't close the path? I like the approachability of the the term sketchOn though.

[adamchalmers]

How often do people draw open sketches? We could also just have a "open" optional bool which default to false.

[jtran]

My intuition says that operations allowed on a closed sketch are very different to those on an open sketch, and therefore, they should be different types.

[adamchalmers]

OK, close could still be an extra step after I guess.

[Irev-Dev]

My thoughts in bullet points.

• Implementing show, and or using Adam's syntax can work. What's important for the sketch mode UI to work is, that we can
  • still do mock execution
  • still access execution results that contains all of the 2d segment data for each of the sketches (even with mock execution) 2d data is needed so that can be drawn client side.
• From a code gen perspective, automatically adding things to show won't be a problem though some situations confuse me, I'll put them below
• New syntax break lots of code mods because the shape of the ast is changing, but can be done ofc.

Things I want to make sure I understand

let sketch1 = startSketchOn('XZ')
  |> startProfileAt([0, 0], %)
  |> lineTo([10, 10], %, $seg01)
  |> lineTo([-9, 0], %)
  |> close(%)
let extrude1 = extrude(20, sketch1)

const myFilletedExtrude = fillet(5, [seg01], extrude1)

show(extrude1)

In this case, the fillet is never applied right? as far as what the user see?

And for the following

let sketch1 = startSketchOn('XZ')
  |> startProfileAt([0, 0], %)
  |> lineTo([10, 10], %, $seg01)
  |> lineTo([-9, 0], %)
  |> close(%)
let extrude1 = extrude(20, sketch1)

const myFilletedExtrude = fillet(5, [seg01], extrude1)

show(extrude1, myFilletedExtrude)

This would ask the engine to create the two bits of geometry that sits on top of each other right?

And for the following

const myShape1 = shape1()
const myShape2 = shape2()
const myShape2 = shape3()

const substractedShape = subtract(myShape1, myShape2)

show(substractedShape)

We only want the final shape in the show function right?

If I'm thinking about the above correctly, like John was saying users will have to add things to the show function, but not only that it's a bit of a moving target, as they create a new variable that takes the old shape and changes it in some way, they need to change the show function, remove the old add the new, could get annoying after a while. less of a problem for code mods since we can do it for them but here are some ideas for how to make it smoother.

We put something in the gutter These are clickable and will add and remove the vars for them.

We dump the show function and instead add some special keyword or symbol for which variables get added to show, so instead of typing out a variable, you just adding a character to the variable declaration (we can still do the gutter helper with this one), $$ is a little goofy but you get the idea

[Irev-Dev]

I think we still need open profiles

[adamchalmers]

Oh I really like the UI for show checkmarks. Great idea.

Also, I think a "variable is not shown, or part of anything that is shown" warning in the IDE will make it easier for users to understand. Because in this plan, variables don't really do anything unless they're put into show or used for calculating something that gets shown.

[Irev-Dev]

btw not sending to the engine early actually give users much more power to process the data that makes up shapes.

Lets say you wanted to write a function called zigZagSktech that takes a sketch and turns all of the straight segments and breaks them up into lots of little zigZaging lines. Well currently since it eagerly sends to the engine, it's too late they've already been created as straight segments, but with the show paradigm, it's able to loop over the segments to break them up before it gets sent to the engine.

I that sense we are currently limiting the creative abstractions of our users.

[jtran]

@Irev-Dev, your code examples and interpretation sound correct to me. So show isn't all sunshine and rainbows. This is exactly why I wanted to write it down. Interesting to explore the consequences.

The check boxes remind me of a spreadsheet or notebook interface.

I really like the zigZagSketch idea.

[jtran]

I know it's a minor thing, but I feel like it would be a lot more convenient if show didn't need parentheses, similar to return, as in:

show myShape1

Or maybe, similar to $$ above, something that's more pictographic 😄 No eyes on myShape2, so it's not visible.

@_@ shape1()
let myShape2 = shape2()
@_@ let myShape3 = shape3()

Functions have the same "moving target problem" when you build something up and want to return it. This is true in general purpose programming languages like TypeScript. The way it's solved in Rust is with implicit return of the last expression. So I think either people can deal with it, or we add implicit show of some kind, maybe of the last expression.

But I've thought a lot about implicit show. I really want to preserve the ability to factor code out from the top level to a function and preserve its functionality. But I also don't think we want implicit show inside functions. So I'm coming around to the idea that just dealing with the moving target is fine.

[jtran]

End-user's non-toy code that uses sharing that has unintuitive results: #4104.

[jtran]

There has been some more discussion on implicit show. I've been meaning to summarize everything and post it here, but I haven't gotten around to it yet. Here's one thread, but there's more.