Uploaded image for project: 'Qt'
  1. Qt
  2. QTBUG-109169

Invalid QColorTrcLut use causes unreadable text with 16-bit X11 visuals

    XMLWordPrintable

Details

    • Bug
    • Resolution: Fixed
    • P3: Somewhat important
    • 5.15.12, 6.2.7, 6.4.3, 6.5.0
    • 5.15.7, 6.4.0
    • GUI: Painting
    • None
    • Linux/X11
    • 9cabb6a70 (tqtc/lts-6.2), 06a7b5ff7 (6.4), a09c33e1f (dev), fcf1ae6cf (tqtc/lts-5.15)

    Description

      After we turned on subpixel rendering via fontconfig, Qt applications connected with Xvnc depth 16 show unreadable text, with the artifacts differing between little and big-endian platforms. This was originally reported downstream at https://bugzilla.opensuse.org/show_bug.cgi?id=1205585, see also the first attached screenshot.

      With a small reproducer I was able to hit this bug even with green text on green background, which should result in a uniform green surface:

      #include <QApplication>
      #include <QLabel>
      
      int main(int argc, char *argv[])
      {
      	QApplication a(argc, argv);
      	QLabel l;
      	l.setText("A");
      	l.setStyleSheet("* {background: green; color: green;}");
      	l.setFont(QFont("Source Sans 3", 15));
      	l.show();
      	return a.exec();
      }
      

      However, it actually shows brown pixels around the text outline, where the subpixel antialiasing should be (second screenshot). This made it easier to debug, as I just had to find the place where the pixels stopped being just green anymore.

      That place turned out to be rgbBlendPixel (for QRgba64) in src/gui/painting/qdrawhelper.cpp:

      static inline void rgbBlendPixel(QRgba64 &dst, int coverage, QRgba64 slinear, const QColorTrcLut *colorProfile)
      {
          // Do a gammacorrected RGB alphablend...
          const QRgba64 dlinear = colorProfile ? colorProfile->toLinear64(dst) : dst;
      
          QRgba64 blend = rgbBlend(dlinear, slinear, coverage);
      
          dst = colorProfile ? colorProfile->fromLinear(blend) : blend;
      }
      

      At start, dst was green but dlinear already had wrong values. The cause for that is a wrong implicit conversion. The method signature is actually QRgba64 QColorTrcLut::toLinear64(QRgb rgb32) const, so it takes a QRgb.

      // from qrgb.h
      typedef unsigned int QRgb;                        // RGB triplet
      // from qrgba64.h
      class QRgba64 {
          Q_DECL_CONSTEXPR operator quint64() const
          {
              return rgba;
          }
      }
      

      In colorProfile->toLinear64(dst), dst's 64bit value is converted to unsigned int and treated as QRgb. This cuts off the upper 64 bits instead of converting the format. The operations performed on the intermediate garbage value (FWICT 0xGGGGRRRR interpreted as 0xAARRGGBB on little-endian and 0xBBBBAAAA interpreted as 0xAARRGGBB on big-endian) cause the artifact color.

      The fix is surprisingly simple (and I plan on submitting that soon):

      diff --git a/src/gui/painting/qdrawhelper.cpp b/src/gui/painting/qdrawhelper.cpp
      index a61793508a..5ba2d277b7 100644
      --- a/src/gui/painting/qdrawhelper.cpp
      +++ b/src/gui/painting/qdrawhelper.cpp
      @@ -6091,7 +6091,7 @@ static inline void alphargbblend_argb32(quint32 *dst, uint coverage, const QRgba
       static inline void rgbBlendPixel(QRgba64 &dst, int coverage, QRgba64 slinear, const QColorTrcLut *colorProfile)
       {
           // Do a gammacorrected RGB alphablend...
      -    const QRgba64 dlinear = colorProfile ? colorProfile->toLinear64(dst) : dst;
      +    const QRgba64 dlinear = colorProfile ? colorProfile->toLinear(dst) : dst;
       
           QRgba64 blend = rgbBlend(dlinear, slinear, coverage);
       
      

      However, I wonder whether this bad implicit conversion (which can happen in both directions) should be addressed and how. There might be more bugs in the code caused by this.

      Attachments

        No reviews matched the request. Check your Options in the drop-down menu of this sections header.

        Activity

          People

            allan.jensen Allan Sandfeld Jensen
            vogtinator Fabian Vogt
            Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved:

              Gerrit Reviews

                There are no open Gerrit changes