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

Calling QStyle::pixelMetric without style options lead to DPI scaling issues

    XMLWordPrintable

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Not Evaluated
    • Resolution: Done
    • Affects Version/s: 5.14.0
    • Fix Version/s: 5.14.2
    • Labels:
      None

      Description

      Greetings –

      Qt 5.14 introduced smarts to allow for QStyleOptions control DPI scaling. However, there is an issue where DPI scaling is wrong if this isn't provided. In QStyleHelpers, if the options aren't provided a fixed value of qstyleBaseDpi is used.

      See "Widget style: Use per-screen DPI in QStyleHelper::dpiScaled()" d603ee689f0e3fdcfa3230b3d75cdce6c5af05c1 https://codereview.qt-project.org/c/qt/qtbase/+/271429 .
       

      \\ Qt 5.14
      Q_WIDGETS_EXPORT qreal dpi(const QStyleOption *option)
      {
      #ifndef Q_OS_DARWIN
          // Prioritize the application override, except for on macOS where
          // we have historically not supported the AA_Use96Dpi flag.
          if (QCoreApplication::testAttribute(Qt::AA_Use96Dpi))
              return 96;
      #endif    // Expect that QStyleOption::QFontMetrics::QFont has the correct DPI set
          if (option)
              return option->fontMetrics.fontDpi();  
          return qstyleBaseDpi;
      }

       
      Previously the default was DPI scaled... albeit by the main monitor's logical DPI.
       

      qreal dpiScaled(qreal value)
      {
      #ifdef Q_OS_MAC
          // On mac the DPI is always 72 so we should not scale it
          return value;
      #else
          static const qreal scale = qreal(qt_defaultDpiX()) / 96.0;
          return value * scale;
      #endif
      }
      

      This is a functional change that has the right idea but it really means that options must be provided everywhere for a visually consistent experience. However, this isn't the case as many places internally like QFusion aren't consistent.

      // ...
      
      // Has option
      int tickSize = proxy()->pixelMetric(PM_SliderTickmarkOffset, option, widget);
      switch (subControl) {
      SC_SliderHandle: {
          if (slider->orientation == Qt::Horizontal) {
      
              // Missing option!!!
              rect.setHeight(proxy()->pixelMetric(PM_SliderThickness));
              rect.setWidth(proxy()->pixelMetric(PM_SliderLength));
      
      // ...

      As a work-around I created a proxy style that created a default style object. This only works in a single-DPI case, though.

      A recommended real fix would be patching all usage points to include options and perhaps doing a better default in the no-options case (which probably should be a logged as a warning as well).

      Cheers!

      NOTE: I see some other bugs claiming this was fixed but I think the root issue still exists.

        Attachments

          Issue Links

          For Gerrit Dashboard: QTBUG-82356
          # Subject Branch Project Status CR V

            Activity

              People

              Assignee:
              sorvig Morten Sørvig
              Reporter:
              kyle.spagnoli Kyle Spagnoli
              Votes:
              2 Vote for this issue
              Watchers:
              5 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:

                  Gerrit Reviews

                  There are no open Gerrit changes