I haven't been following the named parameters discussions too closely, so forgive my ignorance, but I'm not sure what the scoping rules are, so a clarification would be very much appreciated. The comments in the code of this PR imply that the scope is based on a group's handle value, but that's not how entity scope is determined today. For regeneration, the scope is based on group order, which is mostly determined by group dependency (through opA). Ideally we should ditch the linear representation of groups in the text window and just draw a tree view of groups to make it more obvious to the user, but in any case, that's how the solver currently solves groups, and basing the scope on a group's handle value will conflict with the way the solver defines known parameters that are considered constants that shouldn't be solved.

Additionally, the code in this PR doesn't deal with named parameter dependencies, so when a named parameter is deleted, any requests and constraints that depend on it are left dangling. I'm not sure what the best way to deal with it is, but we should at least trigger a regeneration and prune such requests or constraints (honouring the scoping rules), similar to how entity deletion is handled today. A more difficult case is parameter rename, which doesn't seem to check for name conflicts, and can also leave dependants dangling.

Lastly, how should we deal with parameters that redefine constants (such as pi)? The expression parsing code seems to just use the constants, but the UI doesn't try to warn the user when a parameter name is set to the name of a known constant.

I only skimmed the code in this PR, so I'm sure I'll have more questions later when I go through the code more thoroughly, but these are the ones that came up during my first reading.

@iscgar the current implementation does not follow sketch generation scope rules. The plan is to create a valid set of names for the current group when generating a sketch. That would start with a copy of the parent group and adding any names from the current group.

It will also crash if you delete or rename a parameter that is used in a constraint. This will be "solved" when we add the ability to dynamically create a parameter in the parser. To do that it will need the handle for the constraint. Parameters can belong to a request, a group, or a constraint. The text window ones belong to the request. Dynamicly created one will belong to the constraint they are first encountered in. Thats also how it will cope with deletions or renaming - constraints will just create a new parameter with the old name. If someone wants to write a renaming algorithm to update all occurrences when a name is changed thats fine but not strictly necessary.

On the fly parameter creation is important for the relation constraints and the old demo. You can also define parameters on the sketch with "width = 12" or similar. That stuff also turns Solvespace into a small symbolic algebra tool.

BTW I tagged you specifically because you know the solver and regeneration process.

@iscgar the current implementation does not follow sketch generation scope rules. The plan is to create a valid set of names for the current group when generating a sketch. That would start with a copy of the parent group and adding any names from the current group.

Am I correct in my understading that that plan is to effectively follow the sketch regeneration scoping rules, and that the reason the current code doesn't do so is merely because it's a WIP draft for getting the ball rolling?

It will also crash if you delete or rename a parameter that is used in a constraint. This will be "solved" when we add the ability to dynamically create a parameter in the parser. To do that it will need the handle for the constraint. Parameters can belong to a request, a group, or a constraint. The text window ones belong to the request. Dynamicly created one will belong to the constraint they are first encountered in.

Since named parameters can also contain expressions, dynamically generated parameters will also need to be generated by requests, not just constraints.

One related concern that I forgot to mention: the bit space in constraint handles doesn't allow creating more than a single parameter (see my comment #1574 (comment) and #1574 (comment)), and the space in request handles is also very limited, although it should probably suffice (the approach in this PR, which reuses the 6th bit, allows creating up to 32 parameters).

What scope should the dynamically generated parameters belong to? Only the request or constraint that uses them? That seems like a recipe for surprises, since the solver will be free to solve parameters with the same name differently. Scoping them to the group that the request or constraint belong to seems more appropriate to me, although I'm likely missing some other corner cases that would be surprising with that behaviour.

This also brings up the issue of defining the same named parameter multiple times in the same group. The current code doesn't seem to handle that case, and from a cursory look at this PR it seems that the last parameter "wins" (because named parameters are iterated in order of request handle values).

Thats also how it will cope with deletions or renaming - constraints will just create a new parameter with the old name. If someone wants to write a renaming algorithm to update all occurrences when a name is changed thats fine but not strictly necessary.

Instead of keeping the expression in the request or constraint as a string, we could store the parsed expression, which would refer to the handle values of the parameters (although note that it would require decoupling the expression system from the sketch's temporary heap, as those expressions are long lived, unlike the current use of expressions only during regeneration, which allows them to be discarded as soon as a group is solved). That way, renaming a parameter doesn't require walking the tree, and deleting a paramater can be dealt with by either pruning the request/constraint during regeneration, as we do for dependant requests and constraints today, or we could just resolve them with dynamic parameter generation (although see my comment in the next paragraph about the scoping of dynamically generated parameters).

On the fly parameter creation is important for the relation constraints and the old demo. You can also define parameters on the sketch with "width = 12" or similar. That stuff also turns Solvespace into a small symbolic algebra tool.

The old demo is presumably this video by Evil-Spirit? Looking at it seems to imply that dynamically generated parameters don't belong to the constraint that mentions them, since all a and angle values are resolved to the same value. It rather seems like the Evil-Spirit implementation simply defined them globally as soon as they were first encountered (where "first encountered" seems to be the order of constraints based on their handle values). I'm also not sure how the solver is expected to handle an expression such as sqrt(a * a + b * b) with dynamically generated parameters, as any random value satisifes the constraint, as long as it's well defined (i.e. the expression under the sqrt doesn't become negative, infinity, or NaN).

