<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/127631/">https://git.reviewboard.kde.org/r/127631/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On April 11th, 2016, 7:45 a.m. CEST, <b>Martin Gräßlin</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><blockquote style="text-rendering: inherit;padding: 0 0 0 1em;border-left: 1px solid #bbb;white-space: normal;margin: 0 0 0 0.5em;line-height: inherit;">
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">otherwise kwindowsystem::self() is nullptr</p>
</blockquote>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">how can KWindowSystem::self() be null? And why should that have anything to do with KWin? KWindowSystem does not depend on a window manager running.</p></pre>
</blockquote>
<p>On April 11th, 2016, 8:24 a.m. CEST, <b>Thomas Lübking</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">The entire thing sounds as if the problem would be that the session restorage in kwin overrides the placement for restored windows (with the restored position)</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">That's however not a bug (at least it's intended) and I've no idea why that's relevant to the weird geometry handling of SNI items (which looks <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">faaaar</em> too complex - the position of remapped windows is usually maintained by QWidget ...)</p></pre>
</blockquote>
<p>On April 11th, 2016, 8:53 a.m. CEST, <b>Anthony Fieroni</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I saw the code, it looks like KWindowSystem not depend on Kwin, but Kwin <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">must</em> be started <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">before</em> use of kwindowsystem. Thomas may right, because setGeometry even not work on sessions restored app.
When session was finish if start apps with kmail --session session-id everything wors fine.</p></pre>
</blockquote>
<p>On April 11th, 2016, 9:08 a.m. CEST, <b>Anthony Fieroni</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Now, after Thomas comment, i even think only widget->show() must works because widget has internal frameGeometry and position.</p></pre>
</blockquote>
<p>On April 11th, 2016, 10:38 a.m. CEST, <b>Martin Gräßlin</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><blockquote style="text-rendering: inherit;padding: 0 0 0 1em;border-left: 1px solid #bbb;white-space: normal;margin: 0 0 0 0.5em;line-height: inherit;">
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">but Kwin must be started before use of kwindowsystem</p>
</blockquote>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">no, really there is no reason for that. KWindowSystem doesn't depend on a window manager running.</p></pre>
</blockquote>
<p>On April 11th, 2016, 10:54 a.m. CEST, <b>Anthony Fieroni</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Then, correct KWin to kwindowsystem to start working. If there is no bug this will works on all cases -> https://git.reviewboard.kde.org/r/127216/ but it's not.</p></pre>
</blockquote>
<p>On April 11th, 2016, 11:01 a.m. CEST, <b>Martin Gräßlin</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I don't know why you have problems, but it's impossible that KWindowSystem::self() returns a nullptr. You can check the code to verify. Something is really broken on your side, but I don't know what. Maybe missing plugins installed?</p></pre>
</blockquote>
<p>On April 11th, 2016, 11:15 a.m. CEST, <b>Thomas Lübking</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">it's not a hard dependecy on the instace, the sni geometry handling is just plain broken.
the workaround operates on configure events on the mapped window which will go nowhere if there's no running wm.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">the client needs to handle the no-wm case, ie. configure the window correctly, but to repeat myself: such requirement implies that either sni or qwidget/qwindow is completely fucked up.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">qwindow/qwidget::setGeometry should justwork(tm)</p></pre>
</blockquote>
<p>On April 11th, 2016, 11:51 a.m. CEST, <b>Anthony Fieroni</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">setGeometry <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">not</em> works, i can confirm since i'm test it. Nothing works if Kwin is started <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">after</em> usage of kwindowsystem::self().
Martin yeah nullprt is impossible.</p></pre>
</blockquote>
<p>On April 11th, 2016, 11:55 a.m. CEST, <b>Martin Gräßlin</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">then this is a bug in Qt's xcb plugin and needs to be fixed there. Setting a geometry without a window manager must be possible.</p></pre>
</blockquote>
<p>On April 11th, 2016, 12:29 p.m. CEST, <b>Anthony Fieroni</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Wait a little, the bug is not in xcb. I'm not clear or what. This <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">happend</em> only on session restored apps, when kwindowsystem::self() is before kwin statup, only in this case. If run a app after kwin is started <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">all</em> works fine setGeometry(), move() - no problems.</p></pre>
</blockquote>
<p>On April 11th, 2016, 12:37 p.m. CEST, <b>Anthony Fieroni</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Thomas Lübking wrote:
The entire thing sounds as if the problem would be that the session restorage in kwin overrides the placement for restored windows (with the restored position)</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">For me, this is best explanation. Adn give you the easiest, no pain, working solution - wait kwin to start completely.</p></pre>
</blockquote>
<p>On April 11th, 2016, 12:45 p.m. CEST, <b>Martin Gräßlin</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><blockquote style="text-rendering: inherit;padding: 0 0 0 1em;border-left: 1px solid #bbb;white-space: normal;margin: 0 0 0 0.5em;line-height: inherit;">
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Adn give you the easiest, no pain, working solution - wait kwin to start completely.</p>
</blockquote>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I disagree. Working around such problems in the session startup seems wrong. I would say excluse the sni windows from session restore, which can be done from Qt API.</p></pre>
</blockquote>
<p>On April 11th, 2016, 5:19 p.m. CEST, <b>Thomas Lübking</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Can we please get few things straight?</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">You say that ::setGeometry doesn't work, but https://git.reviewboard.kde.org/r/127216/diff/5#index_header makes use of it (uses the propsed internally saved position and restores it with QWidget::move what's basically a ::setGeometry special case)</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;"><div class="codehilite" style="background: #f8f8f8"><pre style="line-height: 125%">=> Does this work (leaving aside session restorage), yes or no?
</pre></div>
</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Assuming it <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">does</em> work (except for session restorage) the claims to require a WM for operative KWindowSystem invocation are gladfully, since you're not using KWindowSystem at all.</p>
<h2 style="font-size: 100%;text-rendering: inherit;padding: 0;white-space: normal;margin: 0;line-height: inherit;">Now onto session restorage:</h2>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">What exactly is the problem here?
a) the windows are <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">not</em> visible when the session restarts. Showing them show them in wrong position
b) the windows <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">are</em> visible when the session restarts, but in wrong position?</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">In either case, the session restored geometry and the SNI internal geometry should <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">not</em> differ.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">If they do there're two possible problems:
1. The window gets stored with the session, but the stored position is wrong (check the kwin session file in ~/.config/session on whether it contains such window and what position is saved)
2. The window does not get saved with the session and picks its new position in a different way</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">(2) has alternative failure points:
a) the window isn't positioned at all
b) the window gets a QWidget::move call from the SNI code, but that's ineffective (ie. QWidget::move fails if there's no WM)</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I agree with Martin that waiting for the WM is ridiculous and only stashes the actual bug.
Either the wrong geometry is stored (and thus falsely restored) => we need to fix that
Or QWidget::move requires a running WM, what's simply a bug in Qt and <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">must</em> be fixed there, despite I severely doubt it will, even if you bang them with tons of patches...</p>
<h2 style="font-size: 100%;text-rendering: inherit;padding: 0;white-space: normal;margin: 0;line-height: inherit;">Last hint:</h2>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">if the affected SNI window(s) are modal (open a nested eventloop) the problems may be an outflow of https://bugreports.qt.io/browse/QTBUG-40584
(the patch likely won't apply anymore and there's obviously no interest in fixing Qt/Desktop, so cc.: please get used to use fullscreen QML apps and forget about things like "windows")</p></pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">To answer what Qt does:
const quint32 mask = XCB_CONFIG_WINDOW_X | XCB_CONFIG_WINDOW_Y | XCB_CONFIG_WINDOW_WIDTH | XCB_CONFIG_WINDOW_HEIGHT;
const qint32 values[] = {
qBound<qint32>(-XCOORD_MAX, wmGeometry.x(), XCOORD_MAX),
qBound<qint32>(-XCOORD_MAX, wmGeometry.y(), XCOORD_MAX),
qBound<qint32>(1, wmGeometry.width(), XCOORD_MAX),
qBound<qint32>(1, wmGeometry.height(), XCOORD_MAX),
};
Q_XCB_CALL(xcb_configure_window(xcb_connection(), m_window, mask, reinterpret_cast<const quint32*>(values)));</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">That code looks correct to me and even if or if not a WM is running should not matter.</p></pre>
<br />
<p>- Martin</p>
<br />
<p>On April 10th, 2016, 9:46 p.m. CEST, Anthony Fieroni wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: 1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
<div>Review request for kwin, Plasma, David Edmundson, Martin Gräßlin, and Thomas Lübking.</div>
<div>By Anthony Fieroni.</div>
<p style="color: grey;"><i>Updated April 10, 2016, 9:46 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
plasma-workspace
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">We must proceed with autoStart0 when kwin process is ended otherwise kwindowsystem::self() is nullptr. Every restored session app cannot use kwindowsystem. This depends of cpu speed, it can be faster enough to start wm before ksmserver restore apps and kwindowsystem will be usable.</p></pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">It's needed to fix this bug -> https://git.reviewboard.kde.org/r/127216/
I don't know how to proceed if kwin fails to start in every way, can we process - i think not?</p></pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>ksmserver/startup.cpp <span style="color: grey">(f118b55)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/127631/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>