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

2019-04-09 Thread Mark De Wit
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  On Behalf Of Elvis 
Stansvik
Sent: 08 April 2019 15:15
To: Olivier Goffart 
Cc: development@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 :
>
> Den sön 7 apr. 2019 kl 20:21 skrev Elvis Stansvik :
> >
> > Den sön 7 apr. 2019 kl 18:45 skrev Olivier Goffart :
> > >
> > > 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 
> #include 
> #include 
>
> 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=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
> >
> > 

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

2019-04-08 Thread Elvis Stansvik
Den mån 8 apr. 2019 kl 15:40 skrev Elvis Stansvik :
>
> Den sön 7 apr. 2019 kl 20:21 skrev Elvis Stansvik :
> >
> > Den sön 7 apr. 2019 kl 18:45 skrev Olivier Goffart :
> > >
> > > 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 
> #include 
> #include 
>
> 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?

I couldn't find an issue quite like this in JIRA, so I went ahead and filed:

https://bugreports.qt.io/browse/QTBUG-75039

Please continue any discussion there.

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 

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

2019-04-08 Thread Elvis Stansvik
Den mån 8 apr. 2019 kl 15:40 skrev Elvis Stansvik :
>
> Den sön 7 apr. 2019 kl 20:21 skrev Elvis Stansvik :
> >
> > Den sön 7 apr. 2019 kl 18:45 skrev Olivier Goffart :
> > >
> > > 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 
> #include 
> #include 
>
> 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=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 

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

2019-04-08 Thread Elvis Stansvik
Den sön 7 apr. 2019 kl 20:21 skrev Elvis Stansvik :
>
> Den sön 7 apr. 2019 kl 18:45 skrev Olivier Goffart :
> >
> > 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 
#include 
#include 

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 

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

2019-04-08 Thread Elvis Stansvik
Den mån 8 apr. 2019 kl 09:35 skrev Mark De Wit :
>
> Are you using Fusion theme by any chance?   I'm facing the same issue, I even 
> tried writing my own Icon Engine to try and improve matters, but turns out 
> widgets are simply requesting the wrong size icon - nothing the icon engine 
> can do to "second-guess" such requests...

We're using the Cocoa theme on Mac.

>
> I have filed one such issue here: 
> https://bugreports.qt.io/browse/QTBUG-74100, and in general I feel that Qt's 
> geometry handling in mixed DPI is still rather poor.   I will often have my 
> application start up at ridiculous sizes (3/4 offscreen etc) because it has 
> completely nonsensical geometry coordinates for windows (Qt applying scaling 
> to coordinates when it's not required etc).

Sounds from the description that you've identified another way this
could be explained (using the wrong pixmap(...) overload), apart from
what Olivier brought up.

But it sounds good to me if our problem is either of these, as it
sounds like it should be fixable.

Elvis

>
> Mark
>
> -Original Message-
> From: Development  On Behalf Of Elvis 
> Stansvik
> Sent: 07 April 2019 19:21
> To: Olivier Goffart 
> Cc: development@qt-project.org
> Subject: Re: [Development] What to expect from QIcon/the icon engine on 
> screen changes
>
> Den sön 7 apr. 2019 kl 18:45 skrev Olivier Goffart :
> >
> > 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.
>
> 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 

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

2019-04-08 Thread Mark De Wit
Are you using Fusion theme by any chance?   I'm facing the same issue, I even 
tried writing my own Icon Engine to try and improve matters, but turns out 
widgets are simply requesting the wrong size icon - nothing the icon engine can 
do to "second-guess" such requests...

I have filed one such issue here: https://bugreports.qt.io/browse/QTBUG-74100, 
and in general I feel that Qt's geometry handling in mixed DPI is still rather 
poor.   I will often have my application start up at ridiculous sizes (3/4 
offscreen etc) because it has completely nonsensical geometry coordinates for 
windows (Qt applying scaling to coordinates when it's not required etc).

Mark

-Original Message-
From: Development  On Behalf Of Elvis 
Stansvik
Sent: 07 April 2019 19:21
To: Olivier Goffart 
Cc: development@qt-project.org
Subject: Re: [Development] What to expect from QIcon/the icon engine on screen 
changes

Den sön 7 apr. 2019 kl 18:45 skrev Olivier Goffart :
>
> 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.

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.
> >
> > 

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

2019-04-07 Thread Elvis Stansvik
Den sön 7 apr. 2019 kl 18:45 skrev Olivier Goffart :
>
> 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.

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@qt-project.org
> > https://lists.qt-project.org/listinfo/development
> >
>
> ___
> Development mailing list
> Development@qt-project.org
> https://lists.qt-project.org/listinfo/development
___
Development mailing list
Development@qt-project.org
https://lists.qt-project.org/listinfo/development


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

2019-04-07 Thread Olivier Goffart

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.







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@qt-project.org
https://lists.qt-project.org/listinfo/development



___
Development mailing list
Development@qt-project.org
https://lists.qt-project.org/listinfo/development


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

2019-04-06 Thread Elvis Stansvik
Den lör 6 apr. 2019 kl 10:40 skrev Elvis Stansvik :
>
> 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

To clarify: This happens when the application is on the external
screen. On the retina screen it looks correct

> 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.
>
> 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@qt-project.org
https://lists.qt-project.org/listinfo/development


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

2019-04-06 Thread Elvis Stansvik
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.

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@qt-project.org
https://lists.qt-project.org/listinfo/development