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

[Elvis Stansvik]

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:

[image: testcase.png]

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?

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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.qt-project.org/pipermail/development/attachments/20190408/42c70ec6/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: testcase.png
Type: image/png
Size: 15708 bytes
Desc: not available
URL: <http://lists.qt-project.org/pipermail/development/attachments/20190408/42c70ec6/attachment.png>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: testcase.zip
Type: application/zip
Size: 3572 bytes
Desc: not available
URL: <http://lists.qt-project.org/pipermail/development/attachments/20190408/42c70ec6/attachment.zip>