Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Rework styling #172

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
Open

Rework styling #172

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
818b56a
Add DI for FlowViewStyle
Quincunx271 Apr 30, 2018
2bb3ff0
DI styling
Quincunx271 May 1, 2018
8ad2d3a
Fix bug in FlowViewStyle setting
Quincunx271 May 1, 2018
da1f6c0
Apply std::move where appropriate
Quincunx271 May 1, 2018
77c4724
Most of the way there
Quincunx271 May 1, 2018
397d98a
Finish removing styling singletons
Quincunx271 May 2, 2018
a5c2430
Merge branch 'master' into di-styling
Quincunx271 May 11, 2018
f88d00b
small misc improvements
Quincunx271 May 11, 2018
008eea5
Use FooStyle objects directly; improve implementation of FooStyle obj…
Quincunx271 May 11, 2018
9f09fad
Add missing using declarations
Quincunx271 May 11, 2018
8cb84ce
Address concerns
Quincunx271 May 14, 2018
468101e
add missing #include <QtCore/QHash>
Quincunx271 May 14, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ set(CPP_SOURCE_FILES
src/NodeState.cpp
src/NodeStyle.cpp
src/Properties.cpp
src/StyleCollection.cpp
src/StyleImport.cpp
)

# If we want to give the option to build a static library,
Expand Down
40 changes: 18 additions & 22 deletions examples/calculator/main.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,27 +60,24 @@ registerDataModels()


static
void
setStyle()
auto
connectionStyle()
{
ConnectionStyle::setConnectionStyle(
R"(
{
"ConnectionStyle": {
"ConstructionColor": "gray",
"NormalColor": "black",
"SelectedColor": "gray",
"SelectedHaloColor": "deepskyblue",
"HoveredColor": "deepskyblue",

"LineWidth": 3.0,
"ConstructionLineWidth": 2.0,
"PointDiameter": 10.0,

"UseDataDefinedColors": true
}
}
)");
auto style = ConnectionStyle::defaultStyle();

style.setConstructionColor(QColor("gray"));
style.setNormalColor(QColor("black"));
style.setSelectedColor(QColor("gray"));
style.setSelectedHaloColor(QColor("deepskyblue"));
style.setHoveredColor(QColor("deepskyblue"));

style.setLineWidth(3.0);
style.setConstructionLineWidth(2.0);
style.setPointDiameter(10.0);

style.useDataDefinedColors(true);

return style;
}


Expand All @@ -89,8 +86,6 @@ main(int argc, char *argv[])
{
QApplication app(argc, argv);

setStyle();

QWidget mainWidget;

auto menuBar = new QMenuBar();
Expand All @@ -101,6 +96,7 @@ main(int argc, char *argv[])

l->addWidget(menuBar);
auto scene = new FlowScene(registerDataModels(), &mainWidget);
scene->setConnectionStyle(connectionStyle());
l->addWidget(new FlowView(scene));
l->setContentsMargins(0, 0, 0, 0);
l->setSpacing(0);
Expand Down
19 changes: 3 additions & 16 deletions examples/connection_colors/main.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,31 +33,18 @@ registerDataModels()
}


static
void
setStyle()
{
ConnectionStyle::setConnectionStyle(
R"(
{
"ConnectionStyle": {
"UseDataDefinedColors": true
}
}
)");
}


//------------------------------------------------------------------------------

