Bug 412738 - Issue with multi-head + fractional scaling
Summary: Issue with multi-head + fractional scaling
Status: RESOLVED FIXED
Alias: None
Product: gwenview
Classification: Applications
Component: general (show other bugs)
Version: 19.08.1
Platform: Other Linux
: NOR normal
Target Milestone: ---
Assignee: Gwenview Bugs
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-10-08 19:32 UTC by Christoph Cullmann
Modified: 2019-10-09 19:51 UTC (History)
3 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Christoph Cullmann 2019-10-08 19:32:48 UTC
SUMMARY

Issue with multi-head + fractional scaling

=> e.g. after entering full screen on second screen no useful repainting anymore (sometimes during normal navigation in folders, too, only on second screen).


STEPS TO REPRODUCE
1. have two displays
2. have set a scale factor of e.g. 1.5, not something like 2 (export QT_SCALE_FACTOR=1.5)
3. open gwenview, open image, go fullscreen => broken

OBSERVED RESULT

screen no longer usefully repainted

EXPECTED RESULT

normal behavior, like on first screen

SOFTWARE/OS VERSIONS

KDE Frameworks 5.63.0
Qt 5.13.1 (built against 5.13.1)
The xcb windowing system
Comment 1 Christoph Cullmann 2019-10-08 20:22:38 UTC
Git commit 769b6e17a0a1e46777ddc87ef1cc9ce3e8807a16 by Christoph Cullmann.
Committed on 08/10/2019 at 20:22.
Pushed by cullmann into branch 'master'.

fix fractional scaling

M  +1    -1    lib/documentview/rasterimageview.cpp
M  +2    -2    lib/thumbnailview/previewitemdelegate.cpp
M  +2    -2    lib/thumbnailview/thumbnailbarview.cpp

https://commits.kde.org/gwenview/769b6e17a0a1e46777ddc87ef1cc9ce3e8807a16
Comment 2 Christoph Cullmann 2019-10-08 20:25:08 UTC
Git commit 113d418203aad7bf6fb09b53a3d70dda30fbd524 by Christoph Cullmann.
Committed on 08/10/2019 at 20:24.
Pushed by cullmann into branch 'Applications/19.08'.

fix fractional scaling

M  +1    -1    lib/documentview/rasterimageview.cpp
M  +2    -2    lib/thumbnailview/previewitemdelegate.cpp
M  +2    -2    lib/thumbnailview/thumbnailbarview.cpp

https://commits.kde.org/gwenview/113d418203aad7bf6fb09b53a3d70dda30fbd524
Comment 3 Christoph Cullmann 2019-10-09 13:34:59 UTC
There seems to be issues with this:

https://github.com/KDE/gwenview/commit/769b6e17a0a1e46777ddc87ef1cc9ce3e8807a16#commitcomment-35426113
Comment 4 Christoph Cullmann 2019-10-09 13:39:22 UTC
I tried my patch with:

1) No scaling => works
2) Scaling 1.5 => works with, without unusable
3) Scaling 2.0 => works

If you can reproduce a crash, please show here, we can then revert the commit.
(especially in the 19.08 branch)

For me the only reproducable "random" crash was before commit

https://commits.kde.org/gwenview/64700e39e989001cc60a179f449695c104bf030b

There I got random crashs during "any" operation.
Comment 5 Christoph Cullmann 2019-10-09 13:39:53 UTC
I will revert the commit in stable just to be sure, if it is faulty, one has more time in master to think about it.
Comment 6 Christoph Cullmann 2019-10-09 13:40:58 UTC
Git commit 2ff212adc2adfc0d50f88abdb7a312250780aa3d by Christoph Cullmann.
Committed on 09/10/2019 at 13:40.
Pushed by cullmann into branch 'Applications/19.08'.

Revert "fix fractional scaling"

This reverts commit 113d418203aad7bf6fb09b53a3d70dda30fbd524.

=> there are reports that this might lead to crashs
I can not reproduce that, but better be safe than sorry

M  +1    -1    lib/documentview/rasterimageview.cpp
M  +2    -2    lib/thumbnailview/previewitemdelegate.cpp
M  +2    -2    lib/thumbnailview/thumbnailbarview.cpp

https://commits.kde.org/gwenview/2ff212adc2adfc0d50f88abdb7a312250780aa3d
Comment 7 Tom Moebert 2019-10-09 13:46:20 UTC
For me crashes in

#4  0x00007ffff7591475 in operator/ (s=..., c=0) at /usr/include/qt5/QtCore/qsize.h:196
194     inline const QSize operator/(const QSize &s, qreal c)
195     {
196         Q_ASSERT(!qFuzzyIsNull(c));
197         return QSize(qRound(s.wd/c), qRound(s.ht/c));
198     }

when c==0 .

Test code:

auto prf= thumbnailPix.devicePixelRatioF();
auto pr = thumbnailPix.devicePixelRatio();

(gdb) print prf
$3 = 0
(gdb) print pr
$2 = 1

(gdb) ptype prf
type = double
(gdb) ptype pr
type = double


Full call stack:

Thread 1 "gwenview" received signal SIGABRT, Aborted.
0x00007fffee76b160 in raise () from /lib64/libc.so.6
(gdb) bt
#0  0x00007fffee76b160 in raise () from /lib64/libc.so.6
#1  0x00007fffee76c741 in abort () from /lib64/libc.so.6
#2  0x00007fffef46b907 in QMessageLogger::fatal(char const*, ...) const () from /usr/lib64/libQt5Core.so.5
#3  0x00007fffef466dd6 in qt_assert(char const*, char const*, int) () from /usr/lib64/libQt5Core.so.5
#4  0x00007ffff7591475 in operator/ (s=..., c=0) at /usr/include/qt5/QtCore/qsize.h:196
#5  0x00007ffff761dcce in Gwenview::ThumbnailBarItemDelegate::sizeHint (this=0x9b4650, index=...)
    at /home/tom/Programme/gwenview/lib/thumbnailview/thumbnailbarview.cpp:182
#6  0x00007ffff1540e5a in ?? () from /usr/lib64/libQt5Widgets.so.5
#7  0x00007ffff15491f4 in ?? () from /usr/lib64/libQt5Widgets.so.5
#8  0x00007ffff1540bf3 in ?? () from /usr/lib64/libQt5Widgets.so.5
#9  0x00007ffff154ac5a in QListView::doItemsLayout() () from /usr/lib64/libQt5Widgets.so.5
#10 0x00007ffff12b9513 in ?? () from /usr/lib64/libQt5Widgets.so.5
#11 0x00007ffff1545160 in QListView::rectForIndex(QModelIndex const&) const () from /usr/lib64/libQt5Widgets.so.5
#12 0x00007ffff15451ae in QListView::visualRect(QModelIndex const&) const () from /usr/lib64/libQt5Widgets.so.5
#13 0x00007ffff154686c in QListView::scrollTo(QModelIndex const&, QAbstractItemView::ScrollHint) ()
   from /usr/lib64/libQt5Widgets.so.5
#14 0x00007ffff7623b67 in Gwenview::ThumbnailView::scrollToSelectedIndex (this=0xa6f0e0)
    at /home/tom/Programme/gwenview/lib/thumbnailview/thumbnailview.cpp:843
#15 0x000000000047545e in Gwenview::MainWindow::slotDirListerCompleted (this=0x7f5670)
    at /home/tom/Programme/gwenview/app/mainwindow.cpp:1302
#16 0x00000000004a61b0 in Gwenview::MainWindow::qt_static_metacall (_o=0x7f5670, 
    _c=QMetaObject::InvokeMetaMethod, _id=15, _a=0x7fffffffd2a0)
    at /home/tom/Programme/gwenview/build/app/gwenview_autogen/EWIEGA46WW/moc_mainwindow.cpp:238
#17 0x00007fffef68e535 in QMetaObject::activate(QObject*, int, int, void**) () from /usr/lib64/libQt5Core.so.5
#18 0x00007ffff6583947 in ?? () from /usr/lib64/libKF5KIOCore.so.5
#19 0x00007ffff6583b36 in ?? () from /usr/lib64/libKF5KIOCore.so.5
#20 0x00007fffef68f0a2 in QObject::event(QEvent*) () from /usr/lib64/libQt5Core.so.5
#21 0x00007ffff12c03dc in QApplicationPrivate::notify_helper(QObject*, QEvent*) ()
   from /usr/lib64/libQt5Widgets.so.5
#22 0x00007ffff12c7ca4 in QApplication::notify(QObject*, QEvent*) () from /usr/lib64/libQt5Widgets.so.5
#23 0x00007fffef65f8d8 in QCoreApplication::notifyInternal2(QObject*, QEvent*) () from /usr/lib64/libQt5Core.so.5
#24 0x00007fffef66204d in QCoreApplicationPrivate::sendPostedEvents(QObject*, int, QThreadData*) ()
   from /usr/lib64/libQt5Core.so.5
#25 0x00007fffef6b9323 in ?? () from /usr/lib64/libQt5Core.so.5
#26 0x00007fffe9152e87 in g_main_context_dispatch () from /usr/lib64/libglib-2.0.so.0
#27 0x00007fffe9153230 in ?? () from /usr/lib64/libglib-2.0.so.0
#28 0x00007fffe91532bc in g_main_context_iteration () from /usr/lib64/libglib-2.0.so.0
#29 0x00007fffef6b894f in QEventDispatcherGlib::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) ()
   from /usr/lib64/libQt5Core.so.5
#30 0x00007fffef65d90a in QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) ()
   from /usr/lib64/libQt5Core.so.5
#31 0x00007fffef6669b4 in QCoreApplication::exec() () from /usr/lib64/libQt5Core.so.5
#32 0x0000000000472361 in main (argc=1, argv=0x7fffffffdbb8) at /home/tom/Programme/gwenview/app/main.cpp:163
Comment 8 Christoph Cullmann 2019-10-09 13:55:01 UTC
Hmm, I tried to look how 

auto prf= thumbnailPix.devicePixelRatioF();
auto pr = thumbnailPix.devicePixelRatio();

are computed internally for pixmaps.
For the that looks normally done via the ::metric function and pixmaps should consistently use there something like

image.devicePixelRatio() * QPaintDevice::devicePixelRatioFScale();

Could you step into the thumbnailPix.devicePixelRatioF() call where it ends up?
Comment 9 Christoph Cullmann 2019-10-09 14:00:14 UTC
I see :/
The issue is: null pixmaps are broken with this, could that be?
Comment 10 Christoph Cullmann 2019-10-09 14:04:19 UTC
Hmm, I am astonished.

