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

Path to hierarchical parametric sketches #1436

Open
phkahler opened this issue Dec 10, 2023 · 14 comments
Open

Path to hierarchical parametric sketches #1436

phkahler opened this issue Dec 10, 2023 · 14 comments

Comments

@phkahler
Copy link
Member

Proposal

I'd like to outline a path to #80 Hierachical sketches that starts with #77 - Parametric sketches.

  1. First we need the named parameters. The evil-spirit approach of adding names to existing solver parameters seems better than creating separate things called Variables. That is being looked at closely as I write this.

  2. Second, we need a way to enter Name/Value pairs for these parameters. I see equation constraints (like comment constraints) as an interesting way to do this which also makes Solvespace start to look like a mathematics scratchpad of sorts. But I also think it's important to be able to specify these in the text window. Each group could have name/value pairs specified in the text window - this might also be a useful place to specify units per parameter, which would default to the currently selected units, but I'm not too concerned about that right now. Handling units is currenly a major topic.

  3. Scope: What happens when a named parameter is used in a group that has no definition for it? We should look up the hierarchy (is it actually just a list right now?) to find its value and treat it as constant if it's defined in a prior group.

  4. Globals: I think adding Name/Value pairs in g001-references is a good place to define globals used throughout the sketch. There isn't much in this group, and no way to add geometry so the text window is mostly empty when clicking this group. It also comes before everything else, and every sketch has this group.

  5. Linking: Linked sketches would behave exactly as-is for now, but we could display their g001 global parameters in the text window of the enclosing sketch in that group. This would be like Mask parameters in simulink models if you're familiar. So a user could create multiple files that are all copies but with different parameters. When linking these into a larger sketch, each linked sketch would show its specific parameter values in the text window when clicked. This would be nothing but a pure UI change to display the globals of a linked sketch - no editing.

  6. No global sketch: The main issue with hierarchical sketches is that Solvespace currently has a single global sketch. A major step is to refactor so sub-sketches can be loaded and solved. The refactoring could be done without affecting the UI, UX, or file format - just internal code changes.

  7. Per-instance parameters: Once the global sketch is eliminated and we think some kind of multi-sketch capability is in place, use that to actually load linked sketches in their entirety and solve them. Then allow editing the top-level parameters within the UI of the enclising sketch on a per-copy basis. In other words, allow the parameter editing that step 5 will have users clamoring for. This does mean makeing a copy of those variables into the link group and using those in the sub-sketch when solving it. But now instances can each be different without changing the linked file.

  8. Beyond that, I think a fundamental change to the internal data structures and file format might be a good thing. There is a lot that could be done for Hierarchical sketches #80 but by follwing the outline above I think a lot of the core functionality can be incrementally built and proven without disruption to users.

Lets keep most of the units discussion in #77 for now, but more general comments on this path forward can go here. People I most want to ping for feedback: @ruevs @jwesthues @dgramop

@phkahler phkahler changed the title Path to hierachical parametric sketches Path to hierarchical parametric sketches Dec 11, 2023
@Evil-Spirit
Copy link
Collaborator

Evil-Spirit commented Dec 26, 2023

Just fork it to SolveSpace 4.0 and support old slvs files as import-export routines.

@phkahler
Copy link
Member Author

Just fork it to SolveSpace 4.0 and support old slvs files as import-export routines.

Yes. But this list could be done incrementally without any compatibility issues. We should get to a version 3.7 or so before we need do that ;-)

@dgramop
Copy link
Contributor

dgramop commented Dec 26, 2023

This looks good, but I think we should take baby steps and play by ear. Let's take the

(1): Not sure I fully understand. But I agree with the evil-spirit approach and I think that's what you mean. How would this work with parameters/variables in expressions (for example 3*c).
(2): Agree, but I think that Eq constraints shouldn't be displayed/edited like comment constraints, they should be in the text window exclusively (to reduce clutter, and make it harder to lose constraints in a sea of 3d objects/space)
(3): Agree with scopes, will be necessary for assemblies etc.

With regard to (3) (4) (5) and (6):
It would be cool to have a semi-functional approach to our parts: Scopes can help with this.

An alternative solution to per-part globals interfering with assembly globals, what if we make parts more functional. All global parameters are automatically considered overriden when the part is imported into a sketch, and these global parameters are like a function's parameters, they are defined/passed down by the parent

