Bug 359662 - Terminal Rows are miscalculated if Menu Bar is turned off
Summary: Terminal Rows are miscalculated if Menu Bar is turned off
Status: RESOLVED FIXED
Alias: None
Product: konsole
Classification: Applications
Component: general (show other bugs)
Version: 15.12.2
Platform: Arch Linux Linux
: NOR normal
Target Milestone: ---
Assignee: Konsole Developer
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-02-22 08:13 UTC by Radics Péter
Modified: 2019-12-03 03:48 UTC (History)
8 users (show)

See Also:
Latest Commit:
Version Fixed In: 19.12.0


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Radics Péter 2016-02-22 08:13:52 UTC
the "Terminal Size" settings was recently fixed (see bug 345403), but the Terminal Rows are miscalculated on start-up if the menu bar is disabled ("Show MenuBar" option). Apparently the calculation is done assuming the menu bar is shown. If I want to get a 80x25 terminal with MenuBar disabled, I have to set Columns to 80 and Rows to 23 (instead of 25).  If I start up konsole with the above settings while the MenuBar is enabled, I get a 80x23 terminal (with menubar).

Reproducible: Always

Steps to Reproduce:
1. In Settings->Configure Konsole->General: 
  - set "Show menubar by default to false" (unchecked)
  - set "Use current window size on next startup" to false (unchecked)
2. In Edit Current Profile->General->Terminal Size, set Columns to 80, Rows to 23.
3. start a new konsole with the above settings


Actual Results:  
A konsole starts up with 80x25 geometry (without menubar)

Expected Results:  
A konsole starts up with 80x23 geometry (without menubar)
Comment 1 Roman Gilg 2016-05-01 11:33:08 UTC
FYI: https://git.reviewboard.kde.org/r/127803/
Comment 2 Kurt Hindenburg 2016-05-21 20:30:45 UTC
Yes this has always been an issue - I thought there was another old report for this.

The patch does appear to fix it.
Comment 3 Kurt Hindenburg 2016-05-21 22:48:03 UTC
Git commit 0a5e2b34971e339561e60c674a6fa5ec7eb941da by Kurt Hindenburg.
Committed on 21/05/2016 at 22:43.
Pushed by hindenburg into branch 'master'.

When setting window size, take into account when menubar is not visible

Take into account menubar visibility when sizing a new window.
Note that this is only used when 'Use current window size on next
startup' is unchecked.

Thanks to Roman Gilg subdiff gmail com for patch
REVIEW: 27803

M  +2    -0    src/MainWindow.cpp

http://commits.kde.org/konsole/0a5e2b34971e339561e60c674a6fa5ec7eb941da
Comment 4 Radics Péter 2018-12-14 16:34:19 UTC
This happens again since the latest update (konsole 18.12.0-1 on arch linux)
Comment 5 Patrick Silva 2019-03-01 19:34:02 UTC
Here konsole 18.12.2 starts up with 112x29 geometry (without menubar) after the steps provided by reporter. I use fractional display scaling (1.2 factor).

Operating System: Arch Linux 
KDE Plasma Version: 5.15.2
KDE Frameworks Version: 5.55.0
Qt Version: 5.12.1
Comment 6 Oleg Girko 2019-06-19 22:34:09 UTC
I've spent several days trying to understand what's going on, but my knowledge of layout mechanics in Qt is not sufficient to find out the root cause. I'll try to summarise my findings here in hope that it will be helpful for somebody more knowledgeable than me to solve this bug.

# My setup

I have konsole5-18.12.3-2.fc30.x86_64 package installed in Fedora 30.
My laptop's screen size is 3200x1800 pixels (295x167 millimeters).
DPI is explicitly set to 192 in KDE settings.
Font in Konsole profile is set to Consolas, size 7 (yes, this is small).
Menu and tabbar in Konsole are disabled.
I've added tons of debug prints to konsole5 source code, but except of debug print statements (using qWarning()), the source code and build procedure is identical to this Fedora package:
https://koji.fedoraproject.org/koji/buildinfo?buildID=1247868
I've set "Use current window size on next startup" to false, and Terminal Size in my only profile to 80x24.

# What's going on

