Comments (15)
Hi hlorus,
Since I already found the solution for the other constraint issue i took a look at this one as well.
The reason here is in the file "operators/add_geometric_constraints.py", the main function of "VIEW3D_OT_slvs_add_horizontal" does indeed not check if the constraint already exists.
I solved it as follows, with two extra imports on the file:
from ..model.horizontal import SlvsHorizontal
from ..model.line_2d import SlvsLine2D
class VIEW3D_OT_slvs_add_horizontal(Operator, GenericConstraintOp):
"""Add a horizontal constraint"""
bl_idname = Operators.AddHorizontal
bl_label = "Horizontal"
bl_options = {"UNDO", "REGISTER"}
type = "HORIZONTAL"
def main(self, context):
# check if the constraint already existed on the selected line or points
hor_constr_exists = False
for c in context.scene.sketcher.constraints.all:
if self.entity1 in c.dependencies() and type(c) == SlvsHorizontal:
if type(self.entity1) == SlvsLine2D:
hor_constr_exists = True
elif self.entity2 in c.dependencies():
hor_constr_exists = True
if not hor_constr_exists:
self.target = context.scene.sketcher.constraints.add_horizontal(
self.entity1,
entity2=self.entity2,
sketch=self.sketch,
)
return super().main(context)
I was still looking to get the constraints from the active sketch only instead of all to save some items from the loop, but could not find it.
It works as intended for all tests i could come up with, please let me know any comments or thoughts.
If everything looks ok, I'll do the same for vertical constraints and create a PR.
Regards,
T
from cad_sketcher.
This problem is likely not specific to horizontal/vertical constraints but rather applies to all constraint tools and probably also to tools that add entities. Therefore a possible solution should try to be somewhat generic, otherwise we end up reinventing the wheel for every tool and have a lot of duplicated code.
from cad_sketcher.
Indeed, I assumed it was only happening on horizontal/vertical constraints.
I took a further look at it, but could not come up with an idea that solves it immediately for all entities in general.
A solution could be something like this in the file "solver.py" you could add a call in the "solve" function to this function (without the printouts):
def _remove_duplicate_constraints(self, context):
constraints_with_deps = []
for c in context.scene.sketcher.constraints.all:
duplicate_found = False
for constraint_with_deps in constraints_with_deps:
if type(c) == constraint_with_deps["constraint_type"]:
if set(c.dependencies()) == set(constraint_with_deps["constraint_dependencies"]):
duplicate_found = True
break
if duplicate_found:
print("duplicate constraint found!")
print(type(c))
print(f" {c.dependencies()}")
print("#######################################")
context.scene.sketcher.constraints.remove(c)
self.solve()
else:
constraint_with_deps = {}
constraint_with_deps["constraint_type"] = type(c)
constraint_with_deps["constraint_dependencies"] = c.dependencies()
constraints_with_deps.append(constraint_with_deps)
This seems to solve it for all constraints. I had to add a call back to the solve method in case there is more than one duplicate, this way it clears all duplicates at once.
For other duplicate entities we could add a separate function looking at lines, arcs etc...
Does this sound like a decent solution?
Update:
The above works when there are duplicate constraints and deletes duplicates while creating/deleting entities.
However I get errors pointing to dangling constraints while dragging existing lines around in this situation.
File "/gizmos/base.py", line 54, in draw_select
if not constr.visible:
AttributeError: 'NoneType' object has no attribute 'visible'
Is "context.scene.sketcher.constraints.remove(c)" the correct way to delete the constraint?
from cad_sketcher.
I would avoid deleting constraints from the solver, this isn't really it's responsibility and could cause issues when we try to delete multiple constraints at once, see: https://docs.blender.org/api/current/info_gotcha.html#no-updates-after-setting-values
Imo the better approach is to avoid creating duplicates in the first place as you've tried first. Maybe just turn the "hor_constr_exists" lookup into a utility function that works for other constraint types as well?
from cad_sketcher.
My focus got a bit too much into trying to clean up existing duplicates instead of just preventing them.
Following your advice I added the following to "class GenericConstraintOp(Operator2d):" in "operators/base_constraint.py"
def exists(self, context, constraint_type=None) -> bool:
new_dependencies = [i for i in [self.entity1, self.entity2, self.sketch] if i is not None]
for c in context.scene.sketcher.constraints.all:
if type(c) == constraint_type:
if set(c.dependencies()) == set(new_dependencies):
print("constraint already exists!")
return True
return False
Then the main method of every class that creates a constraint gets an extra import for the type and a check to the above function. So for all classes in "operators/add_geometric_constraints.py" and also for "operators/add_distance.py", "operators/add_diameter.py" and "operators/add_angle.py".
Example for horizontal:
from ..model.horizontal import SlvsHorizontal
class VIEW3D_OT_slvs_add_horizontal(Operator, GenericConstraintOp):
"""Add a horizontal constraint"""
bl_idname = Operators.AddHorizontal
bl_label = "Horizontal"
bl_options = {"UNDO", "REGISTER"}
type = "HORIZONTAL"
def main(self, context):
if not self.exists(context, SlvsHorizontal):
self.target = context.scene.sketcher.constraints.add_horizontal(
self.entity1,
entity2=self.entity2,
sketch=self.sketch,
)
return super().main(context)
This works perfectly for preventing the issue on all tests I did, does it look good to you?
Thanks again for pointing out the Blender specifics, still have much to learn about those.
from cad_sketcher.
I'd say that looks perfect! Would be great to see a PR :)
from cad_sketcher.
Not completely, just found an edge case: If you put a distance on 2 points you should be able to have both a horizontal and vertical distance constraint, this code doesn't allow that. So I'm going to iron that out first.
Let me know if you can think of other edge cases like this.
On that note I found another bug/missing feature, you can't set dimensions to horizontal or vertical when its a line.
from cad_sketcher.
If I update the "exists" function this way I think everything is covered. Let me know if I can create a PR this way.
def exists(self, context, constraint_type=None) -> bool:
new_dependencies = [i for i in [self.entity1, self.entity2, self.sketch] if i is not None]
distance_dof = 0
has_3D_entity = ((SlvsPoint3D == type(self.entity1) or SlvsPoint3D == type(self.entity2)) or
(SlvsLine3D == type(self.entity1) or SlvsLine3D == type(self.entity2)))
if has_3D_entity:
max_distance_dof = 3
else:
max_distance_dof = 2
if (type(self.entity1) == SlvsCircle or type(self.entity1) == SlvsArc) and type(self.entity2) == SlvsLine2D:
max_distance_dof = 1
for c in context.scene.sketcher.constraints.all:
if type(c) == constraint_type:
if set(c.dependencies()) == set(new_dependencies):
if constraint_type == SlvsDistance:
distance_dof +=1
else:
return True
if distance_dof < max_distance_dof:
return False
else:
return True
I was searching for the status of 3D entities, and their constraints, but currently it only seems half implemented? This code should also cover future plans with it.
Since I went quite far researching this I think I also have the solution that you cannot set line distance constraints to vertical/horizontal: on creation if it is a line you can get the endpoints and set entity1 and 2 to those instead. Then you'll never have duplicates of dimensions on the endpoints + the line, and in case you delete the line the dimension will not be deleted along with it but stay on the points. It would also be one click less for the user (select 1 line vs 2x point).
I'll create a new bug report with the solution for this (have not yet tested it though).
from cad_sketcher.
I don't exactly get the approach with the DoF, why would that matter? 3d constraints aren't supported, there would have to be a bigger effort to get it, therefore i wouldn't look too much into that.
Can't we just check if the constraints settings are the same in the exist method? Afaik constraints keep a list of their main settings in constraint.props, maybe we can use that? Or just hardcode it in the operator.
from cad_sketcher.
The dof (maybe this name is a bit confusing) is just to check how many degrees of freedom are taken by a distance constraint. Following the matrix in model/distance.py I checked how many dimensions each type could have and modified the exists method accordingly (3D was just added in case it would be implemented at some point in the future):
# e1 e2
# ----------------
# line [none] -> max 2 dimensions (hor/vert available)
# point point -> max 2 dimensions (hor/vert available)
# point line -> max 1 dimension (forgot to add a check for this one in code above defaulting to max 1 and checking for max 2 would be better)
# arc point -> max 1 dimension
# arc line -> max 1 dimension
# circle point -> max 1 dimension
# circle line -> max 1 dimension
Using the constraint.props as it is now would not be sufficient, since they only seem to hold the value.
e.g. a distance between two points at 45degrees would have the same value for a horizontal and vertical dimension, thus incorrectly triggering that it already exists.
Maybe we can add a new property there, but I don't see the advantage since we would still need some way to count them (aka the dof way I used).
from cad_sketcher.
Updated for the one I forgot and added a check for entity2 since this doesn't exits on a Diameter/Radius it would look like this:
def exists(self, context, constraint_type=None) -> bool:
if hasattr(self, "entity2"):
new_dependencies = [i for i in [self.entity1, self.entity2, self.sketch] if i is not None]
if ((SlvsPoint3D == type(self.entity1) or SlvsPoint3D == type(self.entity2)) or
(SlvsLine3D == type(self.entity1) or SlvsLine3D == type(self.entity2))):
max_distance_dof = 3
elif ((type(self.entity1) == SlvsLine2D and self.entity2 == None) or
type(self.entity1) == SlvsPoint2D and type(self.entity2) == SlvsPoint2D):
max_distance_dof = 2
else:
max_distance_dof = 1
else:
new_dependencies = [i for i in [self.entity1, self.sketch] if i is not None]
max_distance_dof = 1
distance_dof = 0
for c in context.scene.sketcher.constraints.all:
if type(c) == constraint_type:
if set(c.dependencies()) == set(new_dependencies):
if constraint_type == SlvsDistance:
distance_dof +=1
else:
return True
if distance_dof < max_distance_dof:
return False
else:
return True
from cad_sketcher.
So you're trying to determine if another constraint of the same type makes sense based on the number of DoFs they constrain together with the intention to avoid overconstraining, am i understanding correctly?
Afaik we can't avoid overconstraining anyway as it also depends on other constraints, right?
I would prefer to just avoid duplicated constraints on a data level, where the operator checks if there's already a constraint with the same entities and properties.
Maybe we can add a list of property names to each constraint that should be respected for comparison.
from cad_sketcher.
If we'd like to prevent overconstraining i'd advise to just run the solver in the fini() method and then abort/undo the operation.
Also note that some constraints can have up to 4 entities. So the exists() method should check if all of them overlap with an existing constraint.
from cad_sketcher.
So you're trying to determine if another constraint of the same type makes sense based on the number of DoFs they constrain together with the intention to avoid overconstraining, am i understanding correctly?
No, the intention has nothing to do with overconstraining.
It has the intention to avoid duplicates based on when it would be a duplicate, defined by its type (the matrix explained previously).
Afaik we can't avoid overconstraining anyway as it also depends on other constraints, right?
Right.
I would prefer to just avoid duplicated constraints on a data level, where the operator checks if there's already a constraint with the same entities and properties.
This is exactly what the code above does...
Maybe we can add a list of property names to each constraint that should be respected for comparison.
I don't see the need as the rules are already defined by its type (matrix), which the code above uses.
The only prop that could be added is what I called above "max_distance_dof" (naming can be changed). The logic in the exists method would then be spread out over all of the constraint operators (for some it would be just adding the property, but for others like distance it would require extra logic).
On top of that the exists method would still need to do an individual hasattr check on self.entity2 (because diameter/radius doesn't have it) or we go to a similar situation as in bug report 459 about overriding entity1 and 2.
Personally I think it is much more clear to have all this logic centralised in the exists method.
If we'd like to prevent overconstraining i'd advise to just run the solver in the fini() method and then abort/undo the operation.
No, the solver itself makes mistakes;
Try adding a circle and a line, give them a tangent+coincident constraint without anything else and it will tell you it is overconstrained. So this is not a good option as long as the solver is not 100%.
Even if the solver would be perfect it would still result in a strange user experience (you might want to overconstrain it intentionally while drawing, to see which other constraint you can remove to improve the design. This is how most major CAD packages behave).
Also note that some constraints can have up to 4 entities. So the exists() method should check if all of them overlap with an existing constraint.
Checking overconstrained is not the intention. Not sure if that statement about 4 entities is correct, or if my interpretation is wrong (please elaborate how or which constraint has 4 entities, maybe you meant an entity can have 4 constraints?).
Conclusion:
I'm not trying to sound like a know-it-all but for me the solution is there. Maybe take another look at it, test, discard or change it.
I honestly don't know how or if I can further improve it at the moment. (Apart from maybe adding that one property everywhere. But I think this just makes it less clear.)
I joined the CAD Sketcher Discord (TimS), we can also talk there if that makes things easier.
from cad_sketcher.
Closed by #461
from cad_sketcher.
Related Issues (20)
- Support Blender 4.20 and above HOT 7
- [BUG] Module not found error? HOT 4
- Cannot set Distance constraint to horizontal or vertical for lines [BUG] HOT 6
- Versioning not working since changes to toml versioning were made [BUG] HOT 3
- [BUG] FreeBSD platform is not supported during install HOT 1
- [BUG] ImportError: cannot import name 'GPUShaderCreateInfo' from 'gpu.types' (unknown location) HOT 1
- [BUG] Pip tries to install solver module globally HOT 2
- [BUG] Copy-pasting sketches loses some entities HOT 3
- [BUG] modifiers on generated objects not preserved HOT 8
- Reduce zip size
- [BUG] copy & paste sketch - python script error HOT 1
- [BUG] Error trying to convert sketch to mesh HOT 6
- [BUG] TypeError: Cannot assign a 'Object' value to the existing 'target_curve_object' Group IDProperty HOT 2
- [BUG] Sketcher tools in the default toolbar do nothing in object mode HOT 1
- [Rewrite] Rewrite variable names for easier contribution HOT 2
- [BUG] Constraints Angle supplementary and Distance switch from Horizontal to Vertical or vice versa edits the sketch badly HOT 2
- [BUG] Copy-Paste and duplicate not working HOT 2
- [Feature] Icons for tools HOT 2
- [BUG] - Add a Sketch Impossible to Select the workplane HOT 26
- [BUG] when converting to mesh the output isn't what I expected HOT 4
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from cad_sketcher.