I've mostly been looking at constraints and expressions before I went on vacation. I haven't dug into the sketch sides of things yet so I withhold my opinion for the other points

@Evil-Spirit
Copy link
Collaborator

Evil-Spirit commented Dec 27, 2023

But this list could be done incrementally without any compatibility issues.

Could be done, but for OpenSource no one can say you what to do (ofc if you are not paid like me when I was working on SolveSpace). Support existing legacy is very costly in supporting and introducing new features. Instead, you can jump over it and just remiplement file format in the different way, for example, using json as syntax storage format. My opinion about SolveSpace - it needs some different approaces how things are adressed in sketches. Like changing id (and remapping) to the id-path approach, some thing like directory structure. You can get some inspiration while reading NoteCAD source codes.

@phkahler
Copy link
Member Author

Support existing legacy is very costly in supporting and introducing new features. Instead, you can jump over it and just remiplement file format in the different way, for example, using json as syntax storage format.

We don't have the development effort IMHO to do big changes. I think what I outlined above goes a long way with minimal effort or disruption. Another reason to delay file format changes is that we should only do it once, so getting closer to the desired features first will make it easier to do thing right. I don't want to change a bunch of stuff and then later decide it needs to change again for some reason we didn't plan for.

My opinion about SolveSpace - it needs some different approaces how things are adressed in sketches. Like changing id (and remapping) to the id-path approach, some thing like directory structure. You can get some inspiration while reading NoteCAD source codes.

@Evil-Spirit I completely agree with this. But honestly, I'm not sure I'm smart enough to make those kind of changes. I'd rather continue to incrementally add functionality until we have to make those kinds of changes. By then maybe we'll attract enough developer brains to do it right.

@phkahler
Copy link
Member Author

(1): Not sure I fully understand. But I agree with the evil-spirit approach and I think that's what you mean. How would this work with parameters/variables in expressions (for example 3*c).

@dgramop I hope you've seen the video
The idea of having expressions on the sketch is quite nice. We might (afterward) want to have an option to display the expressions as a number instead.

(2): Agree, but I think that Eq constraints shouldn't be displayed/edited like comment constraints, they should be in the text window exclusively (to reduce clutter, and make it harder to lose constraints in a sea of 3d objects/space)

I strongly disagree. Most designers won't be using many constraint equations, but they probably should be on the sketch. This will also expand another use for Solvespace as a sort of geometry/mathematics sketchpad. But I think it's important to have Name/Value pairs in the text window as well for a more structured approach. That will help with linking parametric parts as I described. But for those who want them on the sketch, an equation like "width = 5" would also be possible if we allow equations on the sketch. I suppose name/expression could be in the text window instead of name/value? Getting back to units, this needs some thought.

(3): Agree with scopes, will be necessary for assemblies etc.

Scopes are already buggy in the current implementation. That's the primary reason it isn't merged. That an the fact we can't specify a value for a parameter with it.

With regard to (3) (4) (5) and (6): It would be cool to have a semi-functional approach to our parts: Scopes can help with this.

An alternative solution to per-part globals interfering with assembly globals, what if we make parts more functional. All global parameters are automatically considered overriden when the part is imported into a sketch, and these global parameters are like a function's parameters, they are defined/passed down by the parent

This is one reason to use g001-references for globals that are passed in from a parent sketch. You could still define things in other groups that would not be overridden in those cases. You're talking about a small variation on how I proposed linking would work, and I like it. That's a variation we can look at when we get there using the incremental approach. I think people will want both options when linking, so it will come down to details. Sketches will definitely need local parameters - meaning ones that are not overridden when linked into an assembly.

@phkahler
Copy link
Member Author

I said scopes are buggy in the current implementation. Try this:

  1. new sketch, draw a rectangle and constraint 2 line equal to make a sqaure

  2. add a dimension "x" to one of the lines

  3. extrude

  4. dimension the length of the extrusion as "x" or "3*x"

  5. drag the lenght of extrusion - the base square does not change (wrong, should not drag)

  6. drag a point on the base sketch, the extrusion snaps back and follows as it should

