From 4bd56fa0f8c863eb3d8829d1a2fac1b6d5876dbe Mon Sep 17 00:00:00 2001 From: Julien Cabieces Date: Tue, 30 Apr 2024 16:22:37 +0200 Subject: [PATCH 01/12] feat(ColorWidget) : Add CMYK support A new setting activeCmykComponent has been introduced, while the old one activeComponent refers now only to the Rgb/Hsv part --- .../gui/auto_additions/qgscolorwidgets.py | 4 + .../gui/auto_generated/qgscolorwidgets.sip.in | 20 +- .../gui/auto_generated/qgscolorwidgets.sip.in | 20 +- src/gui/qgscolorwidgets.cpp | 303 ++-- src/gui/qgscolorwidgets.h | 20 +- src/gui/qgscompoundcolorwidget.cpp | 90 +- src/gui/qgscompoundcolorwidget.h | 10 +- src/ui/qgscompoundcolorwidget.ui | 1441 +++++++++-------- tests/src/gui/CMakeLists.txt | 1 + tests/src/gui/testqgscompoundcolorwidget.cpp | 92 ++ tests/src/python/test_qgscolorwidget.py | 172 ++ 11 files changed, 1361 insertions(+), 812 deletions(-) create mode 100644 tests/src/gui/testqgscompoundcolorwidget.cpp create mode 100644 tests/src/python/test_qgscolorwidget.py diff --git a/python/PyQt6/gui/auto_additions/qgscolorwidgets.py b/python/PyQt6/gui/auto_additions/qgscolorwidgets.py index 06e7160642ee..e004c681cafc 100644 --- a/python/PyQt6/gui/auto_additions/qgscolorwidgets.py +++ b/python/PyQt6/gui/auto_additions/qgscolorwidgets.py @@ -7,6 +7,10 @@ QgsColorWidget.Saturation = QgsColorWidget.ColorComponent.Saturation QgsColorWidget.Value = QgsColorWidget.ColorComponent.Value QgsColorWidget.Alpha = QgsColorWidget.ColorComponent.Alpha +QgsColorWidget.Cyan = QgsColorWidget.ColorComponent.Cyan +QgsColorWidget.Magenta = QgsColorWidget.ColorComponent.Magenta +QgsColorWidget.Yellow = QgsColorWidget.ColorComponent.Yellow +QgsColorWidget.Black = QgsColorWidget.ColorComponent.Black QgsColorRampWidget.Horizontal = QgsColorRampWidget.Orientation.Horizontal QgsColorRampWidget.Vertical = QgsColorRampWidget.Orientation.Vertical QgsColorTextWidget.HexRgb = QgsColorTextWidget.ColorTextFormat.HexRgb diff --git a/python/PyQt6/gui/auto_generated/qgscolorwidgets.sip.in b/python/PyQt6/gui/auto_generated/qgscolorwidgets.sip.in index 9c00acd07c4f..8072da189a42 100644 --- a/python/PyQt6/gui/auto_generated/qgscolorwidgets.sip.in +++ b/python/PyQt6/gui/auto_generated/qgscolorwidgets.sip.in @@ -33,7 +33,11 @@ set to a color with an ambiguous hue (e.g., black or white shades). Hue, Saturation, Value, - Alpha + Alpha, + Cyan, + Magenta, + Yellow, + Black }; QgsColorWidget( QWidget *parent /TransferThis/ = 0, ColorComponent component = Multiple ); @@ -145,7 +149,7 @@ Returns the range of valid values for the color widget's component :return: maximum value allowed for color component, or -1 if widget has multiple components %End - int componentRange( ColorComponent component ) const; + static int componentRange( ColorComponent component ); %Docstring Returns the range of valid values a color component @@ -172,7 +176,7 @@ as QColor returns a hue of -1 if the color's hue is ambiguous (e.g., if the satu :return: explicitly set hue for widget %End - void alterColor( QColor &color, QgsColorWidget::ColorComponent component, int newValue ) const; + static void alterColor( QColor &color, QgsColorWidget::ColorComponent component, int newValue ); %Docstring Alters a color by modifying the value of a specific color component @@ -182,6 +186,16 @@ Alters a color by modifying the value of a specific color component valid range for the color component. %End + bool isCmyk() const; +%Docstring +Returns true if the color widget component is either Cyan, Magenta, Yellow or Black +%End + + static bool isCmyk( QgsColorWidget::ColorComponent component ); +%Docstring +Returns true if ``component`` is either Cyan, Magenta, Yellow or Black +%End + static const QPixmap &transparentBackground(); %Docstring Generates a checkboard pattern pixmap for use as a background to transparent colors diff --git a/python/gui/auto_generated/qgscolorwidgets.sip.in b/python/gui/auto_generated/qgscolorwidgets.sip.in index f898ca867c46..fd73e363e6d3 100644 --- a/python/gui/auto_generated/qgscolorwidgets.sip.in +++ b/python/gui/auto_generated/qgscolorwidgets.sip.in @@ -33,7 +33,11 @@ set to a color with an ambiguous hue (e.g., black or white shades). Hue, Saturation, Value, - Alpha + Alpha, + Cyan, + Magenta, + Yellow, + Black }; QgsColorWidget( QWidget *parent /TransferThis/ = 0, ColorComponent component = Multiple ); @@ -145,7 +149,7 @@ Returns the range of valid values for the color widget's component :return: maximum value allowed for color component, or -1 if widget has multiple components %End - int componentRange( ColorComponent component ) const; + static int componentRange( ColorComponent component ); %Docstring Returns the range of valid values a color component @@ -172,7 +176,7 @@ as QColor returns a hue of -1 if the color's hue is ambiguous (e.g., if the satu :return: explicitly set hue for widget %End - void alterColor( QColor &color, QgsColorWidget::ColorComponent component, int newValue ) const; + static void alterColor( QColor &color, QgsColorWidget::ColorComponent component, int newValue ); %Docstring Alters a color by modifying the value of a specific color component @@ -182,6 +186,16 @@ Alters a color by modifying the value of a specific color component valid range for the color component. %End + bool isCmyk() const; +%Docstring +Returns true if the color widget component is either Cyan, Magenta, Yellow or Black +%End + + static bool isCmyk( QgsColorWidget::ColorComponent component ); +%Docstring +Returns true if ``component`` is either Cyan, Magenta, Yellow or Black +%End + static const QPixmap &transparentBackground(); %Docstring Generates a checkboard pattern pixmap for use as a background to transparent colors diff --git a/src/gui/qgscolorwidgets.cpp b/src/gui/qgscolorwidgets.cpp index e2db43bf74c6..543f7197f332 100644 --- a/src/gui/qgscolorwidgets.cpp +++ b/src/gui/qgscolorwidgets.cpp @@ -102,6 +102,14 @@ int QgsColorWidget::componentValue( const QgsColorWidget::ColorComponent compone return mCurrentColor.value(); case QgsColorWidget::Alpha: return mCurrentColor.alpha(); + case QgsColorWidget::Cyan: + return mCurrentColor.cyan(); + case QgsColorWidget::Yellow: + return mCurrentColor.yellow(); + case QgsColorWidget::Magenta: + return mCurrentColor.magenta(); + case QgsColorWidget::Black: + return mCurrentColor.black(); default: return -1; } @@ -112,7 +120,7 @@ int QgsColorWidget::componentRange() const return componentRange( mComponent ); } -int QgsColorWidget::componentRange( const QgsColorWidget::ColorComponent component ) const +int QgsColorWidget::componentRange( const QgsColorWidget::ColorComponent component ) { if ( component == QgsColorWidget::Multiple ) { @@ -144,42 +152,133 @@ int QgsColorWidget::hue() const } } -void QgsColorWidget::alterColor( QColor &color, const QgsColorWidget::ColorComponent component, const int newValue ) const +void QgsColorWidget::alterColor( QColor &color, const QgsColorWidget::ColorComponent component, const int newValue ) { - int h, s, v, a; - color.getHsv( &h, &s, &v, &a ); - //clip value to sensible range const int clippedValue = std::min( std::max( 0, newValue ), componentRange( component ) ); + if ( isCmyk( component ) ) + { + int c, m, y, k, a; + color.getCmyk( &c, &m, &y, &k, &a ); + + switch ( component ) + { + case QgsColorWidget::Cyan: + if ( c == clippedValue ) + { + return; + } + color.setCmyk( clippedValue, m, y, k, a ); + break; + case QgsColorWidget::Magenta: + if ( m == clippedValue ) + { + return; + } + color.setCmyk( c, clippedValue, y, k, a ); + break; + case QgsColorWidget::Yellow: + if ( y == clippedValue ) + { + return; + } + color.setCmyk( c, m, clippedValue, k, a ); + break; + case QgsColorWidget::Black: + if ( k == clippedValue ) + { + return; + } + color.setCmyk( c, m, y, clippedValue, a ); + break; + default: + return; + } + } + else + { + int r, g, b, a; + color.getRgb( &r, &g, &b, &a ); + int h, s, v; + color.getHsv( &h, &s, &v ); + + switch ( component ) + { + case QgsColorWidget::Red: + if ( r == clippedValue ) + { + return; + } + color.setRed( clippedValue ); + break; + case QgsColorWidget::Green: + if ( g == clippedValue ) + { + return; + } + color.setGreen( clippedValue ); + break; + case QgsColorWidget::Blue: + if ( b == clippedValue ) + { + return; + } + color.setBlue( clippedValue ); + break; + case QgsColorWidget::Hue: + if ( h == clippedValue ) + { + return; + } + color.setHsv( clippedValue, s, v, a ); + break; + case QgsColorWidget::Saturation: + if ( s == clippedValue ) + { + return; + } + color.setHsv( h, clippedValue, v, a ); + break; + case QgsColorWidget::Value: + if ( v == clippedValue ) + { + return; + } + color.setHsv( h, s, clippedValue, a ); + break; + case QgsColorWidget::Alpha: + if ( a == clippedValue ) + { + return; + } + color.setAlpha( clippedValue ); + break; + default: + return; + } + } +} + +bool QgsColorWidget::isCmyk( QgsColorWidget::ColorComponent component ) +{ switch ( component ) { - case QgsColorWidget::Red: - color.setRed( clippedValue ); - return; - case QgsColorWidget::Green: - color.setGreen( clippedValue ); - return; - case QgsColorWidget::Blue: - color.setBlue( clippedValue ); - return; - case QgsColorWidget::Hue: - color.setHsv( clippedValue, s, v, a ); - return; - case QgsColorWidget::Saturation: - color.setHsv( h, clippedValue, v, a ); - return; - case QgsColorWidget::Value: - color.setHsv( h, s, clippedValue, a ); - return; - case QgsColorWidget::Alpha: - color.setAlpha( clippedValue ); - return; + case Cyan: + case Magenta: + case Yellow: + case Black: + return true; default: - return; + return false; } } +bool QgsColorWidget::isCmyk() const +{ + return isCmyk( mComponent ); +} + const QPixmap &QgsColorWidget::transparentBackground() { static QPixmap sTranspBkgrd; @@ -269,72 +368,19 @@ void QgsColorWidget::setComponentValue( const int value ) return; } - //clip value to valid range - int valueClipped = std::min( value, componentRange() ); - valueClipped = std::max( valueClipped, 0 ); - - int r, g, b, a; - mCurrentColor.getRgb( &r, &g, &b, &a ); - int h, s, v; - mCurrentColor.getHsv( &h, &s, &v ); //overwrite hue with explicit hue if required - h = hue(); - - switch ( mComponent ) + if ( mComponent == QgsColorWidget::Saturation || mComponent == QgsColorWidget::Value ) { - case QgsColorWidget::Red: - if ( r == valueClipped ) - { - return; - } - mCurrentColor.setRed( valueClipped ); - break; - case QgsColorWidget::Green: - if ( g == valueClipped ) - { - return; - } - mCurrentColor.setGreen( valueClipped ); - break; - case QgsColorWidget::Blue: - if ( b == valueClipped ) - { - return; - } - mCurrentColor.setBlue( valueClipped ); - break; - case QgsColorWidget::Hue: - if ( h == valueClipped ) - { - return; - } - mCurrentColor.setHsv( valueClipped, s, v, a ); - break; - case QgsColorWidget::Saturation: - if ( s == valueClipped ) - { - return; - } - mCurrentColor.setHsv( h, valueClipped, v, a ); - break; - case QgsColorWidget::Value: - if ( v == valueClipped ) - { - return; - } - mCurrentColor.setHsv( h, s, valueClipped, a ); - break; - case QgsColorWidget::Alpha: - if ( a == valueClipped ) - { - return; - } - mCurrentColor.setAlpha( valueClipped ); - break; - default: - return; + int h, s, v, a; + mCurrentColor.getHsv( &h, &s, &v, &a ); + + h = hue(); + + mCurrentColor.setHsv( h, s, v, a ); } + alterColor( mCurrentColor, mComponent, value ); + //update recorded hue if ( mCurrentColor.hue() >= 0 ) { @@ -839,30 +885,19 @@ void QgsColorBox::setComponent( const QgsColorWidget::ColorComponent component ) void QgsColorBox::setColor( const QColor &color, const bool emitSignals ) { //check if we need to redraw the box image - if ( mComponent == QgsColorWidget::Red && mCurrentColor.red() != color.red() ) - { - mDirty = true; - } - else if ( mComponent == QgsColorWidget::Green && mCurrentColor.green() != color.green() ) - { - mDirty = true; - } - else if ( mComponent == QgsColorWidget::Blue && mCurrentColor.blue() != color.blue() ) - { - mDirty = true; - } - else if ( mComponent == QgsColorWidget::Hue && color.hsvHue() >= 0 && hue() != color.hsvHue() ) - { - mDirty = true; - } - else if ( mComponent == QgsColorWidget::Saturation && mCurrentColor.hsvSaturation() != color.hsvSaturation() ) - { - mDirty = true; - } - else if ( mComponent == QgsColorWidget::Value && mCurrentColor.value() != color.value() ) - { - mDirty = true; - } + mDirty |= ( + ( mComponent == QgsColorWidget::Red && mCurrentColor.red() != color.red() ) || + ( mComponent == QgsColorWidget::Green && mCurrentColor.green() != color.green() ) || + ( mComponent == QgsColorWidget::Blue && mCurrentColor.blue() != color.blue() ) || + ( mComponent == QgsColorWidget::Hue && color.hsvHue() >= 0 && hue() != color.hsvHue() ) || + ( mComponent == QgsColorWidget::Saturation && mCurrentColor.hsvSaturation() != color.hsvSaturation() ) || + ( mComponent == QgsColorWidget::Value && mCurrentColor.value() != color.value() ) || + ( mComponent == QgsColorWidget::Cyan && mCurrentColor.cyan() != color.cyan() ) || + ( mComponent == QgsColorWidget::Magenta && mCurrentColor.magenta() != color.magenta() ) || + ( mComponent == QgsColorWidget::Yellow && mCurrentColor.yellow() != color.yellow() ) || + ( mComponent == QgsColorWidget::Black && mCurrentColor.black() != color.black() ) + ); + QgsColorWidget::setColor( color, emitSignals ); } @@ -949,14 +984,22 @@ QgsColorWidget::ColorComponent QgsColorBox::yComponent() const { case QgsColorWidget::Red: return QgsColorWidget::Green; - case QgsColorWidget:: Green: - case QgsColorWidget:: Blue: + case QgsColorWidget::Green: + case QgsColorWidget::Blue: return QgsColorWidget::Red; + case QgsColorWidget::Hue: - return QgsColorWidget:: Saturation; - case QgsColorWidget:: Saturation: - case QgsColorWidget:: Value: - return QgsColorWidget::Hue; + return QgsColorWidget::Saturation; + case QgsColorWidget::Saturation: + case QgsColorWidget::Value: + return QgsColorWidget::Hue; + + case QgsColorWidget::Magenta: + return QgsColorWidget::Yellow; + case QgsColorWidget::Yellow: + case QgsColorWidget::Cyan: + return QgsColorWidget::Magenta; + default: //should not occur return QgsColorWidget::Red; @@ -972,16 +1015,24 @@ QgsColorWidget::ColorComponent QgsColorBox::xComponent() const { switch ( mComponent ) { - case QgsColorWidget::Red: - case QgsColorWidget:: Green: + case QgsColorWidget::Red: + case QgsColorWidget::Green: return QgsColorWidget::Blue; - case QgsColorWidget:: Blue: + case QgsColorWidget::Blue: return QgsColorWidget::Green; + case QgsColorWidget::Hue: - case QgsColorWidget:: Saturation: - return QgsColorWidget:: Value; - case QgsColorWidget:: Value: - return QgsColorWidget::Saturation; + case QgsColorWidget::Saturation: + return QgsColorWidget::Value; + case QgsColorWidget::Value: + return QgsColorWidget::Saturation; + + case QgsColorWidget::Magenta: + case QgsColorWidget::Yellow: + return QgsColorWidget::Cyan; + case QgsColorWidget::Cyan: + return QgsColorWidget::Yellow; + default: //should not occur return QgsColorWidget::Red; diff --git a/src/gui/qgscolorwidgets.h b/src/gui/qgscolorwidgets.h index fd32563df263..02ef4775c2d0 100644 --- a/src/gui/qgscolorwidgets.h +++ b/src/gui/qgscolorwidgets.h @@ -53,7 +53,11 @@ class GUI_EXPORT QgsColorWidget : public QWidget Hue, //!< Hue component of color (based on HSV model) Saturation, //!< Saturation component of color (based on HSV model) Value, //!< Value component of color (based on HSV model) - Alpha //!< Alpha component (opacity) of color + Alpha, //!< Alpha component (opacity) of color + Cyan, //!< Cyan component (based on CMYK model) of color + Magenta, //!< Magenta component (based on CMYK model) of color + Yellow, //!< Yellow component (based on CMYK model) of color + Black //!< Black component (based on CMYK model) of color }; /** @@ -155,7 +159,7 @@ class GUI_EXPORT QgsColorWidget : public QWidget * Returns the range of valid values a color component * \returns maximum value allowed for color component */ - int componentRange( ColorComponent component ) const; + static int componentRange( ColorComponent component ); /** * Returns the value of a component of the widget's current color. This method correctly @@ -180,7 +184,17 @@ class GUI_EXPORT QgsColorWidget : public QWidget * \param newValue new value of color component. Values are automatically clipped to a * valid range for the color component. */ - void alterColor( QColor &color, QgsColorWidget::ColorComponent component, int newValue ) const; + static void alterColor( QColor &color, QgsColorWidget::ColorComponent component, int newValue ); + + /** + * Returns true if the color widget component is either Cyan, Magenta, Yellow or Black + */ + bool isCmyk() const; + + /** + * Returns true if \a component is either Cyan, Magenta, Yellow or Black + */ + static bool isCmyk( QgsColorWidget::ColorComponent component ); /** * Generates a checkboard pattern pixmap for use as a background to transparent colors diff --git a/src/gui/qgscompoundcolorwidget.cpp b/src/gui/qgscompoundcolorwidget.cpp index ced8cd09d013..b7fd1697c057 100644 --- a/src/gui/qgscompoundcolorwidget.cpp +++ b/src/gui/qgscompoundcolorwidget.cpp @@ -53,6 +53,14 @@ QgsCompoundColorWidget::QgsCompoundColorWidget( QWidget *parent, const QColor &c connect( mTabWidget, &QTabWidget::currentChanged, this, &QgsCompoundColorWidget::mTabWidget_currentChanged ); connect( mActionShowInButtons, &QAction::toggled, this, &QgsCompoundColorWidget::mActionShowInButtons_toggled ); + connect( mColorModel, static_cast( &QComboBox::currentIndexChanged ), this, [this]( int index ) + { + if ( index ) + setColor( this->color().toCmyk() ); + else + setColor( this->color().toRgb() ); + } ); + if ( widgetLayout == LayoutVertical ) { // shuffle stuff around @@ -112,7 +120,7 @@ QgsCompoundColorWidget::QgsCompoundColorWidget( QWidget *parent, const QColor &c mSchemeToolButton->setMenu( schemeMenu ); connect( mSchemeComboBox, static_cast( &QComboBox::currentIndexChanged ), this, &QgsCompoundColorWidget::schemeIndexChanged ); - connect( mSchemeList, &QgsColorSchemeList::colorSelected, this, &QgsCompoundColorWidget::setColor ); + connect( mSchemeList, &QgsColorSchemeList::colorSelected, this, &QgsCompoundColorWidget::_setColor ); mOldColorLabel->hide(); @@ -127,6 +135,10 @@ QgsCompoundColorWidget::QgsCompoundColorWidget( QWidget *parent, const QColor &c mSaturationSlider->setComponent( QgsColorWidget::Saturation ); mValueSlider->setComponent( QgsColorWidget::Value ); mAlphaSlider->setComponent( QgsColorWidget::Alpha ); + mCyanSlider->setComponent( QgsColorWidget::Cyan ); + mMagentaSlider->setComponent( QgsColorWidget::Magenta ); + mYellowSlider->setComponent( QgsColorWidget::Yellow ); + mBlackSlider->setComponent( QgsColorWidget::Black ); mSwatchButton1->setShowMenu( false ); mSwatchButton1->setBehavior( QgsColorButton::SignalOnly ); @@ -259,34 +271,38 @@ QgsCompoundColorWidget::QgsCompoundColorWidget( QWidget *parent, const QColor &c mTabWidget->setCurrentIndex( currentTab ); //setup connections - connect( mColorBox, &QgsColorWidget::colorChanged, this, &QgsCompoundColorWidget::setColor ); - connect( mColorWheel, &QgsColorWidget::colorChanged, this, &QgsCompoundColorWidget::setColor ); - connect( mColorText, &QgsColorWidget::colorChanged, this, &QgsCompoundColorWidget::setColor ); - connect( mVerticalRamp, &QgsColorWidget::colorChanged, this, &QgsCompoundColorWidget::setColor ); - connect( mRedSlider, &QgsColorWidget::colorChanged, this, &QgsCompoundColorWidget::setColor ); - connect( mGreenSlider, &QgsColorWidget::colorChanged, this, &QgsCompoundColorWidget::setColor ); - connect( mBlueSlider, &QgsColorWidget::colorChanged, this, &QgsCompoundColorWidget::setColor ); - connect( mHueSlider, &QgsColorWidget::colorChanged, this, &QgsCompoundColorWidget::setColor ); - connect( mValueSlider, &QgsColorWidget::colorChanged, this, &QgsCompoundColorWidget::setColor ); - connect( mSaturationSlider, &QgsColorWidget::colorChanged, this, &QgsCompoundColorWidget::setColor ); - connect( mAlphaSlider, &QgsColorWidget::colorChanged, this, &QgsCompoundColorWidget::setColor ); - connect( mColorPreview, &QgsColorWidget::colorChanged, this, &QgsCompoundColorWidget::setColor ); - connect( mSwatchButton1, &QgsColorButton::colorClicked, this, &QgsCompoundColorWidget::setColor ); - connect( mSwatchButton2, &QgsColorButton::colorClicked, this, &QgsCompoundColorWidget::setColor ); - connect( mSwatchButton3, &QgsColorButton::colorClicked, this, &QgsCompoundColorWidget::setColor ); - connect( mSwatchButton4, &QgsColorButton::colorClicked, this, &QgsCompoundColorWidget::setColor ); - connect( mSwatchButton5, &QgsColorButton::colorClicked, this, &QgsCompoundColorWidget::setColor ); - connect( mSwatchButton6, &QgsColorButton::colorClicked, this, &QgsCompoundColorWidget::setColor ); - connect( mSwatchButton7, &QgsColorButton::colorClicked, this, &QgsCompoundColorWidget::setColor ); - connect( mSwatchButton8, &QgsColorButton::colorClicked, this, &QgsCompoundColorWidget::setColor ); - connect( mSwatchButton9, &QgsColorButton::colorClicked, this, &QgsCompoundColorWidget::setColor ); - connect( mSwatchButton10, &QgsColorButton::colorClicked, this, &QgsCompoundColorWidget::setColor ); - connect( mSwatchButton11, &QgsColorButton::colorClicked, this, &QgsCompoundColorWidget::setColor ); - connect( mSwatchButton12, &QgsColorButton::colorClicked, this, &QgsCompoundColorWidget::setColor ); - connect( mSwatchButton13, &QgsColorButton::colorClicked, this, &QgsCompoundColorWidget::setColor ); - connect( mSwatchButton14, &QgsColorButton::colorClicked, this, &QgsCompoundColorWidget::setColor ); - connect( mSwatchButton15, &QgsColorButton::colorClicked, this, &QgsCompoundColorWidget::setColor ); - connect( mSwatchButton16, &QgsColorButton::colorClicked, this, &QgsCompoundColorWidget::setColor ); + connect( mColorBox, &QgsColorWidget::colorChanged, this, &QgsCompoundColorWidget::_setColor ); + connect( mColorWheel, &QgsColorWidget::colorChanged, this, &QgsCompoundColorWidget::_setColor ); + connect( mColorText, &QgsColorWidget::colorChanged, this, &QgsCompoundColorWidget::_setColor ); + connect( mVerticalRamp, &QgsColorWidget::colorChanged, this, &QgsCompoundColorWidget::_setColor ); + connect( mRedSlider, &QgsColorWidget::colorChanged, this, &QgsCompoundColorWidget::_setColor ); + connect( mGreenSlider, &QgsColorWidget::colorChanged, this, &QgsCompoundColorWidget::_setColor ); + connect( mBlueSlider, &QgsColorWidget::colorChanged, this, &QgsCompoundColorWidget::_setColor ); + connect( mHueSlider, &QgsColorWidget::colorChanged, this, &QgsCompoundColorWidget::_setColor ); + connect( mValueSlider, &QgsColorWidget::colorChanged, this, &QgsCompoundColorWidget::_setColor ); + connect( mCyanSlider, &QgsColorWidget::colorChanged, this, &QgsCompoundColorWidget::_setColor ); + connect( mMagentaSlider, &QgsColorWidget::colorChanged, this, &QgsCompoundColorWidget::_setColor ); + connect( mYellowSlider, &QgsColorWidget::colorChanged, this, &QgsCompoundColorWidget::_setColor ); + connect( mBlackSlider, &QgsColorWidget::colorChanged, this, &QgsCompoundColorWidget::_setColor ); + connect( mSaturationSlider, &QgsColorWidget::colorChanged, this, &QgsCompoundColorWidget::_setColor ); + connect( mAlphaSlider, &QgsColorWidget::colorChanged, this, &QgsCompoundColorWidget::_setColor ); + connect( mColorPreview, &QgsColorWidget::colorChanged, this, &QgsCompoundColorWidget::_setColor ); + connect( mSwatchButton1, &QgsColorButton::colorClicked, this, &QgsCompoundColorWidget::_setColor ); + connect( mSwatchButton2, &QgsColorButton::colorClicked, this, &QgsCompoundColorWidget::_setColor ); + connect( mSwatchButton3, &QgsColorButton::colorClicked, this, &QgsCompoundColorWidget::_setColor ); + connect( mSwatchButton4, &QgsColorButton::colorClicked, this, &QgsCompoundColorWidget::_setColor ); + connect( mSwatchButton5, &QgsColorButton::colorClicked, this, &QgsCompoundColorWidget::_setColor ); + connect( mSwatchButton6, &QgsColorButton::colorClicked, this, &QgsCompoundColorWidget::_setColor ); + connect( mSwatchButton7, &QgsColorButton::colorClicked, this, &QgsCompoundColorWidget::_setColor ); + connect( mSwatchButton8, &QgsColorButton::colorClicked, this, &QgsCompoundColorWidget::_setColor ); + connect( mSwatchButton9, &QgsColorButton::colorClicked, this, &QgsCompoundColorWidget::_setColor ); + connect( mSwatchButton10, &QgsColorButton::colorClicked, this, &QgsCompoundColorWidget::_setColor ); + connect( mSwatchButton11, &QgsColorButton::colorClicked, this, &QgsCompoundColorWidget::_setColor ); + connect( mSwatchButton12, &QgsColorButton::colorClicked, this, &QgsCompoundColorWidget::_setColor ); + connect( mSwatchButton13, &QgsColorButton::colorClicked, this, &QgsCompoundColorWidget::_setColor ); + connect( mSwatchButton14, &QgsColorButton::colorClicked, this, &QgsCompoundColorWidget::_setColor ); + connect( mSwatchButton15, &QgsColorButton::colorClicked, this, &QgsCompoundColorWidget::_setColor ); + connect( mSwatchButton16, &QgsColorButton::colorClicked, this, &QgsCompoundColorWidget::_setColor ); } QgsCompoundColorWidget::~QgsCompoundColorWidget() @@ -707,6 +723,14 @@ void QgsCompoundColorWidget::stopPicking( QPoint eventPos, const bool takeSample } void QgsCompoundColorWidget::setColor( const QColor &color ) +{ + mColorModel->setCurrentIndex( color.spec() == QColor::Cmyk ? 1 : 0 ); + mRGB->setVisible( color.spec() != QColor::Cmyk ); + mCMYK->setVisible( color.spec() == QColor::Cmyk ); + _setColor( color ); +} + +void QgsCompoundColorWidget::_setColor( const QColor &color ) { if ( !color.isValid() ) { @@ -719,6 +743,12 @@ void QgsCompoundColorWidget::setColor( const QColor &color ) //opacity disallowed, so don't permit transparent colors fixedColor.setAlpha( 255 ); } + + if ( mColorModel->currentIndex() && fixedColor.spec() != QColor::Cmyk ) + { + fixedColor = fixedColor.toCmyk(); + } + const QList colorWidgets = this->findChildren(); const auto constColorWidgets = colorWidgets; for ( QgsColorWidget *widget : constColorWidgets ) @@ -731,6 +761,8 @@ void QgsCompoundColorWidget::setColor( const QColor &color ) widget->setColor( fixedColor ); widget->blockSignals( false ); } + + emit currentColorChanged( fixedColor ); } diff --git a/src/gui/qgscompoundcolorwidget.h b/src/gui/qgscompoundcolorwidget.h index 4683344b6952..2b7a527ca0bf 100644 --- a/src/gui/qgscompoundcolorwidget.h +++ b/src/gui/qgscompoundcolorwidget.h @@ -167,10 +167,14 @@ class GUI_EXPORT QgsCompoundColorWidget : public QgsPanelWidget, private Ui::Qgs void mSampleButton_clicked(); void mTabWidget_currentChanged( int index ); - private slots: - void mActionShowInButtons_toggled( bool state ); + /** + * Internal color setter. Set \a color without changing current color model (RGB or CMYK), + * contrary to public setColor() + */ + void _setColor( const QColor &color ); + private: static QScreen *findScreenAt( QPoint pos ); @@ -224,6 +228,8 @@ class GUI_EXPORT QgsCompoundColorWidget : public QgsPanelWidget, private Ui::Qgs //! Updates the state of actions for the current selected scheme void updateActionsForCurrentScheme(); + + friend class TestQgsCompoundColorWidget; }; #endif // QGSCOMPOUNDCOLORWIDGET_H diff --git a/src/ui/qgscompoundcolorwidget.ui b/src/ui/qgscompoundcolorwidget.ui index b23d000a4f07..aa1a016851ce 100644 --- a/src/ui/qgscompoundcolorwidget.ui +++ b/src/ui/qgscompoundcolorwidget.ui @@ -6,568 +6,163 @@ 0 0 - 597 - 329 + 588 + 481 - - - 0 - - - 0 - - - 0 - - - 0 - + 6 - - - - 2 - - - - 16 - 16 - + + + + + 0 + 0 + - - - - :/images/themes/default/mIconColorBox.svg:/images/themes/default/mIconColorBox.svg - - - - - - Color ramp - - - - - - - - - - - - - - :/images/themes/default/mIconColorWheel.svg:/images/themes/default/mIconColorWheel.svg - - - - - - Color wheel - - - - - - - - - - - :/images/themes/default/mIconColorSwatches.svg:/images/themes/default/mIconColorSwatches.svg - - - - - - Color swatches - - - - 2 - - - - - 1 - - - - - - - - … - - - QToolButton::InstantPopup - - - - - - - - - - - - 1 - - - - - Qt::Horizontal - - - - 40 - 20 - - - - - - - - Add current color - - - - - - - :/images/themes/default/symbologyAdd.svg:/images/themes/default/symbologyAdd.svg - - - - - - - Remove selected color - - - - - - - :/images/themes/default/symbologyRemove.svg:/images/themes/default/symbologyRemove.svg - - - - - - - - - - - :/images/themes/default/mIconColorPicker.svg:/images/themes/default/mIconColorPicker.svg - - - - - - Color picker - - - - - - - - Sample average radius - - - - - - - px - - - 1 - - - 100 - - - - - - - - - Sample color - - - - - - - <i>Press space to sample a color from under the mouse cursor</i> - - - Qt::AutoText - - - true - - - - - - - QFrame::StyledPanel - - - QFrame::Raised - - - - 1 - - - 1 - - - 1 - - - 1 - - - - - - 0 - 40 - - - - - - - - - - - Qt::Vertical - - - - 20 - 0 - - - - - - - - - - - - - - - - - - - - - - - - H - - - - - - - - - - - - - - - - - - - - - S - - - - - - - - - - - - - - - - - - - - - V - - - - - - - - - - - - - - - - - - - - - R - - - - - - - - - - - - - - - - - - - - - G - - - - - - - - - - - - - - - - - - - - - B - - - - - - - - - - - - - - Opacity - - - - - - - - - - - - - - HTML notation - - - - - - - + + + 1 + + + + + + 38 + 30 + + + + + 38 + 30 + + + + + + - - - - Qt::Vertical + + + + + 38 + 30 + - + - 20 - 0 + 38 + 30 - - - - - - - - - - 0 - 0 - - - - - 16777215 - 80 - - - - - - Current - - - Qt::AlignRight|Qt::AlignTrailing|Qt::AlignVCenter + - - - - Old + + + + + 38 + 30 + - - Qt::AlignRight|Qt::AlignTrailing|Qt::AlignVCenter + + + 38 + 30 + + + + - - - - - 0 - 0 - - + + - 28 - 0 + 38 + 30 - 28 - 16777215 + 38 + 30 - - Add color to swatch + + + + + + + + + + 38 + 30 + + + + + 38 + 30 + - - - :/images/themes/default/mActionAtlasNext.svg:/images/themes/default/mActionAtlasNext.svg + + + + + + + 38 + 30 + - + - 24 - 24 + 38 + 30 + + + - - - - QFrame::StyledPanel + + + + + 38 + 30 + - - QFrame::Raised + + + 38 + 30 + + + + - - - 1 - - - 1 - - - 1 - - - 1 - - - - - - 0 - 40 - - - - - - - - - - - - - 0 - 0 - - - - - 1 - - - + + 38 @@ -585,8 +180,8 @@ - - + + 38 @@ -604,8 +199,8 @@ - - + + 38 @@ -623,8 +218,8 @@ - - + + 38 @@ -642,8 +237,8 @@ - - + + 38 @@ -661,8 +256,8 @@ - - + + 38 @@ -680,8 +275,8 @@ - - + + 38 @@ -699,8 +294,8 @@ - - + + 38 @@ -718,8 +313,8 @@ - - + + 38 @@ -735,140 +330,694 @@ - - - - + + + + + + + + + + 0 + 0 + + + + + 16777215 + 80 + + + + + + + Current + + + Qt::AlignRight|Qt::AlignTrailing|Qt::AlignVCenter + + + + + + + Old + + + Qt::AlignRight|Qt::AlignTrailing|Qt::AlignVCenter + + + + + + + + 0 + 0 + + - 38 - 30 + 28 + 0 - 38 - 30 + 28 + 16777215 + + Add color to swatch + - - - - - - - 38 - 30 - + + + :/images/themes/default/mActionAtlasNext.svg:/images/themes/default/mActionAtlasNext.svg - + - 38 - 30 + 24 + 24 - - + + + + + + QFrame::StyledPanel + + + QFrame::Raised + + + 1 + + + 1 + + + 1 + + + 1 + + + + + + 0 + 40 + + + + + - - - - - 38 - 30 - - - - - 38 - 30 - - - - - + + + + + + + 2 + + + + 16 + 16 + + + + + + :/images/themes/default/mIconColorBox.svg:/images/themes/default/mIconColorBox.svg + + + + + + Color ramp + + + + + + + + + + + + + + :/images/themes/default/mIconColorWheel.svg:/images/themes/default/mIconColorWheel.svg + + + + + + Color wheel + + + + + + + + + + + :/images/themes/default/mIconColorSwatches.svg:/images/themes/default/mIconColorSwatches.svg + + + + + + Color swatches + + + + 2 + + + + + 1 + + + + + + + + … + + + QToolButton::InstantPopup + + + + + + + + + + + + 1 + + + + + Qt::Horizontal + + + + 40 + 20 + + + + + + + + Add current color + + + + + + + :/images/themes/default/symbologyAdd.svg:/images/themes/default/symbologyAdd.svg + + + + + + + Remove selected color + + + + + + + :/images/themes/default/symbologyRemove.svg:/images/themes/default/symbologyRemove.svg + + + + + + + + + + + :/images/themes/default/mIconColorPicker.svg:/images/themes/default/mIconColorPicker.svg + + + + + + Color picker + + + + + + + + Sample average radius + + + + + + + px + + + 1 + + + 100 + + + + + + + + + Sample color + + + + + + + <i>Press space to sample a color from under the mouse cursor</i> + + + Qt::AutoText + + + true + + + + + + + QFrame::StyledPanel + + + QFrame::Raised + + + + 1 + + + 1 + + + 1 + + + 1 + + + + + + 0 + 40 + + + + + + + + + + + Qt::Vertical + + + + 20 + 0 + + + + + + + + + + + + + + + + + Color model + + + + + + + + RGB + + + + + CMYK + + + + + + + + Qt::Horizontal + + + + 228 + 20 + + + + + + + + + + + 0 + + + 0 + + + 0 + + + 0 + + + + + + + + + + + + + + H + + + + + + + + + + + + + + + + + + + + + S + + + + + + + + + + + + + + + + + + + + + V + + + + + + + + + + + + + + + + + + + + + R + + + + + + + + + + + + + + + + + + + + + G + + + + + + + + + + + + + + + + + + + + + B + + + + + + + + + - - - - - 38 - 30 - - - - - 38 - 30 - - - - - + + + + + 6 + + + 0 + + + 0 + + + 0 + + + 0 + + + + + + + + + + + + + + C + + + + + + + + + + + + + + + + + + + + + M + + + + + + + + + + + + + + + + + + + + + Y + + + + + + + + + + + + + + + + + + + + + K + + + + + + + + + - - - - - 38 - 30 - - - - - 38 - 30 - - - - - - + + + + + + Opacity + + + + + + + - - - - - 38 - 30 - - - - - 38 - 30 - - - - - - + + + + + + HTML notation + + + + + + + - - - - - 38 - 30 - + + + + Qt::Vertical - + - 38 - 30 + 20 + 0 - - - - + @@ -942,17 +1091,17 @@ - - QgsSpinBox - QSpinBox -
qgsspinbox.h
- QgsColorButton QToolButton
qgscolorbutton.h
1 + + QgsSpinBox + QSpinBox +
qgsspinbox.h
+ QgsColorBox QWidget diff --git a/tests/src/gui/CMakeLists.txt b/tests/src/gui/CMakeLists.txt index f2b2111d66a7..fcf1d67ebd3f 100644 --- a/tests/src/gui/CMakeLists.txt +++ b/tests/src/gui/CMakeLists.txt @@ -74,6 +74,7 @@ set(TESTS testqgsexternalresourcewidgetwrapper.cpp testqgsquerybuilder.cpp testqgsqueryresultwidget.cpp + testqgscompoundcolorwidget.cpp ) foreach(TESTSRC ${TESTS}) diff --git a/tests/src/gui/testqgscompoundcolorwidget.cpp b/tests/src/gui/testqgscompoundcolorwidget.cpp new file mode 100644 index 000000000000..2c7cdca1ad38 --- /dev/null +++ b/tests/src/gui/testqgscompoundcolorwidget.cpp @@ -0,0 +1,92 @@ +/*************************************************************************** + testqgscompoundcolorwidget.cpp + --------------------- + begin : 2024/05/07 + copyright : (C) 2024 by Julien Cabieces + email : julien dot cabieces at oslandia dot com + *************************************************************************** + * * + * This program is free software; you can redistribute it and/or modify * + * it under the terms of the GNU General Public License as published by * + * the Free Software Foundation; either version 2 of the License, or * + * (at your option) any later version. * + * * + ***************************************************************************/ + +#include "qgstest.h" + +#include "qgscompoundcolorwidget.h" + +class TestQgsCompoundColorWidget : public QgsTest +{ + Q_OBJECT + + public: + + TestQgsCompoundColorWidget() : QgsTest( QStringLiteral( "Compound color widget Tests" ) ) {} + + private slots: + void initTestCase();// will be called before the first testfunction is executed. + void cleanupTestCase();// will be called after the last testfunction was executed. + void init();// will be called before each testfunction is executed. + void cleanup();// will be called after every testfunction. + void testCmykConversion(); +}; + +void TestQgsCompoundColorWidget::initTestCase() +{ +} + +void TestQgsCompoundColorWidget::cleanupTestCase() +{ +} + +void TestQgsCompoundColorWidget::init() +{ +} + +void TestQgsCompoundColorWidget::cleanup() +{ +} + +void TestQgsCompoundColorWidget::testCmykConversion() +{ + QgsCompoundColorWidget w( nullptr, QColor( 10, 20, 30, 50 ) ); + w.setVisible( true ); + + QCOMPARE( w.color(), QColor( 10, 20, 30, 50 ) ); + QCOMPARE( w.mColorModel->currentIndex(), 0 ); + QVERIFY( w.mRGB->isVisible() ); + QVERIFY( !w.mCMYK->isVisible() ); + + // switch to CMYK + w.mColorModel->setCurrentIndex( 1 ); + QVERIFY( !w.mRGB->isVisible() ); + QVERIFY( w.mCMYK->isVisible() ); + QCOMPARE( w.mColorModel->currentIndex(), 1 ); + QCOMPARE( w.color(), QColor::fromCmyk( 170, 85, 0, 225, 50 ) ); + + // switch back to RGB + w.mColorModel->setCurrentIndex( 0 ); + QVERIFY( w.mRGB->isVisible() ); + QVERIFY( !w.mCMYK->isVisible() ); + QCOMPARE( w.mColorModel->currentIndex(), 0 ); + QCOMPARE( w.color(), QColor( 10, 20, 30, 50 ) ); + + // edit color in CMYK mode + w.mColorModel->setCurrentIndex( 1 ); + QVERIFY( !w.mRGB->isVisible() ); + QVERIFY( w.mCMYK->isVisible() ); + QCOMPARE( w.mColorModel->currentIndex(), 1 ); + QCOMPARE( w.color(), QColor::fromCmyk( 170, 85, 0, 225, 50 ) ); + + w.mCyanSlider->setColor( QColor::fromCmyk( 120, 85, 0, 225, 50 ), true ); + QCOMPARE( w.color(), QColor::fromCmyk( 120, 85, 0, 225, 50 ) ); + + // edit color in RGB, the returned color is still CMYK + w.mColorWheel->setColor( QColor( 10, 20, 30, 50 ), true ); + QCOMPARE( w.color(), QColor::fromCmyk( 170, 85, 0, 225, 50 ) ); +} + +QGSTEST_MAIN( TestQgsCompoundColorWidget ) +#include "testqgscompoundcolorwidget.moc" diff --git a/tests/src/python/test_qgscolorwidget.py b/tests/src/python/test_qgscolorwidget.py new file mode 100644 index 000000000000..6b57f033543d --- /dev/null +++ b/tests/src/python/test_qgscolorwidget.py @@ -0,0 +1,172 @@ +"""QGIS Unit tests for QgsColorWidget + +.. note:: This program is free software; you can redistribute it and/or modify +it under the terms of the GNU General Public License as published by +the Free Software Foundation; either version 2 of the License, or +(at your option) any later version. +""" +__author__ = 'Julien Cabieces' +__date__ = '02/05/2024' +__copyright__ = 'Copyright 2024, The QGIS Project' + +from qgis.PyQt.QtGui import QColor +from qgis.gui import QgsColorWidget + +import unittest +from qgis.testing import start_app, QgisTestCase + +start_app() + + +class TestQgsWidget(QgisTestCase): + + def testAlterColor(self): + """ + test alterColor method + """ + + # rgb + color = QColor(12, 34, 56) + QgsColorWidget.alterColor(color, QgsColorWidget.ColorComponent.Red, 112) + self.assertEqual(color, QColor(112, 34, 56, 255)) + self.assertEqual(color.spec(), QColor.Spec.Rgb) + QgsColorWidget.alterColor(color, QgsColorWidget.ColorComponent.Green, 134) + self.assertEqual(color.spec(), QColor.Spec.Rgb) + self.assertEqual(color, QColor(112, 134, 56, 255)) + QgsColorWidget.alterColor(color, QgsColorWidget.ColorComponent.Blue, 156) + self.assertEqual(color.spec(), QColor.Spec.Rgb) + self.assertEqual(color, QColor(112, 134, 156, 255)) + QgsColorWidget.alterColor(color, QgsColorWidget.ColorComponent.Alpha, 210) + self.assertEqual(color.spec(), QColor.Spec.Rgb) + self.assertEqual(color, QColor(112, 134, 156, 210)) + + # hsv + QgsColorWidget.alterColor(color, QgsColorWidget.ColorComponent.Hue, 100) + self.assertEqual(color.spec(), QColor.Spec.Hsv) + self.assertEqual(color, QColor.fromHsv(100, 72, 156, 210)) + QgsColorWidget.alterColor(color, QgsColorWidget.ColorComponent.Saturation, 150) + self.assertEqual(color.spec(), QColor.Spec.Hsv) + self.assertEqual(color, QColor.fromHsv(100, 150, 156, 210)) + QgsColorWidget.alterColor(color, QgsColorWidget.ColorComponent.Value, 200) + self.assertEqual(color.spec(), QColor.Spec.Hsv) + self.assertEqual(color, QColor.fromHsv(100, 150, 200, 210)) + + # clipped value + QgsColorWidget.alterColor(color, QgsColorWidget.ColorComponent.Value, 300) + self.assertEqual(color.spec(), QColor.Spec.Hsv) + self.assertEqual(color, QColor.fromHsv(100, 150, 255, 210)) + QgsColorWidget.alterColor(color, QgsColorWidget.ColorComponent.Value, -2) + self.assertEqual(color.spec(), QColor.Spec.Hsv) + self.assertEqual(color, QColor.fromHsv(100, 150, 0, 210)) + + # cmyk + QgsColorWidget.alterColor(color, QgsColorWidget.ColorComponent.Cyan, 22) + self.assertEqual(color.spec(), QColor.Spec.Cmyk) + self.assertEqual(color, QColor.fromCmyk(22, 0, 0, 255, 210)) + QgsColorWidget.alterColor(color, QgsColorWidget.ColorComponent.Magenta, 33) + self.assertEqual(color.spec(), QColor.Spec.Cmyk) + self.assertEqual(color, QColor.fromCmyk(22, 33, 0, 255, 210)) + QgsColorWidget.alterColor(color, QgsColorWidget.ColorComponent.Yellow, 44) + self.assertEqual(color.spec(), QColor.Spec.Cmyk) + self.assertEqual(color, QColor.fromCmyk(22, 33, 44, 255, 210)) + QgsColorWidget.alterColor(color, QgsColorWidget.ColorComponent.Black, 55) + self.assertEqual(color.spec(), QColor.Spec.Cmyk) + self.assertEqual(color, QColor.fromCmyk(22, 33, 44, 55, 210)) + + def testSetComponentValue(self): + """ + test setComponentValue method + """ + w = QgsColorWidget() + + w.setColor(QColor(12, 34, 56)) + self.assertEqual(w.color(), QColor(12, 34, 56)) + + # rgb + w.setComponent(QgsColorWidget.ColorComponent.Red) + w.setComponentValue(112) + self.assertEqual(w.color(), QColor(112, 34, 56, 255)) + self.assertEqual(w.color().spec(), QColor.Spec.Rgb) + w.setComponent(QgsColorWidget.ColorComponent.Green) + w.setComponentValue(134) + self.assertEqual(w.color().spec(), QColor.Spec.Rgb) + self.assertEqual(w.color(), QColor(112, 134, 56, 255)) + w.setComponent(QgsColorWidget.ColorComponent.Blue) + w.setComponentValue(156) + self.assertEqual(w.color().spec(), QColor.Spec.Rgb) + self.assertEqual(w.color(), QColor(112, 134, 156, 255)) + w.setComponent(QgsColorWidget.ColorComponent.Alpha) + w.setComponentValue(210) + self.assertEqual(w.color().spec(), QColor.Spec.Rgb) + self.assertEqual(w.color(), QColor(112, 134, 156, 210)) + + # hsv + w.setComponent(QgsColorWidget.ColorComponent.Hue) + w.setComponentValue(100) + self.assertEqual(w.color().spec(), QColor.Spec.Hsv) + self.assertEqual(w.color(), QColor.fromHsv(100, 72, 156, 210)) + w.setComponent(QgsColorWidget.ColorComponent.Saturation) + w.setComponentValue(150) + self.assertEqual(w.color().spec(), QColor.Spec.Hsv) + self.assertEqual(w.color(), QColor.fromHsv(100, 150, 156, 210)) + w.setComponent(QgsColorWidget.ColorComponent.Value) + w.setComponentValue(200) + self.assertEqual(w.color().spec(), QColor.Spec.Hsv) + self.assertEqual(w.color(), QColor.fromHsv(100, 150, 200, 210)) + + # clipped value + w.setComponent(QgsColorWidget.ColorComponent.Value) + w.setComponentValue(300) + self.assertEqual(w.color().spec(), QColor.Spec.Hsv) + self.assertEqual(w.color(), QColor.fromHsv(100, 150, 255, 210)) + w.setComponent(QgsColorWidget.ColorComponent.Value) + w.setComponentValue(-2) + self.assertEqual(w.color().spec(), QColor.Spec.Hsv) + self.assertEqual(w.color(), QColor.fromHsv(100, 150, 0, 210)) + + # Multiple component has no effect + w.setComponent(QgsColorWidget.ColorComponent.Multiple) + w.setComponentValue(18) + self.assertEqual(w.color().spec(), QColor.Spec.Hsv) + self.assertEqual(w.color(), QColor.fromHsv(100, 150, 0, 210)) + + # set an achromatic color to check it keeps the hue + w.setColor(QColor(130, 130, 130, 255)) + self.assertEqual(w.color().spec(), QColor.Spec.Rgb) + self.assertEqual(w.color(), QColor(130, 130, 130, 255)) + w.setComponent(QgsColorWidget.ColorComponent.Value) + w.setComponentValue(42) + self.assertEqual(w.color(), QColor.fromHsv(100, 0, 42, 255)) + self.assertEqual(w.componentValue(QgsColorWidget.ColorComponent.Hue), 100) + + # cmyk + w.setComponent(QgsColorWidget.ColorComponent.Cyan) + w.setComponentValue(22) + self.assertEqual(w.color().spec(), QColor.Spec.Cmyk) + self.assertEqual(w.color(), QColor.fromCmyk(22, 0, 0, 213, 255)) + w.setComponent(QgsColorWidget.ColorComponent.Magenta) + w.setComponentValue(33) + self.assertEqual(w.color().spec(), QColor.Spec.Cmyk) + self.assertEqual(w.color(), QColor.fromCmyk(22, 33, 0, 213, 255)) + w.setComponent(QgsColorWidget.ColorComponent.Yellow) + w.setComponentValue(44) + self.assertEqual(w.color().spec(), QColor.Spec.Cmyk) + self.assertEqual(w.color(), QColor.fromCmyk(22, 33, 44, 213, 255)) + w.setComponent(QgsColorWidget.ColorComponent.Black) + w.setComponentValue(55) + self.assertEqual(w.color().spec(), QColor.Spec.Cmyk) + self.assertEqual(w.color(), QColor.fromCmyk(22, 33, 44, 55, 255)) + + # set an achromatic color to check it keeps the hue (set from former cmyk values) + self.assertEqual(w.color().hue(), 30) + w.setColor(QColor(130, 130, 130, 255)) + self.assertEqual(w.color().spec(), QColor.Spec.Rgb) + self.assertEqual(w.color(), QColor(130, 130, 130, 255)) + w.setComponent(QgsColorWidget.ColorComponent.Value) + w.setComponentValue(42) + self.assertEqual(w.color(), QColor.fromHsv(30, 0, 42, 255)) + self.assertEqual(w.componentValue(QgsColorWidget.ColorComponent.Hue), 30) + + +if __name__ == '__main__': + unittest.main() From 4c7cb10415a76b3ad4c6bbdb7c7b23be7ee57038 Mon Sep 17 00:00:00 2001 From: Julien Cabieces Date: Wed, 22 May 2024 15:13:49 +0200 Subject: [PATCH 02/12] connect radio buttons --- src/gui/qgscompoundcolorwidget.cpp | 40 ++++++++++++++++++++ src/gui/qgscompoundcolorwidget.h | 4 ++ src/ui/qgscompoundcolorwidget.ui | 33 ++++++---------- tests/src/gui/testqgscompoundcolorwidget.cpp | 35 +++++++++++++++++ 4 files changed, 90 insertions(+), 22 deletions(-) diff --git a/src/gui/qgscompoundcolorwidget.cpp b/src/gui/qgscompoundcolorwidget.cpp index b7fd1697c057..8bb871126f86 100644 --- a/src/gui/qgscompoundcolorwidget.cpp +++ b/src/gui/qgscompoundcolorwidget.cpp @@ -47,6 +47,10 @@ QgsCompoundColorWidget::QgsCompoundColorWidget( QWidget *parent, const QColor &c connect( mRedRadio, &QRadioButton::toggled, this, &QgsCompoundColorWidget::mRedRadio_toggled ); connect( mGreenRadio, &QRadioButton::toggled, this, &QgsCompoundColorWidget::mGreenRadio_toggled ); connect( mBlueRadio, &QRadioButton::toggled, this, &QgsCompoundColorWidget::mBlueRadio_toggled ); + connect( mCyanRadio, &QRadioButton::toggled, this, &QgsCompoundColorWidget::mCyanRadio_toggled ); + connect( mMagentaRadio, &QRadioButton::toggled, this, &QgsCompoundColorWidget::mMagentaRadio_toggled ); + connect( mYellowRadio, &QRadioButton::toggled, this, &QgsCompoundColorWidget::mYellowRadio_toggled ); + connect( mBlackRadio, &QRadioButton::toggled, this, &QgsCompoundColorWidget::mBlackRadio_toggled ); connect( mAddColorToSchemeButton, &QPushButton::clicked, this, &QgsCompoundColorWidget::mAddColorToSchemeButton_clicked ); connect( mAddCustomColorButton, &QPushButton::clicked, this, &QgsCompoundColorWidget::mAddCustomColorButton_clicked ); connect( mSampleButton, &QPushButton::clicked, this, &QgsCompoundColorWidget::mSampleButton_clicked ); @@ -936,6 +940,42 @@ void QgsCompoundColorWidget::mBlueRadio_toggled( bool checked ) } } +void QgsCompoundColorWidget::mCyanRadio_toggled( bool checked ) +{ + if ( checked ) + { + mColorBox->setComponent( QgsColorWidget::Cyan ); + mVerticalRamp->setComponent( QgsColorWidget::Cyan ); + } +} + +void QgsCompoundColorWidget::mMagentaRadio_toggled( bool checked ) +{ + if ( checked ) + { + mColorBox->setComponent( QgsColorWidget::Magenta ); + mVerticalRamp->setComponent( QgsColorWidget::Magenta ); + } +} + +void QgsCompoundColorWidget::mYellowRadio_toggled( bool checked ) +{ + if ( checked ) + { + mColorBox->setComponent( QgsColorWidget::Yellow ); + mVerticalRamp->setComponent( QgsColorWidget::Yellow ); + } +} + +void QgsCompoundColorWidget::mBlackRadio_toggled( bool checked ) +{ + if ( checked ) + { + mColorBox->setComponent( QgsColorWidget::Black ); + mVerticalRamp->setComponent( QgsColorWidget::Black ); + } +} + void QgsCompoundColorWidget::mAddColorToSchemeButton_clicked() { mSchemeList->addColor( mColorPreview->color(), QgsSymbolLayerUtils::colorToName( mColorPreview->color() ) ); diff --git a/src/gui/qgscompoundcolorwidget.h b/src/gui/qgscompoundcolorwidget.h index 2b7a527ca0bf..c9e71afa4e3f 100644 --- a/src/gui/qgscompoundcolorwidget.h +++ b/src/gui/qgscompoundcolorwidget.h @@ -152,6 +152,10 @@ class GUI_EXPORT QgsCompoundColorWidget : public QgsPanelWidget, private Ui::Qgs void mRedRadio_toggled( bool checked ); void mGreenRadio_toggled( bool checked ); void mBlueRadio_toggled( bool checked ); + void mCyanRadio_toggled( bool checked ); + void mMagentaRadio_toggled( bool checked ); + void mYellowRadio_toggled( bool checked ); + void mBlackRadio_toggled( bool checked ); void mAddColorToSchemeButton_clicked(); diff --git a/src/ui/qgscompoundcolorwidget.ui b/src/ui/qgscompoundcolorwidget.ui index aa1a016851ce..6771d99a8d6c 100644 --- a/src/ui/qgscompoundcolorwidget.ui +++ b/src/ui/qgscompoundcolorwidget.ui @@ -448,7 +448,7 @@ - 2 + 0 @@ -701,18 +701,7 @@ - - - - RGB - - - - - CMYK - - - + @@ -894,7 +883,7 @@ - + @@ -915,7 +904,7 @@ - + @@ -936,7 +925,7 @@ - + @@ -957,7 +946,7 @@ - + @@ -1091,17 +1080,17 @@ + + QgsSpinBox + QSpinBox +
qgsspinbox.h
+ QgsColorButton QToolButton
qgscolorbutton.h
1 - - QgsSpinBox - QSpinBox -
qgsspinbox.h
- QgsColorBox QWidget diff --git a/tests/src/gui/testqgscompoundcolorwidget.cpp b/tests/src/gui/testqgscompoundcolorwidget.cpp index 2c7cdca1ad38..0361835ca394 100644 --- a/tests/src/gui/testqgscompoundcolorwidget.cpp +++ b/tests/src/gui/testqgscompoundcolorwidget.cpp @@ -16,6 +16,7 @@ #include "qgstest.h" #include "qgscompoundcolorwidget.h" +#include "qgssettings.h" class TestQgsCompoundColorWidget : public QgsTest { @@ -31,6 +32,7 @@ class TestQgsCompoundColorWidget : public QgsTest void init();// will be called before each testfunction is executed. void cleanup();// will be called after every testfunction. void testCmykConversion(); + void testComponentChange(); }; void TestQgsCompoundColorWidget::initTestCase() @@ -88,5 +90,38 @@ void TestQgsCompoundColorWidget::testCmykConversion() QCOMPARE( w.color(), QColor::fromCmyk( 170, 85, 0, 225, 50 ) ); } +void TestQgsCompoundColorWidget::testComponentChange() +{ + QgsSettings().setValue( QStringLiteral( "Windows/ColorDialog/activeComponent" ), 3 ); + + QgsCompoundColorWidget w( nullptr, QColor( 10, 20, 30, 50 ) ); + w.setVisible( true ); + + QCOMPARE( w.mColorBox->component(), QgsColorWidget::Red ); + QCOMPARE( w.mVerticalRamp->component(), QgsColorWidget::Red ); + + const QList> colors = + { + { w.mHueRadio, QgsColorWidget::Hue }, + { w.mSaturationRadio, QgsColorWidget::Saturation }, + { w.mValueRadio, QgsColorWidget::Value }, + { w.mRedRadio, QgsColorWidget::Red }, + { w.mGreenRadio, QgsColorWidget::Green }, + { w.mBlueRadio, QgsColorWidget::Blue }, + { w.mCyanRadio, QgsColorWidget::Cyan }, + { w.mMagentaRadio, QgsColorWidget::Magenta }, + { w.mYellowRadio, QgsColorWidget::Yellow }, + { w.mBlackRadio, QgsColorWidget::Black } + }; + + for ( QPair color : colors ) + { + color.first->setChecked( true ); + QCOMPARE( w.mColorBox->component(), color.second ); + QCOMPARE( w.mVerticalRamp->component(), color.second ); + } + +} + QGSTEST_MAIN( TestQgsCompoundColorWidget ) #include "testqgscompoundcolorwidget.moc" From a8023948bab83e54f540d4aaa76bdfce285cac34 Mon Sep 17 00:00:00 2001 From: Julien Cabieces Date: Wed, 22 May 2024 15:27:18 +0200 Subject: [PATCH 03/12] use static and explicit color model --- src/gui/qgscompoundcolorwidget.cpp | 10 +++++++--- tests/src/gui/testqgscompoundcolorwidget.cpp | 16 ++++++++++++++++ 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/src/gui/qgscompoundcolorwidget.cpp b/src/gui/qgscompoundcolorwidget.cpp index 8bb871126f86..8fd948dd7b85 100644 --- a/src/gui/qgscompoundcolorwidget.cpp +++ b/src/gui/qgscompoundcolorwidget.cpp @@ -57,9 +57,12 @@ QgsCompoundColorWidget::QgsCompoundColorWidget( QWidget *parent, const QColor &c connect( mTabWidget, &QTabWidget::currentChanged, this, &QgsCompoundColorWidget::mTabWidget_currentChanged ); connect( mActionShowInButtons, &QAction::toggled, this, &QgsCompoundColorWidget::mActionShowInButtons_toggled ); - connect( mColorModel, static_cast( &QComboBox::currentIndexChanged ), this, [this]( int index ) + mColorModel->addItem( tr( "RGB" ), QColor::Spec::Rgb ); + mColorModel->addItem( tr( "CMYK" ), QColor::Spec::Cmyk ); + connect( mColorModel, static_cast( &QComboBox::currentIndexChanged ), this, [this]( int ) { - if ( index ) + const QColor::Spec spec = static_cast< QColor::Spec >( mColorModel->currentData().toInt() ); + if ( spec == QColor::Spec::Cmyk ) setColor( this->color().toCmyk() ); else setColor( this->color().toRgb() ); @@ -728,7 +731,8 @@ void QgsCompoundColorWidget::stopPicking( QPoint eventPos, const bool takeSample void QgsCompoundColorWidget::setColor( const QColor &color ) { - mColorModel->setCurrentIndex( color.spec() == QColor::Cmyk ? 1 : 0 ); + const QColor::Spec colorSpec = color.spec() == QColor::Cmyk ? QColor::Cmyk : QColor::Rgb; + mColorModel->setCurrentIndex( mColorModel->findData( colorSpec ) ); mRGB->setVisible( color.spec() != QColor::Cmyk ); mCMYK->setVisible( color.spec() == QColor::Cmyk ); _setColor( color ); diff --git a/tests/src/gui/testqgscompoundcolorwidget.cpp b/tests/src/gui/testqgscompoundcolorwidget.cpp index 0361835ca394..10f6c52e9175 100644 --- a/tests/src/gui/testqgscompoundcolorwidget.cpp +++ b/tests/src/gui/testqgscompoundcolorwidget.cpp @@ -33,6 +33,7 @@ class TestQgsCompoundColorWidget : public QgsTest void cleanup();// will be called after every testfunction. void testCmykConversion(); void testComponentChange(); + void testModelChange(); }; void TestQgsCompoundColorWidget::initTestCase() @@ -123,5 +124,20 @@ void TestQgsCompoundColorWidget::testComponentChange() } +void TestQgsCompoundColorWidget::testModelChange() +{ + QgsCompoundColorWidget w( nullptr, QColor( 10, 20, 30, 50 ) ); + w.setVisible( true ); + + QCOMPARE( w.mColorModel->currentData(), QColor::Rgb ); + + w.mColorModel->setCurrentIndex( w.mColorModel->findData( QColor::Cmyk ) ); + QCOMPARE( w.mColorModel->currentData(), QColor::Cmyk ); + + w.setColor( QColor( 1, 2, 3 ) ); + QCOMPARE( w.mColorModel->currentData(), QColor::Rgb ); +} + + QGSTEST_MAIN( TestQgsCompoundColorWidget ) #include "testqgscompoundcolorwidget.moc" From 3bcfa9568bba8fe2789a29259c302b76d9ef8000 Mon Sep 17 00:00:00 2001 From: Julien Cabieces Date: Wed, 22 May 2024 15:27:49 +0200 Subject: [PATCH 04/12] replace isCmyk with colorSpec --- .../gui/auto_generated/qgscolorwidgets.sip.in | 8 ++--- .../gui/auto_generated/qgscolorwidgets.sip.in | 8 ++--- src/gui/qgscolorwidgets.cpp | 29 ++++++++++++++----- src/gui/qgscolorwidgets.h | 8 ++--- 4 files changed, 33 insertions(+), 20 deletions(-) diff --git a/python/PyQt6/gui/auto_generated/qgscolorwidgets.sip.in b/python/PyQt6/gui/auto_generated/qgscolorwidgets.sip.in index 8072da189a42..1e6d75c86a9b 100644 --- a/python/PyQt6/gui/auto_generated/qgscolorwidgets.sip.in +++ b/python/PyQt6/gui/auto_generated/qgscolorwidgets.sip.in @@ -186,14 +186,14 @@ Alters a color by modifying the value of a specific color component valid range for the color component. %End - bool isCmyk() const; + QColor::Spec colorSpec() const; %Docstring -Returns true if the color widget component is either Cyan, Magenta, Yellow or Black +Returns color widget type of color, either RGB, HSV, CMYK, or Invalid if this component value is Multiple or Alpha %End - static bool isCmyk( QgsColorWidget::ColorComponent component ); + static QColor::Spec colorSpec( QgsColorWidget::ColorComponent component ); %Docstring -Returns true if ``component`` is either Cyan, Magenta, Yellow or Black +Returns ``component`` type of color, either RGB, HSV, CMYK, or Invalid if ``component`` value is Multiple or Alpha %End static const QPixmap &transparentBackground(); diff --git a/python/gui/auto_generated/qgscolorwidgets.sip.in b/python/gui/auto_generated/qgscolorwidgets.sip.in index fd73e363e6d3..fae83ac032b8 100644 --- a/python/gui/auto_generated/qgscolorwidgets.sip.in +++ b/python/gui/auto_generated/qgscolorwidgets.sip.in @@ -186,14 +186,14 @@ Alters a color by modifying the value of a specific color component valid range for the color component. %End - bool isCmyk() const; + QColor::Spec colorSpec() const; %Docstring -Returns true if the color widget component is either Cyan, Magenta, Yellow or Black +Returns color widget type of color, either RGB, HSV, CMYK, or Invalid if this component value is Multiple or Alpha %End - static bool isCmyk( QgsColorWidget::ColorComponent component ); + static QColor::Spec colorSpec( QgsColorWidget::ColorComponent component ); %Docstring -Returns true if ``component`` is either Cyan, Magenta, Yellow or Black +Returns ``component`` type of color, either RGB, HSV, CMYK, or Invalid if ``component`` value is Multiple or Alpha %End static const QPixmap &transparentBackground(); diff --git a/src/gui/qgscolorwidgets.cpp b/src/gui/qgscolorwidgets.cpp index 543f7197f332..7195598452a5 100644 --- a/src/gui/qgscolorwidgets.cpp +++ b/src/gui/qgscolorwidgets.cpp @@ -157,7 +157,7 @@ void QgsColorWidget::alterColor( QColor &color, const QgsColorWidget::ColorCompo //clip value to sensible range const int clippedValue = std::min( std::max( 0, newValue ), componentRange( component ) ); - if ( isCmyk( component ) ) + if ( colorSpec( component ) == QColor::Spec::Cmyk ) { int c, m, y, k, a; color.getCmyk( &c, &m, &y, &k, &a ); @@ -260,23 +260,34 @@ void QgsColorWidget::alterColor( QColor &color, const QgsColorWidget::ColorCompo } } -bool QgsColorWidget::isCmyk( QgsColorWidget::ColorComponent component ) +QColor::Spec QgsColorWidget::colorSpec( QgsColorWidget::ColorComponent component ) { switch ( component ) { + case Red: + case Green: + case Blue: + return QColor::Spec::Rgb; + + case Hue: + case Saturation: + case Value: + return QColor::Spec::Hsv; + case Cyan: case Magenta: case Yellow: case Black: - return true; + return QColor::Spec::Cmyk; + default: - return false; + return QColor::Spec::Invalid; } } -bool QgsColorWidget::isCmyk() const +QColor::Spec QgsColorWidget::colorSpec() const { - return isCmyk( mComponent ); + return colorSpec( mComponent ); } const QPixmap &QgsColorWidget::transparentBackground() @@ -378,8 +389,10 @@ void QgsColorWidget::setComponentValue( const int value ) mCurrentColor.setHsv( h, s, v, a ); } - - alterColor( mCurrentColor, mComponent, value ); + else + { + alterColor( mCurrentColor, mComponent, value ); + } //update recorded hue if ( mCurrentColor.hue() >= 0 ) diff --git a/src/gui/qgscolorwidgets.h b/src/gui/qgscolorwidgets.h index 02ef4775c2d0..7bfb579ddf88 100644 --- a/src/gui/qgscolorwidgets.h +++ b/src/gui/qgscolorwidgets.h @@ -187,14 +187,14 @@ class GUI_EXPORT QgsColorWidget : public QWidget static void alterColor( QColor &color, QgsColorWidget::ColorComponent component, int newValue ); /** - * Returns true if the color widget component is either Cyan, Magenta, Yellow or Black + * Returns color widget type of color, either RGB, HSV, CMYK, or Invalid if this component value is Multiple or Alpha */ - bool isCmyk() const; + QColor::Spec colorSpec() const; /** - * Returns true if \a component is either Cyan, Magenta, Yellow or Black + * Returns \a component type of color, either RGB, HSV, CMYK, or Invalid if \a component value is Multiple or Alpha */ - static bool isCmyk( QgsColorWidget::ColorComponent component ); + static QColor::Spec colorSpec( QgsColorWidget::ColorComponent component ); /** * Generates a checkboard pattern pixmap for use as a background to transparent colors From a6dd92abd81667ab2227bb301324d9f7997caf50 Mon Sep 17 00:00:00 2001 From: Julien Cabieces Date: Wed, 22 May 2024 19:03:19 +0200 Subject: [PATCH 05/12] Update and test cmyk radio buttons --- .../gui/auto_generated/qgscolorwidgets.sip.in | 1 + .../gui/auto_generated/qgscolorwidgets.sip.in | 1 + src/gui/qgscolorwidgets.h | 2 + src/gui/qgscompoundcolorwidget.cpp | 170 ++++++------------ src/gui/qgscompoundcolorwidget.h | 15 +- src/ui/qgscompoundcolorwidget.ui | 30 ++++ tests/src/gui/testqgscompoundcolorwidget.cpp | 69 ++++++- 7 files changed, 160 insertions(+), 128 deletions(-) diff --git a/python/PyQt6/gui/auto_generated/qgscolorwidgets.sip.in b/python/PyQt6/gui/auto_generated/qgscolorwidgets.sip.in index 1e6d75c86a9b..cd1786f04d48 100644 --- a/python/PyQt6/gui/auto_generated/qgscolorwidgets.sip.in +++ b/python/PyQt6/gui/auto_generated/qgscolorwidgets.sip.in @@ -215,6 +215,7 @@ Generates a checkboard pattern pixmap for use as a background to transparent col virtual void mouseReleaseEvent( QMouseEvent *e ); + }; diff --git a/python/gui/auto_generated/qgscolorwidgets.sip.in b/python/gui/auto_generated/qgscolorwidgets.sip.in index fae83ac032b8..3dea5fec41ed 100644 --- a/python/gui/auto_generated/qgscolorwidgets.sip.in +++ b/python/gui/auto_generated/qgscolorwidgets.sip.in @@ -215,6 +215,7 @@ Generates a checkboard pattern pixmap for use as a background to transparent col virtual void mouseReleaseEvent( QMouseEvent *e ); + }; diff --git a/src/gui/qgscolorwidgets.h b/src/gui/qgscolorwidgets.h index 7bfb579ddf88..7ad88f2a9f20 100644 --- a/src/gui/qgscolorwidgets.h +++ b/src/gui/qgscolorwidgets.h @@ -211,6 +211,8 @@ class GUI_EXPORT QgsColorWidget : public QWidget void mouseMoveEvent( QMouseEvent *e ) override; void mousePressEvent( QMouseEvent *e ) override; void mouseReleaseEvent( QMouseEvent *e ) override; + + friend class TestQgsCompoundColorWidget; }; diff --git a/src/gui/qgscompoundcolorwidget.cpp b/src/gui/qgscompoundcolorwidget.cpp index 8fd948dd7b85..806d083bac57 100644 --- a/src/gui/qgscompoundcolorwidget.cpp +++ b/src/gui/qgscompoundcolorwidget.cpp @@ -22,6 +22,7 @@ #include "qgsscreenhelper.h" #include "qgsguiutils.h" +#include #include #include #include @@ -41,16 +42,36 @@ QgsCompoundColorWidget::QgsCompoundColorWidget( QWidget *parent, const QColor &c mScreenHelper = new QgsScreenHelper( this ); - connect( mHueRadio, &QRadioButton::toggled, this, &QgsCompoundColorWidget::mHueRadio_toggled ); - connect( mSaturationRadio, &QRadioButton::toggled, this, &QgsCompoundColorWidget::mSaturationRadio_toggled ); - connect( mValueRadio, &QRadioButton::toggled, this, &QgsCompoundColorWidget::mValueRadio_toggled ); - connect( mRedRadio, &QRadioButton::toggled, this, &QgsCompoundColorWidget::mRedRadio_toggled ); - connect( mGreenRadio, &QRadioButton::toggled, this, &QgsCompoundColorWidget::mGreenRadio_toggled ); - connect( mBlueRadio, &QRadioButton::toggled, this, &QgsCompoundColorWidget::mBlueRadio_toggled ); - connect( mCyanRadio, &QRadioButton::toggled, this, &QgsCompoundColorWidget::mCyanRadio_toggled ); - connect( mMagentaRadio, &QRadioButton::toggled, this, &QgsCompoundColorWidget::mMagentaRadio_toggled ); - connect( mYellowRadio, &QRadioButton::toggled, this, &QgsCompoundColorWidget::mYellowRadio_toggled ); - connect( mBlackRadio, &QRadioButton::toggled, this, &QgsCompoundColorWidget::mBlackRadio_toggled ); + mRgbRadios = + { + { mHueRadio, QgsColorWidget::ColorComponent::Hue }, + { mSaturationRadio, QgsColorWidget::ColorComponent::Saturation }, + { mValueRadio, QgsColorWidget::ColorComponent::Value }, + { mRedRadio, QgsColorWidget::ColorComponent::Red }, + { mGreenRadio, QgsColorWidget::ColorComponent::Green }, + { mBlueRadio, QgsColorWidget::ColorComponent::Blue } + }; + + mCmykRadios = + { + { mCyanRadio, QgsColorWidget::ColorComponent::Cyan }, + { mMagentaRadio, QgsColorWidget::ColorComponent::Magenta }, + { mYellowRadio, QgsColorWidget::ColorComponent::Yellow }, + { mBlackRadio, QgsColorWidget::ColorComponent::Black } + }; + + QButtonGroup *rgbGroup = new QButtonGroup( this ); + int i = 0; + for ( auto colorRadio : mRgbRadios ) + rgbGroup->addButton( colorRadio.first, i++ ); + + QButtonGroup *cmykGroup = new QButtonGroup( this ); + i = 0; + for ( auto colorRadio : mCmykRadios ) + cmykGroup->addButton( colorRadio.first, i++ ); + + connect( rgbGroup, &QButtonGroup::idToggled, this, &QgsCompoundColorWidget::onRgbButtonGroupToggled ); + connect( cmykGroup, &QButtonGroup::idToggled, this, &QgsCompoundColorWidget::onCmykButtonGroupToggled ); connect( mAddColorToSchemeButton, &QPushButton::clicked, this, &QgsCompoundColorWidget::mAddColorToSchemeButton_clicked ); connect( mAddCustomColorButton, &QPushButton::clicked, this, &QgsCompoundColorWidget::mAddCustomColorButton_clicked ); connect( mSampleButton, &QPushButton::clicked, this, &QgsCompoundColorWidget::mSampleButton_clicked ); @@ -251,29 +272,15 @@ QgsCompoundColorWidget::QgsCompoundColorWidget( QWidget *parent, const QColor &c setColor( color ); } - //restore active component radio button - const int activeRadio = settings.value( QStringLiteral( "Windows/ColorDialog/activeComponent" ), 2 ).toInt(); - switch ( activeRadio ) - { - case 0: - mHueRadio->setChecked( true ); - break; - case 1: - mSaturationRadio->setChecked( true ); - break; - case 2: - mValueRadio->setChecked( true ); - break; - case 3: - mRedRadio->setChecked( true ); - break; - case 4: - mGreenRadio->setChecked( true ); - break; - case 5: - mBlueRadio->setChecked( true ); - break; - } + // restore active Rgb/Cmyk component radio button + const int activeRgbRadio = settings.value( QStringLiteral( "Windows/ColorDialog/activeComponent" ), 2 ).toInt(); + if ( QAbstractButton *rgbRadio = rgbGroup->button( activeRgbRadio ) ) + rgbRadio->setChecked( true ); + + const int activeCmykRadio = settings.value( QStringLiteral( "Windows/ColorDialog/activeCmykComponent" ), 0 ).toInt(); + if ( QAbstractButton *cmykRadio = cmykGroup->button( activeCmykRadio ) ) + cmykRadio->setChecked( true ); + const int currentTab = settings.value( QStringLiteral( "Windows/ColorDialog/activeTab" ), 0 ).toInt(); mTabWidget->setCurrentIndex( currentTab ); @@ -624,12 +631,9 @@ void QgsCompoundColorWidget::mTabWidget_currentChanged( int index ) { //disable radio buttons if not using the first tab, as they have no meaning for other tabs const bool enabled = index == 0; - mRedRadio->setEnabled( enabled ); - mBlueRadio->setEnabled( enabled ); - mGreenRadio->setEnabled( enabled ); - mHueRadio->setEnabled( enabled ); - mSaturationRadio->setEnabled( enabled ); - mValueRadio->setEnabled( enabled ); + const QList colorRadios{ mHueRadio, mSaturationRadio, mValueRadio, mRedRadio, mGreenRadio, mBlueRadio, mCyanRadio, mMagentaRadio, mYellowRadio, mBlackRadio }; + for ( QRadioButton *colorRadio : colorRadios ) + colorRadio->setEnabled( enabled ); } void QgsCompoundColorWidget::mActionShowInButtons_toggled( bool state ) @@ -890,95 +894,27 @@ void QgsCompoundColorWidget::keyPressEvent( QKeyEvent *e ) stopPicking( QCursor::pos(), e->key() == Qt::Key_Space ); } -void QgsCompoundColorWidget::mHueRadio_toggled( bool checked ) -{ - if ( checked ) - { - mColorBox->setComponent( QgsColorWidget::Hue ); - mVerticalRamp->setComponent( QgsColorWidget::Hue ); - } -} - -void QgsCompoundColorWidget::mSaturationRadio_toggled( bool checked ) -{ - if ( checked ) - { - mColorBox->setComponent( QgsColorWidget::Saturation ); - mVerticalRamp->setComponent( QgsColorWidget::Saturation ); - } -} - -void QgsCompoundColorWidget::mValueRadio_toggled( bool checked ) -{ - if ( checked ) - { - mColorBox->setComponent( QgsColorWidget::Value ); - mVerticalRamp->setComponent( QgsColorWidget::Value ); - } -} - -void QgsCompoundColorWidget::mRedRadio_toggled( bool checked ) -{ - if ( checked ) - { - mColorBox->setComponent( QgsColorWidget::Red ); - mVerticalRamp->setComponent( QgsColorWidget::Red ); - } -} - -void QgsCompoundColorWidget::mGreenRadio_toggled( bool checked ) -{ - if ( checked ) - { - mColorBox->setComponent( QgsColorWidget::Green ); - mVerticalRamp->setComponent( QgsColorWidget::Green ); - } -} - -void QgsCompoundColorWidget::mBlueRadio_toggled( bool checked ) -{ - if ( checked ) - { - mColorBox->setComponent( QgsColorWidget::Blue ); - mVerticalRamp->setComponent( QgsColorWidget::Blue ); - } -} -void QgsCompoundColorWidget::mCyanRadio_toggled( bool checked ) +void QgsCompoundColorWidget::onRgbButtonGroupToggled( int id, bool checked ) { - if ( checked ) + if ( checked && id >= 0 && id < mRgbRadios.count() && static_cast( mColorModel->currentData().toInt() ) == QColor::Rgb ) { - mColorBox->setComponent( QgsColorWidget::Cyan ); - mVerticalRamp->setComponent( QgsColorWidget::Cyan ); + const QgsColorWidget::ColorComponent component = mRgbRadios.at( id ).second; + mColorBox->setComponent( component ); + mVerticalRamp->setComponent( component ); } } -void QgsCompoundColorWidget::mMagentaRadio_toggled( bool checked ) +void QgsCompoundColorWidget::onCmykButtonGroupToggled( int id, bool checked ) { - if ( checked ) + if ( checked && id >= 0 && id < mCmykRadios.count() && static_cast( mColorModel->currentData().toInt() ) == QColor::Cmyk ) { - mColorBox->setComponent( QgsColorWidget::Magenta ); - mVerticalRamp->setComponent( QgsColorWidget::Magenta ); + const QgsColorWidget::ColorComponent component = mCmykRadios.at( id ).second; + mColorBox->setComponent( component ); + mVerticalRamp->setComponent( component ); } } -void QgsCompoundColorWidget::mYellowRadio_toggled( bool checked ) -{ - if ( checked ) - { - mColorBox->setComponent( QgsColorWidget::Yellow ); - mVerticalRamp->setComponent( QgsColorWidget::Yellow ); - } -} - -void QgsCompoundColorWidget::mBlackRadio_toggled( bool checked ) -{ - if ( checked ) - { - mColorBox->setComponent( QgsColorWidget::Black ); - mVerticalRamp->setComponent( QgsColorWidget::Black ); - } -} void QgsCompoundColorWidget::mAddColorToSchemeButton_clicked() { diff --git a/src/gui/qgscompoundcolorwidget.h b/src/gui/qgscompoundcolorwidget.h index c9e71afa4e3f..ea69b587f384 100644 --- a/src/gui/qgscompoundcolorwidget.h +++ b/src/gui/qgscompoundcolorwidget.h @@ -146,16 +146,8 @@ class GUI_EXPORT QgsCompoundColorWidget : public QgsPanelWidget, private Ui::Qgs private slots: - void mHueRadio_toggled( bool checked ); - void mSaturationRadio_toggled( bool checked ); - void mValueRadio_toggled( bool checked ); - void mRedRadio_toggled( bool checked ); - void mGreenRadio_toggled( bool checked ); - void mBlueRadio_toggled( bool checked ); - void mCyanRadio_toggled( bool checked ); - void mMagentaRadio_toggled( bool checked ); - void mYellowRadio_toggled( bool checked ); - void mBlackRadio_toggled( bool checked ); + void onRgbButtonGroupToggled( int id, bool checked ); + void onCmykButtonGroupToggled( int id, bool checked ); void mAddColorToSchemeButton_clicked(); @@ -193,6 +185,9 @@ class GUI_EXPORT QgsCompoundColorWidget : public QgsPanelWidget, private Ui::Qgs bool mDiscarded = false; + QList> mRgbRadios; + QList> mCmykRadios; + /** * Saves all widget settings */ diff --git a/src/ui/qgscompoundcolorwidget.ui b/src/ui/qgscompoundcolorwidget.ui index 6771d99a8d6c..b9b58abff757 100644 --- a/src/ui/qgscompoundcolorwidget.ui +++ b/src/ui/qgscompoundcolorwidget.ui @@ -740,6 +740,9 @@ + + false + @@ -761,6 +764,9 @@ + + false + @@ -782,6 +788,9 @@ + + false + @@ -803,6 +812,9 @@ + + false + @@ -824,6 +836,9 @@ + + false + @@ -845,6 +860,9 @@ + + false + @@ -887,6 +905,9 @@ + + false + @@ -908,6 +929,9 @@ + + false + @@ -929,6 +953,9 @@ + + false + @@ -950,6 +977,9 @@ + + false + diff --git a/tests/src/gui/testqgscompoundcolorwidget.cpp b/tests/src/gui/testqgscompoundcolorwidget.cpp index 10f6c52e9175..584f1e81fb76 100644 --- a/tests/src/gui/testqgscompoundcolorwidget.cpp +++ b/tests/src/gui/testqgscompoundcolorwidget.cpp @@ -33,7 +33,10 @@ class TestQgsCompoundColorWidget : public QgsTest void cleanup();// will be called after every testfunction. void testCmykConversion(); void testComponentChange(); + void testComponentSettings_data(); + void testComponentSettings(); void testModelChange(); + void testTabChange(); }; void TestQgsCompoundColorWidget::initTestCase() @@ -91,6 +94,39 @@ void TestQgsCompoundColorWidget::testCmykConversion() QCOMPARE( w.color(), QColor::fromCmyk( 170, 85, 0, 225, 50 ) ); } +void TestQgsCompoundColorWidget::testComponentSettings_data() +{ + QTest::addColumn( "settingsComponent" ); + QTest::addColumn( "expectedComponent" ); + + QTest::newRow( "hue" ) << 0 << QgsColorWidget::ColorComponent::Hue; + QTest::newRow( "saturation" ) << 1 << QgsColorWidget::ColorComponent::Saturation; + QTest::newRow( "value" ) << 2 << QgsColorWidget::ColorComponent::Value; + QTest::newRow( "red" ) << 3 << QgsColorWidget::ColorComponent::Red; + QTest::newRow( "green" ) << 4 << QgsColorWidget::ColorComponent::Green; + QTest::newRow( "blue" ) << 5 << QgsColorWidget::ColorComponent::Blue; + QTest::newRow( "cyan" ) << 0 << QgsColorWidget::ColorComponent::Cyan; + QTest::newRow( "magenta" ) << 1 << QgsColorWidget::ColorComponent::Magenta; + QTest::newRow( "yellow" ) << 2 << QgsColorWidget::ColorComponent::Yellow; + QTest::newRow( "black" ) << 3 << QgsColorWidget::ColorComponent::Black; +} + +void TestQgsCompoundColorWidget::testComponentSettings() +{ + QFETCH( int, settingsComponent ); + QFETCH( QgsColorWidget::ColorComponent, expectedComponent ); + + QgsSettings().setValue( QgsColorWidget::colorSpec( expectedComponent ) == QColor::Cmyk ? + QStringLiteral( "Windows/ColorDialog/activeCmykComponent" ) : QStringLiteral( "Windows/ColorDialog/activeComponent" ), settingsComponent ); + + QgsCompoundColorWidget w( nullptr, QgsColorWidget::colorSpec( expectedComponent ) == QColor::Cmyk ? + QColor::fromCmyk( 1, 2, 3, 4 ) : QColor( 10, 20, 30, 50 ) ); + w.setVisible( true ); + + QCOMPARE( w.mColorBox->component(), expectedComponent ); + QCOMPARE( w.mVerticalRamp->component(), expectedComponent ); +} + void TestQgsCompoundColorWidget::testComponentChange() { QgsSettings().setValue( QStringLiteral( "Windows/ColorDialog/activeComponent" ), 3 ); @@ -117,11 +153,13 @@ void TestQgsCompoundColorWidget::testComponentChange() for ( QPair color : colors ) { + if ( QgsColorWidget::colorSpec( color.second ) != w.mColorModel->currentData() ) + w.mColorModel->setCurrentIndex( w.mColorModel->findData( QgsColorWidget::colorSpec( color.second ) ) ); + color.first->setChecked( true ); QCOMPARE( w.mColorBox->component(), color.second ); QCOMPARE( w.mVerticalRamp->component(), color.second ); } - } void TestQgsCompoundColorWidget::testModelChange() @@ -138,6 +176,35 @@ void TestQgsCompoundColorWidget::testModelChange() QCOMPARE( w.mColorModel->currentData(), QColor::Rgb ); } +void TestQgsCompoundColorWidget::testTabChange() +{ + QgsCompoundColorWidget w( nullptr, QColor( 10, 20, 30, 50 ) ); + w.setVisible( true ); + + QCOMPARE( w.mTabWidget->currentIndex(), 0 ); + QVERIFY( w.mRedRadio->isEnabled() ); + QVERIFY( w.mBlueRadio->isEnabled() ); + QVERIFY( w.mGreenRadio->isEnabled() ); + QVERIFY( w.mHueRadio->isEnabled() ); + QVERIFY( w.mSaturationRadio->isEnabled() ); + QVERIFY( w.mCyanRadio->isEnabled() ); + QVERIFY( w.mMagentaRadio->isEnabled() ); + QVERIFY( w.mYellowRadio->isEnabled() ); + QVERIFY( w.mBlackRadio->isEnabled() ); + + w.mTabWidget->setCurrentIndex( 1 ); + QVERIFY( !w.mRedRadio->isEnabled() ); + QVERIFY( !w.mBlueRadio->isEnabled() ); + QVERIFY( !w.mGreenRadio->isEnabled() ); + QVERIFY( !w.mHueRadio->isEnabled() ); + QVERIFY( !w.mSaturationRadio->isEnabled() ); + QVERIFY( !w.mCyanRadio->isEnabled() ); + QVERIFY( !w.mMagentaRadio->isEnabled() ); + QVERIFY( !w.mYellowRadio->isEnabled() ); + QVERIFY( !w.mBlackRadio->isEnabled() ); +} + + QGSTEST_MAIN( TestQgsCompoundColorWidget ) #include "testqgscompoundcolorwidget.moc" From 65e22eded0f1a85b7b68d729e0d54b4ca752a509 Mon Sep 17 00:00:00 2001 From: Julien Cabieces Date: Wed, 22 May 2024 19:12:35 +0200 Subject: [PATCH 06/12] add an helper method to avoid duplicate code --- src/gui/qgscompoundcolorwidget.cpp | 19 ++++++++++--------- src/gui/qgscompoundcolorwidget.h | 10 ++++++++++ 2 files changed, 20 insertions(+), 9 deletions(-) diff --git a/src/gui/qgscompoundcolorwidget.cpp b/src/gui/qgscompoundcolorwidget.cpp index 806d083bac57..d225c42df344 100644 --- a/src/gui/qgscompoundcolorwidget.cpp +++ b/src/gui/qgscompoundcolorwidget.cpp @@ -895,24 +895,25 @@ void QgsCompoundColorWidget::keyPressEvent( QKeyEvent *e ) } -void QgsCompoundColorWidget::onRgbButtonGroupToggled( int id, bool checked ) +void QgsCompoundColorWidget::onColorButtonGroupToggled( const QList> &colorRadios, + const QColor::Spec colorSpec, const int id, const bool checked ) { - if ( checked && id >= 0 && id < mRgbRadios.count() && static_cast( mColorModel->currentData().toInt() ) == QColor::Rgb ) + if ( checked && id >= 0 && id < colorRadios.count() && static_cast( mColorModel->currentData().toInt() ) == colorSpec ) { - const QgsColorWidget::ColorComponent component = mRgbRadios.at( id ).second; + const QgsColorWidget::ColorComponent component = colorRadios.at( id ).second; mColorBox->setComponent( component ); mVerticalRamp->setComponent( component ); } } +void QgsCompoundColorWidget::onRgbButtonGroupToggled( int id, bool checked ) +{ + onColorButtonGroupToggled( mRgbRadios, QColor::Rgb, id, checked ); +} + void QgsCompoundColorWidget::onCmykButtonGroupToggled( int id, bool checked ) { - if ( checked && id >= 0 && id < mCmykRadios.count() && static_cast( mColorModel->currentData().toInt() ) == QColor::Cmyk ) - { - const QgsColorWidget::ColorComponent component = mCmykRadios.at( id ).second; - mColorBox->setComponent( component ); - mVerticalRamp->setComponent( component ); - } + onColorButtonGroupToggled( mCmykRadios, QColor::Cmyk, id, checked ); } diff --git a/src/gui/qgscompoundcolorwidget.h b/src/gui/qgscompoundcolorwidget.h index ea69b587f384..9ae19c2cc5e5 100644 --- a/src/gui/qgscompoundcolorwidget.h +++ b/src/gui/qgscompoundcolorwidget.h @@ -228,6 +228,16 @@ class GUI_EXPORT QgsCompoundColorWidget : public QgsPanelWidget, private Ui::Qgs //! Updates the state of actions for the current selected scheme void updateActionsForCurrentScheme(); + /** + * Helper method to implement slots called when color radio button has been toggled + * \param colorRadios related to the toggled button + * \param colorSpec color type of the toggled button + * \param id of the toggled button + * \param checked TRUE is the button is checked + */ + void onColorButtonGroupToggled( const QList> &colorRadios, + const QColor::Spec colorSpec, const int id, const bool checked ); + friend class TestQgsCompoundColorWidget; }; From d33c1c0a75a85454228d10b397ffdf710975a78b Mon Sep 17 00:00:00 2001 From: Julien Cabieces Date: Wed, 22 May 2024 19:16:31 +0200 Subject: [PATCH 07/12] Rename python test --- tests/src/python/test_qgscolorwidget.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/src/python/test_qgscolorwidget.py b/tests/src/python/test_qgscolorwidget.py index 6b57f033543d..948797d8e935 100644 --- a/tests/src/python/test_qgscolorwidget.py +++ b/tests/src/python/test_qgscolorwidget.py @@ -18,7 +18,7 @@ start_app() -class TestQgsWidget(QgisTestCase): +class TestQgsColorWidget(QgisTestCase): def testAlterColor(self): """ From bb97a02b0d743887465b30737082cf36576ba71f Mon Sep 17 00:00:00 2001 From: Julien Cabieces Date: Wed, 22 May 2024 19:37:55 +0200 Subject: [PATCH 08/12] fix color component setting save --- src/gui/qgscompoundcolorwidget.cpp | 34 ++++++------------- src/gui/qgscompoundcolorwidget.h | 2 ++ tests/src/gui/testqgscompoundcolorwidget.cpp | 35 ++++++++++++++------ 3 files changed, 37 insertions(+), 34 deletions(-) diff --git a/src/gui/qgscompoundcolorwidget.cpp b/src/gui/qgscompoundcolorwidget.cpp index d225c42df344..baf74d196fe1 100644 --- a/src/gui/qgscompoundcolorwidget.cpp +++ b/src/gui/qgscompoundcolorwidget.cpp @@ -60,18 +60,18 @@ QgsCompoundColorWidget::QgsCompoundColorWidget( QWidget *parent, const QColor &c { mBlackRadio, QgsColorWidget::ColorComponent::Black } }; - QButtonGroup *rgbGroup = new QButtonGroup( this ); + mRgbGroup = new QButtonGroup( this ); int i = 0; for ( auto colorRadio : mRgbRadios ) - rgbGroup->addButton( colorRadio.first, i++ ); + mRgbGroup->addButton( colorRadio.first, i++ ); - QButtonGroup *cmykGroup = new QButtonGroup( this ); + mCmykGroup = new QButtonGroup( this ); i = 0; for ( auto colorRadio : mCmykRadios ) - cmykGroup->addButton( colorRadio.first, i++ ); + mCmykGroup->addButton( colorRadio.first, i++ ); - connect( rgbGroup, &QButtonGroup::idToggled, this, &QgsCompoundColorWidget::onRgbButtonGroupToggled ); - connect( cmykGroup, &QButtonGroup::idToggled, this, &QgsCompoundColorWidget::onCmykButtonGroupToggled ); + connect( mRgbGroup, &QButtonGroup::idToggled, this, &QgsCompoundColorWidget::onRgbButtonGroupToggled ); + connect( mCmykGroup, &QButtonGroup::idToggled, this, &QgsCompoundColorWidget::onCmykButtonGroupToggled ); connect( mAddColorToSchemeButton, &QPushButton::clicked, this, &QgsCompoundColorWidget::mAddColorToSchemeButton_clicked ); connect( mAddCustomColorButton, &QPushButton::clicked, this, &QgsCompoundColorWidget::mAddCustomColorButton_clicked ); connect( mSampleButton, &QPushButton::clicked, this, &QgsCompoundColorWidget::mSampleButton_clicked ); @@ -274,11 +274,11 @@ QgsCompoundColorWidget::QgsCompoundColorWidget( QWidget *parent, const QColor &c // restore active Rgb/Cmyk component radio button const int activeRgbRadio = settings.value( QStringLiteral( "Windows/ColorDialog/activeComponent" ), 2 ).toInt(); - if ( QAbstractButton *rgbRadio = rgbGroup->button( activeRgbRadio ) ) + if ( QAbstractButton *rgbRadio = mRgbGroup->button( activeRgbRadio ) ) rgbRadio->setChecked( true ); const int activeCmykRadio = settings.value( QStringLiteral( "Windows/ColorDialog/activeCmykComponent" ), 0 ).toInt(); - if ( QAbstractButton *cmykRadio = cmykGroup->button( activeCmykRadio ) ) + if ( QAbstractButton *cmykRadio = mCmykGroup->button( activeCmykRadio ) ) cmykRadio->setChecked( true ); const int currentTab = settings.value( QStringLiteral( "Windows/ColorDialog/activeTab" ), 0 ).toInt(); @@ -668,21 +668,9 @@ void QgsCompoundColorWidget::saveSettings() QgsSettings settings; - //record active component - int activeRadio = 0; - if ( mHueRadio->isChecked() ) - activeRadio = 0; - if ( mSaturationRadio->isChecked() ) - activeRadio = 1; - if ( mValueRadio->isChecked() ) - activeRadio = 2; - if ( mRedRadio->isChecked() ) - activeRadio = 3; - if ( mGreenRadio->isChecked() ) - activeRadio = 4; - if ( mBlueRadio->isChecked() ) - activeRadio = 5; - settings.setValue( QStringLiteral( "Windows/ColorDialog/activeComponent" ), activeRadio ); + // record active component + settings.setValue( QStringLiteral( "Windows/ColorDialog/activeComponent" ), mRgbGroup->checkedId() ); + settings.setValue( QStringLiteral( "Windows/ColorDialog/activeCmykComponent" ), mCmykGroup->checkedId() ); //record current scheme settings.setValue( QStringLiteral( "Windows/ColorDialog/activeScheme" ), mSchemeComboBox->currentIndex() ); diff --git a/src/gui/qgscompoundcolorwidget.h b/src/gui/qgscompoundcolorwidget.h index 9ae19c2cc5e5..f9a20af4de00 100644 --- a/src/gui/qgscompoundcolorwidget.h +++ b/src/gui/qgscompoundcolorwidget.h @@ -187,6 +187,8 @@ class GUI_EXPORT QgsCompoundColorWidget : public QgsPanelWidget, private Ui::Qgs QList> mRgbRadios; QList> mCmykRadios; + QButtonGroup *mCmykGroup = nullptr; + QButtonGroup *mRgbGroup = nullptr; /** * Saves all widget settings diff --git a/tests/src/gui/testqgscompoundcolorwidget.cpp b/tests/src/gui/testqgscompoundcolorwidget.cpp index 584f1e81fb76..11bd9a7ff0b2 100644 --- a/tests/src/gui/testqgscompoundcolorwidget.cpp +++ b/tests/src/gui/testqgscompoundcolorwidget.cpp @@ -98,23 +98,27 @@ void TestQgsCompoundColorWidget::testComponentSettings_data() { QTest::addColumn( "settingsComponent" ); QTest::addColumn( "expectedComponent" ); - - QTest::newRow( "hue" ) << 0 << QgsColorWidget::ColorComponent::Hue; - QTest::newRow( "saturation" ) << 1 << QgsColorWidget::ColorComponent::Saturation; - QTest::newRow( "value" ) << 2 << QgsColorWidget::ColorComponent::Value; - QTest::newRow( "red" ) << 3 << QgsColorWidget::ColorComponent::Red; - QTest::newRow( "green" ) << 4 << QgsColorWidget::ColorComponent::Green; - QTest::newRow( "blue" ) << 5 << QgsColorWidget::ColorComponent::Blue; - QTest::newRow( "cyan" ) << 0 << QgsColorWidget::ColorComponent::Cyan; - QTest::newRow( "magenta" ) << 1 << QgsColorWidget::ColorComponent::Magenta; - QTest::newRow( "yellow" ) << 2 << QgsColorWidget::ColorComponent::Yellow; - QTest::newRow( "black" ) << 3 << QgsColorWidget::ColorComponent::Black; + QTest::addColumn( "newComponent" ); + QTest::addColumn( "newSettingsComponent" ); + + QTest::newRow( "hue" ) << 0 << QgsColorWidget::ColorComponent::Hue << QgsColorWidget::ColorComponent::Saturation << 1; + QTest::newRow( "saturation" ) << 1 << QgsColorWidget::ColorComponent::Saturation << QgsColorWidget::ColorComponent::Value << 2; + QTest::newRow( "value" ) << 2 << QgsColorWidget::ColorComponent::Value << QgsColorWidget::ColorComponent::Red << 3; + QTest::newRow( "red" ) << 3 << QgsColorWidget::ColorComponent::Red << QgsColorWidget::ColorComponent::Green << 4; + QTest::newRow( "green" ) << 4 << QgsColorWidget::ColorComponent::Green << QgsColorWidget::ColorComponent::Blue << 5; + QTest::newRow( "blue" ) << 5 << QgsColorWidget::ColorComponent::Blue << QgsColorWidget::ColorComponent::Hue << 0; + QTest::newRow( "cyan" ) << 0 << QgsColorWidget::ColorComponent::Cyan << QgsColorWidget::ColorComponent::Magenta << 1; + QTest::newRow( "magenta" ) << 1 << QgsColorWidget::ColorComponent::Magenta << QgsColorWidget::ColorComponent::Yellow << 2; + QTest::newRow( "yellow" ) << 2 << QgsColorWidget::ColorComponent::Yellow << QgsColorWidget::ColorComponent::Black << 3; + QTest::newRow( "black" ) << 3 << QgsColorWidget::ColorComponent::Black << QgsColorWidget::ColorComponent::Cyan << 0; } void TestQgsCompoundColorWidget::testComponentSettings() { QFETCH( int, settingsComponent ); QFETCH( QgsColorWidget::ColorComponent, expectedComponent ); + QFETCH( QgsColorWidget::ColorComponent, newComponent ); + QFETCH( int, newSettingsComponent ); QgsSettings().setValue( QgsColorWidget::colorSpec( expectedComponent ) == QColor::Cmyk ? QStringLiteral( "Windows/ColorDialog/activeCmykComponent" ) : QStringLiteral( "Windows/ColorDialog/activeComponent" ), settingsComponent ); @@ -125,6 +129,15 @@ void TestQgsCompoundColorWidget::testComponentSettings() QCOMPARE( w.mColorBox->component(), expectedComponent ); QCOMPARE( w.mVerticalRamp->component(), expectedComponent ); + + ( QgsColorWidget::colorSpec( expectedComponent ) == QColor::Cmyk ? w.mCmykRadios : w.mRgbRadios ).at( newSettingsComponent ).first->setChecked( true ); + QCOMPARE( w.mColorBox->component(), newComponent ); + QCOMPARE( w.mVerticalRamp->component(), newComponent ); + + w.saveSettings(); + const int newValue = QgsSettings().value( QgsColorWidget::colorSpec( expectedComponent ) == QColor::Cmyk ? + QStringLiteral( "Windows/ColorDialog/activeCmykComponent" ) : QStringLiteral( "Windows/ColorDialog/activeComponent" ), -1 ).toInt(); + QCOMPARE( newValue, newSettingsComponent ); } void TestQgsCompoundColorWidget::testComponentChange() From 0dd77658efe64fd97ce6eb2ee61bf500c1542f12 Mon Sep 17 00:00:00 2001 From: Julien Cabieces Date: Wed, 22 May 2024 19:48:41 +0200 Subject: [PATCH 09/12] remove else around alterColor --- src/gui/qgscolorwidgets.cpp | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/gui/qgscolorwidgets.cpp b/src/gui/qgscolorwidgets.cpp index 7195598452a5..c709b93c3e45 100644 --- a/src/gui/qgscolorwidgets.cpp +++ b/src/gui/qgscolorwidgets.cpp @@ -389,10 +389,8 @@ void QgsColorWidget::setComponentValue( const int value ) mCurrentColor.setHsv( h, s, v, a ); } - else - { - alterColor( mCurrentColor, mComponent, value ); - } + + alterColor( mCurrentColor, mComponent, value ); //update recorded hue if ( mCurrentColor.hue() >= 0 ) From c1025c5ebc6c5eb26ad20b0aa22ab61f9a09a779 Mon Sep 17 00:00:00 2001 From: Julien Cabieces Date: Thu, 23 May 2024 11:39:25 +0200 Subject: [PATCH 10/12] declare color component --- tests/src/gui/testqgscompoundcolorwidget.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/src/gui/testqgscompoundcolorwidget.cpp b/tests/src/gui/testqgscompoundcolorwidget.cpp index 11bd9a7ff0b2..3d9e96cbbd3c 100644 --- a/tests/src/gui/testqgscompoundcolorwidget.cpp +++ b/tests/src/gui/testqgscompoundcolorwidget.cpp @@ -18,6 +18,8 @@ #include "qgscompoundcolorwidget.h" #include "qgssettings.h" +Q_DECLARE_METATYPE( QgsColorWidget::ColorComponent ) + class TestQgsCompoundColorWidget : public QgsTest { Q_OBJECT From a2924f38101bef4faf85abe5620ac0374ccbf1b1 Mon Sep 17 00:00:00 2001 From: Julien Cabieces Date: Thu, 23 May 2024 11:40:42 +0200 Subject: [PATCH 11/12] Set information for settings --- tests/src/gui/testqgscompoundcolorwidget.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/src/gui/testqgscompoundcolorwidget.cpp b/tests/src/gui/testqgscompoundcolorwidget.cpp index 3d9e96cbbd3c..a6f48b5f1ac3 100644 --- a/tests/src/gui/testqgscompoundcolorwidget.cpp +++ b/tests/src/gui/testqgscompoundcolorwidget.cpp @@ -43,6 +43,10 @@ class TestQgsCompoundColorWidget : public QgsTest void TestQgsCompoundColorWidget::initTestCase() { + // Set up the QgsSettings environment + QCoreApplication::setOrganizationName( QStringLiteral( "QGIS" ) ); + QCoreApplication::setOrganizationDomain( QStringLiteral( "qgis.org" ) ); + QCoreApplication::setApplicationName( QStringLiteral( "QGIS-TEST" ) ); } void TestQgsCompoundColorWidget::cleanupTestCase() From 1ad2b4bc672af8fe7b614c5652f762e50648dc17 Mon Sep 17 00:00:00 2001 From: Julien Cabieces Date: Thu, 23 May 2024 16:18:01 +0200 Subject: [PATCH 12/12] Fix(ColorWidget): Update current component on color model change --- src/gui/qgscompoundcolorwidget.cpp | 29 +++++++++++++++-------------- src/gui/qgscompoundcolorwidget.h | 19 +++++++------------ 2 files changed, 22 insertions(+), 26 deletions(-) diff --git a/src/gui/qgscompoundcolorwidget.cpp b/src/gui/qgscompoundcolorwidget.cpp index baf74d196fe1..c4a5137dde6b 100644 --- a/src/gui/qgscompoundcolorwidget.cpp +++ b/src/gui/qgscompoundcolorwidget.cpp @@ -70,8 +70,8 @@ QgsCompoundColorWidget::QgsCompoundColorWidget( QWidget *parent, const QColor &c for ( auto colorRadio : mCmykRadios ) mCmykGroup->addButton( colorRadio.first, i++ ); - connect( mRgbGroup, &QButtonGroup::idToggled, this, &QgsCompoundColorWidget::onRgbButtonGroupToggled ); - connect( mCmykGroup, &QButtonGroup::idToggled, this, &QgsCompoundColorWidget::onCmykButtonGroupToggled ); + connect( mRgbGroup, &QButtonGroup::idToggled, this, &QgsCompoundColorWidget::onColorButtonGroupToggled ); + connect( mCmykGroup, &QButtonGroup::idToggled, this, &QgsCompoundColorWidget::onColorButtonGroupToggled ); connect( mAddColorToSchemeButton, &QPushButton::clicked, this, &QgsCompoundColorWidget::mAddColorToSchemeButton_clicked ); connect( mAddCustomColorButton, &QPushButton::clicked, this, &QgsCompoundColorWidget::mAddCustomColorButton_clicked ); connect( mSampleButton, &QPushButton::clicked, this, &QgsCompoundColorWidget::mSampleButton_clicked ); @@ -87,6 +87,8 @@ QgsCompoundColorWidget::QgsCompoundColorWidget( QWidget *parent, const QColor &c setColor( this->color().toCmyk() ); else setColor( this->color().toRgb() ); + + updateComponent(); } ); if ( widgetLayout == LayoutVertical ) @@ -883,28 +885,27 @@ void QgsCompoundColorWidget::keyPressEvent( QKeyEvent *e ) } -void QgsCompoundColorWidget::onColorButtonGroupToggled( const QList> &colorRadios, - const QColor::Spec colorSpec, const int id, const bool checked ) +void QgsCompoundColorWidget::updateComponent() { - if ( checked && id >= 0 && id < colorRadios.count() && static_cast( mColorModel->currentData().toInt() ) == colorSpec ) + const bool isCmyk = mColorModel->currentData().toInt() == QColor::Spec::Cmyk; + const auto radios = isCmyk ? mCmykRadios : mRgbRadios; + const QButtonGroup *group = isCmyk ? mCmykGroup : mRgbGroup; + + const int id = group->checkedId(); + if ( id >= 0 && id < radios.count() ) { - const QgsColorWidget::ColorComponent component = colorRadios.at( id ).second; + const QgsColorWidget::ColorComponent component = radios.at( group->checkedId() ).second; mColorBox->setComponent( component ); mVerticalRamp->setComponent( component ); } } -void QgsCompoundColorWidget::onRgbButtonGroupToggled( int id, bool checked ) -{ - onColorButtonGroupToggled( mRgbRadios, QColor::Rgb, id, checked ); -} - -void QgsCompoundColorWidget::onCmykButtonGroupToggled( int id, bool checked ) +void QgsCompoundColorWidget::onColorButtonGroupToggled( int, bool checked ) { - onColorButtonGroupToggled( mCmykRadios, QColor::Cmyk, id, checked ); + if ( checked ) + updateComponent(); } - void QgsCompoundColorWidget::mAddColorToSchemeButton_clicked() { mSchemeList->addColor( mColorPreview->color(), QgsSymbolLayerUtils::colorToName( mColorPreview->color() ) ); diff --git a/src/gui/qgscompoundcolorwidget.h b/src/gui/qgscompoundcolorwidget.h index f9a20af4de00..a99683d78873 100644 --- a/src/gui/qgscompoundcolorwidget.h +++ b/src/gui/qgscompoundcolorwidget.h @@ -146,8 +146,7 @@ class GUI_EXPORT QgsCompoundColorWidget : public QgsPanelWidget, private Ui::Qgs private slots: - void onRgbButtonGroupToggled( int id, bool checked ); - void onCmykButtonGroupToggled( int id, bool checked ); + void onColorButtonGroupToggled( int, bool checked ); void mAddColorToSchemeButton_clicked(); @@ -175,6 +174,12 @@ class GUI_EXPORT QgsCompoundColorWidget : public QgsPanelWidget, private Ui::Qgs static QScreen *findScreenAt( QPoint pos ); + /** + * Helper method to update current widget display with current component according to + * color model and selected color component radio button + */ + void updateComponent(); + QgsScreenHelper *mScreenHelper = nullptr; bool mAllowAlpha = true; @@ -230,16 +235,6 @@ class GUI_EXPORT QgsCompoundColorWidget : public QgsPanelWidget, private Ui::Qgs //! Updates the state of actions for the current selected scheme void updateActionsForCurrentScheme(); - /** - * Helper method to implement slots called when color radio button has been toggled - * \param colorRadios related to the toggled button - * \param colorSpec color type of the toggled button - * \param id of the toggled button - * \param checked TRUE is the button is checked - */ - void onColorButtonGroupToggled( const QList> &colorRadios, - const QColor::Spec colorSpec, const int id, const bool checked ); - friend class TestQgsCompoundColorWidget; };