Wrong placement on simple copy of object inside parts in v 0.17

Post here for help on using FreeCAD's graphical user interface (GUI).
Forum rules
and Helpful information
IMPORTANT: Please click here and read this first, before asking for help

Also, be nice to others! Read the FreeCAD code of conduct!
galou_breizh
Posts: 325
Joined: Wed Sep 15, 2010 9:38 am

Wrong placement on simple copy of object inside parts in v 0.17

Postby galou_breizh » Wed Feb 28, 2018 2:38 pm

Hello,

I may have discovered a bug in simple copy of objects inside parts in v0.17. In the attached file, I tried three methods to show the bug:
- "Part / Create simple copy"
- "Draft / Clone"
- Ctrl-C + Ctrl-V
after having selected a cube with a non-null placement inside a part with a non-null placement.

It looks that the part placement is ignored during the copy.

Cf. attached file.

Any comments? Thanks!
Gaël
Attachments
bug_simple_copy.fcstd
(13.8 KiB) Downloaded 22 times
User avatar
easyw-fc
Posts: 2916
Joined: Thu Jul 09, 2015 9:34 am

Re: Wrong placement on simple copy of object inside parts in v 0.17

Postby easyw-fc » Wed Feb 28, 2018 3:03 pm

galou_breizh wrote:
Wed Feb 28, 2018 2:38 pm
Hello,

I may have discovered a bug in simple copy of objects inside parts in v0.17. In the attached file, I tried three methods to show the bug:
- "Part / Create simple copy"
- "Draft / Clone"
- Ctrl-C + Ctrl-V
after having selected a cube with a non-null placement inside a part with a non-null placement.

It looks that the part placement is ignored during the copy.

Cf. attached file.

Any comments? Thanks!
Gaël
this is a known issue ... simple copy ATM doen't work with PDN
I posted already a mantis bug request
https://www.freecadweb.org/tracker/view.php?id=2905
unfortunately it is still unassigned
galou_breizh
Posts: 325
Joined: Wed Sep 15, 2010 9:38 am

Re: Wrong placement on simple copy of object inside parts in v 0.17

Postby galou_breizh » Wed Feb 28, 2018 4:43 pm

Thanks for the quick reply. I must have searched the forum with the wrong keywords because I hadn't found anything.

In the meantime, I wrote a macro to work around this problem:

Code: Select all

__Name__ = 'Deep Copy'
__Comment__ = 'Takes a part and makes a compound out of it'
__Web__ = ''
__Wiki__ = ''
__Icon__ = ''
__Help__ = 'Select a part and launch'
__Author__ = 'galou_breizh'
__Version__ = '0.1'
__Status__ = 'Beta'
__Requires__ = ''

from freecad import app
from freecad import gui


def deep_copy(doc):
    for sel_object in gui.Selection.getSelectionEx():
        deep_copy_part(doc, sel_object.Object)


def deep_copy_part(doc, part):
    if part.TypeId != 'App::Part':
        # Part is not a part, return.
        return

    copied_subobjects = []
    for o in get_all_subobjects(part):
        wrong_types = [
                'App::Origin',
                'App::Plane',
                'App::Line',
                ]
        if o.TypeId in wrong_types:
            continue
        copied_subobjects += copy_subobject(doc, o)

    compound = doc.addObject('Part::Compound', 'Copy of ' + part.Label)
    compound.Links = copied_subobjects
    doc.recompute()


def get_all_subobjects(part):
    """Recursively get all subobjects"""
    l = part.OutList
    for o in l:
        l += get_all_subobjects(o)
    return l


def copy_subobject(doc, o):
    gui_doc = gui.getDocument(doc.Name)
    gui_o = o.ViewObject
    copied_object = []
    try:
        copy = doc.addObject(o.TypeId, o.Name)
        copy.Shape = o.Shape
        copy.Label = 'Copy of ' + o.Label
        copy.Placement = get_recursive_inverse_placement(o).inverse()

        gui_copy = copy.ViewObject
        gui_copy.ShapeColor = gui_o.ShapeColor
        gui_copy.LineColor = gui_o.LineColor
        gui_copy.PointColor = gui_o.PointColor
        gui_copy.DiffuseColor = gui_o.DiffuseColor
    except AttributeError:
        pass
    else:
        copied_object = [copy]
    return copied_object

