Introduce a config option whether applications are allowed to block compositing
ClosedPublic
Actions

Authored by graesslin on Aug 26 2016, 8:54 AM.

Details

Reviewers

Group Reviewers

VDG
Plasma on Wayland
KWin

Commits

R108:853020336f13: Introduce a config option whether applications are allowed to block compositing

Summary

From feedback we got it seems that not all users agree to games and
other applications blocking compositing. Some users prefer to have
compositing always on even if this gives a small performance penelity.

This change introduces a dedicated config option to specify whether games
are allowed to block compositing. By default this option is enabled.

The setting can be overwritten with a window specific rule. So usecases
like all windows except this very specific one are supported.

In the user interface the config option is shown where previously the
unredirect fullscreen option was shown.

Test Plan

Run a game which should block compositing, verified it blocks.
Changed the setting, run the game again, verified it doesn't block. And
once more for with allowing to block.

Diff Detail

Repository

R108 KWin

Branch

blocking-compositing

Lint

No Linters Available

Unit

No Unit Test Coverage

graesslin updated this revision to Diff 6286.Aug 26 2016, 8:54 AM

graesslin retitled this revision from to Introduce a config option whether applications are allowed to block compositing.

graesslin updated this object.

graesslin edited the test plan for this revision. (Show Details)

graesslin added reviewers: KWin, Plasma on Wayland, VDG.

Restricted Application added projects: Plasma on Wayland, KWin. · View Herald TranscriptAug 26 2016, 8:54 AM

Restricted Application added subscribers: kwin, plasma-devel. · View Herald Transcript

bshah added a subscriber: bshah.Aug 26 2016, 8:59 AM

bshah added inline comments.

kcmkwin/kwincompositing/compositing.ui
294	Even though windows is more correct term here technically, IMO it makes more sense to have it as "Allow applications to block compositing". what do you think?

graesslin added a dependent revision: D2586: [kcmkwin/compositing] Hide config options which don't make sense if Compositing is required.Aug 26 2016, 11:01 AM

yeah, should be "allow applications" even if not 100% correct

luebking added a subscriber: luebking.Aug 26 2016, 12:11 PM

luebking added inline comments.

kcmkwin/kwincompositing/compositing.h
63	The "is" prefix is a QML mandate, right? isObeyingCompositorBlockRequests isCompositorBlockAllowed However, afaiu the biggest complaint was that it was broken (due to shadow deletion) yesno? Really worth a setting (rather than a blind rule to achieve the same)?

reword to applications instead of windows

graesslin added inline comments.Aug 26 2016, 12:16 PM

kcmkwin/kwincompositing/compositing.h
63	However, afaiu the biggest complaint was that it was broken (due to shadow deletion) yesno? No, I also had quite a fair amount of feedback unrelated to the shadow.

Actually, I'm not really convinced of this. Blocking compositing is done via an official, cross-desktop API, isn't it? It's not a KWin-specific feature, right?
If so, then KWin should not go alone in offering this config option. If I'm an application developer and have set my application to block compositing, I'd expect that to be respected. If most compositors always respect it, but for one compositor, it depends on a user setting, this introduces another layer of complexity for application developers.
Knowing our users, some of them will turn this off, knowing it will impact performance, but will then still go ahead and file bug reports about a game being too slow, or whatever happening because the game developer just expected blocking compositing to work, but for those users, it doesn't.

If it's made configurable in all major compositors, then application developers at least know that they cannot expect blocking compositing to work. If we're the only ones allowing to configure it, it will get messy.

Plasma is the only major desktop environment that still supports a non-composited uscase. Unity and Gnome cannot disable it in the first place.

In D2584#48381, @broulik wrote:

Plasma is the only major desktop environment that still supports a non-composited uscase. Unity and Gnome cannot disable it in the first place.

Oh, so they just ignore that request from applications completely?
In that case, we're fine with doing whatever we want ;)

They'll unredirect the window. The property is however not mandarory:

"The compositing manager MAY bypass compositing for both fullscreen and non-fullscreen windows if bypassing is requested" (the supplemental "but MUST NOT bypass if it would cause differences from the composited appearance" btw. effectively means that the hint must be ignored in all cases or adjusted with every painted frame. It's a really shitty protocol)

sebas added a subscriber: sebas.Sep 12 2016, 8:02 PM

sebas added inline comments.

kcmkwin/kwincompositing/compositing.ui
10	resize the window before saving, that means it saves the minimum size, rather than a random one
24	dunno where this comes from, and what it means ... also in the following places
290	improvements (plural)
options.h
193	without the is* ? (It's not required by QML, unlike Thomas suggests)

colomar added inline comments.Sep 12 2016, 8:32 PM

kcmkwin/kwincompositing/compositing.ui
291	window-specific

graesslin added inline comments.Sep 13 2016, 6:07 AM

options.h
193	Using is in the case of boolean getters is normally correct. This follows the property naming suggestions by Qt. Please see https://doc.qt.io/archives/qq/qq13-apis.html#theartofnaming and also the examples in https://doc.qt.io/qt-5/properties.html - in this specific case the wording should be changed to windowsBlockCompositing

graesslin marked 4 inline comments as done.Sep 13 2016, 7:52 AM

graesslin added inline comments.

kcmkwin/kwincompositing/compositing.ui
10	That is the minimum size ;-)
24	Neither do I, designer added it - not going to modify the file manually, though

Adjusted to suggested:

rewording changes (improvements, window-specific)
renamed all the isWindowsBlockingCompositing to windowsBlockCompositing

sebas accepted this revision.Sep 13 2016, 12:45 PM

sebas added a reviewer: sebas.

sebas added inline comments.

kcmkwin/kwincompositing/compositing.ui
10	So your change made the window's minimum width more than 300px wider? That sounds wrong... What I mean is to resize the form in designer to the smallest possible size before you save it there, perhaps that got lost in translation.

This revision is now accepted and ready to land.Sep 13 2016, 12:45 PM

luebking added inline comments.Sep 13 2016, 12:58 PM

kcmkwin/kwincompositing/compositing.ui
10	The stored size should be rather irrelevant, for it is tied to the style used by the developer at creation of the widget anyway. Ie. if there's desire to use the minimum size at some point, one should ::adjustSize() at runtime. Otherwise the size is (hopefully still attempted) to be restored from the last user provided size anyway.

graesslin added inline comments.Sep 13 2016, 1:46 PM

kcmkwin/kwincompositing/compositing.ui
10	So your change made the window's minimum width more than 300px wider? That sounds wrong... Figured out how that happened: high dpi. I just started designer with low dpi settings and have it now small and tiny again.

Closed by commit R108:853020336f13: Introduce a config option whether applications are allowed to block compositing (authored by graesslin). · Explain WhySep 13 2016, 1:49 PM

This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

			Path	Packages
M			client.cpp (2 lines)
M			kcmkwin/kwincompositing/compositing.h (5 lines)
M			kcmkwin/kwincompositing/compositing.cpp (20 lines)
M			kcmkwin/kwincompositing/compositing.ui (38 lines)
M			kcmkwin/kwincompositing/main.cpp (5 lines)
M			kwin.kcfg (3 lines)
M			options.h (9 lines)
M			options.cpp (13 lines)

Diff	ID	Base	Description	Created	Lint	Unit
Base			Base
Diff 1	6286	6e8a891		Aug 26 2016, 8:54 AM	★	★
Diff 2	6296	6e8a891	reword to applications instead of windows	Aug 26 2016, 12:14 PM	★	★
Diff 3	6680	97b5945	Adjusted to suggested:	Sep 13 2016, 7:54 AM	★	★
Diff 4	6693	c6a8404	rKWIN853020336f13af2d91f007ba7eccca3239d5e7a3	Sep 13 2016, 1:47 PM	★	★

Commit	Tree	Parents	Author	Summary	Date
d37fc05cad3c	05b0c2b8c055	6e8a8913d1b4	Martin Gräßlin	Introduce a config option whether applications are allowed to block compositing (Show More…)	Aug 26 2016, 6:56 AM

	Status	Author	Revision
	Closed	graesslin	D2586 [kcmkwin/compositing] Hide config options which don't make sense if Compositing is required
	Closed	graesslin	D2584 Introduce a config option whether applications are allowed to block compositing