Re: Graphics properties as objects

[Shai Ayal]

On Jan 8, 2008 4:09 PM, Michael Goffioul <address@hidden> wrote:
> On 1/8/08, John W. Eaton <address@hidden> wrote:
> > | class property
> > | {
> > | public:
> > |     property (base_property *p, bool persist = false);
> > |     ...
> > | };
> >
> > What is the rest of this class?  Does it include reference counting?
> > How is it expected to be used?
>
> See the attached file for a full patch corresponding to my current
> status (note: this is unfinished work-in-progress, it does not compile).
> The class "property" is a wrapper around base_property with reference
> counting. It provides a value-typed generic interface for accessing
> properties, suitable for storing in a map.
>
> > | class base_properties
> > | {
> > | public:
> > |     ...
> > |
> > |     // this method is to add dynamic properties into the map
> > |     void insert_property (const std::string& name, property p)
> > |       {
> > |         p.set_name (name);
> > |         p.set_parent (__myhandle__);
> > |         all_props[name] = p;
> > |       }
> >
> > I'm surprised to see the property argument above not be a reference or
> > pointer.  Doesn't this cause a copy to be made when insert_property is
> > called?
>
> It just increases the reference counting of the underlying base_property
> object.
>
> > |     Matrix get_position (void) const { return position.matrix_value (); }
> >
> > Would the backend never need access to the additional information
> > stored in the matrix_property object?  If not then I think this is
> > OK.  Otherwise, why not just return the matrix_property object
> > directly?
>
> That's why I also added a "get_property" method to the base_properties
> class (see attached patch), to keep the possibility to access the
> property object. I'm not sure about that part of my proposal: initially, this
> was intended to make life easier in the C++ backend, because it would
> typically access position with something like "get_position().matrix_value()"
> (provided get_position returns matrix_property). For radio properties,
> the equivalent accessor would be something like
> "bool xlimmode_is (const std::string& val) const;". So in the end, while
> maybe making the C++ code easier to write, it would make genprops.awk
> more complex, because the accessors will be different for every kind of
> property.

There are also other cases where access to the property is important
-- e.g. in color_property, you should have access to the
is_rgb/is_radio members and also it's nice to have access to the rgb
member for easy conversion to the OpenGL 3dv type functions. Together
with making genprops more complicated, I think this is reason enough
to provide direct access to the properties.

Shai