As for sketch parameters, I'm assuming defining parameters on the sketch is the same as defining them in the references group, since you can't sketch anything in that group anyway, and it's the root of the group tree.

On the fly parameter creation is important for the relation constraints and the old demo. You can also define parameters on the sketch with "width = 12" or similar. That stuff also turns Solvespace into a small symbolic algebra tool.

The old demo is presumably this video by Evil-Spirit?

@iscgar yes. See also the https://github.com/solvespace/solvespace/commits/parameters/ which is the "modern", unfinished implementation. And (in case you are not aware of it) the O.G. issue and discussion #77

And this #1482 where @dgramop has a WIP of using parameters for the number of repeats in "Step trans/rotating" groups.

@iscgar yes. See also the https://github.com/solvespace/solvespace/commits/parameters/ which is the "modern", unfinished implementation. And (in case you are not aware of it) the O.G. issue and discussion #77

And this #1482 where @dgramop has a WIP of using parameters for the number of repeats in "Step trans/rotating" groups.

Thanks for the links! I did read both of those discussions, but it's always good to referesh my memory. After reading the discussions I'm still not sure how scoping for dynamically generated paremeters should be handled, especially for partial regeneration. This is because we'd need to collect named parameters from parent groups even if they are not being solved, just so we don't end up introducing free parameters where the user expects to reuse the solved ones from the parent groups. If the dynamically generated parameters are scoped to their group rather than to their request/constraint, that would be trvivial to implement.

Additionally, what should be the interactions with linked assemblies? Right now their solved form is being used and they never get regenerated during a parent regeneration, so we don't really need to do anything regarding their named parameters. However, if we ever get hierarchical sketches, I suppose we'd like them to be dynamic and be affected by parameters defined in the parent sketch or vice versa? Supposedly child sketches will be handled differently than linked assemblies anyway (i.e. they won't be groups of static entities), so we can just defer the decision to when we get hierarchical sketches.

I would love to implmenet the regeneration part, but coming back to my points above, it would be great to properly specify the behaviour of dynamically generated parameters and the scoping rules in general. If we really need constraints to dynamically generate parameters, we might want to hold off the implementation of named parameters until 64-bit handles land, since with the current encoding of contraint handles it's impossible to encode more than a single parameter as I mentioned above. Alternatively, we can let the constraint's group be the owner of these parameters, and in that case there shouldn't be any issue with their encoding. However, we still need to decide what to do in some of the weird cases that I mantioned above, such as multiple parameters with the same name defined ina single group, or worse -- a named parameter getting renamed to the name of an existing named parameter in the same group. If allowed, that last case will be sure to cause confusion since the parameter handles are not exposed in the UI, and it'll not be obvious which parameter shadows which.

The behaviour for the rest of the cases I presented (such as shadowing a known constant, which we might want to disallow, and also reserve a bunch of names to give us the option to use them as constants in the future) would also need to be defiend, but I feel that it shouldn't be a blocker for an initial implementation.

@iscgar each group should inherit (copy) the name/handle dictionary of its parent group. Then add any names from requests in the current group, overwriting the handle of any duplicates - most recent wins.

Then when generating equations for constraints we would ideally create new parameters for names that don't exist yet. These would also get copied to subsequent child groups. This would allow writing "width=5" on a sketch (relation constraint) and using it throughout, without needing to use a request.

Are parameters that belong to constraints recreated on regeneration? Are they saved to files? (I think they probably are?)

You noted in the old video a triangle gets a side "sqrt(aa+bb)" and the other sides are "a" and "b". We don't care which constraint those 2 parameters belong to, they could belong to the group. What matters is that all instances of "a" in that group refer to the same parameter. But notice the expression was used instead of "c". If it were called c then we'd have aa + bb = c*c - think pythagorous. After that expression was added it became a right triangle. A and B could be independently changed, but the angle stayed 90 degrees!

As for linked sketches. I wrote a plan for those recently ( issue #1436 ). If we have file A and in group 5 we link file B, we want to show parameters defined in B references group in File A group 5. Once we can load and solve linked sketches, those parameters in File A group 5 will be copies of those if File B and we will allow them to be edited. Their values will be copied to that instance of file B prior to solving. In this way you'll be able to create say an extrusion of 8020 with a length defined in references. Then you can use multiple instances of that part in another sketch by linking multiple times, and each one could be a different length. Reusable paramePythagoras.

We can do all of this without dynamic creation of new parameters in the parser, using text window definitions. But 1) it can be nice to just write them on a sketch. And 2) we will need to handle renaming and deletion somehow. Those operation will orphan a name in other parts of the sketch and the expressions won't parse. They can't just revert to a previous value either as there is no meaningful number in valA.

we will need to handle renaming and deletion somehow. Those operation will orphan a name in other parts of the sketch and the expressions won't parse. They can't just revert to a previous value either as there is no meaningful number in valA.

It just occurred to me that if a stored constraint expression becomes invalid because a variable got deleted or renamed, we could simply not generate an equation for that constraint. The sketch would gain a degree of freedom, but nothing should be moved by the solver. We might have a way to inform the user that the constraint is broken and maybe indicate "err" for the group too. Would it be possible to do this with a generic equation like 1=0 ?

Are parameters that belong to constraints recreated on regeneration? Are they saved to files? (I think they probably are?)

Currently parameters belong to requests, and the only constraint that can generate a (single) parameter is the point on line constraint (see cc07058 for the reason).

