'Re: Review Request 127631: [ksmserver] We must be sure that kwin process is ended'

[prev in list] [next in list] [prev in thread] [next in thread] 

List: kde-panel-devel
Subject: Re: Review Request 127631: [ksmserver] We must be sure that kwin process is ended
From: Thomas_Lübking <thomas.luebking () gmail ! com>
Date: 2016-04-15 16:39:13
Message-ID: 20160415163913.26068.46680 () mimi ! kde ! org
[Download RAW message or body]

--===============2583704461426026755==
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 8bit

> On April 11, 2016, 5:45 vorm., 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. 
> Anthony Fieroni wrote:
> 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 :) 
> Thomas Lübking wrote:
> That patch only works around a bug in Qt (for KXMLGui windows, everything is still affected)
> That bug *might* be related to your problems with positioning - reverting the patch rather \
> won't help you here. 
> What you want to know is whether the windows that fail to reposition during session start \
> have those WM_SIZE_HINTS set. For if not, it's no wonder that the WM positions them and \
> QWidget::move is simply broken for unmanaged non-override-redirects. 
> Anthony Fieroni wrote:
> Can i set WM_SIZE_HINTS explicit?
> 
> Thomas Lübking wrote:
> Not w/o operating on X11 code, but one could try to cheat Qt into doing it likewise (since i \
> frankly doubt, there's any upstream interest in fixing deprecated things like desktop \
> functionality - rather on the contrary) 
> Ftr: they're not set?
> 
> Anthony Fieroni wrote:
> How can i test it, easy, i don't get it.
> 
> Martin Gräßlin wrote:
> xprop the window - it will have a section like
> WM_NORMAL_HINTS(WM_SIZE_HINTS):
> program specified minimum size: 300 by 166
> program specified maximum size: 32767 by 32767
> window gravity: NorthWest
> 
> Anthony Fieroni wrote:
> [toni@toni-sony ~]$ xprop 
> _KDE_NET_WM_FRAME_STRUT(CARDINAL) = 2, 2, 24, 4
> _NET_FRAME_EXTENTS(CARDINAL) = 2, 2, 24, 4
> _NET_WM_DESKTOP(CARDINAL) = 0
> WM_STATE(WM_STATE):
> window state: Normal
> icon window: 0x0
> _NET_WM_STATE(ATOM) = 
> _NET_WM_ICON_GEOMETRY(CARDINAL) = 502, 736, 256, 30
> _NET_WM_ALLOWED_ACTIONS(ATOM) = _NET_WM_ACTION_MOVE, _NET_WM_ACTION_RESIZE, \
> _NET_WM_ACTION_MINIMIZE, _NET_WM_ACTION_SHADE, _NET_WM_ACTION_MAXIMIZE_VERT, \
> _NET_WM_ACTION_MAXIMIZE_HORZ, _NET_WM_ACTION_FULLSCREEN, _NET_WM_ACTION_CHANGE_DESKTOP, \
> _NET_WM_ACTION_CLOSE _KDE_NET_WM_ACTIVITIES(STRING) = "153b4652-e23a-44c6-baf7-e7a0d812498a"
> _NET_WM_USER_TIME(CARDINAL) = 372774774
> _NET_WM_ICON_NAME(UTF8_STRING) = 
> XdndAware(ATOM) = BITMAP
> WM_NAME(STRING) = 
> _NET_WM_NAME(UTF8_STRING) = "Akregator"
> _MOTIF_WM_HINTS(_MOTIF_WM_HINTS) = 0x3, 0x3e, 0x7e, 0x0, 0x0
> _NET_WM_WINDOW_TYPE(ATOM) = _NET_WM_WINDOW_TYPE_NORMAL
> _XEMBED_INFO(_XEMBED_INFO) = 0x0, 0x1
> WM_CLIENT_LEADER(WINDOW): window id # 0x5000006
> WM_HINTS(WM_HINTS):
> Client accepts input or input focus: True
> Initial state is Normal State.
> _NET_WM_PID(CARDINAL) = 3834
> _NET_WM_SYNC_REQUEST_COUNTER(CARDINAL) = 83886085
> WM_CLASS(STRING) = "akregator", "akregator"
> WM_PROTOCOLS(ATOM): protocols  WM_DELETE_WINDOW, WM_TAKE_FOCUS, _NET_WM_PING, \
> _NET_WM_SYNC_REQUEST WM_NORMAL_HINTS(WM_SIZE_HINTS):
> user specified size: 980 by 602
> program specified minimum size: 325 by 237
> window gravity: Static
> 
> Thomas Lübking wrote:
> As predicted the client does not request some position, thus is necessarily subject to \
> positioning. QWidget/QWindow bug for sure. No idea whether one can cheat QWidget into setting \
> the flag or how many years it will take to convince them that this is important :-( 
> Anthony Fieroni wrote:
> It must contribute to Qt to fix this issue, i'm not right person to do it.
> So since this patch is unwanted i will discart reviview
> 
> Andreas Hartmetz wrote:
> If there is no or little regression potential, I don't think it will be hard to convince Qt \
> maintainers to merge a simple patch. 
> Thomas Lübking wrote:
> LOL
> 
> Andreas Hartmetz wrote:
> I have landed a few patches in Qt and, that is quite important, I don't know of any \
> particularly "difficult" maintainers for the XCB QPA.

Good for you. I've completely given up on Qt.
I though doubt it's the xcb qpa, but rather QWidget (see \
https://bugreports.qt.io/browse/QTBUG-40584 - it's pretty much  the same category, just that \
that bug prevented and prevents KDE file dialogs from restoring their size, this afaik still \
lives on the workaround) and QWidget is "done", ie. "dead".

- Thomas

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127631/#review94513
-----------------------------------------------------------

On April 15, 2016, 4:33 nachm., Anthony Fieroni wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127631/
> -----------------------------------------------------------
> 
> (Updated April 15, 2016, 4:33 nachm.)
> 
> 
> 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
> 
> 

--===============2583704461426026755==
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;">
 On April 11th, 2016, 5:45 vorm. UTC, Martin Gräßlin \
wrote: <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;"> otherwise kwindowsystem::self() is nullptr \
</blockquote> 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.</pre> \
</blockquote>

On April 11th, 2016, 6:24 vorm. UTC, Thomas Lübking wrote:
 <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;">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 ...)</pre> </blockquote>

On April 11th, 2016, 6:53 vorm. UTC, Anthony Fieroni wrote:
 <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;">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.</pre> </blockquote>

On April 11th, 2016, 7:08 vorm. UTC, Anthony Fieroni wrote:
 <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;">Now, after Thomas comment, i even \
think only widget-&gt;show() must works because widget has internal frameGeometry and \
position.</pre> </blockquote>

On April 11th, 2016, 8:38 vorm. UTC, Martin Gräßlin wrote:
 <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;"> but Kwin must be started before use of \
kwindowsystem </blockquote>
no, really there is no reason for that. KWindowSystem doesn't depend on a window \
manager running.</pre> </blockquote>

On April 11th, 2016, 8:54 vorm. UTC, Anthony Fieroni wrote:
 <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;">Then, correct KWin to \
kwindowsystem to start working. If there is no bug this will works on all cases -&gt; \
https://git.reviewboard.kde.org/r/127216/ but it's not.</pre> </blockquote>

On April 11th, 2016, 9:01 vorm. UTC, Martin Gräßlin wrote:
 <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;">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?</pre> </blockquote>

On April 11th, 2016, 9:15 vorm. UTC, Thomas Lübking wrote:
 <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;">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)</pre> \
</blockquote>

On April 11th, 2016, 9:51 vorm. UTC, Anthony Fieroni wrote:
 <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;">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.</pre>
 </blockquote>

On April 11th, 2016, 9:55 vorm. UTC, Martin Gräßlin wrote:
 <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;">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.</pre> </blockquote>

On April 11th, 2016, 10:29 vorm. UTC, Anthony Fieroni wrote:
 <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;">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.</pre> </blockquote>

On April 11th, 2016, 10:37 vorm. UTC, Anthony Fieroni wrote:
 <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;">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.</pre> </blockquote>

On April 11th, 2016, 10:45 vorm. UTC, Martin Gräßlin wrote:
 <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;"> Adn give you the easiest, no pain, working \
solution - wait kwin to start completely. </blockquote>
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.</pre> \
</blockquote>

On April 11th, 2016, 3:19 nachm. UTC, Thomas Lübking wrote:
 <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;">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) <div class="codehilite" style="background: #f8f8f8"><pre \
style="line-height: 125%">=&gt; Does this work (leaving aside session restorage), yes or no? \
</pre></div> 
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. <h2 style="font-size: 100%;text-rendering: inherit;padding: \
0;white-space: normal;margin: 0;line-height: inherit;">Now onto session restorage:</h2> 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) =&gt; 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... <h2 style="font-size: 100%;text-rendering: \
inherit;padding: 0;white-space: normal;margin: 0;line-height: inherit;">Last hint:</h2> 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")</pre> </blockquote>

 <p>On April 11th, 2016, 3:34 nachm. UTC, <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&lt;qint32&gt;(-XCOORD_MAX, wmGeometry.x(),      XCOORD_MAX),
            qBound&lt;qint32&gt;(-XCOORD_MAX, wmGeometry.y(),      XCOORD_MAX),
            qBound&lt;qint32&gt;(1,           wmGeometry.width(),  XCOORD_MAX),
            qBound&lt;qint32&gt;(1,           wmGeometry.height(), XCOORD_MAX),
        };
        Q_XCB_CALL(xcb_configure_window(xcb_connection(), m_window, mask, \
reinterpret_cast&lt;const quint32*&gt;(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 April 11th, 2016, 4:12 nachm. UTC, <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: "=&gt; 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" -&gt; 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>

On April 11th, 2016, 4:18 nachm. UTC, Thomas Lübking wrote:
 <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;"> A: setGeometry NOT work </blockquote>
In total? Ie. the other patch is completely \
dysfunctional?</pre> </blockquote>

On April 11th, 2016, 4:45 nachm. UTC, Anthony Fieroni wrote:
 <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;">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 =&gt; placement
2. setGeometry(), move() differs internal widgets =&gt; works i.e. move(geometry().topLeft()) \
causing the bug.</pre> </blockquote>

On April 11th, 2016, 4:47 nachm. UTC, Anthony Fieroni wrote:
 <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;">I mean i.e. \
move(frameGeometry().topLeft()) causing the bug. move(geometry().topLeft()) =&gt; \
placement</pre> </blockquote>

On April 11th, 2016, 11:56 nachm. UTC, Thomas Lübking wrote:
 <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;">&gt; move() works when app is started after \
kwin and not before it

Means that the no-WM path in QWidget/QWindow doesn&#39;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: &lt;x&gt; &lt;y&gt;
or
      program specified location: &lt;x&gt; &lt;y&gt;

If not, then the window doesn&#39;t signal the WM that it was explicitly positioned (and will \
be placed on map requests)</pre>  </blockquote>

On April 12th, 2016, 12:43 vorm. UTC, Andreas Hartmetz wrote:
 <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;">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.</pre> </blockquote>

On April 12th, 2016, 4:46 vorm. UTC, Anthony Fieroni wrote:
 <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;">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 -&gt; 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 :)</pre> </blockquote>

On April 13th, 2016, 7:25 vorm. UTC, Thomas Lübking wrote:
 <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;">That patch only works around a \
bug in Qt (for KXMLGui windows, everything is still affected) That bug might be \
related to your problems with positioning - reverting the patch rather won't help you here. \
What you want to know is whether the windows that fail to reposition during session \
start have those WM_SIZE_HINTS set. For if not, it's no wonder that the WM positions them and \
QWidget::move is simply broken for unmanaged non-override-redirects.</pre> </blockquote>

On April 13th, 2016, 7:49 vorm. UTC, Anthony Fieroni wrote:
 <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;">Can i set WM_SIZE_HINTS \
explicit?</pre> </blockquote>

On April 13th, 2016, 7:53 vorm. UTC, Thomas Lübking wrote:
 <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;">Not w/o operating on X11 code, \
but one could try to cheat Qt into doing it likewise (since i frankly doubt, there's any \
upstream interest in fixing deprecated things like desktop functionality - rather on the \
contrary) Ftr: they're not set?</pre> </blockquote>

On April 14th, 2016, 4:57 vorm. UTC, Anthony Fieroni wrote:
 <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;">How can i test it, easy, i don't \
get it.</pre> </blockquote>

 <p>On April 14th, 2016, 5:45 vorm. UTC, <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;">xprop the window - it will have a \
section like  WM_NORMAL_HINTS(WM_SIZE_HINTS):
                program specified minimum size: 300 by 166
                program specified maximum size: 32767 by 32767
                window gravity: NorthWest</p></pre>
 </blockquote>

 <p>On April 14th, 2016, 4:57 nachm. UTC, <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;">[toni@toni-sony ~]$ xprop  \
_KDE_NET_WM_FRAME_STRUT(CARDINAL) = 2, 2, 24, 4 _NET_FRAME_EXTENTS(CARDINAL) = 2, 2, 24, 4
_NET_WM_DESKTOP(CARDINAL) = 0
WM_STATE(WM_STATE):
                window state: Normal
                icon window: 0x0
_NET_WM_STATE(ATOM) = 
_NET_WM_ICON_GEOMETRY(CARDINAL) = 502, 736, 256, 30
_NET_WM_ALLOWED_ACTIONS(ATOM) = _NET_WM_ACTION_MOVE, _NET_WM_ACTION_RESIZE, \
_NET_WM_ACTION_MINIMIZE, _NET_WM_ACTION_SHADE, _NET_WM_ACTION_MAXIMIZE_VERT, \
_NET_WM_ACTION_MAXIMIZE_HORZ, _NET_WM_ACTION_FULLSCREEN, _NET_WM_ACTION_CHANGE_DESKTOP, \
_NET_WM_ACTION_CLOSE _KDE_NET_WM_ACTIVITIES(STRING) = "153b4652-e23a-44c6-baf7-e7a0d812498a"
_NET_WM_USER_TIME(CARDINAL) = 372774774
_NET_WM_ICON_NAME(UTF8_STRING) = 
XdndAware(ATOM) = BITMAP
WM_NAME(STRING) = 
_NET_WM_NAME(UTF8_STRING) = "Akregator"
_MOTIF_WM_HINTS(_MOTIF_WM_HINTS) = 0x3, 0x3e, 0x7e, 0x0, 0x0
_NET_WM_WINDOW_TYPE(ATOM) = _NET_WM_WINDOW_TYPE_NORMAL
_XEMBED_INFO(_XEMBED_INFO) = 0x0, 0x1
WM_CLIENT_LEADER(WINDOW): window id # 0x5000006
WM_HINTS(WM_HINTS):
                Client accepts input or input focus: True
                Initial state is Normal State.
_NET_WM_PID(CARDINAL) = 3834
_NET_WM_SYNC_REQUEST_COUNTER(CARDINAL) = 83886085
WM_CLASS(STRING) = "akregator", "akregator"
WM_PROTOCOLS(ATOM): protocols  WM_DELETE_WINDOW, WM_TAKE_FOCUS, _NET_WM_PING, \
_NET_WM_SYNC_REQUEST WM_NORMAL_HINTS(WM_SIZE_HINTS):
                user specified size: 980 by 602
                program specified minimum size: 325 by 237
                window gravity: Static</p></pre>
 </blockquote>

On April 14th, 2016, 6:27 nachm. UTC, Thomas Lübking wrote:
 <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;">As predicted the client does not \
request some position, thus is necessarily subject to positioning. QWidget/QWindow bug for \
sure. No idea whether one can cheat QWidget into setting the flag or how many years it will \
take to convince them that this is important :-(</pre> </blockquote>

On April 15th, 2016, 3:50 vorm. UTC, Anthony Fieroni wrote:
 <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;">It must contribute to Qt to fix \
this issue, i'm not right person to do it. So since this patch is unwanted i will discart \
reviview</pre> </blockquote>

On April 15th, 2016, 9:05 vorm. UTC, Andreas Hartmetz wrote:
 <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;">If there is no or little regression \
potential, I don&#39;t think it will be hard to convince Qt maintainers to merge a simple \
patch.</pre> </blockquote>

On April 15th, 2016, 9:11 vorm. UTC, Thomas Lübking wrote:
 <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;">LOL</pre> </blockquote>

On April 15th, 2016, 9:38 vorm. UTC, Andreas Hartmetz wrote:
 <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;">I have landed a few patches in Qt and, that \
is quite important, I don&#39;t know of any particularly &quot;difficult&quot; maintainers for \
the XCB QPA.</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;">Good for you. I've completely \
given up on Qt. I though doubt it's the xcb qpa, but rather QWidget (see \
https://bugreports.qt.io/browse/QTBUG-40584 - it's pretty much the same category, just that \
that bug prevented and prevents KDE file dialogs from restoring their size, this afaik still \
lives on the workaround) and QWidget is "done", ie. "dead".</pre>

<p>- Thomas</p>

<br />
<p>On April 15th, 2016, 4:33 nachm. UTC, 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 15, 2016, 4:33 nachm.</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;">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.</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 -&gt; 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>

--===============2583704461426026755==--

[Attachment #3 (text/plain)]

_______________________________________________
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

[prev in list] [next in list] [prev in thread] [next in thread] 