Why does App::MaterialObjectPython needs a Shape?

[furti]

I don't know exactly when it happend but since some time a Material created with the Arch Workbench has a Shape.

This lead to a problem when texturing objects (https://forum.freecadweb.org/viewtopic. ... 22#p328398). It was an easy fix so no problem here.

But I'm wondering what the purpose of a Shape for a material might be?

Version: 0.19.17505 (Git)

Code: Select all

>>> mat = FreeCAD.ActiveDocument.getObject('Material')
>>> mat.Shape
Traceback (most recent call last):
  File "<input>", line 1, in <module>
AttributeError: 'FeaturePython' object has no attribute 'Shape'
>>> mat
<App::MaterialObjectPython object>
>>> 

Version: 0.19.17803 (Git)

Code: Select all

>>> mat = FreeCAD.ActiveDocument.getObject('Material')
>>> mat.Shape
<Shape object at 000002C15A1118E0>
>>> mat
<App::MaterialObjectPython object>

[DeepSOIC]

It looks like everything 3d now has a shape property:

Code: Select all

>>> o = App.ActiveDocument.addObject("App::GeoFeature")
created object
>>> o.Shape
<Shape object at 000001B917B439D0>
>>> 

[DeepSOIC]

No, it's literally everything:

Code: Select all

>>> o = App.ActiveDocument.addObject("App::DocumentObject")
created object
>>> o.Shape
<Shape object at 000001B917B42590>

[DeepSOIC]

Here's the explanation:
https://github.com/realthunder/FreeCAD_ ... anges#part

PropertyContainerPy also contains a modification to return Shape attribute of any object that does not have a Shape property using Part.getShape(). This modification allows most Part Python feature to work with Link without code modification.

There's one strange thing though. Document is a PropertyContainer too. But:

Code: Select all

>>> App.ActiveDocument.Shape
Traceback (most recent call last):
  File "<input>", line 1, in <module>
AttributeError: 'App.Document' object has no attribute 'Shape'
>>> 

[furti]

I don't know the internals of the App::Link implementation. But why does everything need a shape to work with linking? What is the benefit of having a Null Shape over simply having no shape at all?

So why does a Material need a Shape to be linked? The Shape does not carry any information that defines a Material. But maybe I'm wrong here.

[DeepSOIC]

But why does everything need a shape to work with linking?

No idea. I would naturally expect Shape attribute for all App::Link extended objects then, not for all objects.

[DeepSOIC]

After thinking about it a bit, I think that everything having a Shape is a harmful change. Like, I think I used the presence of Shape attribute as a validity test too. Now, such a validity test is being bypassed, and a null shape is then fed to the algorithm instead. Which is very likely to crash, as I think there are still holes in our API that may let null shape pass to OCC algos, which usually crash then.

[yorik]

After thinking about it a bit, I think that everything having a Shape is a harmful change. Like, I think I used the presence of Shape attribute as a validity test too. Now, such a validity test is being bypassed, and a null shape is then fed to the algorithm instead. Which is very likely to crash, as I think there are still holes in our API that may let null shape pass to OCC algos, which usually crash then.

Agree, I use the same test in several places... And it's also not logical IMHO, we shouldn't suppose every FreeCAD object has a shape. @realthunder what's your take on this?

ping

[realthunder]

After thinking about it a bit, I think that everything having a Shape is a harmful change. Like, I think I used the presence of Shape attribute as a validity test too. Now, such a validity test is being bypassed, and a null shape is then fed to the algorithm instead. Which is very likely to crash, as I think there are still holes in our API that may let null shape pass to OCC algos, which usually crash then.

I'll modify PropertyContainerPy to return the Shape if it is not null. If there is really a property called Shape, the code in PropertyContainerPy will not be reached, and it just returns whatever is inside. If there isn't, it uses Part.getShape() to obtain shape, so you can now simply use App.Link, App.Part, or even plain group to do modeling if you want, for Python code that is. C++ code such as PartDesign still need modification. But I have already modified Part to work with it.

On the other hand, you may also want to know that I have modified all PropertyContainer object to be able to accept dynamic property. Adding a Shape property is of course possible, so you might as well check for null when using the shape. I imagine this extended dynamic property to be especially useful for container type of objects, so that the container can inject any type of property to its children for various purpose. To avoid name conflict, you can form the property name by prepend it with the container's internal name.

[ezzieyguywuf]

After thinking about it a bit, I think that everything having a Shape is a harmful change. Like, I think I used the presence of Shape attribute as a validity test too. Now, such a validity test is being bypassed, and a null shape is then fed to the algorithm instead. Which is very likely to crash, as I think there are still holes in our API that may let null shape pass to OCC algos, which usually crash then.

Agree, I use the same test in several places... And it's also not logical IMHO, we shouldn't suppose every FreeCAD object has a shape. @realthunder what's your take on this?

ping

While I'm not nearly as close to this "the big merge" as any of you are, I feel that I've mucked about in the internals of FreeCAD long enough to merit an opinion.

I agree that that it is not logical that everything have a Shape.

Rather, in keeping with general Object Oriented philosophy: something that is a Shape (or manages the lifetime of one), should contain a Shape.