int
main(int argc, char* argv[])
{
QApplication app(argc, argv);

setStyle();
auto connectionStyle = ConnectionStyle::defaultStyle();
connectionStyle.useDataDefinedColors(true);

FlowScene scene(registerDataModels());
scene.setConnectionStyle(std::move(connectionStyle));

FlowView view(&scene);

Expand Down
18 changes: 8 additions & 10 deletions examples/styles/main.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,9 @@ registerDataModels()

static
void
setStyle()
setStyle(FlowScene& scene, FlowView& view)
{
FlowViewStyle::setStyle(
view.setStyle(FlowViewStyle::fromJson(
R"(
{
"FlowViewStyle": {
Expand All @@ -41,9 +41,9 @@ setStyle()
"CoarseGridColor": [235, 235, 220]
}
}
)");
)"));

NodeStyle::setNodeStyle(
scene.setNodeStyle(NodeStyle::fromJson(
R"(
{
"NodeStyle": {
Expand All @@ -63,9 +63,9 @@ setStyle()
"Opacity": 1.0
}
}
)");
)"));

ConnectionStyle::setConnectionStyle(
scene.setConnectionStyle(ConnectionStyle::fromJson(
R"(
{
"ConnectionStyle": {
Expand All @@ -82,7 +82,7 @@ setStyle()
"UseDataDefinedColors": false
}
}
)");
)"));
}


Expand All @@ -93,11 +93,9 @@ main(int argc, char* argv[])
{
QApplication app(argc, argv);

setStyle();

FlowScene scene(registerDataModels());

FlowView view(&scene);
setStyle(scene, view);

view.setWindowTitle("Style example");
view.resize(800, 600);
Expand Down
13 changes: 10 additions & 3 deletions include/nodes/internal/Connection.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ namespace QtNodes
class Node;
class NodeData;
class ConnectionGraphicsObject;
class ConnectionStyle;

///
class NODE_EDITOR_PUBLIC Connection
Expand All @@ -36,15 +37,17 @@ class NODE_EDITOR_PUBLIC Connection

/// New Connection is attached to the port of the given Node.
/// The port has parameters (portType, portIndex).
/// The opposite connection end will require anothre port.
/// The opposite connection end will require another port.
Connection(PortType portType,
Node& node,
PortIndex portIndex);
PortIndex portIndex,
ConnectionStyle const &style);

Connection(Node& nodeIn,
PortIndex portIndexIn,
Node& nodeOut,
PortIndex portIndexOut,
ConnectionStyle const &style,
TypeConverter converter =
TypeConverter{});

Expand Down Expand Up @@ -118,6 +121,9 @@ class NODE_EDITOR_PUBLIC Connection
void
setTypeConverter(TypeConverter converter);

ConnectionStyle const &
connectionStyle() const;

public: // data propagation

void
Expand All @@ -139,10 +145,11 @@ class NODE_EDITOR_PUBLIC Connection
PortIndex _inPortIndex;

private:

ConnectionState _connectionState;
ConnectionGeometry _connectionGeometry;

ConnectionStyle const *_connectionStyle;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, maybe a stupid question -- why pointer?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What other options were you thinking of?

  • ConnectionStyle _connectionStyle; This is expensive. We don't need a copy of the ConnectionStyle object for each Connection, especially when they're all going to be the same
  • std::shared_ptr<ConnectionStyle const> _connectionStyle; This is unnecessary. The ConnectionStyle outlives the connections anyway (correction: they can, but currently don't; that's something I will fix).
  • ConnectionStyle const &_connectionStyle; I don't use references as class members, as references can't be rebound (they are effectively const). It doesn't matter here, as Connection can't be copied anyway, but I used a pointer instead for consistency.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here ConnectionStyle const & connectionStyle() const we convert this pointer back to a reference.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True. The member is stored as a pointer, but is semantically a reference (that's the style I use anyway).

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. I did not see any setter function
  2. The copy constructor for Connection is deleted.

Therefore I suggested that this member shouldn't be rebound.

I'd simply use a const ref.

BTW, we should explicitly delete move constructor and move assignment operator for this class


std::unique_ptr<ConnectionGraphicsObject>_connectionGraphicsObject;

TypeConverter _converter;
Expand Down
7 changes: 5 additions & 2 deletions include/nodes/internal/ConnectionGeometry.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,12 @@
namespace QtNodes
{

class ConnectionStyle;

class ConnectionGeometry
{
public:

ConnectionGeometry();
ConnectionGeometry(ConnectionStyle const &style);

public:

Expand Down Expand Up @@ -56,5 +57,7 @@ class ConnectionGeometry
double _lineWidth;

bool _hovered;

ConnectionStyle const* _style;
};
}
62 changes: 35 additions & 27 deletions include/nodes/internal/ConnectionStyle.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,37 +2,28 @@

#include <QtGui/QColor>

#include <QByteArray>
#include <QObject>
#include <QString>

#include "Export.hpp"
#include "Style.hpp"

namespace QtNodes
{

class NODE_EDITOR_PUBLIC ConnectionStyle : public Style
class NODE_EDITOR_PUBLIC ConnectionStyle
{
public:

ConnectionStyle();

ConnectionStyle(QString jsonText);

public:

static void setConnectionStyle(QString jsonText);

private:

void loadJsonText(QString jsonText) override;
static ConnectionStyle const&
defaultStyle();

void loadJsonFile(QString fileName) override;

void loadJsonFromByteArray(QByteArray const &byteArray) override;

public:
static ConnectionStyle
fromJson(QString const& jsonText);

QColor constructionColor() const;
QColor normalColor() const;
QColor normalColor(QString typeId) const;
QColor selectedColor() const;
QColor selectedHaloColor() const;
QColor hoveredColor() const;
Expand All @@ -43,18 +34,35 @@ class NODE_EDITOR_PUBLIC ConnectionStyle : public Style

bool useDataDefinedColors() const;

void setConstructionColor(QColor);
void setNormalColor(QColor);
void setSelectedColor(QColor);
void setSelectedHaloColor(QColor);
void setHoveredColor(QColor);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we go further with the Qt-kung-fu and describe all the members in the class with Q_PROPERTY ?

As I understand, something like this

Q_PROPERTY(QColor color MEMBER m_color NOTIFY colorChanged)

would define a private member and generate setters and getters.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a bad idea. E.g. when we implement a QML frontend, that could come in handy. IIUC, Q_PROPERTY only has meaning inside something that inherits from QObject, which the FooStyles currently don't. QObjects can't be copied, so there are some options:

  • Pass around std::unique_ptr<FooStyle> instead of FooStyle directly.
  • Make FooStyle take a QObject* parent = Q_NULLPTR in its constructor.
  • Maybe this could work?: Write explicit copy constructors for FooStyles


void setLineWidth(float);
void setConstructionLineWidth(float);
void setPointDiameter(float);

void useDataDefinedColors(bool);

static QColor
computeNormalColor(QString const& typeId);

private:
ConnectionStyle(QByteArray const& jsonBytes);
void loadJson(QByteArray const& jsonBytes);

QColor ConstructionColor;
QColor NormalColor;
QColor SelectedColor;
QColor SelectedHaloColor;
QColor HoveredColor;
QColor _constructionColor;
QColor _normalColor;
QColor _selectedColor;
QColor _selectedHaloColor;
QColor _hoveredColor;

float LineWidth;
float ConstructionLineWidth;
float PointDiameter;
float _lineWidth;
float _constructionLineWidth;
float _pointDiameter;

bool UseDataDefinedColors;
bool _useDataDefinedColors;
};
}
13 changes: 12 additions & 1 deletion include/nodes/internal/FlowScene.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@
#include "TypeConverter.hpp"
#include "memory.hpp"

#include "ConnectionStyle.hpp"
#include "NodeStyle.hpp"

namespace QtNodes
{

Expand All @@ -22,7 +25,6 @@ class Node;
class NodeGraphicsObject;
class Connection;
class ConnectionGraphicsObject;
class NodeStyle;

/// Scene holds connections and nodes.
class NODE_EDITOR_PUBLIC FlowScene
Expand Down Expand Up @@ -97,6 +99,12 @@ class NODE_EDITOR_PUBLIC FlowScene

void loadFromMemory(const QByteArray& data);

ConnectionStyle const& connectionStyle() const;
NodeStyle const& nodeStyle() const;

void setConnectionStyle(ConnectionStyle style);
void setNodeStyle(NodeStyle style);

signals:

void nodeCreated(Node &n);
Expand Down Expand Up @@ -128,6 +136,9 @@ class NODE_EDITOR_PUBLIC FlowScene
std::unordered_map<QUuid, SharedConnection> _connections;
std::unordered_map<QUuid, UniqueNode> _nodes;
std::shared_ptr<DataModelRegistry> _registry;

ConnectionStyle _connectionStyle;
NodeStyle _nodeStyle;
};

Node*
Expand Down