Proposal: Object Extensions

[ickby]

Hello,

starting with the discussions about the new CoordinateSystem and Container handling it became obvious that freecads current object modeling system is, while very powerful in general, limited to linear inheritance. Over the last weeks I tried to find away around that.

The Problem
Currently every Object in FreeCAD can have exactly one Parent from which it inherits functionality, Properties and Methods. This means that functionality can only be shared if the Objects in question can have the same base class. This is limiting for cases where one can clearly seperate a certain kind of functionality into a interface, but still needs some other functionality and hence must decide which base to use. Best example is the Coordinate System Discussion, but one can think about:

• Attacher: Some Part::Features should be attachable, some not
• Groups: Some Arch tools should be Groups, some not or some even OriginGroups
• Expressions: Some objects may want to be able to define Variables, some not?

Proposed Solution
I extended the FreeCAD type system with a new paradigm called "Extensions". They allow to create special objects, called extensions, which have properties and methods to achieve their functionality. Those extensions can than be added to absolutely any FreeCAD object, and it can be done from c++ and python! In c++ one simply uses multiple inheritance, in python one simply calls "addExtension".

• All Extension properties are added to the object and are accessible from c++ and python
• All Extension methods are added to the object (in c++ really all, in python all methods of the ExtensionPy type, so the designed python interface)
• It is possible to override Extensions methods in c++ and python when they are prepared for it in the extension class. This allow extensions to be customizable by the extended objects.

To see how simple it is an example implementation of a container like App::Part, one time on c++ and the equivalent in python:

Code: Select all

class AppExport Part : public App::DocumentObject, public App::OriginGroupExtension
{
    PROPERTY_HEADER_WITH_EXTENSIONS(App::Part);
};
PROPERTY_SOURCE_WITH_EXTENSIONS(App::Part, App::DocumentObject, (App::OriginGroupExtension))
Part::Part(void){
    GroupExtension::initExtension(this);
}

Code: Select all

class Part:
	def __init__(self, obj):
		obj.addExtension("App::OriginGroupExtensionPython", self)

obj = doc.addObject("App::DocumentObject")
part = Part(obj);

Howto handle Extended Objects
Now every document object can also be for example a group. Hence one cannot anymore check for the GroupObject by type to get all groups. Therefore new functions have been introduced to iterate over all Extensions of an Object and get the wanted extension if available:

Code: Select all

App::DocumentObject* obj = ...;
if(obj.hasExtension(App::GroupExtension::getClassTypeId())) {
	App::GroupExtension* group = dynamic_cast<App::GroupExtension*>(obj.getExtension(App::GroupExtension::getClassTypeId()));
	group->addObject(...);
}

The same works in python. This allows to handle Extensions very abstract, it is unimportant to which object they have been added! This means this also works for objects of other workbenches, one only needs to know the Extension!

The Status
The current Implementation is not perfect, but the fine tuning I like to to when I know it is something that can be included. The Proof of concept is done: https://github.com/ickby/FreeCAD_sf_mas ... Extensions

There you can find the basic extension infrastructure as well as the porting of the app part of group/coordinatesystem classes to the new system. ViewProviders are still old schema.

The Question
Is this something to include into FreeCAD? It does touch the inner workings for sure, and it is not the most subtle change. But IMHO very powerful.
Love to hear your opinions!

[DeepSOIC]

Interesting. I need to digest it.
It seems like with this system, it would be finally possible to make a python derivative of SketchObject. For example, I'd like to make one that presents itself as a face to the outside world. Or even one that replaces certain configurations of geometry with Bezier splines. Right?

[DeepSOIC]

Hi again!
Digesting...
I tried to understand the execute() model of extensions.

First, I think there is a memory leak here:

Code: Select all

DocumentObjectExecReturn *DocumentObject::execute(void)
{
    //call all extensions
    auto vector = getExtensionsDerivedFromType<App::DocumentObjectExtension>();
    for(auto ext : vector)
        if(ext->extensionMustExecute()) 
            ext->extensionExecute();
        
    return StdReturn;
}

ext->extensionExecute returns a pointer to an object, like DocumentObject::execute(). In case of successful execution, the pointer is not "new" and doesn't need to be deleted eventually. However if the execution failed, a pointer to a new error object is returned. This pointer is simply discarded by the loop, so the error object never gets deleted... Or I am simply not understanding the execution system (I don't understand, why all this pointer mess is here in the first place, instead of just throwing a specialized exception).

More to come..

[DeepSOIC]

Second. Let's consider a hypothetical example.
Assumptions:
* SketchObject is not attachable (not as it is now - just imagine)
I want to make a specialized version of SketchObject, "SketchFace", using extensions (for now, in C++...). I want it to:
* I want the SketchFace be attachable. Let's say ExtensionAttachableObject is made already, so I add it to inheritance.
* I want the SketchFace to present itself as a face in its shape property, not as a wire as SketchObject does. So I write up an ExtensionFaceMaker, and add it to inheritance.

So, I must execute the things in this order:
1. Execute ExtensionAttachableObject. It is needed first, because Placement needs to be updated to do projections of external geometry links correctly.
2. Execute SketchObject
3. Execute FaceMakerExtension.

