Summary:
So far we were following a bit unique and rare doxygen comment style:
/**
* Contents of the comment.
**/
Doxygen comments with this style look balanced and neat, but many people
that contribute to KWin don't follow this style. Instead, they prefer
more traditional doxygen comment style, i.e.
/**
* Contents of the comment.
*/
Reviewing such changes has been a bit frustrating for me (so selfish!)
and for other contributors.
This change switches doxygen comment style in KWin to a more traditional
style. The main reason for doing this is to make code review process easier
for new contributors as well us.
Reviewers: #kwin, davidedmundson
Reviewed By: #kwin, davidedmundson
Subscribers: kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D22812
Summary:
Currently code base of kwin can be viewed as two pieces. One is very
ancient, and the other one is more modern, which uses new C++ features.
The main problem with the ancient code is that it was written before
C++11 era. So, no override or final keywords, lambdas, etc.
Quite recently, KDE compiler settings were changed to show a warning if
a virtual method has missing override keyword. As you might have already
guessed, this fired back at us because of that ancient code. We had
about 500 new compiler warnings.
A "solution" was proposed to that problem - disable -Wno-suggest-override
and the other similar warning for clang. It's hard to call a solution
because those warnings are disabled not only for the old code, but also
for new. This is not what we want!
The main argument for not actually fixing the problem was that git
history will be screwed as well because of human factor. While good git
history is a very important thing, we should not go crazy about it and
block every change that somehow alters git history. git blame allows to
specify starting revision for a reason.
The other argument (human factor) can be easily solved by using tools
such as clang-tidy. clang-tidy is a clang-based linter for C++. It can
be used for various things, e.g. fixing coding style(e.g. add missing
braces to if statements, readability-braces-around-statements check),
or in our case add missing override keywords.
Test Plan: Compiles.
Reviewers: #kwin, davidedmundson
Reviewed By: #kwin, davidedmundson
Subscribers: davidedmundson, apol, romangg, kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D22371
Summary:
Currently, if a script relies on clientAdded to setup some required
connections, then it probably won't work with Wayland clients because
clientAdded is emitted only for X11 clients.
Test Plan:
* Installed sticky window snapping script, it works;
* Installed a couple of tiling scripts, with some small changes, they work.
Reviewers: #kwin, davidedmundson
Reviewed By: #kwin, davidedmundson
Subscribers: strobach, TomButler, davidedmundson, mart, graesslin, kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D17097
Currently, clientArea(ClientAreaOption, const Client *) and clientList()
are quite X11-specific, which results in having some tiling scripts not
working with Wayland clients. This change is the first step towards working
tiling scripts in the Wayland session.
Summary:
In 403038 the user expected the DBus call to run to finish before
processing the next request. For a singleshot script that makes a lot of
sense, otherwise you have no idea when it finishes.
This also allows us to return errors.
CCBUG: 403038
Test Plan:
NUM=$(qdbus org.kde.KWin /Scripting org.kde.kwin.Scripting.loadScript /noFile.js Minimize)
qdbus org.kde.KWin /$NUM org.kde.kwin.Scripting.run
Error: org.kde.kwin.Scripting.FileError
Could not open /noFile.js
Running a real script behaved effectively the same as before.
Reviewers: #kwin, graesslin
Reviewed By: #kwin, graesslin
Subscribers: zzag, kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D18127
Summary:
We have a mix of different doxygen comment styles, e.g.
/*!
Foo bar.
*/
/**
* Foo bar.
*/
/** Foo bar.
*/
/**
* Foo bar.
*/
/**
* Foo bar.
**/
To make the code more consistent, this change updates the style of all
doxygen comments to the last one.
Test Plan: Compiles.
Reviewers: #kwin, davidedmundson
Reviewed By: #kwin, davidedmundson
Subscribers: kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D18683
Summary:
KWin::Script::loadScriptFromFile ran in it's own thread and accessed
member variables of KWin::Script without any guards.
Potentially script could be destroyed whilst the file is loading.
Rather than adding mutexes everywhere, this patch scopes the QFile
object to be local to the threaded function making it independent.
BUG: 403038
Test Plan: Ran a script from a file
Reviewers: #kwin, graesslin
Reviewed By: #kwin, graesslin
Subscribers: kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D18126
Summary:
Effects that prefer to manipulate direction of animations sometimes need
to create animations in some particular state so later on they can be
played backward (swapping from and to is not enough and it would be wrong).
The proposed complete function lets such effects to fast-forward animations to
to the target position so they can be played backwards later on.
Reviewers: #kwin, davidedmundson
Reviewed By: #kwin, davidedmundson
Subscribers: davidedmundson, kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D16450
Summary:
Consider current implementation of the Squash effect: if a window was
minimized, an animation will be started; if the window is unminimized
and the animation is still active (that can happen when user clicks on
app's icon really fast), the animation will be stopped and a new one will
be created. Such behavior can lead to rapid jumps in the observed
"animation".
A better approach would be first try to **reverse** the already active
animation, and if that attempt wasn't successful, start a new animation.
This patch introduces a new function to the scripted effects API that
lets JavaScript effects to control direction of animations. The
prototype of the function looks as follows:
redirect(<animation id(s)>, <direction>, [<termination policy>])
the first argument is an animation id or a list of animation ids, the
second argument specifies the new direction of the animation or
animations if a list of ids was passed as the first argument. The
third argument specifies whether the animation(s) should be terminated
when it(they) reaches the source position, currently it's relevant only
for animations that are created with set() function. The termination
policy argument is optional, by default it's Effect.TerminateAtSource.
We can use this function to fix issues with rapid jumps in the Squash
effect. Also, redirect() lets us to write effects for simple animations
in slightly different style: first, we have to start the main animation
(e.g. for the Dialog Parent effect, it would be dimming of main windows)
and then change direction of the animation depending on external events,
e.g. when the Desktop Cube effect is activated.
Reviewers: #kwin, davidedmundson
Reviewed By: #kwin, davidedmundson
Subscribers: davidedmundson, abetts, kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D16449
Summary:
Some JavaScript based effects need to grab particular windows in order
to avoid conflicts with other effects.
Example usage:
```lang=js
effects.windowAdded.connect(function (window) {
if (effect.grab(window, Effect.WindowAddedGrabRole)) {
window.coolWindowTypeAnimation = animate({
...
});
}
});
```
Reviewers: #kwin, davidedmundson
Reviewed By: #kwin, davidedmundson
Subscribers: romangg, graesslin, davidedmundson, kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D13153
Summary:
If a modal window is closed and some alternative effect that animates
the disappearing of windows is enabled(e.g. the Glide effect, or the
Scale effect), the Dialog Parent effect can cause flickering of the
parent window because its animation duration doesn't match duration of
those alternative effects.
Also, if the Fade effect, the Glide effect, and the Scale effect are
disabled, the Dialog Parent will keep the parent window alive for no
good reason.
This change addresses that problem by adding keepAlive property to
`animate` function so scripted effects have more control over lifetime
of animated windows.
If both a modal window and its parent window are closed at the same time
(and there is no effect that animates the disappearing of windows), the
Dialog Parent will stop immediately(because windowDeleted will be
emitted right after windowClosed signal).
If both a modal window and its parent window are closed at the same time
(and there is effect that animates the disappearing of windows), the
Dialog Parent won't reference the latter window. Thus, it won't cause
flickering. I.e. it will "passively" animate parent windows.
BUG: 355036
FIXED-IN: 5.15.0
Reviewers: #kwin, davidedmundson
Reviewed By: #kwin, davidedmundson
Subscribers: davidedmundson, kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D14919
Summary:
Getter is exposed as a property on scripted effect in a way that hides
pointers from the scripting side.
Setter is implicitly handled as a property of newly created animations
and holds the activeFullScreenEffect whilst any of them are active. Like
existing effects it remains up to the effect author to avoid the
problems of multiple full screen effects. The RAII lock pattern is
somewhat overkill currently, but it's the direction I hope we can take
EffectsHandler in next API break.
BUG: 396790
--
This patch is against the QJSEngine port, though it's not conceptually a
requirement.
Test Plan: Unit test
Reviewers: #kwin, zzag
Reviewed By: #kwin, zzag
Subscribers: zzag, kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D14688
Summary:
Qt render loops behave quite differently to each other.
KWin scripting as a workaround for another situation cleans
(17553e5a1f) handles tracking of script
windows by deleting the underlying window handle on hide.
This currently happens before the window gets the hideEvent.
Arguably this is a quirk with Qt, but in the current state:
- QSGThreadedRenderLoop deletes the platform window and cleans up
- We then get the hide() event. This no-ops because there's no window.
(else branch of case WM_TryRelease in qsgthreadedrenderloop.cpp)
- We carry on rendering animations despite having no platform
- undefined behaviour
Normally this isn't a problem as typically destruction of the platform window
happens only when a window is being deleted, we're messing with Qt
internals here.
If we make sure the QHideEvent is processed by the render loop first,
things seem fine.
BUG: 397767
Test Plan:
Ran QSG_RENDER_LOOP=threaded
Read output with scenegraph logging rules on
Reviewers: #kwin, broulik
Reviewed By: broulik
Subscribers: kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D15025
Summary:
One cannot use a non metatype frrom an external class inside an
invokable. https://bugreports.qt.io/browse/QTBUG-58454
End result is the script engine arguments wouldn't match up and MOC
would just use the default value.
As far as I can tell this has been broken for 6 years.
The global animate method that unboxes a QJSValue as an object is
unaffected.
No shipped kwin effect actually used it.
To some extent we didn't even actually want to enforce the enum as we
also accept custom value of ScriptedEffect::GuassianCurve, so it has
been switched for an int.
Test Plan: Unit test
Reviewers: #kwin, broulik
Reviewed By: broulik
Subscribers: kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D14669
Summary:
Ready for QJSEngine port and upcoming other fixes.
Split as it makes it easier to do any before/after testing.
Test Plan:
All tests pass with the current QScriptEngine
Verified expected API against a wiki page and current code.
Reviewers: #kwin
Subscribers: kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D14482
Summary:
KWin::displayWidth and KWin::displayHeight are bound to X11 which
doesn't make much sense on X11. In addition KWin internally knows
the overall display dimensions through the Screens singleton class.
Reviewers: #kwin
Subscribers: kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D1798
Summary:
They were only delegating into switchWindow(Direction), so let's do the
same through std::bind in kwinbindings and delegate to it directly in
scripting's WorkspaceWrapper.
Test Plan: Not yet, want to add test case for Workspace::switchWindow
Reviewers: #kwin, #plasma
Subscribers: plasma-devel
Tags: #plasma
Differential Revision: https://phabricator.kde.org/D6791
Summary:
Thanks to std::bind we don't need that many different slots to setup the
global shortcut connections. Instead we can have one shared
implementation which takes the argument and passes it to the window.
To support std::bind arguments in kwinbindings the initShortcut method
and dependencies are adjusted as well as a new macro is added.
As I don't want to include abstract_client.h in workspace.h a new enum
is created for the quick tiling flags used in Workspace. This caused a
larger refactoring as the change to an enum class also caused quite some
changes.
Test Plan: Affected test cases still pass
Reviewers: #kwin, #plasma
Subscribers: plasma-devel, kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D6783
I did some research with the result that such comparisons are indeed
not standardized to be legal before C++14. In C++14 they are.
These microoptimizations can thus be re-enabled when C++14 becomes
the required version.
Summary:
Exposes two new global JS functions to register and unregister touch
screen edges.
Test Plan: Added test case
Reviewers: #kwin, #plasma
Subscribers: plasma-devel
Tags: #plasma
Differential Revision: https://phabricator.kde.org/D5277
Summary:
This change introduces a new property mode in ScreenEdgeItem to switch
between Pointer and Touch mode. The touch mode interacts with the new
touch screen edge gestures through a QAction.
Test Plan: New test case added
Reviewers: #kwin, #plasma
Subscribers: plasma-devel
Tags: #plasma
Differential Revision: https://phabricator.kde.org/D5272
By changing the loading of scripted effect config to interact with
kwinApp we broke the tests which do not use a kwinApp.
This change turns the access to the KSharedConfigPtr go through the
property system, so that the tests can also install a dummy
KSharedConfigPtr. With that scripted effects loader test passes and no
longer crashes and the integration/effects tests also still pass.
Summary:
So far ScriptedEffect used EffectsHandler::effectConfig to get the
KConfigGroup for the ScriptedEffect. This has the disadvantage that the
config file name is hardcoded to kwinrc in EffectsHandler::effectConfig.
Inside KWin a KSharedConfigPtr is used which can point to somwhere else
than kwinrc. If that was the case the ScriptedEffects were not able to
pick up this customized config and instead continued to read values from
kwinrc.
With this change the ScriptedEffects use the KSharedConfigPtr provided
by KWin. Thus in e.g. autotests we can use the general way to configure
the effects and don't need to write to the config.
Reviewers: #kwin, #plasma
Subscribers: plasma-devel, kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D3383
The export of the WorkspaceWrapper was changed to be a subclass with all
the elements still being in the parent class. But the "workspace" was
exported with QScriptEngine::ExcludeSuperClassContents. Thus all usages
of workspace were broken and our tests started to fail on build.kde.org.
This change removes the ExcludeSuperClassContents which means that also
the QObject properties and slots are now exposed which was previously
not the case.
CCMAIL: github@chilon.net
Summary:
The version provided is only compatible with QtScript so it became
necessary to split WorkspaceWrapper into a base class and two child
classes, one for QtScript and one for QmlScript.
BUG: 340125
FIXED-IN: 5.8.4
REVIEW: D3185
Summary:
registerShortcut is available to javascript KWin scripts but was not
available to those written in declarative script.
BUG: 340225
FIXED-IN: 5.8.4
REVIEW: 129250
Summary:
Modify the kwinscreenedges KCM to also list scripts which support screen
edge activation and read/write the appropriate value in the script's
config.
In order to only show relevant scripts an additional .desktop metadata
field is added.
Test Plan:
Opened KCM set a hot corner for minimize all.
Tested it
unset it, and set on another corner
Tested again
Reviewers: #plasma, graesslin
Reviewed By: #plasma, graesslin
Subscribers: graesslin, plasma-devel, kwin
Tags: #plasma
Differential Revision: https://phabricator.kde.org/D2972
Summary:
It was consistently wrong so it worked before, but not having a typo is
better.
Test Plan: Compiles, grepped that it's not invokable/public
Reviewers: #plasma, graesslin
Reviewed By: #plasma, graesslin
Subscribers: kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D2266
Summary:
This change introduces a Scripting::findScript method which returns
the AbstractScript. Thus a test can load a script, retrieve it and
trigger run on it. As the test would also need to know when finally
the test is running a signal is introduced to notify about it.
This makes the scripting ScreenEdgeTest way more reliable. The test
had been failing on both build.kde.org and build.neon.kde.org due to
not knowing when the script is loaded.
Reviewers: #kwin
Subscribers: kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D2497
Summary:
A new method to tell the effects system whether the compositor scene
is able to drive animations. E.g. on software emulation (llvmpipe) it's
better to not do any animations at all.
This information can be used by effects to adjust their behavior, e.g.
PresentWindows could skip transitions or effects can use it in their
supported check to completely disable themselves.
As a first step all scripted effects are considered to be unsupported
if animations are not supported. They inherit AnimationEffect and are
all about driving animations.
The information whether animations are supported comes from the Scene.
It's implemented in the following way:
* XRender: animations are always supported
* QPainter: animations are never supported
* OpenGL: animations are supported, except for software emulation
In addition - for easier testing - there is a new env variable
KWIN_EFFECTS_FORCE_ANIMATIONS to overwrite the selection.
Reviewers: #kwin, #plasma
Subscribers: kwin
Tags: #kwin
Differential Revision: https://phabricator.kde.org/D2386
A new sub-directory scripting is added in autotests/integration to
gather all test cases related to scripting.
The first added test case verifies the activation of screen edges. For
that it loads a helper script, which reserves an edge based on config.
When the edge is triggered showing desktop is activated.
Canceling the animation in the animationEnded handler triggers a crash.
This is due to multiple lists being iterated and manipulated at the same
time.
This adds a test case which simulates the crashy situation.
REVIEW: 126975
the result of animate is an array of animids: in order for
dumb QScript to understand what it is, we have to build it
as an actual JavaScript Array
reviewed-by: Martin Gräßlin <mgraesslin@kde.org>