But yes, parameters get saved into the file, although that's only for the benefit of assemblies. When loading a model into a sketch (as opposed to linking), everything gets regenerated from groups, requests, and constraints.

You noted in the old video a triangle gets a side "sqrt(a*a+b*b)" and the other sides are "a" and "b". We don't care which constraint those 2 parameters belong to, they could belong to the group. What matters is that all instances of "a" in that group refer to the same parameter. But notice the expression was used instead of "c". If it were called c then we'd have a*a + b*b = c*c - think pythagorous. After that expression was added it became a right triangle. A and B could be independently changed, but the angle stayed 90 degrees!

What I meant was to ask about a case where there aren't any other constraints that use a or b, and the expression is used in the only constraint that the solver needs to solve (say a dimension constraint). In that case the parameters are free, and the solver can choose any real value to arrive at a valid solution. With parameters defined in requests, the user has control over the parameter value (which can be edited, and inspected by the user), so the solver won't do anything surprising as long as the user chooses meaningful values. However, with dynamically generated params, the solver will be free to keep the initial value (which the user has no direct access to), resulting in the user fighting the solver when dragging, with no control over the result unless a named parameter request or a relation constraint is introduced. This feels a bit too magical for me (as an aside, I also dislike the fact that with dynamic parameter generation there are effectively two ways to define a parameter, with named parameter request being inferior in the current implementation because it does not allow an expression).

[snip]

We can do all of this without dynamic creation of new parameters in the parser, using text window definitions. But 1) it can be nice to just write them on a sketch. And 2) we will need to handle renaming and deletion somehow. Those operation will orphan a name in other parts of the sketch and the expressions won't parse. They can't just revert to a previous value either as there is no meaningful number in valA.

[snip]

It just occurred to me that if a stored constraint expression becomes invalid because a variable got deleted or renamed, we could simply not generate an equation for that constraint. The sketch would gain a degree of freedom, but nothing should be moved by the solver. We might have a way to inform the user that the constraint is broken and maybe indicate "err" for the group too. Would it be possible to do this with a generic equation like 1=0 ?

Why wouldn't the expression parse? We can store named parameter references as new expression nodes (lazily bound parameters, resolvable by name), and fix their references only during regeneration, when they are being solved. This is similar to a resolution step that is already taking place today during solving, where parameter handles in the expression are resolved to direct parameter pointers. That way we can fail at resolution time with a nice message about an unknown parameter, allowing the user to define it and retry, similar to how the solver can fail when a set of constraints is unsolvable, and the user can make changes to let the solver retry. To support falling back on the old value while the user fixes things, we can keep a shadow copy of named parameters when a user renames or deletes a parameter, which will be discarded after the first successful regeneration (as a successful regeneration means the operation didn't orphan any parameters that later expressions depend on).

But yes, parameters get saved into the file, although that's only for the benefit of assemblies. When loading a model into a sketch (as opposed to linking), everything gets regenerated from groups, requests, and constraints.

Parameters need to be saved in a file so the sketch has reasonable dimensions. When you draw a line the endpoints use 2 parameters each (x, y). The line entity has handles for the 2 point entities and then each point has handles for 2 parameters. All coordinate parameters have to be saved because of that. I was wondering if the constraint-created parameters are also saved, but I suspect the answer is yes for similar reasons. They also should not be discarded and recreated during sketch regeneration for the same reason. I am not very familiar with the whole regenerate process.

You noted in the old video a triangle gets a side "sqrt(aa+bb)" and the other sides are "a" and "b". We don't care which constraint those 2 parameters belong to, they could belong to the group. What matters is that all instances of "a" in that group refer to the same parameter. But notice the expression was used instead of "c". If it were called c then we'd have aa + bb = c*c - think pythagorous. After that expression was added it became a right triangle. A and B could be independently changed, but the angle stayed 90 degrees!

What I meant was to ask about a case where there aren't any other constraints that use a or b, and the expression is used in the only constraint that the solver needs to solve (say a dimension constraint). In that case the parameters are free, and the solver can choose any real value to arrive at a valid solution.

But it won't. When you type "a" into a dimension constraint (in the old demo), a parameter gets created and is substituted into an equation defining the distance between 2 points. That parameter value will change as you drag an end of the line and change its length. Currently this will also happen with my named parameters. If you define one in the text window of a group and then use it as a distance for the length of a line in that same group, you can still drag the line length and the parameter value in the text window will change. If the line is in a later group, the length will be fixed because the parameter is treated as constant since it's from a previous group.

I don't know if I like this behavior. We may want to "lock" the parameter to the typed value. Relation constraints will also allow writing "x=5" on the sketch, so locking in the text window would prevent that. For that reason I think there needs to be an option to lock them in the text window. The original issue #77 author suggested names could define a variable or a constant. I think one request type is enough, but a checkbox to lock it might be needed. I'm not sure what the internal mechanism would be for that - a request can generate a constraint, so that may work. I thought there might be a flag on parameters that prevent the solver from changing them but I don't know the solver well enough to be sure.

With parameters defined in requests, the user has control over the parameter value (which can be edited, and inspected by the user), so the solver won't do anything surprising as long as the user chooses meaningful values. However, with dynamically generated params, the solver will be free to keep the initial value (which the user has no direct access to), resulting in the user fighting the solver when dragging, with no control over the result unless a named parameter request or a relation constraint is introduced. This feels a bit too magical for me (as an aside, I also dislike the fact that with dynamic parameter generation there are effectively two ways to define a parameter, with named parameter request being inferior in the current implementation because it does not allow an expression).

