Bug 411965 - Rendering issue in dual screen hidpi setup
Summary: Rendering issue in dual screen hidpi setup
Status: RESOLVED FIXED
Alias: None
Product: konsole
Classification: Applications
Component: kpart (show other bugs)
Version: 19.08.1
Platform: Other Linux
: NOR normal
Target Milestone: ---
Assignee: Konsole Developer
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-09-16 12:42 UTC by Guo Yunhe
Modified: 2020-05-18 15:02 UTC (History)
4 users (show)

See Also:
Latest Commit:
Version Fixed In: 19.08.2
Sentry Crash Report:


Attachments
rendering issue when open file in git project under hidpi dual screen setup (1.06 MB, image/png)
2019-09-16 12:42 UTC, Guo Yunhe
Details
konsole with two tabs (51.71 KB, image/png)
2019-09-17 19:15 UTC, Karol Bryd
Details
virtualbox is also affected (131.82 KB, image/png)
2019-09-17 19:15 UTC, Karol Bryd
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Guo Yunhe 2019-09-16 12:42:49 UTC
Created attachment 122680 [details]
rendering issue when open file in git project under hidpi dual screen setup

SUMMARY


STEPS TO REPRODUCE
1. Set up dual screen with different resolution: 1920x1080 and 3840x2160. Place side by side, bottom aligned.
2. Set scale factor to 1.5.
3. Launch Kate.
4. Open a file from Git repository.

OBSERVED RESULT

Project panel opened, and the window content rendering is not usable.


EXPECTED RESULT

Should render properly.


SOFTWARE/OS VERSIONS
Operating System: openSUSE Tumbleweed 20190909
KDE Plasma Version: 5.16.5
KDE Frameworks Version: 5.61.0
Qt Version: 5.13.1
Kernel Version: 5.2.11-1-default
OS Type: 64-bit
Processors: 8 × Intel® Core™ i7-8550U CPU @ 1.80GHz
Memory: 31.2 GiB


ADDITIONAL INFORMATION

The issue only happens when you have dual screen hidpi with different sizes. If you only have one screen, it is okay. If you don't use hidpi scaling, it is okay. If the two screen have same resolution, it is also okay.
Comment 1 Guo Yunhe 2019-09-16 12:46:34 UTC
If you open a normal file not in Git/SVN repo, it is still okay. KWrite and KDevelop is always fine. So I think this is something caused by the project panel of Kate.
Comment 2 Guo Yunhe 2019-09-16 16:09:34 UTC
Comment out this line can resolve the rendering issue:

addTab(new KateProjectInfoViewTerminal(pluginView, projectPath), i18n("Terminal (.kateproject)"));

Looking into KateProjectInfoViewTerminal class now.
Comment 3 Guo Yunhe 2019-09-16 16:38:51 UTC
Disable this single line solved the issue (addons/project/kateprojectinfoviewterminal.cpp):

    qobject_cast<TerminalInterface*>(m_konsolePart)->showShellInDir(m_directory);
Comment 4 Christoph Cullmann 2019-09-16 16:44:25 UTC
Hmm, interesting :/
Perhaps KonsolePart does strange things then.
Does the internal console show up at all?
Comment 5 Guo Yunhe 2019-09-16 16:45:53 UTC
Yes, it shows. After disabling that line, the console is not at your project root directory. But it works.
Comment 6 Guo Yunhe 2019-09-16 16:49:43 UTC
And I already find a fix:

Change:

qobject_cast<TerminalInterface *>(m_konsolePart)->showShellInDir(m_directory);

to:

qobject_cast<TerminalInterface *>(m_konsolePart)->sendInput(QStringLiteral(" cd ") + KShell::quoteArg(m_directory) + QStringLiteral(" && clear\n"));

Work in the same way.

KonsolePart might need a fix, too. Because showShellInDir() is a very normal function call and shouldn't cause rendering issues.
Comment 7 Kai Uwe Broulik 2019-09-16 17:06:15 UTC
It ends up calling winId() which I have seen cause strange havoc in applications in Dolphin and KMail in conjunction with high dpi and multi screen. I can't find the bug report unfortunately and I was pretty sure it got fixed in Qt at some point :/
Comment 9 Guo Yunhe 2019-09-16 17:17:30 UTC
I feel Qt often breaks things again when they introduce something new.

Here is the patch for Kate https://phabricator.kde.org/D23998

Not sure how we can fix it in Konsole or Qt.
Comment 10 Christoph Cullmann 2019-09-16 18:13:26 UTC
Before we start to merge this:

https://invent.kde.org/kde/kate/merge_requests/19

Could this be worked around directly inside that API function in Konsole before all users convert to such workaround like Dolphin?
Comment 11 Guo Yunhe 2019-09-17 13:21:00 UTC
Find this line in QuickEditor.cpp:

setGeometry(0, 0, static_cast<int>(mPixmap.width() * dprI), static_cast<int>(mPixmap.height() * dprI));

I have a 1920x1080 and 2560x1440 setup with x1.5 scale side by side. If I change the width to 2560 or less, it works:

setGeometry(0, 0, 2560, static_cast<int>(mPixmap.height() * dprI));

Anything bigger causes rendering issue:

setGeometry(0, 0, 2561, static_cast<int>(mPixmap.height() * dprI));
Comment 12 Guo Yunhe 2019-09-17 13:21:48 UTC
Oops, wrong ticket. Sorry.
Comment 13 Karol Bryd 2019-09-17 19:15:11 UTC
Created attachment 122697 [details]
konsole with two tabs
Comment 14 Karol Bryd 2019-09-17 19:15:51 UTC
Created attachment 122698 [details]
virtualbox is also affected
Comment 15 Karol Bryd 2019-09-17 19:17:42 UTC
Comment on attachment 122697 [details]
konsole with two tabs

konsole is also affected when there are two screens (hidpi) with enabled scaling (1.5) but with the same resolution. Attached screenshot shows maximized konsole window with two tabs, the other screenshot shows virtualbox maximized on the same display.
Comment 16 Christoph Cullmann 2019-09-17 19:29:42 UTC
I will have access to some hidpi screens in the next weeks, I can try to trace this then.
Comment 17 Karol Bryd 2019-09-17 19:32:18 UTC
It started happening yesterday after I updated my Suse Tumbleweed, so whatever update was yesterday for QT/KDE broke it.
Comment 18 Karol Bryd 2019-09-22 09:00:04 UTC
According to the zypper update history what broke Konsole, Virtualbox and possibly other applications is QT 13.1 update.
Comment 19 Guo Yunhe 2019-09-22 10:26:45 UTC
I can also confirm that Qt 5.13.1 (openSUSE Tumbleweed) broke VirtualBox and Qt Creator. But Konsole is fine on my computer.
Comment 20 Karol Bryd 2019-09-22 20:25:35 UTC
I did some more investigation and turned out that I had custom CSS enabled for konsole to more visibly mark the active tab and following CSS broke it in conjunction with QT 5.13.1:

QWidget, QTabWidget::pane, QTabWidget::tab-bar {
    background-color: rgb(45, 45, 45);
}

I removed it and it is OK now. Sorry for confusion.
Comment 21 Guo Yunhe 2019-09-27 19:57:44 UTC
I track down the issue and the source is this line in konsole:

return window->winId();

https://cgit.kde.org/konsole.git/tree/src/Session.cpp#n216
Comment 22 Christoph Cullmann 2019-09-29 18:12:31 UTC
Git commit 343bc39548f0c42386ba983167b075b2940d626b by Christoph Cullmann.
Committed on 29/09/2019 at 18:10.
Pushed by cullmann into branch 'fix-win-id-rendering'.

fix rendering artifacts introduced by calling winId

winId shall not be called on non-native widgets

just call effectiveWinId that will handle this for us

M  +10   -9    src/Session.cpp

https://invent.kde.org/kde/konsole/commit/343bc39548f0c42386ba983167b075b2940d626b
Comment 23 Christoph Cullmann 2019-09-29 18:12:34 UTC
Git commit 343bc39548f0c42386ba983167b075b2940d626b by Christoph Cullmann.
Committed on 29/09/2019 at 18:10.
Pushed by scmsync into branch 'fix-win-id-rendering'.

fix rendering artifacts introduced by calling winId

winId shall not be called on non-native widgets

just call effectiveWinId that will handle this for us

M  +10   -9    src/Session.cpp

https://commits.kde.org/konsole/343bc39548f0c42386ba983167b075b2940d626b
Comment 24 Christoph Cullmann 2019-09-29 18:14:38 UTC
I think I have some fix:

https://invent.kde.org/kde/konsole/merge_requests/33

For me that removes the distortion.

I think winId introduces native widgets in the wrong places.
Comment 25 Guo Yunhe 2019-09-29 19:14:38 UTC
Yes, I can confirm that patch works.

I am not sure if the winId() function is working as described:

> If a widget is non-native (alien) and winId() is invoked on it, that widget will be provided a native handle.

