D14688: [libkwineffects] Expose getting/setting activeFullScript to scripted effects

zzag accepted this revision.
zzag added a comment.
This revision is now accepted and ready to land.


  Mostly, whitespace nitpicks.

INLINE COMMENTS

> scripted_effects_test.cpp:357
> +
> +     auto *effectMain = new ScriptedEffectWithDebugSpy; // cleaned up in ::clean
> +    QSignalSpy effectOutputSpy(effectMain, &ScriptedEffectWithDebugSpy::testOutput);

please delte extra space

> scripted_effects_test.cpp:363
> +    //shown as being someone else
> +    auto effectOther =  new ScriptedEffectWithDebugSpy();
> +    QVERIFY(effectOther->load("screenEdgeTouchTest"));

extra space

> scripted_effects_test.cpp:395
> +
> +    //after 1500ms  (+a safetey margin) we should have no full screen effect
> +    QTest::qWait(500+100);

extra space before `(`

> anidata.cpp:48
>  AniData::AniData(AnimationEffect::Attribute a, int meta_, int ms, const FPx2 &to_,
> -                 QEasingCurve curve_, int delay, const FPx2 &from_, bool waitAtSource_, bool keepAtTarget_ )
> +                 QEasingCurve curve_, int delay, const FPx2 &from_, bool waitAtSource_, bool keepAtTarget_ ,
> +                 FullScreenEffectLockPtr fullScreenEffectLock_)

please delete whitespace before comma

> kwinanimationeffect.cpp:245
> +        if (d->m_fullScreenEffectLock.isNull()) {
> +            fullscreen = FullScreenEffectLockPtr(new FullScreenEffectLock(this));
> +            d->m_fullScreenEffectLock = fullscreen.toWeakRef();

Can we use the create method? e.g.

  fullScreenLock = FullScreenEffectLockPtr::create(this);

> kwinanimationeffect.h:99
> +    ~FullScreenEffectLock();
> +private:
> +    void *d; //unused currently

Maybe, we have to delete copy constructor and copy assignment.

IIRC, QSharedPointer doesn't make copies of the pointed object so it should compile.

> kwinanimationeffect.h:177
> +     * @param from - The starting value, the default is invalid, ie. the attribute for the window is not transformed in the beginning
> +     * @param fullScreen - sets this effect as the active full screen effect for the duration of the animation
>       * @return an ID that you can use to cancel a running animation

Capitalize "sets"

> kwinanimationeffect.h:228
>  private:
> -    quint64 p_animate( EffectWindow *w, Attribute a, uint meta, int ms, FPx2 to, QEasingCurve curve, int delay, FPx2 from, bool keepAtTarget );
> +    quint64 p_animate(EffectWindow *w, Attribute a, uint meta, int ms, FPx2 to, QEasingCurve curve, int delay, FPx2 from, bool keepAtTarget , bool fullScreenEffect);
>      QRect clipRect(const QRect &windowRect, const AniData&) const;

please dlete extra space before comma

> scriptedeffect.cpp:99
> +
> +    QJSValue isFullScreen = object.property(QStringLiteral("fullScreen"));
> +    if (isFullScreen.isBool()) {

Can be const.

REPOSITORY
  R108 KWin

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D14688

To: davidedmundson, #kwin, zzag
Cc: zzag, kwin, mkulinski, ragreen, jackyalcine, Pitel, iodelay, bwowk, ZrenBot, lesliezhai, ali-mohamed, hardening, jensreuterberg, abetts, sebas, apol, mart