We can do relation constraints without dynamic parameter creation. If we find a way to do it later it should be possible to add without breaking compatibility.

[snip]
We can do all of this without dynamic creation of new parameters in the parser, using text window definitions. But 1) it can be nice to just write them on a sketch. And 2) we will need to handle renaming and deletion somehow. Those operation will orphan a name in other parts of the sketch and the expressions won't parse. They can't just revert to a previous value either as there is no meaningful number in valA.
[snip]
It just occurred to me that if a stored constraint expression becomes invalid because a variable got deleted or renamed, we could simply not generate an equation for that constraint. The sketch would gain a degree of freedom, but nothing should be moved by the solver. We might have a way to inform the user that the constraint is broken and maybe indicate "err" for the group too. Would it be possible to do this with a generic equation like 1=0 ?

Why wouldn't the expression parse? We can store named parameter references as new expression nodes (lazily bound parameters, resolvable by name), and fix their references only during regeneration, when they are being solved. This is similar to a resolution step that is already taking place today during solving, where parameter handles in the expression are resolved to direct parameter pointers. That way we can fail at resolution time with a nice message about an unknown parameter, allowing the user to define it and retry, similar to how the solver can fail when a set of constraints is unsolvable, and the user can make changes to let the solver retry. To support falling back on the old value while the user fixes things, we can keep a shadow copy of named parameters when a user renames or deletes a parameter, which will be discarded after the first successful regeneration (as a successful regeneration means the operation didn't orphan any parameters that later expressions depend on).

The parser throws an error for parameter/constant/function names it doesn't recognize. We validate an expression in mouse.cpp when it is typed. We store the expression string in a constraint, but it is re-evaluated in constrainteq.cpp during regeneration. If the name is changed and the sketch regenerated there will be an expression that won't parse. This currently causes a crash. As I suggested in a previous comment, we could simply not create an equation for the constraint in that case, or even create an unsolvable one to trigger existing warnings. I'll try that tonight if I have time.

I've read everything but have not thought carefully. That said I want your opinion on what should happen in the following situation:

0. CTRL-N
1. Draw two "free" line segments.
2. Add dimension constraints to both with a value "x".

To clarify "x" is NOT defined in any of the "explicit" ways you are discussing.

Bonus questions:

• Change one constraint to "x=3"

• Change one constraint to "2*x"

• Add a second dimension constraint to one segment and make it "4" (yes it is possible in the parameters branch and I still have not looked into why).

• Change one constraint to "x*x"

• Change one constraint to "y=x+8"

I prevented the crash when a named parameter is deleted but still used in constraints. The red background will appear and then somehow it will figure out which are the bad constraints and delete them with a message box.

@ruevs as it is right now you can only use parameters that are defined in the text window, and parameters defined in the current group are not fixed value. So you can do the entire demo/video so long as you define all the names (x,y,a,b) in the text window of g002 and draw everything in g002. Dragging a line of length x will change the value of x - you can see that value change in the text window. Adding a second constraint to the length of a line and typing in a number will force the line to that length, and hence x will become that value as well.

Drawing a triangle and giving the sides length a, b, sqrt(a * a+b * b) will force legs a and b to be perpendicular but not of fixed length.

The only thing missing here is full scope rules. It does obey the rule that params defined in a previous group are held constant for the current group - the solver enforces that because these parameters are defined correctly. All we need now is for the list of parameters to include ONLY those that can be used in the current group (not future groups) and override the handles in the appropriate way if there are duplicate names from different groups.

So in expr.cpp and EditControlDone we need to pass in a "current" list of names.

Parameters need to be saved in a file so the sketch has reasonable dimensions. When you draw a line the endpoints use 2 parameters each (x, y). The line entity has handles for the 2 point entities and then each point has handles for 2 parameters. All coordinate parameters have to be saved because of that. I was wondering if the constraint-created parameters are also saved, but I suspect the answer is yes for similar reasons. They also should not be discarded and recreated during sketch regeneration for the same reason. I am not very familiar with the whole regenerate process.

Right, the constraint created param is also saved. During regeneration all of the parameters are solved again, although the values loaded from the file are used as initial guesses, so in practice the regeneration should converge to almost exactly the same values as in the file.

What I meant was to ask about a case where there aren't any other constraints that use a or b, and the expression is used in the only constraint that the solver needs to solve (say a dimension constraint). In that case the parameters are free, and the solver can choose any real value to arrive at a valid solution.

But it won't. When you type "a" into a dimension constraint (in the old demo), a parameter gets created and is substituted into an equation defining the distance between 2 points. That parameter value will change as you drag an end of the line and change its length. Currently this will also happen with my named parameters. If you define one in the text window of a group and then use it as a distance for the length of a line in that same group, you can still drag the line length and the parameter value in the text window will change. If the line is in a later group, the length will be fixed because the parameter is treated as constant since it's from a previous group.