I think there is a single parameter named "x" here, and the solver is modifying it in the extrude group and we don't regenerate the 2d sketch group so it doesn't follow. As soon as you do something that regenerates the 2d sketch everything follows.

The solution here is to hold parameters from previous groups as constant in the group currently being solved. I'm not sure where this should take place. There is a call to generate equations, and one to copy equations? Not sure at which point, but I think the solution is to check if a named parameter belongs to a prior group and substitute it's value into the expression rather than putting the parameter itself in that expression. To do this you need to look at the way parameter handles are composed - the group number is in there. We also need to verify these handles are getting created that way too (meaning they include the group number).

Fixing this should give us proper scope handling for parameters.
Adding a way to set values (either in text window or equations or both) should make this fit for merging.

@dgramop
Copy link
Contributor

dgramop commented Dec 30, 2023

@phkahler

The idea of having expressions on the sketch is quite nice.
Ah yeah, I misunderstood. I agree with what's in the video, that behavior is intuitive and ideal

What I am against is having equations in the text window (expressions are OK, since they're attached to some entity). In particular, I'm against having equations that go into the solver that aren't attached to an actual measurement in the sketch in the graphics window. For example, "a^2 + b^2 = c^2" wouldn't be in the graphics window, but it should be in the text window. I'm not deadset on this, and if I write that code I'll probably make a config option to hide unattached equations.

Let me make a few more changes and then I'll take a look at the scope bug.

@phkahler
Copy link
Member Author

@dgramop

What I am against is having equations in the text window (expressions are OK, since they're attached to some entity). In particular, I'm against having equations that go into the solver that aren't attached to an actual measurement in the sketch in the graphics window. For example, "a^2 + b^2 = c^2" wouldn't be in the graphics window, but it should be in the text window.

I feel the opposite way. I think a free-floating equation in the sketch window makes a lot of sense, and I intend to add them if nobody else does. It seems very consistent with the solvespace way of doing as much in the sketch as possible. It also means the information is not hidden and can be shown on an exported image of the sketch.

I think the text window is a good place for putting parameter definitions as name/value pairs. Possible allowing an expression instead of just a value. I realize this could also be handled with generic equaitons in the text window like "radius=5". But per my outline above, we still need a way to put name/values into linked sketches and I'm not sure generic equations are a good way to do that. How would you see top-level parameters of a linked sketch being set in a top level sketch?

BTW, once the parameters thing is merged I'm probably going to add equation constraints (like comments) that can be placed on the sketch, so we're going to have that as an option eventually anyway ;-)

@dgramop
Copy link
Contributor

dgramop commented Dec 30, 2023

Can do. Given how constraints are implemented right now, I think I'll need to implement free-floating on the way to writing code for text-window anyhow so that code might as well keep existing.

@phkahler
Copy link
Member Author

@dgramop I think adding comment constraint equations may be trivial. Just go here:

case Type::COMMENT:

  1. make sure "reference" stuff doesn't exit that function early.
  2. split the comment text at the "=" character if there is one.
  3. use the parser to convert each substring to an expression.
  4. take the difference of the 2 expressions (as an expression) and hand make a constraint equation of it.

Obviously this should do nothing if the string is not a valid equation ( more or less than one "=" or either expression fails to parse ) and it will just be a regular comment that doesn't produce an equation.

I think the scope problems in the existing branch are due to the creation of parameters on-the-fly and they are not associated with a particular group, but I've got some possible ideas on that. This comment thing should work as well as the existing expression stuff.

@phkahler
Copy link
Member Author

I was looking for the scope bug with not much luck but I noticed this addition in the patch:

Expr::From(expression.c_str(), false, l);

That line parses a string to get an expression, but discards the expression. The only reason I can think of to call it would be to find and create new named parameters and add them to the list "l". But it doesn't pass in a handle to the constraint or group, so any parameters created will not have that "link" to their origin provided by their handle.

BTW the handles in solvespace are weird and seem to encode information about whom an object belongs to, or what created it. I was digging through all that and just couldn't put it all together in one sitting.

@dgramop
Copy link
Contributor

dgramop commented Jan 21, 2024

Hey sounds good, I'll take a look soon. Was AFK for a cross-country move.

@ruevs
Copy link
Member

ruevs commented Apr 17, 2024

For cross reference #1433

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants