D26457: Introduce shadow API

2020-01-12 Thread Vlad Zahorodnii 
zzag updated this revision to Diff 73352.
zzag added a comment.


  - Make KWindowShadow a QObject subclass (for memory management)
  - Clarify that it is okay to call destroy() after window() had been removed
  - Check whether QWindow has QPlatformWindow associated with it

REPOSITORY
  R278 KWindowSystem

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D26457?vs=72880=73352

BRANCH
  kwindowshadow

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

AFFECTED FILES
  src/CMakeLists.txt
  src/kwindowshadow.cpp
  src/kwindowshadow.h
  src/kwindowshadow_dummy_p.h
  src/kwindowshadow_p.h
  src/kwindowsystemplugininterface.cpp
  src/kwindowsystemplugininterface_p.h
  src/platforms/xcb/CMakeLists.txt
  src/platforms/xcb/kwindowshadow.cpp
  src/platforms/xcb/kwindowshadow_p_x11.h
  src/platforms/xcb/plugin.cpp
  src/platforms/xcb/plugin.h
  src/pluginwrapper.cpp
  src/pluginwrapper_p.h

To: zzag, #kwin
Cc: davidedmundson, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, 
bruns

D26457: Introduce shadow API

2020-01-06 Thread Vlad Zahorodnii 
zzag added inline comments.

INLINE COMMENTS

> zzag wrote in kwindowshadow.h:201
> Oh, that one... Yeah, perhaps it will be worth to handle surface 
> creation/destruction in kwayland-integration.

Okay, looks like it's do-able. However, we would need to call 
`QWidget::winId()` in `Breeze::ShadowHelper::installShadows()` to create a 
`QWindow`...

REPOSITORY
  R278 KWindowSystem

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

To: zzag, #kwin
Cc: davidedmundson, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, 
bruns

D26457: Introduce shadow API

2020-01-06 Thread Vlad Zahorodnii 
zzag added inline comments.

INLINE COMMENTS

> davidedmundson wrote in kwindowshadow.h:201
> kwayland-integration
> 
> 0a0c3f23fce265f36e5b8fb2b4b8bd311c7c1beb 
> 
> src/windowsystem/windoweffects.cpp

Oh, that one... Yeah, perhaps it will be worth to handle surface 
creation/destruction in kwayland-integration.

REPOSITORY
  R278 KWindowSystem

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

To: zzag, #kwin
Cc: davidedmundson, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, 
bruns

D26457: Introduce shadow API

2020-01-06 Thread David Edmundson 
davidedmundson added inline comments.

INLINE COMMENTS

> zzag wrote in kwindowshadow.h:201
> Could you please point me to that code?

kwayland-integration

0a0c3f23fce265f36e5b8fb2b4b8bd311c7c1beb 

src/windowsystem/windoweffects.cpp

REPOSITORY
  R278 KWindowSystem

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

To: zzag, #kwin
Cc: davidedmundson, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, 
bruns

D26457: Introduce shadow API

2020-01-06 Thread Vlad Zahorodnii 
zzag added inline comments.

INLINE COMMENTS

> davidedmundson wrote in kwindowshadow.h:201
> For KWindowEffects I did put window tracking into the wayland plugin and it 
> improved the readability of all the client code considerably.
> 
> I would recommend looking at doing so, especially as we could probably even 
> share the implementation that kwayland-integration windoweffects.cpp has.

Could you please point me to that code?

REPOSITORY
  R278 KWindowSystem

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

To: zzag, #kwin
Cc: davidedmundson, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, 
bruns

D26457: Introduce shadow API

2020-01-06 Thread David Edmundson 
davidedmundson added a comment.


  Seems generally very clean and sensible. +1
  
  I want to see plasma-framework/dialog.cpp and breeze ports before we merge.

INLINE COMMENTS

> kwindowshadow.h:201
> + *
> + * Note that the KWindowShadow does not keep track of the platform 
> surface. If for whatever
> + * reason the native platform surface is deleted and then created, you 
> must to destroy() the

For KWindowEffects I did put window tracking into the wayland plugin and it 
improved the readability of all the client code considerably.

I would recommend looking at doing so, especially as we could probably even 
share the implementation that kwayland-integration windoweffects.cpp has.

REPOSITORY
  R278 KWindowSystem

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

To: zzag, #kwin
Cc: davidedmundson, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, 
bruns

D26457: Introduce shadow API

2020-01-06 Thread Vlad Zahorodnii 
zzag updated this revision to Diff 72880.
zzag added a comment.


  Remove pointless default member initializers in KWindowShadowPrivate.

REPOSITORY
  R278 KWindowSystem

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D26457?vs=72862=72880

BRANCH
  kwindowshadow

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

AFFECTED FILES
  src/CMakeLists.txt
  src/kwindowshadow.cpp
  src/kwindowshadow.h
  src/kwindowshadow_dummy_p.h
  src/kwindowshadow_p.h
  src/kwindowsystemplugininterface.cpp
  src/kwindowsystemplugininterface_p.h
  src/platforms/xcb/CMakeLists.txt
  src/platforms/xcb/kwindowshadow.cpp
  src/platforms/xcb/kwindowshadow_p_x11.h
  src/platforms/xcb/plugin.cpp
  src/platforms/xcb/plugin.h
  src/pluginwrapper.cpp
  src/pluginwrapper_p.h

To: zzag, #kwin
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D26457: Introduce shadow API

2020-01-06 Thread Vlad Zahorodnii 
zzag added a task: T12496: Shadows API in KWindowSystem.

REPOSITORY
  R278 KWindowSystem

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

To: zzag, #kwin
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D26457: Introduce shadow API

2020-01-06 Thread Vlad Zahorodnii 
zzag created this revision.
zzag added a reviewer: KWin.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
zzag requested review of this revision.

REVISION SUMMARY
  The new API provides a platform-independent way for setting drop-shadows
  on a window. X11 and Wayland details are completely hidden from the users
  of the new API, but you still need to be cautious about QPAs that destroy
  the underlying native resources(e.g. wl_surface) upon a window becoming
  hidden. It is highly recommended to install an event filter that reacts
  to events of type QEvent::PlatformSurface.

REPOSITORY
  R278 KWindowSystem

BRANCH
  kwindowshadow

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

AFFECTED FILES
  src/CMakeLists.txt
  src/kwindowshadow.cpp
  src/kwindowshadow.h
  src/kwindowshadow_dummy_p.h
  src/kwindowshadow_p.h
  src/kwindowsystemplugininterface.cpp
  src/kwindowsystemplugininterface_p.h
  src/platforms/xcb/CMakeLists.txt
  src/platforms/xcb/kwindowshadow.cpp
  src/platforms/xcb/kwindowshadow_p_x11.h
  src/platforms/xcb/plugin.cpp
  src/platforms/xcb/plugin.h
  src/pluginwrapper.cpp
  src/pluginwrapper_p.h

To: zzag, #kwin
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns