Don't know. This is the real issue here IMO.adrianinsaval wrote: ↑Tue Jan 25, 2022 7:06 pm but why is removeSplitter() allowed without using copy()?
Ticket #6266: recompute undos removeSplitter
Forum rules
Be nice to others! Read the FreeCAD code of conduct!
Be nice to others! Read the FreeCAD code of conduct!
Re: Ticket #4754: recompute undos removeSplitter
-
- Veteran
- Posts: 5513
- Joined: Thu Apr 05, 2018 1:53 am
Re: Ticket #4754: recompute undos removeSplitter
Well, obj.Shape.removeSplitter() does not modify obj.Shape, but instead returns a new refined shape object, so there's no need to disallow calling it on obj.Shape. Unless I'm not thinking of some good reason to disallow it.
Re: Ticket #4754: recompute undos removeSplitter
You're totally right.TheMarkster wrote: ↑Thu Jan 27, 2022 3:36 am Well, obj.Shape.removeSplitter() does not modify obj.Shape, but instead returns a new refined shape object, so there's no need to disallow calling it on obj.Shape. Unless I'm not thinking of some good reason to disallow it.
I pushed the test and the problem is far more generic. To reproduce :
* New blank document, add default Part/Cube and default Part/Cylinder
* Move Cylinder with it's Placement value (say [10,0,0])
* Enter App.ActiveDocument.Box.Shape = App.ActiveDocument.Cylinder.Shape
* Recompute => The Cube shape is kept, but it is moved to Cylinder placement
I have absolutely no idea where the magics take place in the code when assigning a new value to '.Shape'.
wmayer wrote: Ping
- adrianinsaval
- Veteran
- Posts: 5553
- Joined: Thu Apr 05, 2018 5:15 pm
Re: Ticket #4754: recompute undos removeSplitter
Yes as I said the issue has nothing to do with removeSplittter() (as such, can we change the title as it is misleading?) however (going off topic here) 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? I understand obj.Shape.scale(3) should be disallowed since this does modify it.TheMarkster wrote: ↑Thu Jan 27, 2022 3:36 am Well, obj.Shape.removeSplitter() does not modify obj.Shape, but instead returns a new refined shape object, so there's no need to disallow calling it on obj.Shape. Unless I'm not thinking of some good reason to disallow it.
So the issue is that when trying to assign a new shape to the object, that placement is taken but not the actual shape. I wonder if it should really take the placement at all, even when changing the shape is allowed, aren't they supposed to be independent?
-
- Veteran
- Posts: 5513
- Joined: Thu Apr 05, 2018 1:53 am
Re: Ticket #4754: recompute undos removeSplitter
I was also thinking the same thing. Any function that does not modify the shape, but returns a new one should be allowed.adrianinsaval wrote: ↑Thu Jan 27, 2022 5:16 pm
Yes as I said the issue has nothing to do with removeSplittter() (as such, can we change the title as it is misleading?) however (going off topic here) 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? I understand obj.Shape.scale(3) should be disallowed since this does modify it.
Shape and Placement are different things, but the Shape also has a placement. Try this: create a new cylinder in Part workbench. Move it to 10,0,0. Select and press Ctrl+Shift+P. In the console enter obj.Placement and obj.Shape.Placement. You'll get the same placement for both the object and the shape.
Re: Ticket #4754: recompute undos removeSplitter
You're right. Maybe open a new topic to fix this ?adrianinsaval wrote: ↑Thu Jan 27, 2022 5:16 pmI 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? I understand obj.Shape.scale(3) should be disallowed since this does modify it.
- adrianinsaval
- Veteran
- Posts: 5553
- Joined: Thu Apr 05, 2018 5:15 pm
Re: Ticket #4754: recompute undos removeSplitter
Ok then here's the thing, you CAN change the shape, it's just that it's regenerated the moment you recompute. Try this:
A new object with the cube shape is created, despite providing the cylinder's shape. So when you assign the shape you also assign the placement, however recomputing then regenerates the correct shape but placement is untouched as recompute has no reason to change it. Should we then actually forbid changing the shape of these objects? Ideally IMO assigning a shape shouldn't change the placement at all but I don't know how complicated that would be.
Code: Select all
box=App.ActiveDocument.addObject("Part::Box","Box")
cyl=App.ActiveDocument.addObject("Part::Cylinder","Cylinder")
cyl.Visibility= False
box.Visibility= False
App.ActiveDocument.recompute()
cyl.Shape=box.Shape
Part.show(cyl.Shape)
Re: Ticket #4754: recompute undos removeSplitter
There is a global problem. I have no idea what the best solution would be. @wmayer probably could be of good advice here, because he may know about the impacts of choosing this or that solution.adrianinsaval wrote: ↑Thu Jan 27, 2022 5:41 pm Should we then actually forbid changing the shape of these objects? Ideally IMO assigning a shape shouldn't change the placement at all but I don't know how complicated that would be.
- adrianinsaval
- Veteran
- Posts: 5553
- Joined: Thu Apr 05, 2018 5:15 pm
Re: Ticket #4754: recompute undos removeSplitter
Yes, I'm talking out of my ass here, I have no idea were any of this is handled in code or the implications. I guess this is way above our paygrade
Re: Ticket #4754: recompute undos removeSplitter
An object's placement is always kept in sync with the placement of its shape. This means when you change the placement of an object it will be applied to the shape.adrianinsaval wrote: ↑Thu Jan 27, 2022 5:41 pm Should we then actually forbid changing the shape of these objects? Ideally IMO assigning a shape shouldn't change the placement at all but I don't know how complicated that would be.
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.
So, this means if you call removeSplitter() inside the execute() method
Code: Select all
class MyFeature:
def execute(self, fp):
shape = ...
fp.Shape = shape.removeSplliter()
Code: Select all
from freecad import app as FreeCAD
import Draft
doc = FreeCAD.newDocument("testDoc")
box1 = doc.addObject("Part::Box", "Box1")
doc.recompute()
box2 = Draft.move(box1, FreeCAD.Vector(1, 0, 0), copy=True)
placement = box2.Placement
box2.Shape = box2.Shape.removeSplitter()
box2.Placement = placement
doc.recompute()
Code: Select all
from freecad import app as FreeCAD
import Draft
doc = FreeCAD.newDocument("testDoc")
box1 = doc.addObject("Part::Box", "Box1")
doc.recompute()
box2 = Draft.move(box1, FreeCAD.Vector(1, 0, 0), copy=True)
shape = box2.Shape.removeSplitter()
shape.Placement = box2.Placement
box2.Shape = shape
doc.recompute()