The solver will generally try to keep the initial values if they still satisfy the constraints. However, when dragging a point of a line segment, it must change either the parameter in the length equation or the parameters of the other point, since the constraint isn't satisfied with the existing value (it has less effect on the dragged point, due to the dragged logic that limits it from making significant changes to dragged parameters, but otherwise it can still affect the dragged point as well), and that's when unexpected things may happen depending on the order of equations provided to the solver. I haven't tested the parameters branch, but you can see that in the old demo Evil-Spirit was forced to put a WHERE_DRAGGED constraint on the other point of the line segment in order to ensure that the solver only changes the constraint's free parameter.

I don't know if I like this behavior. We may want to "lock" the parameter to the typed value. Relation constraints will also allow writing "x=5" on the sketch, so locking in the text window would prevent that. For that reason I think there needs to be an option to lock them in the text window. The original issue #77 author suggested names could define a variable or a constant. I think one request type is enough, but a checkbox to lock it might be needed. I'm not sure what the internal mechanism would be for that - a request can generate a constraint, so that may work. I thought there might be a flag on parameters that prevent the solver from changing them but I don't know the solver well enough to be sure.

Locking parameters should be easy enough to do. All that is needed is for the generation code to mark them as known, which causes the solver to resolve them as constants. So if a "lock" check box is introduced for the named parameter request, it can easily be implemented within the existing regeneration framework.

The parser throws an error for parameter/constant/function names it doesn't recognize. We validate an expression in mouse.cpp when it is typed. We store the expression string in a constraint, but it is re-evaluated in constrainteq.cpp during regeneration. If the name is changed and the sketch regenerated there will be an expression that won't parse. This currently causes a crash.

Sure, but what I'm suggesting is to modify the parser to allow parsing such expressions without crashing. This can be done by introducing a new expression node (which I termed "lazily bound parameters") that can be resolved at evaluation time by passing a list of params to use for the resolution. This is an easy change to do, and is the most logical IMO. That way we also easily deal with renames and deletions, since resolution will be delayed until we actually evaluate the expression. It'll also allow us to bail at solution time and point directly to the broken constraint, without the need for hacks like 0 = 1 which the solver can only identify by brute force.

@phkahler I took your code and created an initial draft of the scoping rules that you described and my idea of the parser changes in a branch. Note that this branch does not yet persist named parameters to file, so any file that uses named parameters will be broken.

The main changes from the code in this PR are:

• Named parameters are a group property rather than requests (this makes it easier to reason about scoping, and avoids redundant iterations over the requests list to gather named parameters)
• The expression parser supports parsing named parameters without resolving them, allowing resolution to be deferred to when needed
• A new solver status for unresolved named parameters was added, and deletion of a named parameter (or referencing an undefined named parameter when editing a constraint) is handled with a descriptive message in the text window

@iscgar I'm going to need some time to review what you're doing. At a glance it seems more invase, but a lot of the changes are small.

Right now the version in this PR can save and load files - there was no new code needed for that. In mine, I see that diameters are not shown correctly.

@iscgar I'm going to need some time to review what you're doing. At a glance it seems more invase, but a lot of the changes are small.

Most of the invasiveness in my branch is due to also decoupling Expr from the sketch and making it standalone, which requires passing ParamList everywhere params need to be resolved. I did it mostly because it seemed a good opportunity to do so with the need to pass named parameter mapping for named parameter resolution, but we can do the decoupling at a later stage and reduce the scope of the changes.

Right now the version in this PR can save and load files - there was no new code needed for that.

That's indeed a plus with your implementation, though I don't think it justifies the added complexity with the request handling all over the place. Adding the ability to save and load named parameters shouldn't be too difficult, and I'll try to work on it during the weekend so the two approaches could be compared.

That's indeed a plus with your implementation, though I don't think it justifies the added complexity with the request handling all over the place. Adding the ability to save and load named parameters shouldn't be too difficult, and I'll try to work on it during the weekend so the two approaches could be compared.

I'm not sure what you mean by "request handling all over the place". The text window needs something - I do want to define them primarily in the text window, but in relation constraints and on-sketch expressions is a very nice to have. I'm going to move the creation of the "dictionary" into the group generation area next, so that should be cleaner and only in one place. None that means they couldn't belong to the group instead of a request, but groups (so far) have fixed number of parameters depending on the type of group.

One thing I hated about the original demo be evil-spirit was that he added names to the parameters themselves and saved those in files. Parameters in Solvespace were never meant to be any kind of object. They are just a double with a handle that can be used by the constraint solver. Objects (groups, requests, constraints) have parameters, not the other way around :-)

I hope to have time to finish my approach in the coming week as well.

I'm not sure what you mean by "request handling all over the place". The text window needs something - I do want to define them primarily in the text window, but in relation constraints and on-sketch expressions is a very nice to have. I'm going to move the creation of the "dictionary" into the group generation area next, so that should be cleaner and only in one place. None that means they couldn't belong to the group instead of a request, but groups (so far) have fixed number of parameters depending on the type of group.

What I meant was that with requests there is no easy way to find the group's parameters other than iterating over the entire request list, and that leads to things like the double iteration when showing the group info in the text window, or during regeneration. As you mentioned, though, my approach isn't without downsides (the biggest of which is the need to add new code for saving and loading parameters, whereas your approach Just Works by reusing the request serialisation logic).

None that means they couldn't belong to the group instead of a request, but groups (so far) have fixed number of parameters depending on the type of group.

In my implementation the named parameters use the upper half of possible group parameter values, in order to allow future groups to use the entire bottom half for fixed parameters (or other uses that may arise) if needed. the total allowed parameter range per group is 2^16, so that should leave plenty of room for future expansion, and the room for named parameters should also be enough, as I don't think users will ever need to define more than 32K named parameters.

