Bug 371568 - Krita opens main window and then maximizes it, instead of showing maximized in one go
Summary: Krita opens main window and then maximizes it, instead of showing maximized i...
Status: RESOLVED FIXED
Alias: None
Product: krita
Classification: Applications
Component: General (show other bugs)
Version: 3.0.1.1
Platform: Other Linux
: NOR normal
Target Milestone: ---
Assignee: Krita Bugs
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-10-24 11:35 UTC by Martin Flöser
Modified: 2016-10-25 07:23 UTC (History)
1 user (show)

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


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Flöser 2016-10-24 11:35:05 UTC
When starting Krita on KWin one can observe that the maximize animation is run. This means that Krita opens a window and afterwards changes it to maximized. I confirmed this by adding debug statements into KWin. This is part of the reason why bug #371284 happens - it hits a condition which normally cannot happen.

Reproducible: Always

Steps to Reproduce:
1. Start Krita on KWin/X11

Actual Results:  
Maximize animation is run

Expected Results:  
Maximize animation is not run, the window opens as maximized directly.

Qt provides API to show a window maximized. E.g. QWindow::showMaximized or QWidget::showMaximized
Comment 1 Halla Rempt 2016-10-24 12:29:25 UTC
Hm, that's "interesting"... The window geometry restoration code is ancient, probably dates back to Qt 1.x and is shared with all the calligra applications. I'm not even sure I really understand what it's trying to do.
Comment 2 Halla Rempt 2016-10-24 12:43:26 UTC
Um, actually, isn't this a Qt bug then? I know of no KDE or Qt application that uses showMaximized() directly, and the Qt documentation says to use 

http://doc.qt.io/qt-5/qwidget.html#saveGeometry

and 

http://doc.qt.io/qt-5/qwidget.html#restoreGeometry

Which is what we do:

void KisMainWindow::closeEvent(QCloseEvent *e)
{
...
    cfg.writeEntry("ko_windowstate", saveState().toBase64());
...
}

and

void KisMainWindow::initializeGeometry()
{
    // if the user didn's specify the geometry on the command line (does anyone do that still?),
    // we first figure out some good default size and restore the x,y position. See bug 285804Z.
    KConfigGroup cfg( KSharedConfig::openConfig(), "MainWindow");
    QByteArray geom = QByteArray::fromBase64(cfg.readEntry("ko_geometry", QByteArray()));
    if (!restoreGeometry(geom)) {
...
}
Comment 3 Martin Flöser 2016-10-24 13:37:20 UTC
> Um, actually, isn't this a Qt bug then?

I'm not excluding the possibility that it's a Qt bug.

> I know of no KDE or Qt application that uses showMaximized() directly

It's a hardly used feature that applications request to open maximized directly.

> void KisMainWindow::initializeGeometry()

Can you point me to the code for that, then I can go through what it does.
Comment 5 Martin Flöser 2016-10-24 14:25:55 UTC
Ok, found the problematic code:
void KisMainWindow::showEvent(QShowEvent *e)
{
    KXmlGuiWindow::showEvent(e);

    if (!d->geometryInitialized) {
        /**
         * We should move the window only *after* it has been shown on
         * screen, otherwise it will become owned by a wrong screen, which
         * will make positioning of all the child widgets wrong.
         * (see bug https://bugs.kde.org/show_bug.cgi?id=362025)
         *
         * This is actually a bug/feature of Qt 5.5.x and it has been
         * fixed in Qt 5.6.0. So we can avoid this delay on newer versions
         * of Qt.
         */
        QTimer::singleShot(1, this, SLOT(initializeGeometry()));
        d->geometryInitialized = true;
    }
}

Thus we have a show (normal geometry) with the initializeGeometry later. Which is exactly what I observe in KWin.
Comment 6 Halla Rempt 2016-10-24 14:35:46 UTC
Ah, right! Well, since Qt 5.6 is now the version used for all our releases and the version on most distributions, I guess we can remove this again. I'll also propose raising the minimum version of Qt, but that might run into some resistance.
Comment 7 Halla Rempt 2016-10-25 07:23:28 UTC
Git commit 3b62983593fbf5c293e603a2b2c92c83d70acc57 by Boudewijn Rempt.
Committed on 25/10/2016 at 07:23.
Pushed by rempt into branch 'master'.

Make Qt 5.6.0 the minimum with an override

Qt 5.6 has fixed the bug for which this reverted commit was
a hacky fix; the hacky fix caused problems for kwin because
we first show, then position the main window.

Revert "Fix wrong positioning of children/popup widgets"

This reverts commit ceff0e241bbd1c14b4b5ee5de11743b92fb7d760.

M  +3    -3    CMakeLists.txt
M  +2    -1    libs/ui/KisApplication.cpp
M  +1    -21   libs/ui/KisMainWindow.cpp
M  +0    -1    libs/ui/KisMainWindow.h

http://commits.kde.org/krita/3b62983593fbf5c293e603a2b2c92c83d70acc57