def get_recursive_inverse_placement(o):
    # We browse the parent in reverse order so we have to multipy the inverse
    # placements and return the inverse placement.
    # Note that we cannot rely on o.InListRecursive because the order there is
    # not reliable.
    p = o.Placement.inverse()
    for parent in o.InList:
        if parent.TypeId == 'App::Part':
            p = p.multiply(get_recursive_inverse_placement(parent))
            break
    return p

if __name__ == "__main__":
    doc = app.activeDocument()
    deep_copy(doc)
I also add the missing information:
OS: Ubuntu 14.04.5 LTS
Word size of OS: 64-bit
Word size of FreeCAD: 64-bit
Version: 0.17.13335 (Git)
Build type: None
Branch: master
Hash: 8aad83f08f7a9e4da9c42b42d14f4ff514ee93c6
Python version: 2.7.6
Qt version: 4.8.6
Coin version: 4.0.0a
OCC version: 7.1.0
Locale: English/UnitedStates (en_US)

Gaël
chrisb
Posts: 29157
Joined: Tue Mar 17, 2015 9:14 am

Re: Wrong placement on simple copy of object inside parts in v 0.17

Postby chrisb » Wed Feb 28, 2018 4:53 pm

galou_breizh wrote:
Wed Feb 28, 2018 2:38 pm
I may have discovered a bug in simple copy of objects inside parts in v0.17.
And there is - also known - one more of this kind: Behaviour is similar with a ShapeBinder. My current workaround is to use Expressions to assign the Placement of the Part to the copy. That makes the copy move together with the Part.
A Sketcher Lecture with in-depth information is available in English, auf deutsch, en français, en español.
User avatar
easyw-fc
Posts: 2916
Joined: Thu Jul 09, 2015 9:34 am

Re: Wrong placement on simple copy of object inside parts in v 0.17

Postby easyw-fc » Wed Feb 28, 2018 10:16 pm

galou_breizh wrote:
Wed Feb 28, 2018 4:43 pm
Thanks for the quick reply. I must have searched the forum with the wrong keywords because I hadn't found anything.

In the meantime, I wrote a macro to work around this problem:

Code: Select all

__Name__ = 'Deep Copy'
....
...

Gaël
Hi Gaël

nice macro...
I have a similar approach with a recursive placement in my kicadStepUp WB...

I modified a bit the macro to manage Part in Part (and for example in a hierarchical STEP) and keep transparency

Code: Select all

__Name__ = 'Deep Copy'
__Comment__ = 'Takes a part and makes a compound out of it'
__Web__ = ''
__Wiki__ = ''
__Icon__ = ''
__Help__ = 'Select a part and launch'
__Author__ = 'galou_breizh'
__Version__ = '0.2'
__Status__ = 'Beta'
__Requires__ = ''

make_compound = True #False

import FreeCAD as app,FreeCADGui as gui

# from FreeCAD import app
# from FreeCAD import gui


def deep_copy(doc):
    for sel_object in gui.Selection.getSelectionEx():
        deep_copy_part(doc, sel_object.Object)


def deep_copy_part(doc, part):
    if part.TypeId != 'App::Part':
        # Part is not a part, return.
        return

    copied_subobjects = []
    for o in get_all_subobjects(part):
        wrong_types = [
                'App::Origin',
                'App::Plane',
                'App::Line',
                'App::Part'
                ]
        if o.TypeId in wrong_types:
            continue
        copied_subobjects += copy_subobject(doc, o)

    if make_compound:
        compound = doc.addObject('Part::Compound', part.Label+'_copy')
        compound.Links = copied_subobjects
    doc.recompute()


def get_all_subobjects(part):
    """Recursively get all subobjects"""
    l = part.OutList
    for o in l:
        l += get_all_subobjects(o)
    return l


