308438 2012-10-15 14:01:01 +0000 When compositing is disabled, opening a window which blocks compositing *enables* it 2013-05-20 09:58:52 +0000 1 1 4 Plasma kwin compositing 4.9.2 Compiled Sources Linux RESOLVED FIXED https://git.reviewboard.kde.org/r/106900/ NOR normal 4.10 1 post kwin-bugs-null eshkrig http://commits.kde.org/kde-workspace/7df368b35d27b27c3cb7da4802daa541a89dc773 4.9.3 0 oldest_to_newest 1306149 0 post 2012-10-15 14:01:01 +0000 When a window sets the "block compositing" property while compositing is disabled, compositing gets enabled. Reproducible: Always Steps to Reproduce: 1. Start with compositing enabled. Open a Konsole. 2. In the window menu, choose "More Actions - Special Window Settings..." 3. In the "Appearance" tab, enable "Block compositing", choose "Force" and "yes" 4. Close all window (at least all Konsole windows and all the others which have the "block composite" property). 5. Verify that opening a Konsole will block compositing, and closing it will re-enable compositing. Make sure it is closed. 6. Hit Ctrl+Alt+F12 to disable compositing 7. Start a Konsole. Actual Results: Compositing is *enabled*. Expected Results: Of course, compositing should stay disabled. 1306154 1 post 2012-10-15 14:15:21 +0000 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. 1306161 2 74560 post 2012-10-15 14:43:44 +0000 Created attachment 74560 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? 1306164 3 thomas.luebking 2012-10-15 14:56:28 +0000 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. 1306177 4 mgraesslin 2012-10-15 15:27:09 +0000 > 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. 1306182 5 post 2012-10-15 15:35:55 +0000 (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? 1306184 6 mgraesslin 2012-10-15 15:41:29 +0000 (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. 1306191 7 thomas.luebking 2012-10-15 15:59:57 +0000 (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. 1306192 8 post 2012-10-15 16:01:56 +0000 (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. 1306197 9 74564 post 2012-10-15 16:06:51 +0000 Created attachment 74564 Fix handling of compositing-blocking windows when compositing is already disabled (v2) Updated patch 1306198 10 thomas.luebking 2012-10-15 16:10:15 +0000 (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. 1306202 11 thomas.luebking 2012-10-15 16:25:58 +0000 (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 1306203 12 post 2012-10-15 16:28:57 +0000 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. 1306212 13 thomas.luebking 2012-10-15 17:09:36 +0000 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 1306229 14 post 2012-10-15 17:45:03 +0000 I see. I will open a review request as this has more side-effects than I anticipated. 1306238 15 post 2012-10-15 18:04:37 +0000 (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. 1306254 16 thomas.luebking 2012-10-15 19:06:43 +0000 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. 1306298 17 post 2012-10-15 20:48:12 +0000 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) 1306304 18 mgraesslin 2012-10-15 20:56:29 +0000 (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) 1311413 19 post 2012-11-01 13:04:43 +0000 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 1311608 20 post 2012-11-01 21:41:46 +0000 The bug is not actually fixed, behaviour got just a bit better. 1311660 21 mgraesslin 2012-11-02 06:45:59 +0000 (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? 1311680 22 post 2012-11-02 09:42:05 +0000 (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. 1320716 23 thomas.luebking 2012-12-03 14:07:03 +0000 *** Bug 311081 has been marked as a duplicate of this bug. *** 1330127 24 mgraesslin 2013-01-09 07:33:53 +0000 @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 1330198 25 post 2013-01-09 11:21:21 +0000 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. 1334792 26 thomas.luebking 2013-01-23 21:12:24 +0000 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 1369918 27 post 2013-05-20 09:53:55 +0000 I see this buggy behaviour in KDE 4.10.3 again - no idea when it came back. 1369921 28 post 2013-05-20 09:58:52 +0000 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. 74560 2012-10-15 14:43:44 +0000 2012-10-15 16:06:51 +0000 Fix handling of compositing-blocking windows when compositing is already disabled 0001-Fix-handling-of-compositing-blocking-windows-when-co.patch text/plain 1753 post RnJvbSBjMWM5NjE3OWQ3NDM4MDMwYWQ2ZWEwYjE1ZTdjZmZlZmYxNjRiY2FjIE1vbiBTZXAgMTcg MDA6MDA6MDAgMjAwMQpGcm9tOiBSYWxmIEp1bmcgPHBvc3RAcmFsZmouZGU+CkRhdGU6IE1vbiwg MTUgT2N0IDIwMTIgMTY6MzE6NTEgKzAyMDAKU3ViamVjdDogW1BBVENIXSBGaXggaGFuZGxpbmcg b2YgY29tcG9zaXRpbmctYmxvY2tpbmcgd2luZG93cyB3aGVuCiBjb21wb3NpdGluZyBpcyBhbHJl YWR5IGRpc2FibGVkCgotLS0KIGt3aW4vY29tcG9zaXRlLmNwcCB8ICAgIDggKysrKy0tLS0KIDEg ZmlsZSBjaGFuZ2VkLCA0IGluc2VydGlvbnMoKyksIDQgZGVsZXRpb25zKC0pCgpkaWZmIC0tZ2l0 IGEva3dpbi9jb21wb3NpdGUuY3BwIGIva3dpbi9jb21wb3NpdGUuY3BwCmluZGV4IDZlZDgyODku LmU2YWI0YzUgMTAwNjQ0Ci0tLSBhL2t3aW4vY29tcG9zaXRlLmNwcAorKysgYi9rd2luL2NvbXBv c2l0ZS5jcHAKQEAgLTMwMiw5ICszMDIsOSBAQCBRU3RyaW5nTGlzdCBXb3Jrc3BhY2U6OmFjdGl2 ZUVmZmVjdHMoKSBjb25zdAogdm9pZCBXb3Jrc3BhY2U6OnVwZGF0ZUNvbXBvc2l0ZUJsb2NraW5n KENsaWVudCAqYykKIHsKICAgICBpZiAoYykgeyAvLyBpZiBjID09IDAgd2UganVzdCBjaGVjayBp ZiB3ZSBjYW4gcmVzdW1lCi0gICAgICAgIGlmIChjLT5pc0Jsb2NraW5nQ29tcG9zaXRpbmcoKSkg ewotICAgICAgICAgICAgaWYgKCFjb21wb3NpdGluZ0Jsb2NrZWQpIC8vIGRvIE5PVCBhdHRlbXB0 IHRvIGNhbGwgc3VzcGVuZENvbXBvc2l0aW5nKHRydWUpOyBmcm9tIHdpdGhpbiB0aGUgZXZlbnRj aGFpbiEKLSAgICAgICAgICAgICAgICBRTWV0YU9iamVjdDo6aW52b2tlTWV0aG9kKHRoaXMsICJz bG90VG9nZ2xlQ29tcG9zaXRpbmciLCBRdDo6UXVldWVkQ29ubmVjdGlvbik7CisgICAgICAgIGlm IChjLT5pc0Jsb2NraW5nQ29tcG9zaXRpbmcoKSAmJiAhY29tcG9zaXRpbmdTdXNwZW5kZWQgJiYg IWNvbXBvc2l0aW5nQmxvY2tlZCkgeworICAgICAgICAgICAgLy8gZG8gTk9UIGF0dGVtcHQgdG8g Y2FsbCBzdXNwZW5kQ29tcG9zaXRpbmcodHJ1ZSk7IGZyb20gd2l0aGluIHRoZSBldmVudGNoYWlu IQorICAgICAgICAgICAgUU1ldGFPYmplY3Q6Omludm9rZU1ldGhvZCh0aGlzLCAic3VzcGVuZENv bXBvc2l0aW5nIiwgUXQ6OlF1ZXVlZENvbm5lY3Rpb24sIFFfQVJHKGJvb2wsIHRydWUpKTsKICAg ICAgICAgICAgIGNvbXBvc2l0aW5nQmxvY2tlZCA9IHRydWU7CiAgICAgICAgIH0KICAgICB9CkBA IC0zMjEsNyArMzIxLDcgQEAgdm9pZCBXb3Jrc3BhY2U6OnVwZGF0ZUNvbXBvc2l0ZUJsb2NraW5n KENsaWVudCAqYykKICAgICAgICAgaWYgKHJlc3VtZSkgeyAvLyBkbyBOT1QgYXR0ZW1wdCB0byBj YWxsIHN1c3BlbmRDb21wb3NpdGluZyhmYWxzZSk7IGZyb20gd2l0aGluIHRoZSBldmVudGNoYWlu IQogICAgICAgICAgICAgY29tcG9zaXRpbmdCbG9ja2VkID0gZmFsc2U7CiAgICAgICAgICAgICBp ZiAoY29tcG9zaXRpbmdTdXNwZW5kZWQpCi0gICAgICAgICAgICAgICAgUU1ldGFPYmplY3Q6Omlu dm9rZU1ldGhvZCh0aGlzLCAic2xvdFRvZ2dsZUNvbXBvc2l0aW5nIiwgUXQ6OlF1ZXVlZENvbm5l Y3Rpb24pOworICAgICAgICAgICAgICAgIFFNZXRhT2JqZWN0OjppbnZva2VNZXRob2QodGhpcywg InN1c3BlbmRDb21wb3NpdGluZyIsIFF0OjpRdWV1ZWRDb25uZWN0aW9uLCBRX0FSRyhib29sLCBm YWxzZSkpOwogICAgICAgICB9CiAgICAgfQogfQotLSAKMS43LjEwLjQKCg== 74564 2012-10-15 16:06:51 +0000 2012-10-15 16:06:51 +0000 Fix handling of compositing-blocking windows when compositing is already disabled (v2) 0001-Fix-handling-of-compositing-blocking-windows-when-co.patch text/plain 2215 post RnJvbSAxNGI0ZTkwN2Y2ZTVlYTIwYzNjMGUzMjRkMGUwMmFjNjgyMDc0OTY3IE1vbiBTZXAgMTcg MDA6MDA6MDAgMjAwMQpGcm9tOiBSYWxmIEp1bmcgPHBvc3RAcmFsZmouZGU+CkRhdGU6IE1vbiwg MTUgT2N0IDIwMTIgMTY6MzE6NTEgKzAyMDAKU3ViamVjdDogW1BBVENIXSBGaXggaGFuZGxpbmcg b2YgY29tcG9zaXRpbmctYmxvY2tpbmcgd2luZG93cyB3aGVuCiBjb21wb3NpdGluZyBpcyBhbHJl YWR5IGRpc2FibGVkCgotLS0KIGt3aW4vY29tcG9zaXRlLmNwcCB8ICAgMTEgKysrKysrLS0tLS0K IDEgZmlsZSBjaGFuZ2VkLCA2IGluc2VydGlvbnMoKyksIDUgZGVsZXRpb25zKC0pCgpkaWZmIC0t Z2l0IGEva3dpbi9jb21wb3NpdGUuY3BwIGIva3dpbi9jb21wb3NpdGUuY3BwCmluZGV4IDZlZDgy ODkuLmZlYjQ4YTAgMTAwNjQ0Ci0tLSBhL2t3aW4vY29tcG9zaXRlLmNwcAorKysgYi9rd2luL2Nv bXBvc2l0ZS5jcHAKQEAgLTMwMiw5ICszMDIsMTAgQEAgUVN0cmluZ0xpc3QgV29ya3NwYWNlOjph Y3RpdmVFZmZlY3RzKCkgY29uc3QKIHZvaWQgV29ya3NwYWNlOjp1cGRhdGVDb21wb3NpdGVCbG9j a2luZyhDbGllbnQgKmMpCiB7CiAgICAgaWYgKGMpIHsgLy8gaWYgYyA9PSAwIHdlIGp1c3QgY2hl Y2sgaWYgd2UgY2FuIHJlc3VtZQotICAgICAgICBpZiAoYy0+aXNCbG9ja2luZ0NvbXBvc2l0aW5n KCkpIHsKLSAgICAgICAgICAgIGlmICghY29tcG9zaXRpbmdCbG9ja2VkKSAvLyBkbyBOT1QgYXR0 ZW1wdCB0byBjYWxsIHN1c3BlbmRDb21wb3NpdGluZyh0cnVlKTsgZnJvbSB3aXRoaW4gdGhlIGV2 ZW50Y2hhaW4hCi0gICAgICAgICAgICAgICAgUU1ldGFPYmplY3Q6Omludm9rZU1ldGhvZCh0aGlz LCAic2xvdFRvZ2dsZUNvbXBvc2l0aW5nIiwgUXQ6OlF1ZXVlZENvbm5lY3Rpb24pOworICAgICAg ICAvLyAiY29tcG9zaXRpbmdCbG9ja2VkID0gdHJ1ZSIgbWVhbnMgIndlIGJsb2NrZWQgY29tcG9z aXRpbmcgYmFzZWQgb24gYSB3aW5kb3cgcnVsZSwgaXQncyBva2F5IHRvIGF1dG9tYXRpY2FsbHkg cmUtZW5hYmxlIGl0IGxhdGVyIgorICAgICAgICBpZiAoYy0+aXNCbG9ja2luZ0NvbXBvc2l0aW5n KCkgJiYgIWNvbXBvc2l0aW5nU3VzcGVuZGVkICYmICFjb21wb3NpdGluZ0Jsb2NrZWQpIHsKKyAg ICAgICAgICAgIC8vIGRvIE5PVCBhdHRlbXB0IHRvIGNhbGwgc3VzcGVuZENvbXBvc2l0aW5nKHRy dWUpOyBmcm9tIHdpdGhpbiB0aGUgZXZlbnRjaGFpbiEKKyAgICAgICAgICAgIFFNZXRhT2JqZWN0 OjppbnZva2VNZXRob2QodGhpcywgInN1c3BlbmRDb21wb3NpdGluZyIsIFF0OjpRdWV1ZWRDb25u ZWN0aW9uLCBRX0FSRyhib29sLCB0cnVlKSk7CiAgICAgICAgICAgICBjb21wb3NpdGluZ0Jsb2Nr ZWQgPSB0cnVlOwogICAgICAgICB9CiAgICAgfQpAQCAtMzIwLDggKzMyMSw3IEBAIHZvaWQgV29y a3NwYWNlOjp1cGRhdGVDb21wb3NpdGVCbG9ja2luZyhDbGllbnQgKmMpCiAgICAgICAgIH0KICAg ICAgICAgaWYgKHJlc3VtZSkgeyAvLyBkbyBOT1QgYXR0ZW1wdCB0byBjYWxsIHN1c3BlbmRDb21w b3NpdGluZyhmYWxzZSk7IGZyb20gd2l0aGluIHRoZSBldmVudGNoYWluIQogICAgICAgICAgICAg Y29tcG9zaXRpbmdCbG9ja2VkID0gZmFsc2U7Ci0gICAgICAgICAgICBpZiAoY29tcG9zaXRpbmdT dXNwZW5kZWQpCi0gICAgICAgICAgICAgICAgUU1ldGFPYmplY3Q6Omludm9rZU1ldGhvZCh0aGlz LCAic2xvdFRvZ2dsZUNvbXBvc2l0aW5nIiwgUXQ6OlF1ZXVlZENvbm5lY3Rpb24pOworICAgICAg ICAgICAgUU1ldGFPYmplY3Q6Omludm9rZU1ldGhvZCh0aGlzLCAic3VzcGVuZENvbXBvc2l0aW5n IiwgUXQ6OlF1ZXVlZENvbm5lY3Rpb24sIFFfQVJHKGJvb2wsIGZhbHNlKSk7CiAgICAgICAgIH0K ICAgICB9CiB9CkBAIC0zMzMsNiArMzMzLDcgQEAgdm9pZCBXb3Jrc3BhY2U6OnN1c3BlbmRDb21w b3NpdGluZygpCiAKIHZvaWQgV29ya3NwYWNlOjpzdXNwZW5kQ29tcG9zaXRpbmcoYm9vbCBzdXNw ZW5kKQogeworICAgIGlmIChjb21wb3NpdGluZ1N1c3BlbmRlZCA9PSBzdXNwZW5kKSByZXR1cm47 IC8vIG5vdGhpbmcgYWN0dWFsbHkgY2hhbmdlcwogICAgIGNvbXBvc2l0aW5nU3VzcGVuZGVkID0g c3VzcGVuZDsKICAgICBmaW5pc2hDb21wb3NpdGluZygpOwogICAgIHNldHVwQ29tcG9zaXRpbmco KTsgLy8gd2lsbCBkbyBub3RoaW5nIGlmIHN1c3BlbmRlZAotLSAKMS43LjEwLjQKCg==