Read only property confusion

Here's the place for discussion related to coding in FreeCAD, C++ or Python. Design, interfaces and structures.
ickby
Posts: 2918
Joined: Wed Oct 05, 2011 7:36 am

Read only property confusion

Postby ickby » Thu Oct 22, 2015 5:30 am

Hello,

while working on the expressions in the property editor I encountered a problem with the read only status of properties. I'm working in this file:
https://github.com/FreeCAD/FreeCAD/blob ... tyItem.cpp

I want to know if a property is read only and tried the following methods (pseudocode, no compiler here right now)

Code: Select all

property->getContainer()->isReadOnly(property)
property->StatusBits.test(2)
Both return false even for read only properties, as can be seen for spreadsheet dynamic properties. Those methods are used in line 95 to detect read only properties, but it does not really work. I wondered, because when one tries to set a read only property via the python console, it is detected as read only and an error is printed. The used detection method is this one:

Code: Select all

proeprty->getContainer()->getPropertyType(prop)  & Prop_ReadOnly
which returns true, even if the methods above return false.

Do I miss something obvious here or is this a bug?
User avatar
DeepSOIC
Posts: 6965
Joined: Fri Aug 29, 2014 12:45 am
Location: Saint-Petersburg, Russia

Re: Read only property confusion

Postby DeepSOIC » Thu Oct 22, 2015 7:00 pm

I don't know, but I think there may be two readonlyness grades involved. Like - one restricts modifying the property in property editor, and the other one indicates a deep readonlyness (the property that can't be modified with python). The first, weak readonlyness, is needed for making read-only properties of python document objects.
wmayer
Site Admin
Posts: 14782
Joined: Thu Feb 19, 2009 10:32 am

Re: Read only property confusion

Postby wmayer » Thu Oct 22, 2015 8:15 pm

There are two ways to set the read-only attribute to a property.
1. The normal way is to set it via the ADD_PROPERTY macro in the constructor of sub-classes of PropertyContainer. This attribute is stored in the member of PropertyContainer PropertyData. For memory efficiency reasons this is a static member which means that the attributes need to be stored only once when the first instance of the class is created. But this also means that these attributes affect all instances, i.e. if in class MyFeature you set a property MyProperty read-only then this affects all instances of MyFeature.

2. Now there are cases where you want to change the read-only attribute on the fly or set it only for certain instances. Since the property attributes in 1.) are static this is not an option. So, therefore the StatusBits bitset of the Property class reserves the bit "2" therefore. But note, this bit is a pure GUI thing where the user can't change values in GUI but in contrast to 1.) values still can be changed in Python.

The property editor takes care of this: https://github.com/FreeCAD/FreeCAD/blob ... em.cpp#L95

Code: Select all

ro &= (parent->isReadOnly(*it) || (*it)->StatusBits.test(2));

Example:

Code: Select all

doc=App.newDocument()
box1=doc.addObject("Part::Box","Box1")
box2=doc.addObject("Part::Box","Box2")
doc.recompute()
box1.setEditorMode("Height",("ReadOnly",))
box1.Height=12 # OK
Select box1 and check the property editor for Height. You can't change it because it's read-only.
Select box2 and check the property editor now. You can change it because only box1 is affected.
ickby
Posts: 2918
Joined: Wed Oct 05, 2011 7:36 am

Re: Read only property confusion

Postby ickby » Thu Nov 12, 2015 9:33 pm

Hello werner,


thanks for the feedback. I still think that there is a oddity. The case in question is the following:

1. Create a spreadsheet
2. Add a number and make a cell alias named e.g. "test"
3. Deselect the spreadsheet in the tree
4. Select the spreadsheet in the tree
5. try to edit the property "test"

You find that the editing is possible in the gui, but the python console gives an error that the property is read only. To test this further I added some test code to PropertyItem to test the different ways for detecting the read only status:

Code: Select all

void PropertyItem::setPropertyData(const std::vector<App::Property*>& items)
{
    for( std::vector<App::Property*>::const_iterator it = items.begin(); it != items.end(); ++it ) {
        
        Base::Console().Message("Property: %s\n", (*it)->getName());
        Base::Console().Message("Container read only: %s\n", (*it)->getContainer()->isReadOnly(*it) ? "true" : "false");
        Base::Console().Message("Status bit read only: %s\n", (*it)->StatusBits.test(2) ? "true" : "false");
        Base::Console().Message("Property Type read only: %s:\n\n", ((*it)->getContainer()->getPropertyType(*it) & App::Prop_ReadOnly) ? "true" : "false");
    };
For the spreadsheet scenario the following output is generated:

Code: Select all

Property: test
Container read only: false
Status bit read only: false
Property Type read only: true
The PropertyItem checks read only with the first two methods, the python code for setting properties uses the last one. They give different results. I really don't know what is the correct thing here, but it does seem odd.
wmayer
Site Admin
Posts: 14782
Joined: Thu Feb 19, 2009 10:32 am

Re: Read only property confusion

Postby wmayer » Fri Nov 13, 2015 5:02 pm

The property item classes work correctly but the problem is somewhere else.

The class PropertyContainer has two methods to get the read-only status of a property: isReadOnly and getPropertyType and both of them are virtual.
The class PropertyItem uses isReadOnly() while DocumentObjectPy uses getPropertyType().

Now the class Sheet uses the mechanism to support dynamic properties and therefore it correctly overrides the method getPropertyType() but does not override isReadOnly().

What happens when PropertyItem uses isReadOnly() is that the method of PropertyContainer is called. This, however, doesn't know about the dynamic property 'test' and just gives 0 as result.

Solution:
1. The easiest would be to override the functions isReadOnly, isHidden, ... in the Sheet class.
2. The API of PropertyContainer is a bit redundant when isReadOnly (& co) and getPropertyType are virtual. Actually it would suffice to make isReadOnly (& co) non-virtual but internally use getPropertyType().
wmayer
Site Admin
Posts: 14782
Joined: Thu Feb 19, 2009 10:32 am

Re: Read only property confusion

Postby wmayer » Sat Nov 14, 2015 1:19 am

eivindkvedalen
Posts: 597
Joined: Tue Jan 29, 2013 10:35 pm

Re: Read only property confusion

Postby eivindkvedalen » Sat Nov 14, 2015 6:55 pm

Thank you for looking into this. While we're at it, it would be nice to solve this viewtopic.php?f=3&t=13140 issue as well. It boils down to somehow signal that a dynamic property has been removed from the system. Currently, if you are unlucky, FreeCAD crashes in the property editor if a spreadsheet cell changes its type, as this is done by first removing the property, and then creating a new one with the same name. Any suggestions on how to solve this?

Eivind
wmayer
Site Admin
Posts: 14782
Joined: Thu Feb 19, 2009 10:32 am

Re: Read only property confusion

Postby wmayer » Sun Nov 15, 2015 12:04 pm

Currently, if you are unlucky, FreeCAD crashes in the property editor if a spreadsheet cell changes its type, as this is done by first removing the property, and then creating a new one with the same name. Any suggestions on how to solve this?
OK, I see what happens. I will work out a solution soon. One could add new boost signals to the document class signalAddProperty and signalRemoveProperty and connect this with some slot function of the property editor.