https://doc.qt.io/qt-5/qwidget.html#winId
Comment 26 Nate Graham 2019-09-30 13:29:22 UTC
The bug only gets closed when the patch is merged. :)
Comment 27 Christoph Cullmann 2019-09-30 13:35:23 UTC
Yep, forgot to reopen ;=)
Comment 28 Christoph Cullmann 2019-09-30 18:30:17 UTC
Git commit 60ab2125e14464b1b8b11155969664703f62c88b by Christoph Cullmann.
Committed on 30/09/2019 at 18:30.
Pushed by cullmann into branch 'master'.

fix rendering artifacts introduced by calling winId

winId shall not be called on non-native widgets

just call effectiveWinId that will handle this for us


(cherry picked from commit 343bc39548f0c42386ba983167b075b2940d626b)

M  +10   -9    src/Session.cpp

https://invent.kde.org/kde/konsole/commit/60ab2125e14464b1b8b11155969664703f62c88b
Comment 29 Christoph Cullmann 2019-09-30 18:30:21 UTC
Git commit 60ab2125e14464b1b8b11155969664703f62c88b by Christoph Cullmann.
Committed on 30/09/2019 at 18:30.
Pushed by scmsync into branch 'master'.

fix rendering artifacts introduced by calling winId

winId shall not be called on non-native widgets

just call effectiveWinId that will handle this for us


(cherry picked from commit 343bc39548f0c42386ba983167b075b2940d626b)

M  +10   -9    src/Session.cpp

https://commits.kde.org/konsole/60ab2125e14464b1b8b11155969664703f62c88b
Comment 30 Christoph Cullmann 2019-09-30 18:33:43 UTC
Keep this open for potential backport.
I run into this now every time I use Kate on my second screen.
If somebody else OKs this, I would backport this.
Comment 31 Christoph Cullmann 2019-09-30 18:48:10 UTC
And the sad truth here is: whereas this avoid any rendering artifacts, I am not sure the id is not in most cases just 0, even thought the docs don't hint to that...
Comment 32 Christoph Cullmann 2019-09-30 18:53:01 UTC
For normal konsole starts, I get a useful WINDOWID exported:

declare -x WINDOWID="69206023"

For konsole parts embedded in e.g. Kate, I get not mostly 0.

Perhaps the problem is the way the session is started before some widget is around (or some widget with proper parent).

Perhaps a fallback should be there to QApplication::activeWindow() if we find no valid id... It's guessing anyways.
Comment 33 Christoph Cullmann 2019-09-30 19:00:35 UTC
I debugged a bit more, and yes, for the Kate case, the widget->nativeParentWidget() function returns nullptr.

Therefore you get some 0 id from the effectiveWinId().

For a normal konsole start, all is fine.

Either one is OK with a "bad" WINDOWID env var or one needs to add e.g. a fallback to QApplication::activeWindow()->winId() (if a window there).

This fallback could be used for the "no views" case, too.

But I think that is something for the konsole devs to decide.
At least the current code doesn't introduce native widgets our of thin air.
Comment 34 Christoph Cullmann 2019-09-30 19:10:31 UTC
Ok, for Kate I can solve that, the part widget() just needs to have some proper parent.
Comment 35 Christoph Cullmann 2019-09-30 19:11:20 UTC
Git commit ec4391fb297ff3f91013c7cd4bc5bf82771f516d by Christoph Cullmann.
Committed on 30/09/2019 at 19:09.
Pushed by cullmann into branch 'master'.

load terminal on demand to have some parent widget for WINDOWID computation

M  +10   -5    addons/project/kateprojectinfoviewterminal.cpp
M  +7    -0    addons/project/kateprojectinfoviewterminal.h

https://invent.kde.org/kde/kate/commit/ec4391fb297ff3f91013c7cd4bc5bf82771f516d
Comment 36 Christoph Cullmann 2019-09-30 19:11:23 UTC
Git commit ec4391fb297ff3f91013c7cd4bc5bf82771f516d by Christoph Cullmann.
Committed on 30/09/2019 at 19:09.
Pushed by scmsync into branch 'master'.

load terminal on demand to have some parent widget for WINDOWID computation

M  +10   -5    addons/project/kateprojectinfoviewterminal.cpp
M  +7    -0    addons/project/kateprojectinfoviewterminal.h

