Bug 403195

Summary:	New check: warn on hidpi issues
Product:	[Developer tools] clazy	Reporter:	Nyall Dawson <nyall.dawson>
Component:	general	Assignee:	Unassigned bugs mailing-list <unassigned-bugs>
Status:	RESOLVED LATER
Severity:	wishlist	CC:	smartins
Priority:	NOR
Version:	unspecified
Target Milestone:	---
Platform:	Other
OS:	Linux
Latest Commit:		Version Fixed In:
Sentry Crash Report:

Description Nyall Dawson 2019-01-13 23:05:13 UTC

I've been working on a large, legacy code base recently on a hi-dpi display. This application is completely un-hdp-friendly, due to a multitude of fixed sizes being set in pixels.

This had me thinking of a possible clazy check which would automatically flag potential hi-dpi issues. A super-basic version of the check could flag whenever calls to methods which set sizes (e.g. QWidget::setFixedSize, QTreeView::setColumnWidth, etc) are called with literal size values. I.e. flag a call to widget->setFixedSize( 16, 16 ), but not widget->setFixedSize( someVariable, someVariable ). (even if someVariable is a const value such as const int someVariable = 16; --- this would definitely be a very rough check only!)

A more advanced check could detect if the arguments to the sizing methods use a variable which has been set based on a result from a QFontMetrics/QFontMetricsF  call -- and throw the warning if this ISN'T the case.

Comment 1 Sergio Martins 2019-01-14 15:03:22 UTC

It's completely ok to use setFixedSize(16, 16), as those 16x16 pixels are not really physical pixels, but logical pixels.

Qt converts from logical to physical internally, depending on your hdpi settings, and there's no need for the user to do the scaling on his own.

Comment 2 Nyall Dawson 2019-01-14 21:54:52 UTC

Sergio - is that also the case for methods like setIconSize?

My experience has been the opposite, under both Windows 10 and Fedora (Gnome Shell), on a Dell XPS 13. ANY fixed sizes are hugely undersized on the display. 

And indeed, the Qt docs state the same: "Replace hard-coded sizes in layouts and drawing code by values calculated from font metrics or screen size." (http://doc.qt.io/qt-5/highdpi.html)

Comment 3 Sergio Martins 2019-04-25 15:57:33 UTC

For icon sizes you need AA_UseHighDpiPixmaps and AA_EnableHighDpiScaling

>> "Replace hard-coded sizes in layouts and drawing code by values calculated from font metrics or screen size."

I don't think this is correct, and instead people are just using AA_EnableHighDpiScaling, even in new projects, not only legacy ones.

Suppose you're using a QComboBox, how do you replace the hardcoded sizes/margins from its implementation which you don't have access to ? Providing your own QStyle is a lot of work.

Furthermore,  there's a bunch of new HDPI work in Qt's codereview, which might change some of these recommendations, and also support floating-point factors.


I don't think we're ready for a clazy check at this time, since even the Qt maintainers send mixed signals about what to use.

See also the mailing lists, there's lots of doubts about all this

I'll move this suggestion to the "Later" queue, and re-open if anything changes upstream, thanks!