[Development] What to expect from QIcon/the icon engine on screen changes
Mark De Wit
mark.dewit at iesve.com
Tue Apr 9 10:33:20 CEST 2019
Interesting find - that fix might have solved my issue with Fusion theme as well. From the looks of it, QWindow* support was added to some theme code since that bug report, but not consistently for all controls and all themes. It would have been great to fix this at the QIcon level rather than change every widget painter for every theme....
Mark
-----Original Message-----
From: Development <development-bounces at qt-project.org> On Behalf Of Elvis Stansvik
Sent: 08 April 2019 15:15
To: Olivier Goffart <olivier at woboq.com>
Cc: development at qt-project.org
Subject: Re: [Development] What to expect from QIcon/the icon engine on screen changes
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
_______________________________________________
Development mailing list
Development at qt-project.org
https://lists.qt-project.org/listinfo/development
More information about the Development
mailing list