https://commits.kde.org/kate/ec4391fb297ff3f91013c7cd4bc5bf82771f516d
Comment 37 Christoph Cullmann 2019-09-30 19:11:47 UTC
With this I think this issue is really solved.
Konsole will never invent bad native widgets and Kate will provide proper parents.
Comment 38 Christoph Cullmann 2019-09-30 19:12:10 UTC
Git commit f295734722808827e3cb0743f6588d01a2ae2a0b by Christoph Cullmann.
Committed on 30/09/2019 at 19:12.
Pushed by cullmann into branch 'Applications/19.08'.

load terminal on demand to have some parent widget for WINDOWID computation


(cherry picked from commit ec4391fb297ff3f91013c7cd4bc5bf82771f516d)

M  +10   -5    addons/project/kateprojectinfoviewterminal.cpp
M  +7    -0    addons/project/kateprojectinfoviewterminal.h

https://invent.kde.org/kde/kate/commit/f295734722808827e3cb0743f6588d01a2ae2a0b
Comment 39 Christoph Cullmann 2019-09-30 19:12:14 UTC
Git commit f295734722808827e3cb0743f6588d01a2ae2a0b by Christoph Cullmann.
Committed on 30/09/2019 at 19:12.
Pushed by scmsync into branch 'Applications/19.08'.

load terminal on demand to have some parent widget for WINDOWID computation


(cherry picked from commit ec4391fb297ff3f91013c7cd4bc5bf82771f516d)

M  +10   -5    addons/project/kateprojectinfoviewterminal.cpp
M  +7    -0    addons/project/kateprojectinfoviewterminal.h

https://commits.kde.org/kate/f295734722808827e3cb0743f6588d01a2ae2a0b
Comment 40 Christoph Cullmann 2019-09-30 19:12:47 UTC
I cherry-picked the Kate change to 19.08.
I will wait for the Konsole devs to judge if the other change is OK.
Comment 41 Nate Graham 2019-09-30 19:15:29 UTC
+1 for backporting this latest version to Konsole's stable branch.
Comment 42 Christoph Cullmann 2019-09-30 19:19:15 UTC
We still have some days before the 19.08.2 tagging, I think we can wait for judgement of the console devs, wrote more stuff to

https://invent.kde.org/kde/konsole/merge_requests/33

Perhaps they want some additional fallbacks.

In any case, without this patch, you can't use Kate on multi-screen setups that scale at all, at least here it is unusable ;=))
Comment 43 Christoph Cullmann 2019-10-02 18:29:21 UTC
Hi, I would like to backport my patch to stable, is that ok?
Otherwise Kate will just startup as a large artifact on multi-head setups in many cases.
Comment 44 Guo Yunhe 2019-10-02 18:53:03 UTC
Why not :-)
Comment 45 Christoph Cullmann 2019-10-03 14:44:15 UTC
I will backport this, without it, Kate and other applications embedding Konsole will be unusable on multi-screen setups with scaling.
Comment 46 Christoph Cullmann 2019-10-03 14:44:59 UTC
Git commit 4056588da2421144ee84deac791400a0f3c0f9b1 by Christoph Cullmann.
Committed on 03/10/2019 at 14:44.
Pushed by cullmann into branch 'Applications/19.08'.

fix rendering artifacts introduced by calling winId

winId shall not be called on non-native widgets

just call effectiveWinId that will handle this for us


(cherry picked from commit 343bc39548f0c42386ba983167b075b2940d626b)

(cherry picked from commit 60ab2125e14464b1b8b11155969664703f62c88b)

M  +10   -9    src/Session.cpp

https://invent.kde.org/kde/konsole/commit/4056588da2421144ee84deac791400a0f3c0f9b1
Comment 47 Christoph Cullmann 2019-10-03 14:45:04 UTC
Git commit 4056588da2421144ee84deac791400a0f3c0f9b1 by Christoph Cullmann.
Committed on 03/10/2019 at 14:44.
Pushed by scmsync into branch 'Applications/19.08'.

fix rendering artifacts introduced by calling winId

winId shall not be called on non-native widgets

just call effectiveWinId that will handle this for us


(cherry picked from commit 343bc39548f0c42386ba983167b075b2940d626b)

(cherry picked from commit 60ab2125e14464b1b8b11155969664703f62c88b)

M  +10   -9    src/Session.cpp

https://commits.kde.org/konsole/4056588da2421144ee84deac791400a0f3c0f9b1
Comment 48 Christoph Cullmann 2019-10-03 14:45:19 UTC
done
Comment 49 Guo Yunhe 2020-05-18 15:02:32 UTC
Here is a similar issue in qBittorrent. But it doesn't have any winId() call. Any ideas?