Everything is happening in Application::newInstance() method.

1. MainWindow is created. Then MainWindow::createSession() is called that in turn calls ViewManager::createView(Session *) that creates TabbedViewContainer.
Constructor of TabbedViewContainer calls TabbedViewContainer::konsoleConfigChanged() that calls tabBar()->setVisible(false);

2. Later in ViewManager::createView(Session *) another overload with the same name is called: ViewManager::createView(Session *, TabbedViewContainer *, int) that creates TerminalDisplay. Then it calls TerminalDisplay::setSize(80, 24) on newly created TerminalDisplay.

I have the following before TerminalDisplay::setSize(80, 24):

ViewSplitter:
- isVisible() = false
- contentsRect() =  QRect(0,0 640x480)
- sizeHint() =  QSize(8, 8)
- size() =  QSize(640, 480)
- geometry() =  QRect(0,0 640x480)
TabbedViewContainer:
- isVisible() = false
- contentsRect() =  QRect(0,0 640x480)
- sizeHint() =  QSize(8, 8)
- size() =  QSize(640, 480)
- geometry() =  QRect(0,0 640x480)
QTabBar:
- isVisible() = false
- contentsRect() =  QRect(0,0 0x0)
- sizeHint() =  QSize(0, 0)
- size() =  QSize(0, 0)
- geometry() =  QRect(0,0 0x0)
TerminalDisplay:
- isVisible() = false
- contentsRect() =  QRect(0,0 640x480)
- sizeHint() =  QSize(-1, -1)
- size() =  QSize(640, 480)
- geometry() =  QRect(0,0 640x480)

As you see, these values are just placeholders, nothing interesting.

I have the following after TerminalDisplay::setSize(80, 24):

ViewSplitter:
- isVisible() = false
- contentsRect() =  QRect(0,0 640x480)
- sizeHint() =  QSize(8, 8)
- size() =  QSize(640, 480)
- geometry() =  QRect(0,0 640x480)
TabbedViewContainer:
- isVisible() = false
- contentsRect() =  QRect(0,0 640x480)
- sizeHint() =  QSize(8, 8)
- size() =  QSize(640, 480)
- geometry() =  QRect(0,0 640x480)
QTabBar:
- isVisible() = false
- contentsRect() =  QRect(0,0 0x0)
- sizeHint() =  QSize(0, 0)
- size() =  QSize(0, 0)
- geometry() =  QRect(0,0 0x0)
TerminalDisplay:
- isVisible() = false
- contentsRect() =  QRect(0,0 640x480)
- sizeHint() =  QSize(819, 482)
- size() =  QSize(640, 480)
- geometry() =  QRect(0,0 640x480)

What changed here is TerminalDisplay::sizeHint() changed to something that makes sense for 80x24 display.

3. Later in ViewManager::createView(Session *, TabbedViewContainer *, int), container->addView() is called. TabbedViewContainer::addView() method just calls QTabWidget::addTab() method with newly created TerminalDisplay as its first argument, but it causes geometry of widgets to be recalculated dramatically. This is what I have after this:

ViewSplitter:
- isVisible() = false
- contentsRect() =  QRect(0,0 640x480)
- sizeHint() =  QSize(827, 535)
- size() =  QSize(640, 480)
- geometry() =  QRect(0,0 640x480)
TabbedViewContainer:
- isVisible() = false
- contentsRect() =  QRect(0,0 640x480)
- sizeHint() =  QSize(827, 535)
- size() =  QSize(640, 480)
- geometry() =  QRect(0,0 640x480)
QTabBar:
- isVisible() = false
- contentsRect() =  QRect(0,0 0x0)
- sizeHint() =  QSize(129, 45)
- size() =  QSize(0, 0)
- geometry() =  QRect(0,0 0x0)
TerminalDisplay:
- isVisible() = false
- contentsRect() =  QRect(0,0 640x480)
- sizeHint() =  QSize(819, 482)
- size() =  QSize(640, 480)
- geometry() =  QRect(0,0 640x480)

Look at sizeHint() of TabbedViewContainer and ViewSplitter that encloses it. 827x535 is way bigger than 819x482 of TerminalDisplay. Height of 535 is even 8 pixels higher that 482 (height of TerminalDisplay) + 45 (height of QTabBar).

