Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Showing and hiding geometry (construction geometry, assemblies) #18

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

nrc
Copy link

@nrc nrc commented Nov 5, 2024

I propose an 'implicit show' syntax (with an explicit version as an alternative), where geometry which is consumed (by assigning into a variable, passing to a function, etc.) is treated as construction geometry and geometry which escapes to the top-level of a module is rendered (or becomes part of an assembly).

Signed-off-by: Nick Cameron <nrc@ncameron.org>
@adamchalmers
Copy link
Collaborator

I originally was quite in favor of an explicit show but I now agree that it'll get a bit repetitive. Personally I would be OK with that but I'm open to being told "that sucks" by the people closer to the product and its users.

I liked explicit show because showing/hiding geometry becomes a very very simple codemod without the need for dummy variable names, but I guess _ = fixes that.

|> extrude()

// Variable is used, result of pipeline is rendered
object
Copy link

Choose a reason for hiding this comment

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

What happens if you have:

object
object

Is one thing displayed or two?

Displaying two things at the same location is not useful. But if we need to keep track of what has been displayed, that may dictate how it's implemented. (And without thinking through it fully, my guess is it would be more complicated.)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Just noting that even if we have an explicit show keyword or function, we'd have the same issue. We should track what is shown I guess.

Copy link
Author

Choose a reason for hiding this comment

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

This is a good question. I think this is up to the engine rather than KCL really (although of course there is the question of what we send the engine). I'm also not sure if there is a difference between displaying twice or once? If it's not named or tagged, then it can't be referred to and if it's at exactly the same spot, the image shouldn't change. So I would argue that how many actual draw calls are made is just an optimisation that cannot be observed. In other words, KCL should not track what has been displayed

@@ -0,0 +1,207 @@
# Showing and hiding geometry (construction geometry, assemblies)

I propose an 'implicit show' syntax (with an explicit version as an alternative), where geometry which is consumed (by assigning into a variable, passing to a function, etc.) is treated as construction geometry and geometry which escapes to the top-level of a module is rendered (or becomes part of an assembly).
Copy link

Choose a reason for hiding this comment

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

I feel like we're misusing terminology.

If construction geometry weren't displayed in the 3D scene, point-and-click users wouldn't be able to use it. So that is a form of rendering, no?

The difference is that construction geometry shouldn't be exported as part of the glTF 3D geometry for use in other software. (Not to be confused with the KCL export keyword that allows import, which is a completely different thing.)

I propose the following terminology:

  • rendered: Displayed in the 3D scene, either in the desktop/web app or in the 2D image snapshot output from the CLI.
  • part-exported: Exported in 3D geometry of the part in glTF format (or equivalent) to be used in other software such as a 3D printer's slicer.
  • KCL-exported: Allowed to be imported from other KCL code using the import statement.

The three things are orthogonal. "Construction geometry" is by definition not part-exported. But it says nothing of the other properties.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Furthermore, "rendered" should really specify whether you mean "rendered as normal geometry" or "rendered as construction geometry".

Copy link
Author

Choose a reason for hiding this comment

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

Yeah, "rendered" is a bit fast and loose. "rendered by default" might be better. There is some property of geometry which is "construction" or not and these two kinds are clearly treated differently. I think rendering and exporting in gITF or whatever follow from that more fundamental property (import/export in KCL is a bit different and I think mostly orthogonal to construction or not). So, most places I've written "rendered" should be read as "!construction".

As for actual rendering, my understanding is that rendering of construction geometry should be a property of viewing the scene, it can be turned on and off, and happens in a distinguished way (e.g., dashed wireframe). So it is not rendered in the same way as non-construction geometry. So yes it might be rendered, but also might not be, and definitely not in the same way as !construction geometry.


Exactly how this is implemented with respect to the engine is left to the [foundations design doc](foundations.md), however, whether something is rendered or not must (I think) be explicit in the API calls.

When a function is declared, nothing is rendered (if an entity escapes usage into the top-level of the function, then there should be an error or warning). When a function is called, the result returned by the function follows the usage rules as described above.
Copy link

@jtran jtran Nov 5, 2024

Choose a reason for hiding this comment

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

if an entity escapes usage into the top-level of the function, then there should be an error or warning

This seems like a pretty important point that should be front and center of the design. It means that if you have code at the top level, and you factor it out into a function, the perfectly good code now has errors or warnings. I generally don't like that code cannot be mechanically factored out in this way.

There's some discussion below in the GUI section about showing construction geometry. These seem related to me. I think we all agree that it's a bad idea to render inside functions. But preventing this completely is very detrimental to users debugging. If you can't show geometry in a function, it's like in Haskell when you have a pure function and you can't debug-print. It can be extremely frustrating as a user. You basically have to rewrite all your code to wire it up to return all the way to the top level, or in the case of Haskell, be in the IO monad. At one point, I proposed a debugShow specifically for this, allowing users to render geometry from anywhere, only for debugging. It somehow wouldn't escape outside the module or part for assemblies.

Copy link
Author

Choose a reason for hiding this comment

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

So, I think mechanical refactoring of code into functions is a good guideline, but not an absolute principle. Consider Java for example, in general you can copy and paste into a function, but it's not 100% straightforward because you might reference variables which are out of scope (the IDE can make these into arguments, so mechanical, but not trivial). Then consider Rust, the same thing is mostly true but actually due to the borrowing rules there are times when it is not possible to do this, so not only not trivial but also not possible (and not even possible to predict without a very complete model of the borrow checker). So, in KCL we can basically copy and paste, but we are going to need to identify arguments, and we are also going to have to insert a return for where something might other wise be rendered. That seems reasonable to me.

Partly because (and addressing your second para) there is not a straightforward way to render geometry in functions, consider fn unitCircle(centre) { circle(centre, radius = 2) }. There is no way to render that. You might say pick a strawman argument (e.g., the origin), but what if radius were the argument? The default is much less obvious and there is no easy way to know whether a default argument is a good idea or not (what if there are two points, picking both to be the origin would probably not work - it might for an elipse, but not for a line). But there's a bigger problem - the circle is a 2d object, how do you choose a plane/surface to render it on?

Anyway, afaict, there is not good way to render geometry in a function and we shouldn't try to do so by default, i.e., an error is the right thing to do. We probably should have some support in the UI for this (e.g., the user selects the geometry in the function and the UI can suggest values for missing variables and the user can manipulate them for debugging).

This proposal focusses on KCL, not the UI, so I won't get too deep into things, but I have the following recommendations/expectations for the UI:

- By default non-rendered geometry is not rendered (surprise!)
- There is some UI to show all construction geometry or a specific construction object rendered as a dashed wireframe, this does not affect the KCL program
Copy link

Choose a reason for hiding this comment

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

If we decided it wasn't part of the code and only the UI, then we're accepting that the CLI and Text-to-CAD cannot use it.

Copy link
Author

Choose a reason for hiding this comment

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

There are many properties of viewing the scene which are not reflected in the code (e.g., camera angle, zoom, highlighting geometry when selected, etc). Rendering of construction geometry is just one more of those. The CLI and T2C can still use it, just not control how it is viewed

Comment on lines +196 to +205
```
sketch(on = XY) {
l1 = line(...)
l2 = line(...)
cnstCircle = circle(...)
cnstLine(...)
l3 = line(...)
enclose(l1, l2, l3)
}
```

Choose a reason for hiding this comment

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

I have only ever used Construction Geometry with regards to sketches, so I'll reserve my comments for only sketches. I don't believe I've ever seen construction geometry for 3D objects, but it may just be a tool I've never used.

The words "Construction Geometry" actually might be causing some miscommunication. I'm not positive. For sketches that use the construction (not rendered) lines, I'll use "References" (used in NX).

Probably no surprise, but this code makes the most sense to me. The distribution of rendered entities vs reference entities is (guessing) 90/10. I'd rather the user be explicit about what needs to not be rendered, and having a reference, construction, etc. is best aligned with their workflow

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