Regardless of whether named parameters are requests or a group property, another question is whether name resolution should be performed at expression parsing time or deferred to expression evaluation time. Your implementation chooses the first option, whereas I went with the second one. Deferring resolution to evaluation time allows the solver to handle resolution failure as a first class citizen, clearly indicating the issue and pointing to the list of bad constraints, resulting in nicer error handling. I feel that requiring them named parameters to be resolved at parsing time is needlessly limiting, and leads to hacks like the unsolvable equation that you needed to add when resolution fails in order to allow the solver to find the bad constraint (which still isn't a nice solution, because the user has no way to tell why the constraint is bad).

I hope to have time to finish my approach in the coming week as well.

Sounds great. I'm looking forward to seeing the complete implementation.

I cleaned up the code a bit, dropped the decoupling of Expr from the sketch for now, and implemented named parameter persistency in a different branch. Please check it out and let me know what you think.

EDIT: I updated my implementation to include some quality of life improvements, such as showing the unresolved parameter names in the text window on resolution failure, as well as fixes to bugs that I discovered while testing.

I cleaned up the code a bit, dropped the decoupling of Expr from the sketch for now, and implemented named parameter persistency in a different branch. Please check it out and let me know what you think.

@phkahler @ruevs did you get a chance to test my branch? I would love to hear your thoughts and get your feedback regarding the approach I took.

@iscgar I played both with yours @phkahler's branches - as a "dumb user". Have not looked at the code of either and therefore have not commented yet.

@iscgar I have not had time yet. How does it handle names that only exist in an expression? Are they added to the group? At what index?

@ruevs my version still needs to inherit a list of names from the parent group to get the scope handled correctly, and also needs a toggle to lock a value in the text window (const vs varaible).

@iscgar I have not had time yet. How does it handle names that only exist in an expression? Are they added to the group? At what index?

No, you have to create named parameters on the group to use them in an expression. Any unresolved references result in an error in my version, with a clear explanation of the issue in the text window:

Once you get this error, you can either edit the expression if there was a typo, or select the group in the text window and add the missing named parameters in order to fix the issue.

[snip] also needs a toggle to lock a value in the text window (const vs varaible).

Is this really needed? I was under the impression you preferred to use the references group for "constants".

I implemented parameter locking functionality in my branch, and it's indeed very useful. While at it I also aligned parameter names and values in the text window to make it look more consistent with different lengths of parameter names and values:

I implemented parameter locking functionality in my branch, and it's indeed very useful. While at it I also aligned parameter names and values in the text window to make it look more consistent with different lengths of parameter names and values:

Off topic: @iscgar by adding tooltips you've touched the infamous TextWindow::Printf - our "GUI framework " :-) - which has inspired some... passionate comments... in the past. Having used it to implement parameters and even modified it to add tool tips - what do you think of it? Sincerely :-)

I'll pull and play with your latest changes tomorrow. Not sure if I'll find time to read the code.

Off topic: @iscgar by adding tooltips you've touched the infamous TextWindow::Printf - our "GUI framework " :-) - which has inspired some... passionate comments... in the past. Having used it to implement parameters and even modified it to add tool tips - what do you think of it? Sincerely :-)

Well, it's different from other GUI libraries I've used in the past. When I first encountered it I wasn't sure what I was looking at (the name Printf definitely didn't help, and the half-line logic felt like a quick and dirty patch to fix a special case). However, as a user, while I still find myself going back to the code in order to refresh my memory about the different modifers of %B, %F, and %L, I've mostly gotten used to it. As a devloper, the tool-tip feature already existed in the text window, so I'm not sure it's representative of actually implementing a new GUI component, since all I had to do was add the new format string and show the tool-tip on hover (I did have to debug the weird interaction with DrawOrHitTestIcons() though, so there's that). The fact that the GUI code is so bare-bones made it in some ways easier to add the tool-tip feature, because I only needed to keep track of all the ways the item matrix is used to make it functional.

While having a proper GUI toolkit abstraction would probably make the code easier to interact with and understand, I'm not sure it justifies the effort that it would require. On the one hand you'd be able to add things like text boxes, and maybe even fancier controls, like combo boxes and proper radio button groups. On the other hand, you'd then need to deal with things like scrollable containers and Z-order, which currently simply aren't a thing.

@iscgar How did you implement the lock/unlock icons? I searched and couldn't find those in the unicode BMP :-) Also, how are you handling units in your parameters branch? Or are you? I like your version of the text window. Also glad you found the value in locking - sometimes you want to have a parameter with a fixed value and just use it in the current group. My intent for the references group would be for "global" or high level parameters to be defined there, and also to reflect those in linking group.

@iscgar How did you implement the lock/unlock icons? I searched and couldn't find those in the unicode BMP :-)

That's because they aren't standard BMP glyphs :)
The way custom icons in the text window are handled is by assigning them to the Private Use Area:

They are then referenced using raw UTF-8 bytes in the strings handed to TextWindow::Printf.

I simply added two new PNG files for the lock states, and assigned them to the next empty slots in the PUA.

Also, how are you handling units in your parameters branch? Or are you?

The same way it's handled in your branch, with the unit multiplier saved in valA of the constraint.

Any updates on this PR? The discussion here sounds to me like you guys are almost there!

