From 27546496f5cd9d0e9a6719dee27315b34d2fdcf5 Mon Sep 17 00:00:00 2001 From: Michael Ferguson Date: Sun, 9 Aug 2020 19:03:29 -0400 Subject: [PATCH 1/4] improve color support for themes based on https://github.com/ros-visualization/rviz/pull/1319 --- rviz_common/help/help.html | 4 ++-- .../rviz_common/properties/status_property.hpp | 4 ---- rviz_common/src/rviz_common/display.cpp | 12 +----------- rviz_common/src/rviz_common/panel_dock_widget.cpp | 3 ++- rviz_common/src/rviz_common/properties/property.cpp | 12 ++++-------- .../properties/property_tree_with_help.cpp | 2 +- .../src/rviz_common/properties/status_property.cpp | 5 +++++ rviz_common/src/rviz_common/view_controller.cpp | 4 ---- 8 files changed, 15 insertions(+), 31 deletions(-) diff --git a/rviz_common/help/help.html b/rviz_common/help/help.html index 7860be089..49174877f 100644 --- a/rviz_common/help/help.html +++ b/rviz_common/help/help.html @@ -8,8 +8,8 @@ diff --git a/rviz_common/include/rviz_common/properties/status_property.hpp b/rviz_common/include/rviz_common/properties/status_property.hpp index 319c888f7..9bc9f92ab 100644 --- a/rviz_common/include/rviz_common/properties/status_property.hpp +++ b/rviz_common/include/rviz_common/properties/status_property.hpp @@ -60,10 +60,6 @@ class RVIZ_COMMON_PUBLIC StatusProperty : public Property Qt::ItemFlags getViewFlags(int column) const override; /// Return the color appropriate for the given status level. - /** - * Returns an invalid QColor for Ok status, meaning we should use the default - * text color. - */ static QColor statusColor(Level level); /// Return the word appropriate for the given status level: "Ok", "Warn", or "Error". diff --git a/rviz_common/src/rviz_common/display.cpp b/rviz_common/src/rviz_common/display.cpp index daf09b123..76f3af618 100644 --- a/rviz_common/src/rviz_common/display.cpp +++ b/rviz_common/src/rviz_common/display.cpp @@ -111,16 +111,6 @@ void Display::queueRender() QVariant Display::getViewData(int column, int role) const { switch (role) { - case Qt::BackgroundRole: - { - /* - QLinearGradient q( 0,0, 0,5); - q.setColorAt( 0.0, QColor(230,230,230)); - q.setColorAt( 1.0, Qt::white); - return QBrush( q); - */ - return QColor(Qt::white); - } case Qt::ForegroundRole: { // if we're item-enabled (not greyed out) and in warn/error state, set appropriate color @@ -134,7 +124,7 @@ QVariant Display::getViewData(int column, int role) const return QColor(40, 120, 197); } } else { - return QColor(Qt::black); + return QApplication::palette().color(QPalette::Text); } } break; diff --git a/rviz_common/src/rviz_common/panel_dock_widget.cpp b/rviz_common/src/rviz_common/panel_dock_widget.cpp index 4004571a9..8aa370237 100644 --- a/rviz_common/src/rviz_common/panel_dock_widget.cpp +++ b/rviz_common/src/rviz_common/panel_dock_widget.cpp @@ -30,6 +30,7 @@ #include "rviz_common/panel_dock_widget.hpp" +#include #include #include #include @@ -47,7 +48,7 @@ PanelDockWidget::PanelDockWidget(const QString & name) QWidget * title_bar = new QWidget(this); QPalette pal(palette()); - pal.setColor(QPalette::Window, QColor(200, 200, 200)); + pal.setColor(QPalette::Window, QApplication::palette().color(QPalette::Mid)); title_bar->setAutoFillBackground(true); title_bar->setPalette(pal); title_bar->setContentsMargins(0, 0, 0, 0); diff --git a/rviz_common/src/rviz_common/properties/property.cpp b/rviz_common/src/rviz_common/properties/property.cpp index a9ae7e70a..f5b1dbb51 100644 --- a/rviz_common/src/rviz_common/properties/property.cpp +++ b/rviz_common/src/rviz_common/properties/property.cpp @@ -34,6 +34,8 @@ #include // for INT_MIN and INT_MAX #include +#include +#include #include // NOLINT: cpplint is unable to handle the include order here #include // NOLINT: cpplint is unable to handle the include order here @@ -251,14 +253,8 @@ void Property::setParent(Property * new_parent) QVariant Property::getViewData(int column, int role) const { - if (role == Qt::ForegroundRole && - ( parent_ && parent_->getDisableChildren() ) ) - { -#if QT_VERSION < QT_VERSION_CHECK(5, 0, 0) - return Qt::gray; -#else - return QColor(Qt::gray); -#endif + if (role == Qt::TextColorRole && parent_ && parent_->getDisableChildren()) { + return QApplication::palette().brush(QPalette::Disabled, QPalette::Text); } switch (column) { diff --git a/rviz_common/src/rviz_common/properties/property_tree_with_help.cpp b/rviz_common/src/rviz_common/properties/property_tree_with_help.cpp index 1bc8d4718..000b8cc47 100644 --- a/rviz_common/src/rviz_common/properties/property_tree_with_help.cpp +++ b/rviz_common/src/rviz_common/properties/property_tree_with_help.cpp @@ -71,7 +71,7 @@ void PropertyTreeWithHelp::showHelpForProperty(const Property * property) if (property) { QString body_text = property->getDescription(); QString heading = property->getName(); - QString html = "" + heading + "
" + + QString html = "" + heading + "
" + body_text + ""; help_->setHtml(html); } else { diff --git a/rviz_common/src/rviz_common/properties/status_property.cpp b/rviz_common/src/rviz_common/properties/status_property.cpp index 39da9dbc1..606e834c4 100644 --- a/rviz_common/src/rviz_common/properties/status_property.cpp +++ b/rviz_common/src/rviz_common/properties/status_property.cpp @@ -28,7 +28,9 @@ * POSSIBILITY OF SUCH DAMAGE. */ +#include #include +#include #include "rviz_common/load_resource.hpp" #include "rviz_common/properties/property_tree_model.hpp" @@ -54,6 +56,9 @@ StatusProperty::StatusProperty( status_icons_[0] = loadPixmap("package://rviz_common/icons/ok.png"); status_icons_[1] = loadPixmap("package://rviz_common/icons/warning.png"); status_icons_[2] = loadPixmap("package://rviz_common/icons/error.png"); + + if (!status_colors_[0].isValid()) // initialize default text color once + status_colors_[0] = QApplication::palette().color(QPalette::Text); } bool StatusProperty::setValue(const QVariant & new_value) diff --git a/rviz_common/src/rviz_common/view_controller.cpp b/rviz_common/src/rviz_common/view_controller.cpp index f47f86afc..c9998d22b 100644 --- a/rviz_common/src/rviz_common/view_controller.cpp +++ b/rviz_common/src/rviz_common/view_controller.cpp @@ -153,10 +153,6 @@ QVariant ViewController::getViewData(int column, int role) const if (is_active_) { switch (role) { - case Qt::BackgroundRole: - { - return QColor(0xba, 0xad, 0xa4); - } case Qt::FontRole: { QFont font; From 9df778a34f01fee8d05f24f61949591d98371032 Mon Sep 17 00:00:00 2001 From: Michael Ferguson Date: Thu, 13 Aug 2020 20:22:08 -0400 Subject: [PATCH 2/4] fixup style issues --- rviz_common/src/rviz_common/properties/property.cpp | 4 ++-- rviz_common/src/rviz_common/properties/status_property.cpp | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/rviz_common/src/rviz_common/properties/property.cpp b/rviz_common/src/rviz_common/properties/property.cpp index f5b1dbb51..a145bf4fe 100644 --- a/rviz_common/src/rviz_common/properties/property.cpp +++ b/rviz_common/src/rviz_common/properties/property.cpp @@ -34,8 +34,8 @@ #include // for INT_MIN and INT_MAX #include -#include -#include +#include // NOLINT: cpplint is unable to handle the include order here +#include // NOLINT: cpplint is unable to handle the include order here #include // NOLINT: cpplint is unable to handle the include order here #include // NOLINT: cpplint is unable to handle the include order here diff --git a/rviz_common/src/rviz_common/properties/status_property.cpp b/rviz_common/src/rviz_common/properties/status_property.cpp index 606e834c4..be3d44e7e 100644 --- a/rviz_common/src/rviz_common/properties/status_property.cpp +++ b/rviz_common/src/rviz_common/properties/status_property.cpp @@ -57,8 +57,9 @@ StatusProperty::StatusProperty( status_icons_[1] = loadPixmap("package://rviz_common/icons/warning.png"); status_icons_[2] = loadPixmap("package://rviz_common/icons/error.png"); - if (!status_colors_[0].isValid()) // initialize default text color once + if (!status_colors_[0].isValid()) { // initialize default text color once status_colors_[0] = QApplication::palette().color(QPalette::Text); + } } bool StatusProperty::setValue(const QVariant & new_value) From ecc4bc02a41a8f81f0f9df6969b43cafcd299d30 Mon Sep 17 00:00:00 2001 From: Michael Ferguson Date: Thu, 13 Aug 2020 21:28:47 -0400 Subject: [PATCH 3/4] try this again --- rviz_common/src/rviz_common/properties/status_property.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rviz_common/src/rviz_common/properties/status_property.cpp b/rviz_common/src/rviz_common/properties/status_property.cpp index be3d44e7e..1c130ef7a 100644 --- a/rviz_common/src/rviz_common/properties/status_property.cpp +++ b/rviz_common/src/rviz_common/properties/status_property.cpp @@ -57,7 +57,7 @@ StatusProperty::StatusProperty( status_icons_[1] = loadPixmap("package://rviz_common/icons/warning.png"); status_icons_[2] = loadPixmap("package://rviz_common/icons/error.png"); - if (!status_colors_[0].isValid()) { // initialize default text color once + if (!status_colors_[0].isValid()) { // initialize default text color once status_colors_[0] = QApplication::palette().color(QPalette::Text); } } From 48e9e36f0cf5f074e20a132614b0e22b05c4cf00 Mon Sep 17 00:00:00 2001 From: Michael Ferguson Date: Wed, 19 Aug 2020 19:53:27 -0400 Subject: [PATCH 4/4] use Qt::ForegroundRole --- rviz_common/src/rviz_common/properties/property.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rviz_common/src/rviz_common/properties/property.cpp b/rviz_common/src/rviz_common/properties/property.cpp index a145bf4fe..1df0c0429 100644 --- a/rviz_common/src/rviz_common/properties/property.cpp +++ b/rviz_common/src/rviz_common/properties/property.cpp @@ -253,7 +253,7 @@ void Property::setParent(Property * new_parent) QVariant Property::getViewData(int column, int role) const { - if (role == Qt::TextColorRole && parent_ && parent_->getDisableChildren()) { + if (role == Qt::ForegroundRole && parent_ && parent_->getDisableChildren()) { return QApplication::palette().brush(QPalette::Disabled, QPalette::Text); }