I think, the key question to understand the cause of the problem is what happened here.

How sizeHint() of TabbedViewContainer (that is essentially QTabWidget) became significantly bigger than one of TerminalDisplay that was just added to it, despite its tabBar() being hidden?

Remember this size: 827x535, it is what will cause problems later.

4. In the end of Application::newInstance(), Application::finalizeNewMainWindow(MainWindow *) is called.
First, it calls:
window->resize(window->sizeHint());
Essentially, it resizes the main window to that problematic size of 827x535 that was miscalculated in the previous step.

5. Then, Application::finalizeNewMainWindow(MainWindow *) calls:
window->show();
This triggers the following chain of events: TerminalDisplay::resizeEvent() and TerminalDisplay::showEvent().

6. TerminalDisplay::resizeEvent() does not detect yet that TerminalDisplay is resized to the wrong size. However, its contentsRect() gets changed somewhere deep inside call stack when layout recalculation is triggered by some innocently looking function.

The stack trace leading to this recalculation is following. I've omited internal details of signal handling and internal Qt methods to simplify it. Also, I've omited line numbers because they mean nothing in code heavily edited with debug prints. Also, I've removed Konsole namespace from method names and added some comments to this stack trace.

QWidget::setGeometry(const QRect & = (0, 0, 826, 534))
# this has TerminalDisplay type
QLayoutPrivate::doResize(const QSize & = (827, 535))
QWidget::setGeometry(const QRect & = (0, 0, 826, 534))
# this has QStackedWidget type
QTabWidget::setUpLayout(false)
# this has TabbedViewContainer type
QTabWidget::setTabText(...)
# this has TabbedViewContainer type
TabbedViewContainer::updateTitle(ViewProperties *)
emit ViewProperties::titleChanged(...)
ViewProperties::setTitle(const QString &)
# this has SessionController type
SessionController::sessionAttributeChanged()
emit sessionAttributeChanged()
Session::setTitle(TitleRole, const QString &)
SessionController::snapshot()
emit started()
Session::run()
emit imageSizeInitialized()
Emulation::setImageSize(24, 80)
Konsole::Session::updateTerminalSize()
emit TerminalDisplay::changedContentSizeSignal(480, 800)
TerminalDisplay::updateImageSize()
TerminalDisplay::resizeEvent(QResizeEvent *event)
# event->size() =  QSize(819, 482)
QWidget::setVisible(...)
# this has QStackedWidget type
QWidget::setVisible(...)
# this has TabbedViewContainer type
QWidget::setVisible(...)
# this has ViewSplitter type
QWidget::setVisible(...)
# this has MainWindow type
Application::finalizeNewMainWindow(MainWindow *)
# calls window->show()
Application::newInstance()
kdemain()

As tou see, TerminalDisplay::resizeEvent() receives QResizeEvent with a good size. Deeper in call stack it calls TerminalDisplay::changedContentSizeSignal(), also with the right size. Deeper Emulation::setImageSize() is called with right args and emits imageSizeInitialized() signal.

However, deeper in call stack TabbedViewContainer::updateTitle(ViewProperties *)
 calls QTabWidget::setTabText() method on TabbedViewController that in turn triggers QTabWidget::setUpLayout(false). This resizes TerminalDIsplay to 826x534 (1x1 pixel smaller than the size miscalculated above).

7. The final act of this tragedy happens in TerminalDisplay::showEvent() method that is also triggered by the same window->show(); in Application::finalizeNewMainWindow(MainWindow *) method.
Here TerminalDisplay notices that its contentsRect() changed to QRect(0,0 827x535) and recalculates its size in TerminalDisplay::calcGeometry(). Essentially, it has the same effect as if the window was resized to 827x535. Even a tooltip showing terminal's new size (80x26) is shown.

That's all I've discovered so far. I hope it will be useful to find out what happened and fix this bug.
Comment 7 Oleg Girko 2019-11-25 16:45:38 UTC
Looks like this problem is fixed.

Probably, by this commit:
https://cgit.kde.org/konsole.git/commit/?id=efb621d091c05f1195982667782507ffe27ed8cf