Actually, QPixmap has a own overload "qreal devicePixelRatio" (https://doc.qt.io/qt-5/qpixmap.html#devicePixelRatio) that overwrites the wrong https://doc.qt.io/qt-5/qpaintdevice.html#devicePixelRatio

The QPixmap variant is save, the other not.
Thought for me this commit did for sure remove all artifacts.
That is strange :/

Let's revert that again.
Comment 11 Christoph Cullmann 2019-10-09 14:06:41 UTC
Git commit 0bdba41e1dbc9ba48b52e43495091c43270d8be9 by Christoph Cullmann.
Committed on 09/10/2019 at 14:05.
Pushed by cullmann into branch 'master'.

Revert "fix fractional scaling"

This reverts commit 769b6e17a0a1e46777ddc87ef1cc9ce3e8807a16.

devicePixelRatioF seems to be a bad idea for QPixmap, thought I can't reproduce
the crashs here

M  +1    -1    lib/documentview/rasterimageview.cpp
M  +2    -2    lib/thumbnailview/previewitemdelegate.cpp
M  +2    -2    lib/thumbnailview/thumbnailbarview.cpp

https://commits.kde.org/gwenview/0bdba41e1dbc9ba48b52e43495091c43270d8be9
Comment 12 Tom Moebert 2019-10-09 14:11:22 UTC
Thread 1 "gwenview" hit Breakpoint 1, Gwenview::ThumbnailBarItemDelegate::sizeHint (this=0x9b3090, index=...)
    at /home/tom/Programme/gwenview/lib/thumbnailview/thumbnailbarview.cpp:180
180             auto prf= thumbnailPix.devicePixelRatioF();
(gdb) s
QPaintDevice::devicePixelRatioF (this=0x7fffffffcd00) at /usr/include/qt5/QtGui/qpaintdevice.h:87
87          qreal devicePixelRatioF()  const { return metric(PdmDevicePixelRatioScaled) / devicePixelRatioFScale(); }
(gdb) s
QPixmap::metric (this=0x7fffffffcd00, metric=QPaintDevice::PdmDevicePixelRatioScaled) at image/qpixmap.cpp:1476
1476        return data ? data->metric(metric) : 0;
(gdb) print data
$7 = {d = 0x0}
(gdb) print devicePixelRatioFScale()
$5 = 65536


It seems that "data" of type QExplicitlySharedDataPointer<QPlatformPixmap> is not set to an instance.
Comment 13 Christoph Cullmann 2019-10-09 14:13:15 UTC
Hmm, is that a null-pixmap?

In any case, I am now confused why this did fix the rendering issues for me.

I need to try this with the correct monitor setup again.

Locally I can not get the bad rendering with any scale factor now...
Comment 14 Tom Moebert 2019-10-09 14:17:26 UTC
Yes:

(gdb) print thumbnailPix.isNull()
$9 = true
Comment 15 Christoph Cullmann 2019-10-09 14:19:23 UTC
Hmm, ok.

Thanks for the helpful debugging.

Then I got just lucky never to hit that case.

I need to retry this later at home.

I still not get why this commit shall fix then my issues at all, as if it works, it should return the same factor as the qreal overload in QPixmap...
Comment 16 Christoph Cullmann 2019-10-09 14:22:08 UTC
Btw., as you are interested here: Is there interest to get the gwenview bug mails?

I think at the moment they just go to a black hole, or?
Comment 17 Tom Moebert 2019-10-09 14:52:34 UTC
Sry, I'm only occasionally coming here and currently don't really have time to look at the bugs into detail.

I thought Nate receives and tracks all the gwenview bugs.
Comment 18 Nate Graham 2019-10-09 16:15:28 UTC
I triage all new bugs filed anywhere, but hadn't gotten to today's crop of bugs yet. :)
Comment 19 Christoph Cullmann 2019-10-09 16:40:42 UTC
I will re-investigate my issue + the "wrong" fix.

I will post a review request if I find again a workable solution.

Perhaps all is already fine after the memory corruption fix 
https://commits.kde.org/gwenview/64700e39e989001cc60a179f449695c104bf030b
and my issues were just after-effects...
Comment 20 Christoph Cullmann 2019-10-09 16:41:18 UTC
And I want to thank Tom for showing that my change was wrong!
Thanks for that!
Comment 21 Christoph Feck 2019-10-09 17:55:52 UTC
Just in case, I am also (still) monitoring all gwenview bug mails. Was the cause for null pixmaps found? If not, I need to respin the 19.08.2 tarball.
Comment 22 Christoph Feck 2019-10-09 17:58:27 UTC
nvm, the commits went in after tars were created.
Comment 23 Christoph Cullmann 2019-10-09 18:25:10 UTC
> nvm, the commits went in after tars were created.
uf :=) sorry for the issue :/

I will first investigate the stuff here more before opening some request.

Somehow this now works for me even with my patch reverted.

Perhaps I made the error to not test enough after the memory corruption fix

https://commits.kde.org/gwenview/64700e39e989001cc60a179f449695c104bf030b

and that was really the main culprit.

I have taken a look in the stable branch, that Q_FOREACH => for conversion that introduced this issue was done after we branched off.
Comment 24 Christoph Cullmann 2019-10-09 19:51:41 UTC
Hmm, I tried to reproduce my issue, I fail.
Perhaps it really just was that corruption :/
Will re-open if I am able to get the behavior again...