def copy_subobject(doc, o):
    gui_doc = gui.getDocument(doc.Name)
    gui_o = o.ViewObject
    copied_object = []
    try:
        copy = doc.addObject(o.TypeId, o.Name)
        copy.Shape = o.Shape
        #copy.Label = 'Copy of ' + o.Label
        copy.Label = o.Label+'_copy'
        copy.Placement = get_recursive_inverse_placement(o).inverse()

        gui_copy = copy.ViewObject
        gui_copy.ShapeColor = gui_o.ShapeColor
        gui_copy.LineColor = gui_o.LineColor
        gui_copy.PointColor = gui_o.PointColor
        gui_copy.DiffuseColor = gui_o.DiffuseColor
        gui_copy.Transparency = gui_o.Transparency
    except AttributeError:
        pass
    else:
        copied_object = [copy]
    return copied_object

def get_recursive_inverse_placement(o):
    # We browse the parent in reverse order so we have to multipy the inverse
    # placements and return the inverse placement.
    # Note that we cannot rely on o.InListRecursive because the order there is
    # not reliable.
    p = o.Placement.inverse()
    for parent in o.InList:
        if parent.TypeId == 'App::Part':
            p = p.multiply(get_recursive_inverse_placement(parent))
            break
    return p

if __name__ == "__main__":
    doc = app.activeDocument()
    deep_copy(doc)
It would be nice to have it also working with Body objects ...

Maurice
galou_breizh
Posts: 325
Joined: Wed Sep 15, 2010 9:38 am

Re: Wrong placement on simple copy of object inside parts in v 0.17

Postby galou_breizh » Thu Mar 01, 2018 9:28 am

Maurice, you probably mean something else than I think because part in part were already supported (a useless copy of parts was done, that's true). Anyway, thanks for your contribution.

Here is a version of the macro that also supports bodies, compounds, and results of boolean operations.

Code: Select all

__Name__ = 'Deep Copy'
__Comment__ = 'Takes a part and makes a compound out of it'
__Web__ = ''
__Wiki__ = ''
__Icon__ = ''
__Help__ = 'Select a part and launch'
__Author__ = 'galou_breizh'
__Version__ = '0.3'
__Status__ = 'Beta'
__Requires__ = 'FreeCAD > v0.17'

from freecad import app
from freecad import gui


def deep_copy(doc):
    for sel_object in gui.Selection.getSelectionEx():
        deep_copy_part(doc, sel_object.Object)


def deep_copy_part(doc, part):
    if part.TypeId != 'App::Part':
        # Part is not a part, return.
        return

    copied_subobjects = []
    for o in get_all_subobjects(part):
        copied_subobjects += copy_subobject(doc, o)

    compound = doc.addObject('Part::Compound', 'Copy of ' + part.Label)
    compound.Links = copied_subobjects
    doc.recompute()


def get_all_subobjects(o):
    """Recursively get all subobjects
    
    Subobjects of objects having a Shape attribute are not included otherwise each
    single feature of the object would be copied. The result is that bodies,
    compounds, and the result of boolean operations will be converted into a
    simple copy of their shape.
    """
    if hasattr(o, 'Shape'):
        return []
    # With the assumption that the attribute InList is ordered, only add the
    # subobject if o is the direct parent, i.e. the first in InList.
    l = [so for so in o.OutList if so.InList and so.InList[0] is o]
    for subobject in l:
        l += get_all_subobjects(subobject)
    return l


def copy_subobject(doc, o):
    copied_object = []
    if not hasattr(o, 'Shape'):
        return copied_object
    vo_o = o.ViewObject
    try:
        copy = doc.addObject('Part::Feature', o.Name + '_Shape')
        copy.Shape = o.Shape
        copy.Label = 'Copy of ' + o.Label
        copy.Placement = get_recursive_inverse_placement(o).inverse()

        vo_copy = copy.ViewObject
        vo_copy.ShapeColor = vo_o.ShapeColor
        vo_copy.LineColor = vo_o.LineColor
        vo_copy.PointColor = vo_o.PointColor
        vo_copy.DiffuseColor = vo_o.DiffuseColor
        vo_copy.Transparency = vo_o.Transparency
    except AttributeError:
        pass
    else:
        copied_object = [copy]
    return copied_object

def get_recursive_inverse_placement(o):
    # We browse the parent in reverse order so we have to multipy the inverse
    # placements and return the inverse placement.
    # Note that we cannot rely on o.InListRecursive because the order there is
    # not reliable.
    # TODO: see if this cannot be replaced with o.getGlobalPlacement().
    p = o.Placement.inverse()
    parent = o.getParentGeoFeatureGroup()
    if parent:
        p = p.multiply(get_recursive_inverse_placement(parent))
    return p

if __name__ == "__main__":
    doc = app.activeDocument()
    deep_copy(doc)
Gaël
User avatar
easyw-fc
Posts: 2916
Joined: Thu Jul 09, 2015 9:34 am

Re: Wrong placement on simple copy of object inside parts in v 0.17

Postby easyw-fc » Thu Mar 01, 2018 10:42 am

galou_breizh wrote:
Thu Mar 01, 2018 9:28 am
Maurice, you probably mean something else than I think because part in part were already supported (a useless copy of parts was done, that's true).
the useless 'Part' was what I meant :D
galou_breizh wrote:
Thu Mar 01, 2018 9:28 am
Here is a version of the macro that also supports bodies, compounds, and results of boolean operations.

Code: Select all

__Name__ = 'Deep Copy'
....
Gaël
thanks... this should be included to useful FC Macros ;)
https://www.freecadweb.org/wiki/Macros_recipes

I'm attaching an icon that could be used (same color of Part DN icon)
deep_copy.svg
(15.01 KiB) Downloaded 123 times
deep-copy.png
deep-copy.png (46.67 KiB) Viewed 1640 times
User avatar
easyw-fc
Posts: 2916
Joined: Thu Jul 09, 2015 9:34 am

Re: Wrong placement on simple copy of object inside parts in v 0.17

Postby easyw-fc » Sun Mar 04, 2018 9:30 am

galou_breizh wrote:
Thu Mar 01, 2018 9:28 am
Here is a version of the macro that also supports bodies, compounds, and results of boolean operations.
....
Gaël
Hi Gaël,
I've found a case where Body seems not to be copied correctly...
I'm attaching the file:
Body-in-Part-failing.FCStd
(43.85 KiB) Downloaded 20 times
'Spacer' is working, 'MainPart' is not.
Thank you for having a look at
Maurice
galou_breizh
Posts: 325
Joined: Wed Sep 15, 2010 9:38 am

Re: Wrong placement on simple copy of object inside parts in v 0.17

Postby galou_breizh » Thu Mar 15, 2018 8:24 am

This happens because the tip of Body001 is not set (i.e. the attribute shape of Body001 is present but the call to isNull() returns True).

I improved the macro so that is this situation, the shape of the body will not be copied.

I'll put the macro on the Macro Recipies page if you feel it's worth it.

Gaël
User avatar
easyw-fc
Posts: 2916
Joined: Thu Jul 09, 2015 9:34 am

Re: Wrong placement on simple copy of object inside parts in v 0.17

Postby easyw-fc » Thu Mar 15, 2018 10:28 am

galou_breizh wrote:
Thu Mar 15, 2018 8:24 am
This happens because the tip of Body001 is not set (i.e. the attribute shape of Body001 is present but the call to isNull() returns True).

I improved the macro so that is this situation, the shape of the body will not be copied.

I'll put the macro on the Macro Recipies page if you feel it's worth it.

Gaël
I think this should be placed on Macro Recipes... :)
Moreover I consider this macro so useful that could be included in Part Design WB Menu as 'Deep Copy with recursive Placement' :D
Very useful for i.e. 3D printing ;)

Please post here when you have placed the macro in recipes...
Thx
Maurice