Any updates on this PR? The discussion here sounds to me like you guys are almost there!

@TerryGeng We've gone in slightly different directions. With holiday vacations coming up I'm planning to pull this together in the next few weeks. My plan is to use my original concept of having requests for parameters in the text window and internally using a per-group dictionary copied from the parent group and extended by new requests (or values overwritten for duplicate names). This avoids adding any new members to C++ classes or anything new to files, just some new request types. I'll use @iscgar layout of the text window because it's clean and he added nice locks. We'll use 2 request types for locked or unlocked parameters (the UI can just change the type). Actually for future-proof (linked sketches) there will be 2 more reserved parameter request types but that stuff will be transparent to the user. That much I want to start on Dec 20th and get done ASAP. After that is in place and working I'll hope to turn @dgramop loose on bringing relation constraints on top of this (if still interested ;-). We will not be allowing new names to be defined in relation constraints yet - that's going to be a little bit tricky (meaning I'm not sure the best way to create the parameter handles yet, but it will be possible to add).

I'd like to get version 3.2 final out this year, and then immediately start putting the named parameters stuff in. If all goes well there could be a 3.3 with that stuff by spring - I'm happy to have a release just for this feature. I'm never as fast as I hope, but we are very close with this like you said. This was first prototyped when? In 2016? so 10 years. Ugh, it's time.

Updated the PR. name dictionary is in the group and populated in group::generate. But the scope rules are not quite right. @ruevs It's using opA as the parent group, but not all groups have an opA. Should it just use the previous group in group order?

@ruevs It's using opA as the parent group, but not all groups have an opA. Should it just use the previous group in group order?

@phkahler I'll take a look when I end up in front of a computer. Merry Christmas!

Updated the PR. name dictionary is in the group and populated in group::generate. But the scope rules are not quite right. @ruevs It's using opA as the parent group, but not all groups have an opA. Should it just use the previous group in group order?

If the intention is to have the scoping rules follow the regeneration scoping rules, as I did in my branch, then yes, it should just use the previous group in the group order. Otherwise, if we really want to have the scoping rules being determined by actual dependency, figuring out the right parent group can become complicated. That's because the other way for a group to depend on another is by having its origin and normal determined by entities, and there are multiple ways to define the group's origin and normal (in some cases they can be formed by entities from different groups). So while group order doesn't actually determine direct dependency (because it's a flat list) it does a pretty good approximation, and using it makes it consistent with entity dependency, which users are already accustomed to.

So while group order doesn't actually determine direct dependency (because it's a flat list) it does a pretty good approximation, and using it makes it consistent with entity dependency, which users are already accustomed to.

That should be the right way for now. People occasionally realize that the construction history is a actually a tree, and eventually we should embrace that more, but for now consistency is the goal.

So I added a second type of parameter request NAMED_CONST_PARAM to my branch. When the named value is changed, the request type is changed. Then during generation the type is checked and if the request is NAMED_CONST_PARAM, the parameter has its "known" flag set to true. The intent was that this would lock the parameter so dragging in the same group it's defined in would not change it. But instead the value immediately goes to zero and I don't understand why.

@ruevs should we just go with the @iscgar version of this? I don't know if I've got the mental energy to evaluate the differences any more.

So I added a second type of parameter request NAMED_CONST_PARAM to my branch. When the named value is changed, the request type is changed. Then during generation the type is checked and if the request is NAMED_CONST_PARAM, the parameter has its "known" flag set to true. The intent was that this would lock the parameter so dragging in the same group it's defined in would not change it. But instead the value immediately goes to zero and I don't understand why.

Setting it on the parameter directly is not enough, because at regeneration time the known flag is reset. See this commit in my branch for the changes I made to implement locking (specifically the changes to SolveSpaceUI::WriteEqSystemForGroup()).

It's a little late in the game, but I recently thought.... Instead of using requests to define variables, what if we used constraints? A constraint is allowed to create a parameter and also has string storage for the name (comment field). It also has a valA member which could be used to store a numeric value, and a reference flag which could be used to indicate if it's a constant or not. The reference flag is used (via false) to indicate that an equation should be generated for the constraint. We could just generate param = valA to get the solver to substitute the constant if it's locked. I'd still want the text window UI stuff, just with a different internal representation.

@iscgar does this make more or less sense to you than using requests for this? BTW I'm not thinking of putting the constraint on the sketch, though that might be a possibility as well. I just figure it should work without resorting to extra code to stuff the correct values into the solver along with setting that "known" flag.

BTW I made a new branch on my fork with most of my previous commits squashed and want to try this change to see what it looks like afterward. Have not done the work yet.

It's a little late in the game, but I recently thought.... Instead of using requests to define variables, what if we used constraints? A constraint is allowed to create a parameter and also has string storage for the name (comment field). It also has a valA member which could be used to store a numeric value, and a reference flag which could be used to indicate if it's a constant or not. The reference flag is used (via false) to indicate that an equation should be generated for the constraint. We could just generate param = valA to get the solver to substitute the constant if it's locked. I'd still want the text window UI stuff, just with a different internal representation.

@iscgar does this make more or less sense to you than using requests for this? BTW I'm not thinking of putting the constraint on the sketch, though that might be a possibility as well. I just figure it should work without resorting to extra code to stuff the correct values into the solver along with setting that "known" flag.

