Ticket #6266: recompute undos removeSplitter

Have some feature requests, feedback, cool stuff to share, or want to know where FreeCAD is going? This is the place.
Forum rules
Be nice to others! Read the FreeCAD code of conduct!
openBrain
Veteran
Posts: 9034
Joined: Fri Nov 09, 2018 5:38 pm
Contact:

Re: Ticket #4754: recompute undos removeSplitter

Post by openBrain »

wmayer wrote: Fri Jan 28, 2022 3:38 pm And when you assign a new shape to an object then its placement will change, too. However, in a feature's execute() function there is a mechanism implemented that checks if the shape is being changed during a recompute and if yes the object's placement is applied to the shape. This is to make sure that object placement and shape placement is equal.
Thanks for deep insight.
However there is still something unclear to me, regarding following snippet :

Code: Select all

doc = App.newDocument()
box = doc.addObject('Part::Box')
cyl = doc.addObject('Part::Cylinder')
doc.recompute()
cyl.Placement.Base.x = 10
box.Shape = cyl.Shape.copy()
doc.recompute()
At the end, shape of the box is still a cube (not a cylinder) but it's now placed at cylinder position... :?
wmayer
Founder
Posts: 20243
Joined: Thu Feb 19, 2009 10:32 am
Contact:

Re: Ticket #4754: recompute undos removeSplitter

Post by wmayer »

At the end, shape of the box is still a cube (not a cylinder) but it's now placed at cylinder position...
Of course. When assigning an arbitrary shape to a box feature it gets touched and on the next recompute it will restore its actual shape again. And when assigning the shape outside a recompute the box will accept and keep the placement of the assigned shape.
openBrain
Veteran
Posts: 9034
Joined: Fri Nov 09, 2018 5:38 pm
Contact:

Re: Ticket #4754: recompute undos removeSplitter

Post by openBrain »

wmayer wrote: Fri Jan 28, 2022 6:26 pm Of course. When assigning an arbitrary shape to a box feature it gets touched and on the next recompute it will restore its actual shape again. And when assigning the shape outside a recompute the box will accept and keep the placement of the assigned shape.
All this may sounds obvious for you, and eventually for me after your explanation and having a look at the code.
But IMO from a user PoV, it's somewhat puzzling behavior. ;)
This is not of high importance though. I'll eventually have a look to see if I can propose something around that. And most probably documenting this somewhere could be good, but not trivial.
User avatar
Kunda1
Veteran
Posts: 13434
Joined: Thu Jan 05, 2017 9:03 pm

Re: Ticket #4754: recompute undos removeSplitter

Post by Kunda1 »

wmayer wrote: Fri Jan 28, 2022 3:38 pm So, issue issue #6266 actually isn't a bug but the pitfalls should be documented.
Hi wmayer, where do you suggest this be documented? Also is there a way to identify the pitfall behavior as it's happening and orient the user in an error message in the console?
Alone you go faster. Together we go farther
Please mark thread [Solved]
Want to contribute back to FC? Checkout:
'good first issues' | Open TODOs and FIXMEs | How to Help FreeCAD | How to report Bugs
User avatar
Roy_043
Veteran
Posts: 8450
Joined: Thu Dec 27, 2018 12:28 pm

Re: Ticket #4754: recompute undos removeSplitter

Post by Roy_043 »

I have missed this discussion before. The use of "Part::Box" in the examples is actually confusing and obscuring the behavior of the removeSplitter() function IMO. Maybe the question should be: Why does the removeSplitter() function reset the Placement of the shape?

Code: Select all

from freecad import app as FreeCAD
import Draft

doc = App.activeDocument()

box1 = Part.show(Part.makeBox(10, 10, 10))
doc.recompute()

box2 = Draft.move(box1, FreeCAD.Vector(1, 0, 0), copy=True)
doc.recompute()
print(box2.Placement.Base)        # Vector (1.0, 0.0, 0.0)
print(box2.Shape.CenterOfGravity) # Vector (6.0, 5.0, 5.0)

box2.Shape = box2.Shape.removeSplitter()
doc.recompute()
print(box2.Placement.Base)        # Vector (0.0, 0.0, 0.0)
print(box2.Shape.CenterOfGravity) # Vector (6.000000000000001, 5.0, 5.0)
User avatar
adrianinsaval
Veteran
Posts: 5541
Joined: Thu Apr 05, 2018 5:15 pm

Re: Ticket #4754: recompute undos removeSplitter

Post by adrianinsaval »

adrianinsaval wrote: Thu Jan 27, 2022 5:16 pm I don't understand what's the rationale behind allowing/disallowing functions, AFAIK obj.Shape.scaled(3) doesn't modify obj.Shape either, it returns a scaled shape, so why disallow?
in case somebody else was left wondering about this it should be fixed now: https://forum.freecadweb.org/viewtopic. ... 23#p604421
User avatar
adrianinsaval
Veteran
Posts: 5541
Joined: Thu Apr 05, 2018 5:15 pm

Re: Ticket #4754: recompute undos removeSplitter

Post by adrianinsaval »

revisiting this issue IMO the problem is that we are allowed to (at least temporarily) assign a value to something that is set as an immutable property, @wmayer is it possible to forbid or reject value assignment for such properties? is it worth doing it or should we just close this issue?
wmayer
Founder
Posts: 20243
Joined: Thu Feb 19, 2009 10:32 am
Contact:

Re: Ticket #4754: recompute undos removeSplitter

Post by wmayer »

adrianinsaval wrote: Wed Jun 22, 2022 3:24 pm revisiting this issue IMO the problem is that we are allowed to (at least temporarily) assign a value to something that is set as an immutable property
The Shape property is not supposed to be immutable because there are uses cases where you have a non-parametric feature and want to change it from outside. As an example consider the base class Part::Feature that provides the Shape property but doesn't re-implement the execute() method. Usually you use this feature when importing a shape from a data file (.brep, .step, .iges, ...).
is it worth doing it or should we just close this issue?
IMO, nothing needs to be done.
User avatar
adrianinsaval
Veteran
Posts: 5541
Joined: Thu Apr 05, 2018 5:15 pm

Re: Ticket #4754: recompute undos removeSplitter

Post by adrianinsaval »

wmayer wrote: Fri Jun 24, 2022 9:16 am The Shape property is not supposed to be immutable because there are uses cases where you have a non-parametric feature and want to change it from outside.
yeah that makes sense, however I get this error when running for example obj.Shape.scale(5)
Traceback (most recent call last):
File "<input>", line 1, in <module>
ReferenceError: This object is immutable, you can not set any attribute or call a non const method
It makes sense for parametric objects but I also get it for a Part simple copy.
IMO, nothing needs to be done.
the issue should be closed then
wmayer
Founder
Posts: 20243
Joined: Thu Feb 19, 2009 10:32 am
Contact:

Re: Ticket #4754: recompute undos removeSplitter

Post by wmayer »

adrianinsaval wrote: Fri Jun 24, 2022 11:27 pm yeah that makes sense, however I get this error when running for example obj.Shape.scale(5)
Doing

Code: Select all

obj.Shape.scale(5)
or

Code: Select all

obj.Shape = shape
are two different things. With the latter the method PropertyPartShape::setPyObject() is called so that it can notify its parent feature class to make sure everything is in sync (like updating the placement) while with the former this won't be possible.
It makes sense for parametric objects but I also get it for a Part simple copy.
This is independent on whether a feature is parametric or not. The only way to do what you want is:

Code: Select all

shape = obj.Shape.copy()
shape.scale(5)
obj.Shape = shape
Post Reply