[konsole] [Bug 359662] Terminal Rows are miscalculated if Menu Bar is turned off

[Oleg Girko]

https://bugs.kde.org/show_bug.cgi?id=359662

--- Comment #6 from Oleg Girko <ol+kde at infoserver.ru> ---
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.

-- 
You are receiving this mail because:
You are the assignee for the bug.