[Development] What to expect from QIcon/the icon engine on screen changes

Elvis Stansvik elvstone at gmail.com
Mon Apr 8 16:14:40 CEST 2019 

Den mån 8 apr. 2019 kl 15:40 skrev Elvis Stansvik <elvstone at gmail.com>:
>
> Den sön 7 apr. 2019 kl 20:21 skrev Elvis Stansvik <elvstone at gmail.com>:
> >
> > Den sön 7 apr. 2019 kl 18:45 skrev Olivier Goffart <olivier at woboq.com>:
> > >
> > > On 06.04.19 10:40, Elvis Stansvik wrote:
> > > > Hi all,
> > > >
> > > > I'm looking for someone who knows the inner workings of QIcon and the
> > > > icon engines.
> > > >
> > > > In our application, we almost exclusively use SVG icons, and we use a
> > > > single SVG file for each icon (no @2x versions) that we try to design
> > > > to work reasonably well at all sizes, since we do not have the
> > > > resources to make custom variations for each target size.
> > > >
> > > > We put these SVG icons into an XDG icon theme, that we ship inside the
> > > > application resources (.qrc) in the expected XDG layout and with an
> > > > icon theme index file. We can then use
> > > > QIcon::fromTheme("our-app-some-icon") to reference an icon (either
> > > > through the .ui file or in code).
> > > >
> > > > The problem we've run into is that when the application is launched in
> > > > a mixed-DPI setup, for example a retina Mac laptop with an external
> > > > lower-DPI monitor, the icons appear too large and get cropped. In
> > > > effect, it seems to always use the DPI of the primary screen (the
> > > > built-in retina screen) when calculating the size of the pixmaps it
> > > > generates for the icons.
> > >
> > >
> > > Not sure if this is your problem, but it seems that
> > > QSvgIconEngine::virtual_hook does not handle the QIconEngine::ScaledPixmapHook
> > > call, which is needed for the QIcon::pixmap(QWindow *window, ...) call.
> >
> > Thanks Olivier,
> >
> > I'm not familiar with the code, but it sounds like that could be it.
> >
> > I'll try to make a minimal test case tomorrow.
>
> I've now created a minimal test case:
>
> dirac:~ insight$ find testcase
> testcase
> testcase/testcase.cpp
> testcase/icons
> testcase/icons/mytheme
> testcase/icons/mytheme/index.theme
> testcase/icons/mytheme/scalable
> testcase/icons/mytheme/scalable/actions
> testcase/icons/mytheme/scalable/actions/test-icon.svg
> testcase/testcase.pro
> testcase/testcase.qrc
> dirac:~ insight$
>
> testcase.cpp:
>
> #include <QApplication>
> #include <QIcon>
> #include <QToolButton>
>
> int main(int argc, char *argv[]) {
>     QApplication app(argc, argv);
>
>     app.setAttribute(Qt::AA_UseHighDpiPixmaps);
>
>     QIcon::setThemeName("mytheme");
>
>     // Create icon from file name -> Size of pixmap is correct on both screens
>     QToolButton button1;
>     button1.setIcon(QIcon("icons/mytheme/scalable/actions/test-icon.svg"));
>     button1.show();
>
>     // Create icon from theme -> Size of pixmap is correct only on primary screen
>     QToolButton button2;
>     button2.setIcon(QIcon::fromTheme("test-icon"));
>     button2.show();
>
>     return app.exec();
> }
>
> This is the result when running on the external lower-DPI monitor:
>
>
> Note how the right button icon (button2), which is created using QIcon::fromTheme has the wrong pixmap size for that monitor (it's appropriate for the internal retina screen).
>
> And it really does seem like the Qt::AA_UseHighDpiPixmaps application attribute has a finger in the game here, because if I remove setting of that attribute, the problem goes away.
>
> Attaching the full test case as a ZIP.
>
> Should I report this as a bug?

As I was browsing through bugs containing the word "UseHighDpiPixmaps"
I came across this comment on a closed one (which was similar, but not
the same):

    https://bugreports.qt.io/browse/QTBUG-50251?focusedCommentId=304908&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-304908

Quoting the comment:

    "I have patched this in late '14 but it did not got integrated.

Problem is icons are loaded by a style. Styles do not have access to a
QWindow*. To load the dpi-correct icon a QWindow* is needed right now,
and in current case it just defaults to app default - biggest scale
factor.

My patch added an API to QIcon which used QPainter* instead of
QWindow* to query and load the right icon. With this addition fixing
the mac style is trivial.

Probably even more generic API can be added to QIcon, where
devicePixelRatio is directly passed as an qreal. This will also make
the problem fixable."

This seems possibly related to my issue.

Elvis

>
>
> Elvis
>
> >
> > Elvis
> >
> > >
> > >
> > >
> > >
> > > >
> > > > To work around this, we've had to put in special code in all of our
> > > > widgets that use icons. The code reacts to screen change events (or
> > > > changes to the underlying QWindow in some cases), and in that code, go
> > > > through each and every one of our icons and do this monkey dance:
> > > >
> > > >          auto pixmap = someButton->icon().pixmap(someButton->iconSize());
> > > >          pixmap.setDevicePixelRatio(window()->windowHandle()->screen()->devicePixelRatio());
> > > >          someButton->setIcon(QIcon(pixmap));
> > > >
> > > > So essentially taking the pixmap out of the icon, set its DPR to that
> > > > of the current screen, and then set that pixmap back on the icon.
> > > >
> > > > This "works", the icons now look correct on both the retina screen and
> > > > the external one, and adjust themselves when the application is moved
> > > > back and forth, or when the external monitor is activated/deactivated.
> > > >
> > > > But surely this kludge should not be necessary? We've provided Qt with
> > > > an SVG, so it should be able to work out on its own that the screen
> > > > has changed, and regenerate an appropriate pixmap in response to that?
> > > >
> > > > Some more details:
> > > >
> > > > - We are running with the Qt::AA_UseHighDpiPixmaps application
> > > > attribute turned on. I'm not sure this is relevant for this issue
> > > > though, because AFAIK that attribute is only about picking 2x versions
> > > > of icons (we have a couple of those, where we have PNGs with 2x
> > > > versions).
> > > >
> > > > - We are not running with Qt's built-in scaling activated, but relying
> > > > on the Mac platform scaling (I'm not even sure Qt's built-in scaling
> > > > is applicable on Mac). The application runs with
> > > > NSHighResolutionCapable set to True in its Info.plist (which I also
> > > > think is the default nowadays).
> > > >
> > > > - I have not investigated yet whether this problem also occurs on a
> > > > mixed-DPI Linux setup, with Qt's high-dpi scaling activated. Nor have
> > > > I checked if it happens on Windows using it's artificial "screen
> > > > scaling" (we do not use Qt's built-in scaling on Windows either,
> > > > trying to follow the advise in the docs to avoid that for new
> > > > applications). So for now this is only about Mac retina + external
> > > > monitor.
> > > >
> > > > Any advise on this would be highly appreciated, because the code
> > > > required to re-trigger pixmap generation on screen changes is a real
> > > > kludge all over our code base, and often it happens that we add
> > > > buttons et.c. with icons, but forget to update this machinery.
> > > >
> > > > I'm not at the office at the moment, but can provide a little test
> > > > program that mimics what we're doing on Monday.
> > > >
> > > > Thanks in advance,
> > > > Elvis
> > > > _______________________________________________
> > > > Development mailing list
> > > > Development at qt-project.org
> > > > https://lists.qt-project.org/listinfo/development
> > > >
> > >
> > > _______________________________________________
> > > Development mailing list
> > > Development at qt-project.org
> > > https://lists.qt-project.org/listinfo/development

More information about the Development mailing list