qskinny icon indicating copy to clipboard operation
qskinny copied to clipboard

QColor vs. QskColor

Open uwerat opened this issue 1 year ago • 0 comments

QColor did not fit into some limitations ( <= 32 bits ) of the early design of the skin hints. That lead to the decision to use QRgb as this is also the format being used on the scene graph.

However QColor and QRgb do not play together well, when using both in the public API. Consider the following:

struct A
{
    void setColor( const QColor& );
} a;

a.setColor( qRgba( 100, 100, 100, 100 )  );

Very early versions of Qt did not support alpha values and - probably for compatibility reasons - QColor( QRgb ) drops the alpha value. So you need to have a second setColor method for QRgb.

However this is not were the issue ends:

struct A
{
    void setColor( const QColor& );
    void setColor( QRgb ) { setColor( QColor::fromRgba( rgb ); }
}  a;

a.setColor( Qt::red );

Now Qt::GlobalColor will be interpreted as QRgb.

So whenever having an API, where you have to pass a color you need 3 methods:

struct A
{
    void setColor( const QColor& );
    void setColor( QRgb rgb ) { setColor( QColor::fromRgba( rgb ); }
    void setColor( Qt::GlobalColor color ) { setColor( QColor( color ); }
} ;

To avoid this totally annoying situation I see 2 possible solutions:

a) Always using QColor and not using QRgb anymore b) Introducing QskColor

class QskColor
{
       QskColor( QRgb rgb ): m_rgb( rgb ) {}
       QskColor( QColorConstants::Svg rgb ): m_rgb( rgb ) {}

       QskColor( QColor& color ): m_rgb( color.rgba() ) {}
       QskColor( Qt::GlobalColor globalColor ): m_rgb( QskRgb::rgb( globalColor ); }
       QskColor( const char *name ) : QskColor( QColor( name ) ) {}

       ...

      operator QColor() const { return QColor::fromRgba( m_rgb ); }
      operator QRgb() const { return m_rgb; }

   private:
       QRgb m_rgb;
};

The argument for QColor is that it is used in all Qt classes - also having special hacks in QVariant or the QML bridge.

However the ability to store a color as a different color model introduces unexpected conversions and some overhead when using it on the scene graph. QColor ( because of supporting 64bit colors ) needs 16 bytes, while QRgb is only 4 bytes. With Qt5 it is also not possible to create a QColor at compile time ( constexpr ).

QColor::isValid() is sometimes useful but often annoying as you always have to handle it when dealing with colors. F.e when invalidating a gradient you can do it by setting no stops. Having to check the color of each stop if it is valid is extra work without adding any extra value.

None of the arguments against QColor is a major issue, but introducing QskColor doesn't sound like much work and allows further improvements/adjustments that make sense for the Qsk context.

uwerat avatar Feb 19 '24 09:02 uwerat