Bug 308438

Looking at the code, this is actually not surprising:
In composite.cpp, Workspace::updateCompositeBlocking *toggles* compositing when seeing the first client with the property set. The suspend call should probably be something like

QMetaObject::invokeMethod(this, "suspendCompositing", Qt::QueuedConnection, Q_ARG(bool, true));

However, if compositing was disabled, this needs to be remembered for when the last blocking window closes. This could be achieved by not setting compositingBlocked to true if it was already blocked, but the comment says

do NOT check for "compositingSuspended" or "!compositing()"

Nevertheless, a few lines down, compositingSuspended *is* checked.

This is all in the KDE/4.9 branch.

Created attachment 74560 [details]
Fix handling of compositing-blocking windows when  compositing is already disabled


Attached patch fixes the problem here. Is it okay to go, should I open a review request or use a different approach?

The patch pollutes the "compositingBlocked" state if compositingSuspended && c->isBlockingCompositing()

Also since there's now a distinct suspend(bool) slot, the compositingSuspended check can be just omitted.

Notice that git master fundamentally differs for the introduction of the compositor class and as of now, composite blocking seems to not wor at all.

> Notice that git master fundamentally differs for the introduction of the
> compositor class and as of now, composite blocking seems to not wor at all.
what? I'm quite sure I tested that (as I was afraid of breaking it).

But given the changes between 4.9 and master in that area we will probably 
need two patches.