If you leverage the param generation capability of constraints, you don't need to store the value in valA, and can instead just use the parameter to store the value (as you do now when you use requests. You also need valA to store the unit multiplier anyway). As for the reference flag -- it doesn't prevent equation generation if the group has "treat all dimensions as reference" enabled (which modifies valA through ConstraintBase::ModifyToSatisfy()), so I think overloading it for const indication will simply require more code changes than you currently have with the request implementation. I also can't say I like the addition of "pointless" equations that just make the solver's work harder when we already have the known framework for dealing with locked parameters, but it will work (it's essentially what the WHERE_DRAGGED constraint does, only for parameters rather than entities).

Other than the above, I don't think it's that much different than your current approach of using requests, and I don't think it'll result in less code. You can already generate an equation for locked values with the request implementation if you prefer to avoid using the known flag for parameter locking, and using requests make more sense IMO because they are meant for generating things explicitly requested by user actions. The only reason constraints even have the ability to generate a (single) parameter is as a hack to avoid the hairy ball problem in PT_ON_LINE, and using it for general parameter generation feels to me like an abuse of that facility.

@iscgar Thank you for your feedback on that idea. I am of similar opinion that requests seem more natural from a conceptual point of view. I was hoping constraints would be a more natural fit on the code side, but it seems not. There was one part of your argument that was flawed though - a variable-defining constraint wouldn't use valA for anything else. variables are unitless. It's dimension constraints only that will use valA for unit scaling. Anyway, your comment was enough to get the idea out of my head, so thanks again for taking the time.

Hey guys, I just can't wait for this feature become real because that's really what I miss from fusion.

Before any discussion comes up, I'm not a big fan of AI slop put to maintainers. And this is not my intention here. It's just to get an idea of how an implementation could look like. I do not assume that this is used as production code, it's more like an inspiration. Please be aware that some of the changes like in cmake.txt where needed because it otherwise would not build on my machine. you can ignore this.

I just played around with codex and asked it to implement user parameters how I imagine it. And to my surprise, it works perfectly after 2 hours of playing around.

IMPORTANT: this is currently only working on a mac as I understood!

The solution looks like this - it's using the configuration view to setup the parameters. It's possible to use other parameters inside of a new parameter. In that case the "formula" is shown additionally, so that the user knows how it's calculated - and of course the resulting value). I've also added auto completion here, so the moment you type a character, the tool looks for any variable with this subtext and shows it as a proposal for autocompletion:

The second part is the usage in the constraints:

Here you can see the auto-complete in action. I also added that if any variable is used in the constraint, only then the used expression is shown in brackets.

The full thing is working, including auto-updates of all sketches the moment you change any user parameter afterwards. Of course I just played around and did no deep testing.

To be very clear: this is fully vibe-coded without any deeper knowledge of C/CPP, it's really just a showcase of how the final feature for most people would already be very useful.

If you're interested to see the code, you can check it out here:
https://github.com/netzmensch/solvespace/tree/user_parameters

If your policy is just to ignore AI stuff, then sorry for my comment :)

@netzmensch In general I'm opposed to AI generated code, but I am following the trend and am increasingly impressed with its capability. My PR here changes under 300 lines of code, while your AI changed over 700. To be fair, mine doesn't have autocomplete. But then I only changed 10 files vs over 20. There is no reason the solution should be MacOS only and there should be no changes to the build system needed. The AI implementation also adds things like a new flag that isn't needed. I suspect the AI solution doesn't handle units properly if you change units to inches for example (try that after building some stuff with variables). I am impressed that it was able to do anything meaningful with the text window since that implementation is unique and even has custom format strings/characters.

Part of the challenge implementing things like this in Solvespace is just forming a mental model of how the code works, and then figuring out how to extend it. If we generate twice the code needed for new features it will make it that much harder to grasp the internals to make future changes. One of my favorite things to do as a software developer is give the code a good going over with the delete key - AI is doing the opposite as if we had a "generate" key. But I'm keeping my eye on it, and might even try something soon myself.

Thanks that you did not just ignore it.

I'm fully aware of everything you say, and I also fully understand this/have the same view. That's why I explicitly said it's just to get an impression, not that should be merged. It's more kind of a proof of how it can work and especially how the ui should look like from a users perspective. I'm developing software since 18 years as my full time job, I just don't do it in cpp. I have some basic knowledge and I really love solvespace for it's simplicity - that's why I try to give any help. At the moment I just evaluate what different coding agents can do, and since I was waiting for those features, I just wanted to test it out. Seeing results that fast was really impressing, so I just wanted to share what is possible to do - it's not to show how cool AI stuff is. I just continued and also added stuff like adding threads easily or starting new sketches by selecting faces instead of points. Those are huge improvements if you're coming from stuff like fusion.

btw: I tested your current branch in this PR and at least on my mac it was not fully working. The values of the variables kind of saved, but the configuration/group settings always showed "0.000000" for each variable. Every time I try to change the value, it seems to get smaller internally. Also it's not possible to combine variables like my variant can do. and the user does not get any feedback if a variable is used in a constraint.

Regarding the build settings: I'm using the build tools from xcode, maybe that's why some changes to the build settings where needed. I had to do the same for your branch here, so it's very likely a misconfiguration of my system.

So again: see my example just as an idea of how the feature can look like, I'm just waiting for any progress here like the other hardcore fans. I really appreciate every work done by any contributor here 🙏