Question. How do I control the order of executes? In this hypothetical example, the order of execution is strictly important. In other cases, it may not be as important, or completely unimportant at all.
Question 2. Who has the power to control executes?
Let's say I attempt to get the order by overriding execute() in SketchFace:

Code: Select all

SketchFace::execute()
{
  extensionAttacher.execute();
  SketchObject::execute(); //<--- executes all extensions
  extensionFaceMaker.execute();
}

For example, SketchObject::Execute may call Part::Feature::execute, which in turn executes all the extensions, messing up the order - extensions are executed twice...

EDIT:
And if SketchObject is to decide the order - FAIL- SketchObject may have no idea what each kind of extension do, and which one to execute first and which last...
And if extensions need to decide if they need to be executed last or first - well, better. But still not terribly good, because what if two extensions say they need to be executed very last? Which "last" is more last?

[ickby]

It was never my attension to let extensions integrate in the execute flow linke you descrobe it, this is not supportet right now. I think oft an extension as a seperate Kind oft functionality which offers properties and methods for this functionality. Those extension can be called via its interface and than also access and change the extendes object, but that must happen fomr someone knowing the extension. There is currently a hook into execute via DocumentObjectExtension but this is intended only to be used for calculating things inside the extension only, and not to change stuff back in the extended object.

I think what you describe needs a different implementation specialised for this purpose, some kind of filters one can register in an object, that are used in the execution. This could than happen in an ordered list.

[ickby]

Deepsoic: i think your example would still work with extension, but a bit different. Extensions are intended for creating new classes, not changing existing ones. Hence one would create a extension doing the convertion stuff, ,meaning an extension providing all needed methods. Than a new object is created deriving from sketchobject with your extension. In the new objects execute you call the extension methods in the correct order to do the convertion. Note that in this case your extension should not derive from DocumentObjectExtension but can derive from Extension directly (oor leave execute unimplented if derived from DocumentObjectExtension)

[ickby]

3rd repliy, sorry, early in the morning and I'm still sleepy I just now realise that you already did what I proposed in my last post.

Code: Select all

SketchFace::execute()
{
  extensionAttacher.execute();
  SketchObject::execute(); //<--- executes all extensions
  extensionFaceMaker.execute();
}

Note that "execute all extensions" is not true. It only calls the special execute methods of DocmentObjectExtension, but not all extensions must be of that type. And e ven if they are they still do not need to override their special extensionExecute() method. Your proposed one for example would directly derive from Extension and not have a execute method as it does not need it at all. You would hence not call it twice.

[wmayer]

It looks like a very powerful approach.

I haven't built & tested your branch. So, does this already fix the problems you wanted to solve?
Classes that don't want to be extended are they affected by this?
And the idea of extensions I guess this applies to all instances of a class and not just certain instances.
What about persistence? Will this information then restored on object creation?

I looked quickly through your code and saw that a lot of static_cast has been replaced by dynamic_cast. Functions like freecad_dynamic_cast will become completely obsolete with this because there is no reason to perform two checks, i.e. do a typeId check first and then do a second check inside dynamic_cast.
So, what is the point of this? Or is the assumption no longer true that if the typeId check passes that the object must be of this type, i.e. that a static_cast is not safe?

[ickby]

I haven't built & tested your branch. So, does this already fix the problems you wanted to solve?

Yes, this does all I need for the grouping (as already implemented)

Classes that don't want to be extended are they affected by this?

Not at all, they work just like before

And the idea of extensions I guess this applies to all instances of a class and not just certain instances.

For c++ yes, multiple inheritance does effect al instances. Python uses dynamic extensions added at runtime, so they are per instance. This is why they should be added when creating a new class in python. In theory it is also possible to add a extension dynamically in c++, but never tried it.

What about persistence? Will this information then restored on object creation?

In c++ yes due to the inheritance. All Properties of the extensions are stored with the extended object automatically. In python the extensions are not restored. They work like the dynamic properties: they must be added on class initialisation. But as save/restore of properties comes after the class init they are again restored too (if the extension was added)

I looked quickly through your code and saw that a lot of static_cast has been replaced by dynamic_cast. Functions like freecad_dynamic_cast will become completely obsolete with this because there is no reason to perform two checks, i.e. do a typeId check first and then do a second check inside dynamic_cast.
So, what is the point of this? Or is the assumption no longer true that if the typeId check passes that the object must be of this type, i.e. that a static_cast is not safe?

No the reason for this lies in the inheritance approach: As ExtensionContainer (and hence documentObject) as well as Extension derive from PropertyContainer to store properties I needed to make the inheritance virtual to handle the occuring diamond pattern. However, briding a virtual inheritance is not allowed with a static_cast, it gives a compiler error. This happens for example for casts from PropertyContainer to DocumentObject as there the virtual inheritance is in between, but not from documentObject to Part::Feature, there static_cast still works.

I have to admid that I strictly replaced static by dynamic casts where needed without rethinking that many checks become obsolete due to this. I will reiterate the places and adopt accordingly.

[ickby]

I worked further on this and I'm pretty close to finish. I added the extension infrastructure to ViewProviders and portet the Groups classes to this new way of working. Now it is possible to make any object in FreeCAD a group/OriginGroup and extend their viewproviders with the needed functionality too with extensions.

I will test this further to see if some regressions occure, but I'm quite pleased with the code so far.
https://github.com/ickby/FreeCAD_sf_mas ... Extensions