(In reply to comment #3)
> The patch pollutes the "compositingBlocked" state if compositingSuspended &&
> c->isBlockingCompositing()
How that? If the window is blocking compositing and compositing was not yet blocked (this merges the formerly two ifs), it will get blocked unless it's manually suspended.
I am available in IRC (#kde-devel).

(In reply to comment #4)
> > Notice that git master fundamentally differs for the introduction of the
> > compositor class and as of now, composite blocking seems to not wor at all.
> what? I'm quite sure I tested that (as I was afraid of breaking it).
Yes, it seems to ignore the blocking here as well - probably a different bug?

(In reply to comment #5)
> (In reply to comment #4)
> > > Notice that git master fundamentally differs for the introduction of the
> > > compositor class and as of now, composite blocking seems to not wor at all.
> > what? I'm quite sure I tested that (as I was afraid of breaking it).
> Yes, it seems to ignore the blocking here as well - probably a different bug?
probably, given the rather large refactoring.

(In reply to comment #4)

> what? I'm quite sure I tested that (as I was afraid of breaking it).
the function is not called, only updateCompositeBlocking(NULL) is.(

In reply to comment #5)
> How that?
As far as i can say from the plaintext patch, "compositingBlocked = true;" is not called if "compositingSuspended == true", despite "c->isBlockingCompositing()" is, thus the state becomes invalid.

(In reply to comment #7)
> In reply to comment #5)
> > How that?
> As far as i can say from the plaintext patch, "compositingBlocked = true;"
> is not called if "compositingSuspended == true", despite
> "c->isBlockingCompositing()" is, thus the state becomes invalid.
That's true, but it is necessary: If compositingBlocked were set to true, then compositing would be resumed after the last compositing-blocking window is closed - even if compositing was manually suspended previously.

Created attachment 74564 [details]
Fix handling of compositing-blocking windows when compositing is already disabled (v2)

Updated patch

(In reply to comment #7)

> > what? I'm quite sure I tested that (as I was afraid of breaking it).
> the function is not called, only updateCompositeBlocking(NULL) is.

workspace.cpp / ::createClient binds 
connect(c, SIGNAL(blockingCompositingChanged(KWin::Client*)), m_compositor, SLOT(updateCompositeBlocking(KWin::Client*)));

after conditional invocation in "!c->manage(w, is_mapped)"

The connects could and likely should happen after the construction and before the management attempt.

(In reply to comment #8)

> That's true, but it is necessary: If compositingBlocked were set to true,
> then compositing would be resumed after the last compositing-blocking window
> is closed - even if compositing was manually suspended previously.

This will require a slight more complex check that store resumeCompositing alongside setting the blocked state and alters it with external susped/resume requests, shifting the variable meaning won't resolve this ;-P

I can't think of a situation in which the behaviour this patch implements is wrong. No matter whether the initial compositing state is enabled or disabled, it will be restored after the last window is closed.
If you could tell me under which circumstances the behaviour should be different, I can try to fix that.

suspend compositing (alt+shift+f12)
launch blocking client // m_blocked = false
resume compositing (alt+shift+f12)
launch blocking client // m_blocked = true
suspend compositing (alt+shift+f12)
close blocking clients // (m_blocked == true)
-> compositing gets resumed

I see. I will open a review request as this has more side-effects than I anticipated.

(In reply to comment #13)
> suspend compositing (alt+shift+f12)
> launch blocking client // m_blocked = false
> resume compositing (alt+shift+f12)
> launch blocking client // m_blocked = true
> suspend compositing (alt+shift+f12)
Actually this does not work - when the blocking client is launched, compositing is automatically suspended. Of course, it will be resumed later. If however I hit alt+shift+F12 two times, then the new patch shows the desired behaviour of not resuming composite.

Sorry, forgot a resume, but just tested and this:

suspend compositing (alt+shift+f12)
launch blocking client // m_blocked = false
resume compositing (alt+shift+f12)
launch blocking client // m_blocked = true
resume compositing (alt+shift+f12)
suspend compositing (alt+shift+f12)
close blocking clients // (m_blocked == true)

resumes the compositor.

That's no wonder, because the system currently does not store the compositing state request from user (shortcut, dbus) and system (blocked for window) independently.

If that's demanded (given the initial bug, it seems so) the m_suspended state either needs to be a flag (ByUser = 1, ByBlockingClient = 2) and the resume after a block may only happen
m_suspended &= ~ByBlockingClient;
if (!m_suspended)
   ::invoke("resume");

or there need to be an additional boolean about the user driven compositing state.

The current patch is at https://git.reviewboard.kde.org/r/106900/ and I will reply over there to avois splitting the thread too much.
(Should've opened a review request from the start, sorry for that)

(In reply to comment #17)
> (Should've opened a review request from the start, sorry for that)
no problem at all, the systems are connected (well at least kind of)

Git commit d61ee825eceac212c4aa4d011c47a82ec92ade9a by Ralf Jung.
Committed on 26/10/2012 at 16:18.
Pushed by ralfjung into branch 'KDE/4.9'.

do not resume compositing when it's suspended and a client blocks it

REVIEW: 106900

M  +6    -3    kwin/composite.cpp

http://commits.kde.org/kde-workspace/d61ee825eceac212c4aa4d011c47a82ec92ade9a

The bug is not actually fixed, behaviour got just a bit better.

(In reply to comment #20)
> The bug is not actually fixed, behaviour got just a bit better.
but it is fixed in master, isn't it?

(In reply to comment #21)
> but it is fixed in master, isn't it?
Last time I checked, no: If compositing is disabled, opening a blocking window will (correctly) do nothing. However, closing that window again will start compositing.
To prevent this, the original compositing state needs to be remembered when a blocking window is opened, and properly restored. However, since the user can manually toggle compositing even when a blocking window is present, it's not entirely clear what the expected behaviour is.
I can propose a patch against master which implements the behaviour I'd expect, and open a review request. As far as I remember, Thomas expects a different behaviour.

*** Bug 311081 has been marked as a duplicate of this bug. ***

@Ralf: what's the current state in 4.10 branch? If it's not fixed we should try to get it fixed before RC 3

The current state is - as far as I know (didn't test in a while) - still as described above: If compositing is disabled, opening a blocking window will (correctly) do nothing. However, closing that window again will start compositing.
We could not agree on the correct behaviour to fix this. I proposed the following behaviour in my patch:
KWin maintains a flag whether compositing was automatically suspended by a window. This flag is reset whenever the user manually toggles compositing.
When a window with composite-blocking is opened, if compositing is already disabled or the flag is set, nothing happens. If compositing is enabled, the flag is set and compositing is disabled.
When the last blocking window is closed, if the flag is set, compositing is enabled and the flag reset.

Git commit 7df368b35d27b27c3cb7da4802daa541a89dc773 by Thomas Lübking.
Committed on 09/01/2013 at 16:03.
Pushed by luebking into branch 'master'.

32 bit compositing suspension

REVIEW: 108304

M  +25   -29   kwin/composite.cpp
M  +15   -8    kwin/composite.h

http://commits.kde.org/kde-workspace/7df368b35d27b27c3cb7da4802daa541a89dc773

I see this buggy behaviour in KDE 4.10.3 again - no idea when it came back.

Oops, I'm sorry. This is just the "compositing is re-enabled when the window is closed" bug, which I thought was fixed in KDE 4.10, but it is not. Sorry for the noise.