Re: [Inkscape-devel] Widget c++-sification
Resending: original message was send accidentally off-list.
A safe way to mitigate many of the C vs C++ issues with Gtkmm is to keep
a proper logical separation between Gtkmm UI objects and the functionality they trigger or state they manifest. The bulk of code ends up in plain C++ classes, while the Gtkmm widgets merely provide a thin layer to expose
in the 'proper' manner. That might including moving certain
functionality out of the widget classes themselves.
I see your point. Could you please name some inkscape UI classes that preserve the separation?
I would like to go on with SPColorSelector refactoring. In Inkscape color can be selected using one of two widgets: sp-color-wheel-selector, sp-color-icc-selector. They share common functionality in sp-color-selector. Currently there are six structures consisting of both Gobjects and c++ classes.
I made a simplified diagram, listing possible options for refactoring [1]. 1) ColorSelector widgets contain a reference to their state (ColorSelector). It is accessible via getColorSelector() method. Optional ColorSelectorWidget abstract class provides the method. Parent UI component would initialize color selector widgets in ctor. Reference to widget would be kept in ctor only. Parent class would contain a pointer to widget state, returned by getColorSelector(). 2) Double inheritance. ColorSelector provides widget state. Concrete implementations inherit ColorSelector and aproppriate Gtk::Widget. 3) Full wrapper. Color selector widgets contain reference to their state and have full set of thin wrapper methods. They only call corresponding methods in ColorSelector.
Which is the best? What is the suggested naming and namespacing of mentioned classes? Which of the options fits the separation between gtkmm and functionality best?
[1] https://www.dropbox.com/s/nvie03p9ee9r8ry/colorSelector.pdf
Regards, Tomasz
On Sat, May 24, 2014 at 12:41 PM, Tomasz Boczkowski <penginsbacon@...400...>wrote:
A safe way to mitigate many of the C vs C++ issues with Gtkmm is to
keep a proper logical separation between Gtkmm UI objects and the functionality they trigger or state they manifest. The bulk of code ends up in plain C++ classes, while the Gtkmm widgets merely provide a thin layer to expose
in the 'proper' manner. That might including moving certain
functionality out of the widget classes themselves.
I see your point. Could you please name some inkscape UI classes that preserve the separation?
I would like to go on with SPColorSelector refactoring. In Inkscape color can be selected using one of two widgets: sp-color-wheel-selector, sp-color-icc-selector. They share common functionality in sp-color-selector. Currently there are six structures consisting of both Gobjects and c++ classes.
I made a simplified diagram, listing possible options for refactoring [1].
- ColorSelector widgets contain a reference to their state
(ColorSelector). It is accessible via getColorSelector() method. Optional ColorSelectorWidget abstract class provides the method. Parent UI component would initialize color selector widgets in ctor. Reference to widget would be kept in ctor only. Parent class would contain a pointer to widget state, returned by getColorSelector(). 2) Double inheritance. ColorSelector provides widget state. Concrete implementations inherit ColorSelector and aproppriate Gtk::Widget. 3) Full wrapper. Color selector widgets contain reference to their state and have full set of thin wrapper methods. They only call corresponding methods in ColorSelector.
Which is the best? What is the suggested naming and namespacing of mentioned classes? Which of the options fits the separation between gtkmm and functionality best?
[1] https://www.dropbox.com/s/nvie03p9ee9r8ry/colorSelector.pdf
On Wed, May 21, 2014 at 8:21 PM, Jon A. Cruz <jon@...18...> wrote:
On Wed, May 21, 2014, at 10:31 AM, Tomasz Boczkowski wrote:
Thank you for the answer.
Also, take care on what widgets you are looking at. We have a few
groupings in our source tree that are meant to stay C.
I started with src/widgets/sp-color-slider. I plan to continue with widgets related to fill and stroke dialog (sp-color-notebook, sp-color-scales, sp-color-wheel-selector etc.). Are they safe to port?
Those should be OK... but in the 'proper' manner. That might including moving certain functionality out of the widget classes themselves. *Additionally* those I recognize as some of the first files I poked at when I initially joined Inkscape (before the project had its name). They're probably quite outdated by this point.
Then also keep in mind that "sp" stands for Sodipodi, so when you move files you can drop such. Just be sure to do it in a source-control friendly way that won't break history. :-)
- Should child widgets be stored as pointers as in glyph.h or
directly as in text-edit.h?
Which file are you referencing as "glyph.h"?
src/ui/dialog/glyphs.h
Ahh, that's an interesting question.
For widgets, the design and intent of Gtkmm is to place children via new
- Gtk::manage(). That ensures consistent object lifetimes and helps reduce
memory leaks. For even stronger encapsulation often it's better to not list pointers as members in the public .h file, but keep them all to setup in the constructor. Since you won't have to delete them, you don't have to track them... for those cases where you set up events and handlers then add the child. That also goes on to event driven programming, and fewer bugs once you wrap your head around it.
-- Jon A. Cruz jon@...18...
"Accelerate Dev Cycles with Automated Cross-Browser Testing - For FREE Instantly run your Selenium tests across 300+ browser/OS combos. Get unparalleled scalability from the best Selenium testing platform available Simple to use. Nothing to install. Get started now for free." http://p.sf.net/sfu/SauceLabs _______________________________________________ Inkscape-devel mailing list Inkscape-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/inkscape-devel
On Sat, May 24, 2014, at 03:43 AM, Tomasz Boczkowski wrote:
Resending: original message was send accidentally off-list.
A safe way to mitigate many of the C vs C++ issues with Gtkmm is to
keep a proper logical separation between Gtkmm UI objects and the functionality they trigger or state they manifest. The bulk of code ends up in plain C++ classes, while the Gtkmm widgets merely provide a thin layer to expose
in the 'proper' manner. That might including moving certain
functionality out of the widget classes themselves.
I see your point. Could you please name some inkscape UI classes that preserve the separation?
I'm going through the codebase to see what examples we might want to follow. There are a some that might need to be taken a step further, though.
I would like to go on with SPColorSelector refactoring. In Inkscape color can be selected using one of two widgets: sp-color-wheel-selector, sp-color-icc-selector. They share common functionality in sp-color-selector. Currently there are six structures consisting of both Gobjects and c++ classes.
I made a simplified diagram, listing possible options for refactoring [1]. 1) ColorSelector widgets contain a reference to their state (ColorSelector). It is accessible via getColorSelector() method. Optional ColorSelectorWidget abstract class provides the method. Parent UI component would initialize color selector widgets in ctor. Reference to widget would be kept in ctor only. Parent class would contain a pointer to widget state, returned by getColorSelector(). 2) Double inheritance. ColorSelector provides widget state. Concrete implementations inherit ColorSelector and aproppriate Gtk::Widget. 3) Full wrapper. Color selector widgets contain reference to their state and have full set of thin wrapper methods. They only call corresponding methods in ColorSelector.
Which is the best? What is the suggested naming and namespacing of mentioned classes? Which of the options fits the separation between gtkmm and functionality best?
The diagrams seem quite helpful. However, in this instance it can also be helpful to also start with some test code, as per standard TDD approaches.
The first question that pops into my mind is "why?" for the getColorSelector() method. Once things are set up that might not come into play. Another factor would be to try to avoid multiple inheritance. In this usage, needing to inherit from a Gtkmm widget plus something different just doesn't quite seem to be clean and logical (aka might not meet 'coherence' criteria for good OO design).
Gtk+ does have the Action object that has a few aspects we might care about. It is an abstract instance, however there are factory methods to get an appropriate widget as needed. This allows one to place a GtkAction in a menu or a toolbar or another contain and have it manifest as a proper widget as needed. In the case of placement in a toolbar, a button widget is normally created. However if actions do not fit in the visible area of the toolbar, they are placed in an overflow menu and manifest as menu item widgets.
With GtkAction the class itself contains factory methods to create needed widget instances. For a clean separation we might want to have either a factory class instance or a set of factory functions to create the needed widgets. The calls would return just a Gtk::Widget * pointer, and thus provide properly encapsulated instances. As a consumer we don't know what they do, just that they can be placed in a UI container. Or rather than a plain Gtk::Widget* we might return Gtk::MenuItem* and Gtk::ToolItem.
The code in the factory method would take in an instance of ColorSelector, wire it up to one or more Gtk::Widget subclasses with signal handlers, etc. then return a final parent Gtk::Widget (MenuItem or ToolItem as appropriate) to be placed in a UI. So we might have factory functions for
Gtk::MenuItem *createMenuItem(ColorSelector &selector)
and
Gtk::ToolItem *createToolItem(ColorSelector &selector)
However we definitely would need at least a base dialog/panel factory:
Gtk::Widget *createPanel(ColorSelector &selector)
All this, of course, is just some initial pondering to get us going on some directions to look at. Actually writing some test code can help to quickly pin down what details will be needed or not.
Then we get to one more problem your UML diagram pointed out. The term "ColorSelector" seems to be very UI centric. That name in and of itself might cloud understanding of a cleaner separation. If we look for some name that more properly indicates the *what* of what it is rather than the *how* it's been implemented in the UI, that name can help us move forward. We might consider "ColorSelection", but that second half is now misleading (as we don't use it to collect up a set of things the user wants to act on). "Color" is too generic as is "ColorData", but "MutableColor" starts to clarify it a bit.
Add in our Panel class for abstract UI container and we might get something like
Panel *createColorSelectorPanel(MutableColor &color);
-- Jon A. Cruz jon@...18...
Thank you for such an exhaustive explanation.
I wrote simplified class declarations ( http://pastebin.com/0gC5vSDN ). They demonstrate the idea of storing reference to ColorData by concrete widgets. It utilizes factory methods to create them, as you suggested. Could you please take a look, and tell if it is sensible now?
To be honest, I have little experience with TDD in conjunction with design. Would you like to see a full cxxtest for ColorData class?
Regards, Tomasz
On Sun, May 25, 2014 at 8:59 PM, Jon A. Cruz <jon@...18...> wrote:
On Sat, May 24, 2014, at 03:43 AM, Tomasz Boczkowski wrote:
Resending: original message was send accidentally off-list.
A safe way to mitigate many of the C vs C++ issues with Gtkmm is to
keep a proper logical separation between Gtkmm UI objects and the functionality they trigger or state they manifest. The bulk of code ends up in plain C++ classes, while the Gtkmm widgets merely provide a thin layer to expose
in the 'proper' manner. That might including moving certain
functionality out of the widget classes themselves.
I see your point. Could you please name some inkscape UI classes that preserve the separation?
I'm going through the codebase to see what examples we might want to follow. There are a some that might need to be taken a step further, though.
I would like to go on with SPColorSelector refactoring. In Inkscape color can be selected using one of two widgets: sp-color-wheel-selector, sp-color-icc-selector. They share common functionality in sp-color-selector. Currently there are six structures consisting of both Gobjects and c++ classes.
I made a simplified diagram, listing possible options for refactoring [1].
- ColorSelector widgets contain a reference to their state
(ColorSelector). It is accessible via getColorSelector() method. Optional ColorSelectorWidget abstract class provides the method. Parent UI component would initialize color selector widgets in ctor. Reference to widget would be kept in ctor only. Parent class would contain a pointer to widget state, returned by getColorSelector(). 2) Double inheritance. ColorSelector provides widget state. Concrete implementations inherit ColorSelector and aproppriate Gtk::Widget. 3) Full wrapper. Color selector widgets contain reference to their state and have full set of thin wrapper methods. They only call corresponding methods in ColorSelector.
Which is the best? What is the suggested naming and namespacing of mentioned classes? Which of the options fits the separation between gtkmm and functionality best?
The diagrams seem quite helpful. However, in this instance it can also be helpful to also start with some test code, as per standard TDD approaches.
The first question that pops into my mind is "why?" for the getColorSelector() method. Once things are set up that might not come into play. Another factor would be to try to avoid multiple inheritance. In this usage, needing to inherit from a Gtkmm widget plus something different just doesn't quite seem to be clean and logical (aka might not meet 'coherence' criteria for good OO design).
Gtk+ does have the Action object that has a few aspects we might care about. It is an abstract instance, however there are factory methods to get an appropriate widget as needed. This allows one to place a GtkAction in a menu or a toolbar or another contain and have it manifest as a proper widget as needed. In the case of placement in a toolbar, a button widget is normally created. However if actions do not fit in the visible area of the toolbar, they are placed in an overflow menu and manifest as menu item widgets.
With GtkAction the class itself contains factory methods to create needed widget instances. For a clean separation we might want to have either a factory class instance or a set of factory functions to create the needed widgets. The calls would return just a Gtk::Widget * pointer, and thus provide properly encapsulated instances. As a consumer we don't know what they do, just that they can be placed in a UI container. Or rather than a plain Gtk::Widget* we might return Gtk::MenuItem* and Gtk::ToolItem.
The code in the factory method would take in an instance of ColorSelector, wire it up to one or more Gtk::Widget subclasses with signal handlers, etc. then return a final parent Gtk::Widget (MenuItem or ToolItem as appropriate) to be placed in a UI. So we might have factory functions for Gtk::MenuItem *createMenuItem(ColorSelector &selector) and Gtk::ToolItem *createToolItem(ColorSelector &selector) However we definitely would need at least a base dialog/panel factory: Gtk::Widget *createPanel(ColorSelector &selector)
All this, of course, is just some initial pondering to get us going on some directions to look at. Actually writing some test code can help to quickly pin down what details will be needed or not.
Then we get to one more problem your UML diagram pointed out. The term "ColorSelector" seems to be very UI centric. That name in and of itself might cloud understanding of a cleaner separation. If we look for some name that more properly indicates the *what* of what it is rather than the *how* it's been implemented in the UI, that name can help us move forward. We might consider "ColorSelection", but that second half is now misleading (as we don't use it to collect up a set of things the user wants to act on). "Color" is too generic as is "ColorData", but "MutableColor" starts to clarify it a bit.
Add in our Panel class for abstract UI container and we might get something like
Panel *createColorSelectorPanel(MutableColor &color);-- Jon A. Cruz jon@...18...
"Accelerate Dev Cycles with Automated Cross-Browser Testing - For FREE Instantly run your Selenium tests across 300+ browser/OS combos. Get unparalleled scalability from the best Selenium testing platform available Simple to use. Nothing to install. Get started now for free." http://p.sf.net/sfu/SauceLabs _______________________________________________ Inkscape-devel mailing list Inkscape-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/inkscape-devel
participants (2)
-
Jon A. Cruz -
Tomasz Boczkowski