Rework styling
Addresses #149
This PR makes a rather large amount of changes, as the singletons were used in many places.
Changes:
- Convert styling singletons into dependency injected styles.
FlowViewStylebecomes a property ofFlowView, andConnectionStyleandNodeStyle(the default one, at least) become a property ofFlowScene.- For
FlowView, the style can be passed as a parameter to the constructor, or via callingmyFlowView->setStyle(...). - For
FlowScene, I chose not to allow the styles to be passed as parameters to the constructor. I am not sure of this decision. The problem was that I couldn't think of any reason to have the styles passed in one order or the other. Styles can still be set viascene->setConnectionStyle(...)andscene->setNodeStyle(...)
- For
- The
*Styles are reworked.- Style settings can be set as properties of the
FooStyleobject, rather than solely through parsing JSON - Implementation: no more macros for parsing. Instead, utility functions from a new
StyleImportclass are used.
- Style settings can be set as properties of the
-
NodeDataModel::setStyleis removed.NodeDataModelis not in charge of managing the style, butNodeis. To use a custom style, one would override the newvirtual NodeStyle const* nodeStyle() const;function.- One thought I had here was that maybe the Creator objects could somehow be in charge of managing this. Currently the Creators are
std::functions, but if we made them our ownNodeModelCreator, there could be a way to set the style on the creator.
- One thought I had here was that maybe the Creator objects could somehow be in charge of managing this. Currently the Creators are
Concerns:
- I don't know what would happen if you change the style in the middle of execution. I believe that some things will update to use the new style, but some things won't. I think that the right path would be to always allow style changes at any time, but the system isn't set up to handle it.
I am very curious about what are you implementing with the Node Editor :-)
I'll look through this PR
Dmitry
Maybe I am missing something. Before, it was possible to define a style per NodeDataModel.
Each model could be effectively styled differently. Even two identical models could be styled differently based on some inherent model data.
Can we repeat the same trick now? There were some users that needed different looks for different models.
I removed NodeDataModel::setStyle. I can't remember the exact reason (I'm currently working on a different branch), but it had something to do with that it was easier to implement and made more sense to me for the NodeDataModel to not be in charge of managing the style.
Instead, users that want to customize the style per model (or internal model data) should implement virtual NodeStyle const* nodeStyle() const; to return the appropriate style.
Should I bump the library version to 3?
This would indeed be a breaking change.
However, it might be beneficial to instead create something like a version-3 branch and merge this into that (once I fix the changes), so that we can implement multiple breaking-change things together (rather than having to bump the major version each time, we could group several changes into some milestone which we call a new major version)
With this commit, note that I did not address the Q_PROPERTY for style objects. I'm not sure which way to go with it.
I haven't looked through the full usage of styles to see if this breaks something, but if these changes are being saved for a future breaking update, could the const be dropped from the StyleCollection getters in the meantime? In my local branch I dropped the consts and forwarded the getters through the NodeStyle/etc. classes. Things seem to be working normally and style properties can be set individually without using json strings. If I am understanding the current version, you can only set styles via json strings? This seems very awkward to me.