[prev in list] [next in list] [prev in thread] [next in thread]
List: kwin
Subject: Re: Review Request 127631: [ksmserver] We must be sure that kwin process is ended
From: Anthony Fieroni <bvbfan () abv ! bg>
Date: 2016-04-12 4:46:39
Message-ID: 20160412044639.25275.27817 () mimi ! kde ! org
[Download RAW message or body]
--===============2376917810165970083==
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 8bit
> On Април 11, 2016, 8:45 преди обяд, Martin Gräßlin wrote:
> > > otherwise kwindowsystem::self() is nullptr
> >
> > 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.
>
> 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)
> 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 *faaaar* too \
> complex - the position of remapped windows is usually maintained by QWidget ...)
> Anthony Fieroni wrote:
> I saw the code, it looks like KWindowSystem not depend on Kwin, but Kwin *must* be \
> started *before* 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.
> Anthony Fieroni wrote:
> Now, after Thomas comment, i even think only widget->show() must works because \
> widget has internal frameGeometry and position.
> Martin Gräßlin wrote:
> > but Kwin must be started before use of kwindowsystem
>
> no, really there is no reason for that. KWindowSystem doesn't depend on a window \
> manager running.
> Anthony Fieroni wrote:
> 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.
> Martin Gräßlin wrote:
> 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?
> Thomas Lübking wrote:
> 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.
> 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.
> qwindow/qwidget::setGeometry should justwork(tm)
>
> Anthony Fieroni wrote:
> setGeometry *not* works, i can confirm since i'm test it. Nothing works if Kwin is \
> started *after* usage of kwindowsystem::self(). Martin yeah nullprt is impossible.
>
> Martin Gräßlin wrote:
> 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.
> Anthony Fieroni wrote:
> Wait a little, the bug is not in xcb. I'm not clear or what. This *happend* only on \
> session restored apps, when kwindowsystem::self() is before kwin statup, only in \
> this case. If run a app after kwin is started *all* works fine setGeometry(), \
> move() - no problems.
> Anthony Fieroni wrote:
> 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)
> For me, this is best explanation. Adn give you the easiest, no pain, working \
> solution - wait kwin to start completely.
> Martin Gräßlin wrote:
> > Adn give you the easiest, no pain, working solution - wait kwin to start \
> > completely.
>
> 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.
> Thomas Lübking wrote:
> Can we please get few things straight?
>
> 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)
> => Does this work (leaving aside session restorage), yes or no?
>
> Assuming it *does* 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.
>
>
> Now onto session restorage:
> ---------------------------
> What exactly is the problem here?
> a) the windows are *not* visible when the session restarts. Showing them show them \
> in wrong position b) the windows *are* visible when the session restarts, but in \
> wrong position?
> In either case, the session restored geometry and the SNI internal geometry should \
> *not* differ.
> 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
> (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)
>
>
> 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 *must* be fixed there, despite I severely doubt it will, even if you bang them \
> with tons of patches...
>
>
> Last hint:
> ----------
> 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")
> Martin Gräßlin wrote:
> 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)));
> That code looks correct to me and even if or if not a WM is running should not \
> matter.
> Anthony Fieroni wrote:
> T: "=> Does this work (leaving aside session restorage), yes or no?"
> A: setGeometry *NOT* work
> T: "What exactly is the problem here?"
> A: a) the windows are not visible when the session restarts. Showing them show them \
> in wrong position Position is depend on "placement" -> \
> https://i.imgur.com/tzwJ1Lk.png "2. The window does not get saved with the session \
> and picks its new position in a different way" First show() always depend on \
> "placement" i.e. if widget was shown when leave session it will shown on right \
> place, if was hidden first show depend on "placement". But if Kwin is not started \
> *always* depend on "placement" "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)"
> Thomas Lübking wrote:
> > A: setGeometry NOT work
>
> In total? Ie. the other patch is *completely* dysfunctional?
>
> Anthony Fieroni wrote:
> What you mean "in total"? setGeometry() and move() works when app is started \
> *after* kwin and not before it. I.e. i can't write in more clear words, my english \
> is till here, Kwin is stared before apps:
> 1. setGeometry(), move() always works
> Kwin not stated before apps:
> 1. setGeometry(), move() if new values are same as internal widget => placement
> 2. setGeometry(), move() differs internal widgets => works i.e. \
> move(geometry().topLeft()) causing the bug.
> Anthony Fieroni wrote:
> I mean
> i.e. move(frameGeometry().topLeft()) causing the bug.
> move(geometry().topLeft()) => placement
>
> Thomas Lübking wrote:
> > move() works when app is started after kwin and not before it
>
> Means that the no-WM path in QWidget/QWindow doesn't set USPosition/PPosition
> Is or is related to QTBUG-40584.
>
> You can check that by running xprop on such (placed) window, it should have some
>
> user specified location: <x> <y>
> or
> program specified location: <x> <y>
>
> If not, then the window doesn't signal the WM that it was explicitly positioned \
> (and will be placed on map requests)
> Andreas Hartmetz wrote:
> Possibly the whole thing is moot. Due to a new bug in Qt 5.6 which has been \
> recently fixed (b77ef8a7e6e4104067d52824e29eadc8c66f5929 / "QtDBus: clean up signal \
> hooks and object tree in closeConnection"), applications got stuck when they were \
> supposed to terminate. This made session exit act strangely, essentially some \
> applications didn't shut down cleanly and were forcibly killed tens of seconds \
> later. ksmserver stuck around, too, waiting for them. I haven't done a real or \
> mental trace through ksmserver but such a situation *can* mess with session saving.
Andreas, i notice it, but this stays before Qt 5.5, it's works correct on Qt 5.4 or \
5.3 i can't remember when exactly was broken. Thomas, if you mean this -> \
https://git.reviewboard.kde.org/r/120078/ i can test it without this patch, but bug \
is removed, i don't know it's fixed :)
- Anthony
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127631/#review94513
-----------------------------------------------------------
On Април 10, 2016, 10:46 след обяд, Anthony Fieroni wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127631/
> -----------------------------------------------------------
>
> (Updated Април 10, 2016, 10:46 след обяд)
>
>
> Review request for kwin, Plasma, David Edmundson, Martin Gräßlin, and Thomas \
> Lübking.
>
> Repository: plasma-workspace
>
>
> Description
> -------
>
> 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.
>
> Diffs
> -----
>
> ksmserver/startup.cpp f118b55
>
> Diff: https://git.reviewboard.kde.org/r/127631/diff/
>
>
> Testing
> -------
>
> 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?
>
> Thanks,
>
> Anthony Fieroni
>
>
--===============2376917810165970083==
MIME-Version: 1.0
Content-Type: text/html; charset="utf-8"
Content-Transfer-Encoding: 8bit
<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 Април 11th, 2016, 8:45 преди обяд \
EEST, <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 Април 11th, 2016, 9:24 преди обяд EEST, <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 Април 11th, 2016, 9:53 преди обяд EEST, <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 Април 11th, 2016, 10:08 преди обяд EEST, <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 Април 11th, 2016, 11:38 преди обяд EEST, <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 Април 11th, 2016, 11:54 преди обяд EEST, <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 Април 11th, 2016, 12:01 след обяд EEST, <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 Април 11th, 2016, 12:15 след обяд EEST, <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 Април 11th, 2016, 12:51 след обяд EEST, <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 Април 11th, 2016, 12:55 след обяд EEST, <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 Април 11th, 2016, 1:29 след обяд EEST, <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 Април 11th, 2016, 1:37 след обяд EEST, <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 Април 11th, 2016, 1:45 след обяд EEST, <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 Април 11th, 2016, 6:19 след обяд EEST, <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>
<p>On Април 11th, 2016, 6:34 след обяд EEST, <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;">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> </blockquote>
<p>On Април 11th, 2016, 7:12 след обяд EEST, <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;">T: "=> Does this work (leaving aside session restorage), \
yes or no?"
A: setGeometry <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: normal;">NOT</em> work
T: "What exactly is the problem here?"
A: a) the windows are not visible when the session restarts. Showing them show them \
in wrong position Position is depend on "placement" -> \
https://i.imgur.com/tzwJ1Lk.png "2. The window does not get saved with the session \
and picks its new position in a different way" First show() always depend on \
"placement" i.e. if widget was shown when leave session it will shown on right place, \
if was hidden first show depend on "placement". But if Kwin is not started <em \
style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
normal;">always</em> depend on "placement" "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></pre> </blockquote>
<p>On Април 11th, 2016, 7:18 след обяд EEST, <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;"><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;">A: setGeometry NOT work</p> </blockquote>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">In total? Ie. the other patch is <em style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
normal;">completely</em> dysfunctional?</p></pre> </blockquote>
<p>On Април 11th, 2016, 7:45 след обяд EEST, <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;">What you mean "in total"? setGeometry() and move() works when app is \
started <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: normal;">after</em> kwin and not before it. I.e. i can't write \
in more clear words, my english is till here, Kwin is stared before apps:
1. setGeometry(), move() always works
Kwin not stated before apps:
1. setGeometry(), move() if new values are same as internal widget => placement
2. setGeometry(), move() differs internal widgets => works i.e. \
move(geometry().topLeft()) causing the bug.</p></pre> </blockquote>
<p>On Април 11th, 2016, 7:47 след обяд EEST, <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 mean i.e. move(frameGeometry().topLeft()) causing the bug.
move(geometry().topLeft()) => placement</p></pre>
</blockquote>
<p>On Април 12th, 2016, 2:56 преди обяд EEST, <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;">> move() \
works when app is started after kwin and not before it
Means that the no-WM path in QWidget/QWindow doesn't set USPosition/PPosition
Is or is related to QTBUG-40584.
You can check that by running xprop on such (placed) window, it should have some
user specified location: <x> <y>
or
program specified location: <x> <y>
If not, then the window doesn't signal the WM that it was explicitly positioned \
(and will be placed on map requests)</pre> </blockquote>
<p>On Април 12th, 2016, 3:43 преди обяд EEST, <b>Andreas Hartmetz</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;">Possibly the whole thing is moot. Due to a new bug in Qt 5.6 which has been \
recently fixed (b77ef8a7e6e4104067d52824e29eadc8c66f5929 / "QtDBus: clean up signal \
hooks and object tree in closeConnection"), applications got stuck when they were \
supposed to terminate. This made session exit act strangely, essentially some \
applications didn't shut down cleanly and were forcibly killed tens of seconds later. \
ksmserver stuck around, too, waiting for them. I haven't done a real or mental trace \
through ksmserver but such a situation <em style="padding: 0;text-rendering: \
inherit;margin: 0;line-height: inherit;white-space: normal;">can</em> mess with \
session saving.</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;">Andreas, i notice it, but this stays before Qt 5.5, it's works correct on \
Qt 5.4 or 5.3 i can't remember when exactly was broken. Thomas, if you mean this \
-> https://git.reviewboard.kde.org/r/120078/ i can test it without this patch, but \
bug is removed, i don't know it's fixed :)</p></pre> <br />
<p>- Anthony</p>
<br />
<p>On Април 10th, 2016, 10:46 след обяд EEST, 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 Април 10, 2016, 10:46 след \
обяд</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>
--===============2376917810165970083==--
[Attachment #3 (text/plain)]
_______________________________________________
kwin mailing list
kwin@kde.org
https://mail.kde.org/mailman/listinfo/kwin
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic