Re: Plasma-Mycroft is in kdereview

[Aditya Mehra]

Hi all,

Plasma-Mycroft has been in KDE review over a month, there have been a few 
additions and bug fixes to the plasmoid but nothing major, all fixes have also 
been made to the build system which were mentioned by Christophe Giboudeaux, I 
am hoping plasma-mycroft can complete its review soon as I am looking forward 
to making a stable release. As there haven't also been any other objections 
currently can this review process move ahead / moved to extra gears.

Regards,
Aditya Mehra

From: Aditya Mehra
Sent: Monday, December 18, 2017 4:56 PM
To: kde-core-de...@kde.org
Subject: Re: Plasma-Mycroft is in kdereview


Hi,

I have fixed the following:
- removed Author, Email, Bug_Address from top CMakeLists.txt
- set destination to use ${SYSCONF_INSTALL_DIR}
- use ecm_install_icons from ecm to install the icons
- in plugin/CMakeLists.txt removed unused pkgconfig
- added a ATTRIBUTION file to top directory to mark usage of images (wikipedia 
icon)
- added a Copying.apache2 license file as MSM.sh, Mycroft are now Apache 2.0 
licensed
- added license headers to all files in plasmoid/content/ui missing the header
- removed .directory file in plasmoid/contents/images

I am not sure about the COPYING file as I think this should be under GPL-2.1 as 
most other plasmoid/applets also seemed to be released under this license does 
this file need to be updated to reflect 2.1 specifically ?

Regards,
Aditya Mehra



From: kde-core-devel  on behalf of Christophe 
Giboudeaux 
Sent: Friday, December 15, 2017 1:40 AM
To: kde-core-de...@kde.org
Subject: Re: Plasma-Mycroft is in kdereview

Hi,

On mardi 5 décembre 2017 08:52:18 CET Aditya Mehra wrote:
> Hi all,
>
> This is a request email for the review process of the mycroft plasmoid, the
> plasma-mycroft project has been moved to kdereview
>
> The repository url: https://cgit.kde.org/plasma-mycroft.git/
>

Just commenting on the build system part.
In the top CMakeLists.txt :
-  AUTHOR, EMAIL, BUG_ADDRESS are set but not used anywhere, just remove them.
- install( DIRECTORY mycroft DESTINATION /etc ) → use ${SYSCONF_INSTALL_DIR}
instead of /etc
- Instead of using install( DIRECTORY ...), please use ecm_install_icons from
ECM.
- in plugin/CMakeLists.txt, you include pkgconfig but don't use it for
anything

other notes :
- COPYING contains the GPL-2.0 license but nothing needs it
- msm.sh is GPL-3.0+ but there's no license file in the repository
- There's a wikipedia icon in the repo, don't forget the terms (the
attribution part) : https://creativecommons.org/licenses/by-sa/3.0/

- some files in plasmoid/content/ui don't have a license header
- there's a .directory file in plasmoid/contents/images, this should probably
not be in the repo :)

That's all.

KDE CI: Plasma plasma-desktop kf5-qt5 FreeBSDQt5.9 - Build # 74 - Failure!

[CI System]

BUILD FAILURE
 Build URL
https://build.kde.org/job/Plasma%20plasma-desktop%20kf5-qt5%20FreeBSDQt5.9/74/
 Project:
Plasma plasma-desktop kf5-qt5 FreeBSDQt5.9
 Date of build:
Wed, 17 Jan 2018 04:03:40 +
 Build duration:
10 min and counting
   CONSOLE OUTPUT
  [...truncated 536.78 KB...]Unknown property type for key "X-KDE-Keywords[en_GB]" -> falling back to stringUnknown property type for key "X-KDE-Keywords[es]" -> falling back to stringUnknown property type for key "X-KDE-Keywords[et]" -> falling back to stringUnknown property type for key "X-KDE-Keywords[eu]" -> falling back to stringUnknown property type for key "X-KDE-Keywords[fi]" -> falling back to stringUnknown property type for key "X-KDE-Keywords[fr]" -> falling back to stringUnknown property type for key "X-KDE-Keywords[ga]" -> falling back to stringUnknown property type for key "X-KDE-Keywords[gl]" -> falling back to stringUnknown property type for key "X-KDE-Keywords[he]" -> falling back to stringUnknown property type for key "X-KDE-Keywords[hu]" -> falling back to stringUnknown property type for key "X-KDE-Keywords[ia]" -> falling back to stringUnknown property type for key "X-KDE-Keywords[id]" -> falling back to stringUnknown property type for key "X-KDE-Keywords[is]" -> falling back to stringUnknown property type for key "X-KDE-Keywords[it]" -> falling back to stringUnknown property type for key "X-KDE-Keywords[ja]" -> falling back to stringUnknown property type for key "X-KDE-Keywords[kk]" -> falling back to stringUnknown property type for key "X-KDE-Keywords[km]" -> falling back to stringUnknown property type for key "X-KDE-Keywords[ko]" -> falling back to stringUnknown property type for key "X-KDE-Keywords[lt]" -> falling back to stringUnknown property type for key "X-KDE-Keywords[mr]" -> falling back to stringUnknown property type for key "X-KDE-Keywords[nb]" -> falling back to stringUnknown property type for key "X-KDE-Keywords[nds]" -> falling back to stringUnknown property type for key "X-KDE-Keywords[nl]" -> falling back to stringUnknown property type for key "X-KDE-Keywords[nn]" -> falling back to stringUnknown property type for key "X-KDE-Keywords[pa]" -> falling back to stringUnknown property type for key "X-KDE-Keywords[pl]" -> falling back to stringUnknown property type for key "X-KDE-Keywords[pt]" -> falling back to stringUnknown property type for key "X-KDE-Keywords[pt_BR]" -> falling back to stringUnknown property type for key "X-KDE-Keywords[ro]" -> falling back to stringUnknown property type for key "X-KDE-Keywords[ru]" -> falling back to stringUnknown property type for key "X-KDE-Keywords[sk]" -> falling back to stringUnknown property type for key "X-KDE-Keywords[sl]" -> falling back to stringUnknown property type for key "X-KDE-Keywords[sr]" -> falling back to stringUnknown property type for key "X-KDE-Keywords[sr@ijekavian]" -> falling back to stringUnknown property type for key "X-KDE-Keywords[sr@ijekavianlatin]" -> falling back to stringUnknown property type for key "X-KDE-Keywords[sr@latin]" -> falling back to stringUnknown property type for key "X-KDE-Keywords[sv]" -> falling back to stringUnknown property type for key "X-KDE-Keywords[tr]" -> falling back to stringUnknown property type for key "X-KDE-Keywords[ug]" -> falling back to stringUnknown property type for key "X-KDE-Keywords[uk]" -> falling back to stringUnknown property type for key "X-KDE-Keywords[vi]" -> falling back to stringUnknown property type for key "X-KDE-Keywords[x-test]" -> falling back to stringUnknown property type for key "X-KDE-Keywords[zh_CN]" -> falling back to stringUnknown property type for key "X-KDE-Keywords[zh_TW]" -> falling back to stringGenerated  "/usr/home/jenkins/workspace/Plasma plasma-desktop kf5-qt5 FreeBSDQt5.9/build/kcms/desktoptheme/kcm_desktoptheme.json" [ 10%] Automatic MOC for target kcm_desktoptheme[ 10%] Built target kcm_desktoptheme_autogenScanning dependencies of target kcm_standard_actions_autogen[ 11%] Automatic MOC for target kcm_standard_actions[ 11%] Built target kcm_standard_actions_autogenScanning dependencies of target kcm_keys_autogen[ 11%] Automatic MOC for target kcm_keys[ 11%] Built target kcm_keys_autogenScanning dependencies of target kcm_smserver_autogen[ 11%] Automatic MOC for target kcm_smserver[ 11%] Built target kcm_smserver_autogenScanning dependencies of target kcm_lookandfeel-kcms-metadata-json[ 12%] Generating kcm_lookandfeel-kcms-metadata.jsonAbout to parse service type file "/usr/home/jenkins/install-prefix/share/kservicetypes5/plasma-generic.desktop"Unknown property type for key "Keywords" -> falling back to stringUnknown property type for key "X-KDE-ParentApp" -> falling back to stringUnknown property type for key "X-Plasma-API" -> falling back to stringUnknown property type for key "X-Plasma-MainScript" -> falling back to stringUnknown property type for key "X-Plasma-RemoteLocation" -> falling back to stringGenerated  

D9930: Require libinput version 1.9

[David Edmundson]

davidedmundson added a comment.
Restricted Application edited projects, added Plasma; removed KWin.


  Edit: please make sure the CI has this before pushing.

REPOSITORY
  R108 KWin

BRANCH
  require-libinput-1-9

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

To: graesslin, #kwin, #plasma, davidedmundson
Cc: plasma-devel, kwin, ZrenBot, progwolff, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D9848: Updated the blur method to use the more efficient dual kawase blur algorithm.

[Alex Nemeth]

anemeth updated this revision to Diff 25511.
anemeth added a comment.
Restricted Application edited projects, added KWin; removed Plasma.


  Added the .arcconfig file

REPOSITORY
  R108 KWin

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D9848?vs=25504=25511

BRANCH
  master

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

AFFECTED FILES
  effects/blur/blur.cpp
  effects/blur/blur.h
  effects/blur/blur.kcfg
  effects/blur/blur_config.ui
  effects/blur/blurshader.cpp
  effects/blur/blurshader.h
  libkwineffects/kwinglutils.cpp
  libkwineffects/kwinglutils.h

To: anemeth, #plasma, #kwin
Cc: luebking, broulik, romangg, zzag, anthonyfieroni, mart, davidedmundson, 
fredrik, ngraham, plasma-devel, kwin, #kwin, iodelay, bwowk, ZrenBot, 
progwolff, lesliezhai, ali-mohamed, hardening, jensreuterberg, abetts, sebas, 
apol

D9930: Require libinput version 1.9

[David Edmundson]

davidedmundson accepted this revision.
This revision is now accepted and ready to land.
Restricted Application edited projects, added KWin; removed Plasma.

REPOSITORY
  R108 KWin

BRANCH
  require-libinput-1-9

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

To: graesslin, #kwin, #plasma, davidedmundson
Cc: plasma-devel, kwin, iodelay, bwowk, ZrenBot, progwolff, lesliezhai, 
ali-mohamed, hardening, jensreuterberg, abetts, sebas, apol, mart

D1231: Add Remote Access interface to KWayland

[Nathaniel Graham]

ngraham added a comment.


  @graesslin, would you mind reviewing this so we can push forward with the 
feature? Thanks!

REPOSITORY
  R127 KWayland

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

To: Kanedias, graesslin, davidedmundson
Cc: romangg, ngraham, alexeymin, #frameworks, davidedmundson, plasma-devel, 
leezu, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
eliasp, sebas, apol, mart, hein

D1230: GBM remote access support for KWin

[Nathaniel Graham]

ngraham added a comment.


  @graesslin, would you mind reviewing this so we can push forward with the 
feature? Thanks!

REPOSITORY
  R108 KWin

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

To: Kanedias, graesslin, davidedmundson, romangg, #kwin
Cc: romangg, ngraham, alexeymin, aacid, kwin, #kwin, davidedmundson, 
plasma-devel, iodelay, bwowk, leezu, ZrenBot, progwolff, lesliezhai, 
ali-mohamed, hardening, jensreuterberg, abetts, eliasp, sebas, apol, mart, hein

D9848: Updated the blur method to use the more efficient dual kawase blur algorithm.

[Alex Nemeth]

anemeth marked an inline comment as done.
Restricted Application edited projects, added Plasma; removed KWin.

REPOSITORY
  R108 KWin

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

To: anemeth, #plasma, #kwin
Cc: luebking, broulik, romangg, zzag, anthonyfieroni, mart, davidedmundson, 
fredrik, ngraham, plasma-devel, kwin, #kwin, ZrenBot, progwolff, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol

D9848: Updated the blur method to use the more efficient dual kawase blur algorithm.

[Alex Nemeth]

anemeth updated this revision to Diff 25504.
anemeth added a comment.
Restricted Application edited projects, added KWin; removed Plasma.


  Added a new function to GLRenderTarget called `setRenderTargets`
  With this function I implemented the changes suggested by @fredrik to remove 
unnecessary gl calls.
  We achieve this by pushing all GLRenderTargets at once.

REPOSITORY
  R108 KWin

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D9848?vs=25490=25504

BRANCH
  master

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

AFFECTED FILES
  .arcconfig
  effects/blur/blur.cpp
  effects/blur/blur.h
  effects/blur/blur.kcfg
  effects/blur/blur_config.ui
  effects/blur/blurshader.cpp
  effects/blur/blurshader.h
  libkwineffects/kwinglutils.cpp
  libkwineffects/kwinglutils.h

To: anemeth, #plasma, #kwin
Cc: luebking, broulik, romangg, zzag, anthonyfieroni, mart, davidedmundson, 
fredrik, ngraham, plasma-devel, kwin, #kwin, iodelay, bwowk, ZrenBot, 
progwolff, lesliezhai, ali-mohamed, hardening, jensreuterberg, abetts, sebas, 
apol

D1230: GBM remote access support for KWin

[Oleg Chernovskiy]

Kanedias added a comment.


  @ngraham I'm still waiting for a review to land this

REPOSITORY
  R108 KWin

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

To: Kanedias, graesslin, davidedmundson, romangg, #kwin
Cc: romangg, ngraham, alexeymin, aacid, kwin, #kwin, davidedmundson, 
plasma-devel, iodelay, bwowk, leezu, ZrenBot, progwolff, lesliezhai, 
ali-mohamed, hardening, jensreuterberg, abetts, eliasp, sebas, apol, mart, hein

D1231: Add Remote Access interface to KWayland

[Oleg Chernovskiy]

Kanedias added a comment.


  @ngraham, yes, he didn't review this after changes were made

REPOSITORY
  R127 KWayland

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

To: Kanedias, graesslin, davidedmundson
Cc: romangg, ngraham, alexeymin, #frameworks, davidedmundson, plasma-devel, 
leezu, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
eliasp, sebas, apol, mart, hein

D9930: Require libinput version 1.9

[Martin Flöser]

graesslin created this revision.
graesslin added reviewers: KWin, Plasma.
Restricted Application added a project: KWin.
Restricted Application added subscribers: kwin, plasma-devel.
graesslin requested review of this revision.
Restricted Application edited projects, added Plasma; removed KWin.

REPOSITORY
  R108 KWin

BRANCH
  require-libinput-1-9

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

AFFECTED FILES
  CMakeLists.txt
  autotests/libinput/CMakeLists.txt
  autotests/libinput/device_test.cpp
  autotests/libinput/mock_libinput.cpp
  config-kwin.h.cmake
  libinput/connection.cpp
  libinput/device.cpp
  libinput/events.cpp
  libinput/events.h

To: graesslin, #kwin, #plasma
Cc: plasma-devel, kwin, ZrenBot, progwolff, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D1230: GBM remote access support for KWin

[Nathaniel Graham]

ngraham added a comment.


  What's the status of this?

REPOSITORY
  R108 KWin

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

To: Kanedias, graesslin, davidedmundson, romangg, #kwin
Cc: romangg, ngraham, alexeymin, aacid, kwin, #kwin, davidedmundson, 
plasma-devel, iodelay, bwowk, leezu, ZrenBot, progwolff, lesliezhai, 
ali-mohamed, hardening, jensreuterberg, abetts, eliasp, sebas, apol, mart, hein

D1231: Add Remote Access interface to KWayland

[Nathaniel Graham]

ngraham added a comment.


  What's the status of this? Are we waiting for something other than 
@graesslin's review?

REPOSITORY
  R127 KWayland

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

To: Kanedias, graesslin, davidedmundson
Cc: romangg, ngraham, alexeymin, #frameworks, davidedmundson, plasma-devel, 
leezu, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
eliasp, sebas, apol, mart, hein

D9414: Implement OSD to select action when unknown monitors is connected

[Andres Betts]

abetts added a comment.


  Will this OSD need any kind of styling? Or will we just use what was shown 
above in the screenshot?

REPOSITORY
  R104 KScreen

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

To: dvratil, sebas, davidedmundson, #vdg
Cc: abetts, broulik, kamathraghavendra, graesslin, ngraham, plasma-devel, 
mlaurent, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, sebas, 
apol, mart

D9414: Implement OSD to select action when unknown monitors is connected

[Martin Flöser]

graesslin added a comment.


  I just had a look at all the OSD's used and they all have outputOnly set to 
true. I have to say the idea here is clearly that OSDs should be 
non-interactive.

REPOSITORY
  R104 KScreen

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

To: dvratil, sebas, davidedmundson, #vdg
Cc: broulik, kamathraghavendra, graesslin, ngraham, plasma-devel, mlaurent, 
ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, mart

D9414: Implement OSD to select action when unknown monitors is connected

[Martin Flöser]

graesslin added a comment.


  In https://phabricator.kde.org/D9414#191973, @broulik wrote:
  
  > According to KWin code, windows of type `OnScreenDisplay` do not accept 
focus and don't take keyboard input but they can still be clicked (unless set 
to `outputOnly`, obviously).
  
  
  The idea of an OnScreenDisplay is to be non-interactive. If KWin allows to 
pass pointer events to it, this is certainly a bug which needs fixing.

REPOSITORY
  R104 KScreen

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

To: dvratil, sebas, davidedmundson, #vdg
Cc: broulik, kamathraghavendra, graesslin, ngraham, plasma-devel, mlaurent, 
ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, mart

Re: LTS and Wayland

[Martin Flöser]

Am 2018-01-15 23:58, schrieb David Edmundson:

That could simply mean avoiding writing Wayland and LTS in the same
sentence.


Totally fine with that. The only thing I want to have expressed is that 
Wayland is no longer exclude in KWin.


Cheers
Martin

D7246: Tooltips

[Martin Flöser]

graesslin requested changes to this revision.
graesslin added a comment.
This revision now requires changes to proceed.


  I get compile errors:
  
/home/martin/src/kf5/kde/workspace/kdecoration/autotests/mockbridge.cpp: In 
member function ‘virtual std::unique_ptr 
MockBridge::createClient(KDecoration2::DecoratedClient*, 
KDecoration2::Decoration*)’:

/home/martin/src/kf5/kde/workspace/kdecoration/autotests/mockbridge.cpp:27:77: 
error: invalid new-expression of abstract class type ‘MockClient’
 auto ptr = std::unique_ptr(new MockClient(client, 
decoration));

 ^
In file included from 
/home/martin/src/kf5/kde/workspace/kdecoration/autotests/mockbridge.cpp:21:0:
/home/martin/src/kf5/kde/workspace/kdecoration/autotests/mockclient.h:27:7: 
note:   because the following virtual functions are pure within ‘MockClient’:
 class MockClient : public QObject, public 
KDecoration2::ApplicationMenuEnabledDecoratedClientPrivate
   ^~
In file included from 
/home/martin/src/kf5/kde/workspace/kdecoration/autotests/mockclient.h:23:0,
 from 
/home/martin/src/kf5/kde/workspace/kdecoration/autotests/mockbridge.cpp:21:

/home/martin/src/kf5/kde/workspace/kdecoration/autotests/../src/private/decoratedclientprivate.h:79:18:
 note:   virtual void 
KDecoration2::DecoratedClientPrivate::requestShowToolTip(const QString&)
 virtual void requestShowToolTip(const QString ) = 0;
  ^~

/home/martin/src/kf5/kde/workspace/kdecoration/autotests/../src/private/decoratedclientprivate.h:80:18:
 note:   virtual void KDecoration2::DecoratedClientPrivate::requestHideToolTip()
 virtual void requestHideToolTip() = 0;
  ^~
[ 72%] Building CXX object 
autotests/CMakeFiles/decorationButtonTest.dir/mockdecoration.cpp.o
autotests/CMakeFiles/decorationTest.dir/build.make:62: recipe for target 
'autotests/CMakeFiles/decorationTest.dir/mockbridge.cpp.o' failed
make[2]: *** [autotests/CMakeFiles/decorationTest.dir/mockbridge.cpp.o] 
Error 1
make[2]: *** Waiting for unfinished jobs

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

To: McPain, #breeze, #plasma, graesslin
Cc: cfeck, davidedmundson, jriddell, ngraham, broulik, plasma-devel, #breeze, 
ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, mart

D9848: Updated the blur method to use the more efficient dual kawase blur algorithm.

[Alex Nemeth]

anemeth marked 13 inline comments as done.
Restricted Application edited projects, added Plasma; removed KWin.

REPOSITORY
  R108 KWin

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

To: anemeth, #plasma, #kwin
Cc: luebking, broulik, romangg, zzag, anthonyfieroni, mart, davidedmundson, 
fredrik, ngraham, plasma-devel, kwin, #kwin, ZrenBot, progwolff, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol

D9848: Updated the blur method to use the more efficient dual kawase blur algorithm.

[Alex Nemeth]

anemeth updated this revision to Diff 25490.
anemeth added a comment.
Restricted Application edited projects, added KWin; removed Plasma.


  Added a new constructor to GLRenderTarget
  Added a new function `detachTexture()` to GLRenderTarget
  Implemented most of the changes suggested by @fredrik

REPOSITORY
  R108 KWin

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D9848?vs=25436=25490

BRANCH
  master

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

AFFECTED FILES
  .arcconfig
  effects/blur/blur.cpp
  effects/blur/blur.h
  effects/blur/blur.kcfg
  effects/blur/blur_config.ui
  effects/blur/blurshader.cpp
  effects/blur/blurshader.h
  libkwineffects/kwinglutils.cpp
  libkwineffects/kwinglutils.h

To: anemeth, #plasma, #kwin
Cc: luebking, broulik, romangg, zzag, anthonyfieroni, mart, davidedmundson, 
fredrik, ngraham, plasma-devel, kwin, #kwin, iodelay, bwowk, ZrenBot, 
progwolff, lesliezhai, ali-mohamed, hardening, jensreuterberg, abetts, sebas, 
apol

D9891: [Style KCM] Sync KConfig before announcing change

[Kai Uwe Broulik]

This revision was automatically updated to reflect the committed changes.
Closed by commit R119:6ae4277121ab: [Style KCM] Sync KConfig before announcing 
change (authored by broulik).

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D9891?vs=25398=25488

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

AFFECTED FILES
  kcms/style/kcmstyle.cpp

To: broulik, #plasma, apol
Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D9914: [Folder View] Don't trash local files

[Kai Uwe Broulik]

This revision was automatically updated to reflect the committed changes.
Closed by commit R119:5551d9be17dd: [Folder View] Dont trash local files 
(authored by broulik).

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D9914?vs=25448=25489

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

AFFECTED FILES
  containments/desktop/plugins/folder/foldermodel.cpp
  containments/desktop/plugins/folder/foldermodel.h

To: broulik, #plasma, hein
Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D9414: Implement OSD to select action when unknown monitors is connected

[Kai Uwe Broulik]

broulik added a comment.


  According to KWin code, windows of type `OnScreenDisplay` do not accept focus 
and don't take keyboard input but they can still be clicked (unless set to 
`outputOnly`, obviously).

REPOSITORY
  R104 KScreen

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

To: dvratil, sebas, davidedmundson, #vdg
Cc: broulik, kamathraghavendra, graesslin, ngraham, plasma-devel, mlaurent, 
ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, mart

D9414: Implement OSD to select action when unknown monitors is connected

[Sebastian Kügler]

sebas added a comment.


  In https://phabricator.kde.org/D9414#191971, @dvratil wrote:
  
  > In https://phabricator.kde.org/D9414#191653, @broulik wrote:
  >
  > > I think `OnScreenDisplay` implies being non-interactive on Wayland
  >
  >
  > I could interact with the dialog on Wayland, so either it's not implied, or 
there's a bug in KWin :-)
  
  
  We might as well remove that flag, since it's confusing to Kai and Martin ;-)

REPOSITORY
  R104 KScreen

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

To: dvratil, sebas, davidedmundson, #vdg
Cc: broulik, kamathraghavendra, graesslin, ngraham, plasma-devel, mlaurent, 
ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, mart

D9414: Implement OSD to select action when unknown monitors is connected

[Daniel Vrátil]

dvratil added a comment.


  In https://phabricator.kde.org/D9414#191653, @broulik wrote:
  
  > I think `OnScreenDisplay` implies being non-interactive on Wayland
  
  
  I could interact with the dialog on Wayland, so either it's not implied, or 
there's a bug in KWin :-)

REPOSITORY
  R104 KScreen

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

To: dvratil, sebas, davidedmundson, #vdg
Cc: broulik, kamathraghavendra, graesslin, ngraham, plasma-devel, mlaurent, 
ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, mart

D9908: Wayland: add support for output type detection

[Daniel Vrátil]

This revision was automatically updated to reflect the committed changes.
Closed by commit R110:b1428d1381e8: Wayland: add support for output type 
detection (authored by dvratil).

REPOSITORY
  R110 KScreen Library

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D9908?vs=25431=25486

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

AFFECTED FILES
  backends/kwayland/CMakeLists.txt
  backends/kwayland/waylandoutput.cpp
  backends/utils.cpp
  backends/utils.h
  backends/xrandr/CMakeLists.txt
  backends/xrandr/xrandroutput.cpp

To: dvratil, sebas, davidedmundson
Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D9906: Wayland: don't update output configuration if there are no changes

[Daniel Vrátil]

This revision was automatically updated to reflect the committed changes.
Closed by commit R110:daf107948d58: Wayland: dont update output 
configuration if there are no changes (authored by dvratil).

REPOSITORY
  R110 KScreen Library

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D9906?vs=25428=25485

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

AFFECTED FILES
  backends/kwayland/waylandconfig.cpp

To: dvratil, sebas, davidedmundson
Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D9905: Don't create multiple in-process backend instances

[Daniel Vrátil]

This revision was automatically updated to reflect the committed changes.
Closed by commit R110:6e37f3034f6c: Dont create multiple in-process 
backend instances (authored by dvratil).

REPOSITORY
  R110 KScreen Library

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D9905?vs=25427=25484

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

AFFECTED FILES
  src/backendmanager.cpp

To: dvratil, sebas, davidedmundson
Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D9904: Wayland: don't share the same Config between backend and clients

[Daniel Vrátil]

This revision was automatically updated to reflect the committed changes.
Closed by commit R110:4abdc5b134a3: Wayland: dont share the same Config 
between backend and clients (authored by dvratil).

REPOSITORY
  R110 KScreen Library

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D9904?vs=25424=25483

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

AFFECTED FILES
  src/backendmanager.cpp
  src/configmonitor.cpp
  src/getconfigoperation.cpp

To: dvratil, sebas, davidedmundson
Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D9903: KDED: respond to output being completely removed

[Daniel Vrátil]

This revision was automatically updated to reflect the committed changes.
Closed by commit R104:25d030dd0db1: KDED: respond to output being completely 
removed (authored by dvratil).

REPOSITORY
  R104 KScreen

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D9903?vs=25423=25482

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

AFFECTED FILES
  kded/daemon.cpp

To: dvratil, sebas, davidedmundson
Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D9902: KDED: restore output scale from config

[Daniel Vrátil]

This revision was automatically updated to reflect the committed changes.
Closed by commit R104:3129f5089ef2: KDED: restore output scale from config 
(authored by dvratil).

REPOSITORY
  R104 KScreen

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D9902?vs=25422=25481

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

AFFECTED FILES
  kded/serializer.cpp

To: dvratil, sebas, davidedmundson
Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D9908: Wayland: add support for output type detection

[Sebastian Kügler]

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


  Nice!

REPOSITORY
  R110 KScreen Library

BRANCH
  master

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

To: dvratil, sebas, davidedmundson
Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D9906: Wayland: don't update output configuration if there are no changes

[Sebastian Kügler]

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


  LGTM.

REPOSITORY
  R110 KScreen Library

BRANCH
  master

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

To: dvratil, sebas, davidedmundson
Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D9904: Wayland: don't share the same Config between backend and clients

[Sebastian Kügler]

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


  LGTM as well.

REPOSITORY
  R110 KScreen Library

BRANCH
  master

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

To: dvratil, sebas, davidedmundson
Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D9903: KDED: respond to output being completely removed

[Sebastian Kügler]

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


  Good catch!

REPOSITORY
  R104 KScreen

BRANCH
  master

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

To: dvratil, sebas, davidedmundson
Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D9463: XRandR: implement scaling support

[Sebastian Kügler]

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


  I'm not suitable to review the X11 bits in here, but since you are the former 
maintainer, I trust you on those.

INLINE COMMENTS

> config.h:69
> +PerOutputScaling = 1 << 2, ///< The backend supports scaling each 
> output individually.
> +NonIntegerScaling = 1 << 3 ///< The backend supports scaling by 
> non-integer factor
>  };

I don't like the negation in the API, might as well tell what is does rather 
than what it doesn't do.

FractionalScaling?

REPOSITORY
  R110 KScreen Library

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

To: dvratil, sebas, davidedmundson
Cc: ngraham, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D7681: Update human-readable tier designation in API dox

[Adriaan de Groot]

This revision was automatically updated to reflect the committed changes.
Closed by commit R169:8a233471fc54: Update human-readable tier designation in 
API dox (authored by adridg).

REPOSITORY
  R169 Kirigami

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D7681?vs=19146=25479

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

AFFECTED FILES
  Mainpage.dox

To: adridg, dfaure, mart
Cc: plasma-devel, apol, davidedmundson, mart, hein

D9848: Updated the blur method to use the more efficient dual kawase blur algorithm.

[Alex Nemeth]

anemeth added inline comments.
Restricted Application edited projects, added Plasma; removed KWin.

INLINE COMMENTS

> fredrik wrote in blur.cpp:145
> Why is this needed?
> 
> I'm probably missing something here, but it looks to me as if the effect 
> copies the contents of the framebuffer to the helper texture, then copies the 
> contents of that texture to m_renderTextures[0], after which the contents of 
> helper texture is not used again. Can't copyScreenSampleTexture() copy 
> directly from the framebuffer to m_renderTextures[0]?

This texture is only used by `copyScreenSampleTexture()`
It could very well be a separate variable, but I just appended another texture 
to the textures vector and used that instead.
The reason for first rendering into m_renderTextures.last() and then copying 
that to m_renderTextures[0] is to eliminate what I call "extended blur".
Extended blur is when windows or other elements that are not under the blurred 
area affect the blur effect. This sounds great in theory, and this is how the 
old blur method worked as well, but it becomes a big issue when the blur radius 
gets big. For example when you maximize a white window, the completely 
transparent taskbar also becomes almost completely white because of this 
effect, so the way I achieve this is by creating a GL_CLAMP_TO_EDGE effect.
F5656755: c3_clamping.png 
For example if we want to blur a window (red) we have to blur a bigger area. To 
avoid extended blur I use the copySample shader to create a GL_CLAMP_TO_EDGE 
effect (blue) when copying the texture from m_renderTextures.last() to 
m_renderTextures[0]

Ideally we could specify to only disable extended blur on the taskbar, but I 
don't see a way to identify a window as the taskbar.

REPOSITORY
  R108 KWin

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

To: anemeth, #plasma, #kwin
Cc: luebking, broulik, romangg, zzag, anthonyfieroni, mart, davidedmundson, 
fredrik, ngraham, plasma-devel, kwin, #kwin, ZrenBot, progwolff, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol

D8816: Show root items in kicker menu

[Eike Hein]

hein added a comment.


  He doesn't (as discussed above).

REPOSITORY
  R119 Plasma Desktop

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

To: lopatin, #plasma, hein
Cc: davidedmundson, ngraham, hein, mart, plasma-devel, ZrenBot, progwolff, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol

D8816: Show root items in kicker menu

[David Edmundson]

davidedmundson added a comment.


  Do you have commit access?

REPOSITORY
  R119 Plasma Desktop

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

To: lopatin, #plasma, hein
Cc: davidedmundson, ngraham, hein, mart, plasma-devel, ZrenBot, progwolff, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol

D8816: Show root items in kicker menu

[Eike Hein]

hein accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R119 Plasma Desktop

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

To: lopatin, #plasma, hein
Cc: ngraham, hein, mart, plasma-devel, ZrenBot, progwolff, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol

D9914: [Folder View] Don't trash local files

[Eike Hein]

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


  Nice! This was based on older KDE 4 code ...

REPOSITORY
  R119 Plasma Desktop

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

To: broulik, #plasma, hein
Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D9917: Add New and Polished Animations

[Aditya Mehra]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit R846:6963245e8619: Add New and Polished Animations (authored 
by Aiix).

REPOSITORY
  R846 Mycroft Plasma integration

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D9917?vs=25454=25455

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

AFFECTED FILES
  plasmoid/contents/ui/CustomIndicator.qml
  plasmoid/contents/ui/main.qml

To: Aiix, #plasma
Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D9917: Add New and Polished Animations

[Aditya Mehra]

Aiix created this revision.
Aiix added a reviewer: Plasma.
Restricted Application added a project: Plasma.
Restricted Application added a subscriber: plasma-devel.
Aiix requested review of this revision.

REVISION SUMMARY
  Ref https://phabricator.kde.org/T7762, https://phabricator.kde.org/T7175
  
  Changed previous static animation to contextual animations based on Mycroft 
Logo. The animation set now includes transition animation, intent 
recognized/connection success animation and intent failure/connection error 
animation.

REPOSITORY
  R846 Mycroft Plasma integration

BRANCH
  master

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

AFFECTED FILES
  plasmoid/contents/ui/CustomIndicator.qml
  plasmoid/contents/ui/main.qml

To: Aiix, #plasma
Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D9019: [Calendar] Adjust the calendar toolbar anchors

[Alexandr Akulich]

This revision was automatically updated to reflect the committed changes.
Closed by commit R242:55efe5ae6357: [Calendar] Adjust the calendar toolbar 
anchors (authored by akulichalexandr).

REPOSITORY
  R242 Plasma Framework (Library)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D9019?vs=23032=25449

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

AFFECTED FILES
  src/declarativeimports/calendar/qml/DaysCalendar.qml

To: akulichalexandr, #plasma, broulik
Cc: aacid, davidedmundson, broulik, plasma-devel, #plasma, #frameworks, 
ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, mart

D9914: [Folder View] Don't trash local files

[Kai Uwe Broulik]

broulik created this revision.
broulik added reviewers: Plasma, hein.
Restricted Application added a project: Plasma.
Restricted Application added a subscriber: plasma-devel.
broulik requested review of this revision.

REVISION SUMMARY
  Send the actual URL (e.g. `desktop:/foo`) to KIO and let it figure it out. 
Otherwise `KDirNotify` emits a change for the backing 
`file:///home/foo/Desktop` URL and `KDirLister` cannot relate the two and 
doesn't notice a deletion immediately.
  Since we don't offer "Move to Trash" for non-local files anyway, this should 
be safe.

TEST PLAN
  This fixes files being moved to trash from the desktop taking half a second, 
now it's instant.
  
  While at it, check for whether files can be moved before offering Trash and 
wehther files can be deleted before offering Delete (Dolphin does that). I can 
split that into a separate commit, if you prefer.
  
  Hitting "Delete" key on a remote file will show "access denied" but that's a 
different issue as FolderView does not (iirc that was done on purpose) 
automatically fall back to "Delete" when "Move to Trash" is unavailable as 
Dolphin would.

REPOSITORY
  R119 Plasma Desktop

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

AFFECTED FILES
  containments/desktop/plugins/folder/foldermodel.cpp
  containments/desktop/plugins/folder/foldermodel.h

To: broulik, #plasma, hein
Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

Re: LTS and Wayland

[Marco Martin]

On lunedì 15 gennaio 2018 23:58:40 CET David Edmundson wrote:
> I want to stick very clear from writing "our wayland support is LTS" on the
> annoucement.
> 
> That could simply mean avoiding writing Wayland and LTS in the same
> sentence.
+1 for that,
we threat it as such, but we don't put it in the announcement as we know it 
will be misinterpreted

-- 
Marco Martin

D9892: cmake: Use the official CMake variable for building as a static plugin.

[Aleix Pol Gonzalez]

apol added a comment.


  In https://phabricator.kde.org/D9892#191707, @mart wrote:
  
  > In https://phabricator.kde.org/D9892#191590, @apol wrote:
  >
  > > I wonder if it would make sense to always ship the qml files as qrc in 
the plugin. Have you considered it?
  >
  >
  > may make sense...
  >  i would save it for another poatch tough this is big enough already :)
  
  
  Okay, can you go through the questions I put in this patch?

REPOSITORY
  R169 Kirigami

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

To: mart, #kirigami, lepagevalleeemmanuel
Cc: apol, plasma-devel, davidedmundson, mart, hein

D9892: cmake: Use the official CMake variable for building as a static plugin.

[Marco Martin]

mart added a comment.


  In https://phabricator.kde.org/D9892#191590, @apol wrote:
  
  > I wonder if it would make sense to always ship the qml files as qrc in the 
plugin. Have you considered it?
  
  
  may make sense...
  i would save it for another poatch tough this is big enough already :)

REPOSITORY
  R169 Kirigami

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

To: mart, #kirigami, lepagevalleeemmanuel
Cc: apol, plasma-devel, davidedmundson, mart, hein

D9848: Updated the blur method to use the more efficient dual kawase blur algorithm.

[Fredrik Höglund]

fredrik added a comment.
Restricted Application edited projects, added Plasma; removed KWin.


  Don't forget to add your name to the license headers.

INLINE COMMENTS

> blur.cpp:46
>  m_simpleShader = 
> ShaderManager::instance()->generateShaderFromResources(ShaderTrait::MapTexture,
>  QString(), QStringLiteral("logout-blur.frag"));
> +m_simpleTarget = new GLRenderTarget(GLTexture(GL_RGBA8, 1, 1));
> +

You should add a GLRenderTarget constructor that takes no arguments.
This is working around deficiencies in the API.

> blur.cpp:116
> +return !target->valid();
> +}) == m_renderTargets.cend();
> +}

I suggest doing this check after creating the render targets, and caching the 
result.

> blur.cpp:135
> +deleteFBOs();
> +
> +for (int i = 0; i <= m_downSampleIterations; i++) {

Call reserve() on m_renderTextures and m_renderTargets here.

> blur.cpp:137
> +for (int i = 0; i <= m_downSampleIterations; i++) {
> +m_renderTextures.append(GLTexture(GL_RGBA8, 
> effects->virtualScreenSize() / qPow(2, i)));
> +m_renderTextures.last().setFilter(GL_LINEAR);

1 << i

> blur.cpp:145
> +// This last set is used as a temporary helper texture
> +m_renderTextures.append(GLTexture(GL_RGBA8, 
> effects->virtualScreenSize()));
> +m_renderTextures.last().setFilter(GL_LINEAR);

Why is this needed?

I'm probably missing something here, but it looks to me as if the effect copies 
the contents of the framebuffer to the helper texture, then copies the contents 
of that texture to m_renderTextures[0], after which the contents of helper 
texture is not used again. Can't copyScreenSampleTexture() copy directly from 
the framebuffer to m_renderTextures[0]?

> blur.cpp:694
> +m_simpleTarget->attachTexture(blurTexture);
> +m_simpleTarget->blitFromFramebuffer(w->geometry(), QRect(QPoint(0, 0), 
> w->size()));
>  

Detach the texture from the render target before you return from this method - 
otherwise the FBO continues to hold a reference to the texture, preventing it 
from being deleted.

> blur.cpp:123
> +{
> +for (int i = 0; i < m_renderTargets.size(); i++) {
> +delete m_renderTargets[i];

You can use qDeleteAll() here.

> blur.cpp:127
> +
> +m_renderTextures[i].discard();
> +}

There is no need to call discard on the textures - just clear the vector.

> blur.cpp:374
> +for (int i = 0; i <= downSampleIterations; i++) {
> +const int divisionRatio = qPow(2, i);
> +

1 << i

> blur.cpp:717
> +vbo->draw(GL_TRIANGLES, blurRectCount * i, blurRectCount);
> +GLRenderTarget::popRenderTarget();
> +}

These three lines of code will expand to an unfortunate sequence of GL calls:

  // Iteration N
  glGetIntegerv(GL_VIEWPORT, ...);
  glBindFramebuffer(GL_FRAMEBUFFER, ...);
  glViewport(...);
  glDrawArrays(...);
  glBindFramebuffer(GL_FRAMEBUFFER, 0);
  glViewport(...);
  
  // Iteration N+1
  glGetIntegerv(GL_VIEWPORT, ..);
  glBindFramebuffer(GL_FRAMEBUFFER, ...);
  glViewport(...);
  glDrawArrays(...);
  glBindFramebuffer(GL_FRAMEBUFFER, 0);
  glViewport(...);

Note the redundant calls to glViewport() and glBindFramebuffer(). The worst 
offender here is glGetIntegerv() however, because it forces serialization of 
the internal driver threads.

Ideally the call sequence should look like this:

  // Iteration N
  glBindFramebuffer(GL_FRAMEBUFFER, ...);
  glViewport(...);
  glDrawArrays(...);
  
  // Iteration N+1
  glBindFramebuffer(GL_FRAMEBUFFER, ...);
  glViewport(...);
  glDrawArrays(...);

This can be fixed in a followup patch though.

> blur.h:41
>  
> +static const int borderSize = 5;
> +

This number could use an explanation.

> blurshader.cpp:241
> +"uniform float offset;\n"
> +"uniform vec2 textureSize;\n";
> +

textureSize is also the name of a built-in function in GLSL, so I suggest 
changing the name to targetSize, renderTargetSize, framebufferSize or something 
similar. That also makes it clear that it is not the size of the texture being 
sampled.

> blurshader.h:95
> +GLShader *m_shaderUpsample = nullptr;
> +GLShader *m_shaderCopysample = nullptr;
> +

I think you could simplify the code quite a bit by having an array of

  struct {
  GLShader *shader;
  int mvpMatrixLocation;
  ...
  };

and use m_activeSampleType as an index into that array.

> blurshader.h:47
> +virtual void setOffset(float offset) = 0;
> +virtual void setTextureSize(QSize textureSize) = 0;
> +virtual void setBlurRect(QRect blurRect, QSize screenSize) = 0;

I suggest changing the name to setTargetSize() to make it clear that the size 
is the size of the render target, and not the size of the texture being sampled.

> blurshader.h:50
>  
> -virtual void bind() = 0;
> +virtual void bind(int sampleType) = 0;
>  virtual void unbind() = 0;

sampleTypeEnum sampleType

> blurshader.h:53
> +
> +enum sampleTypeEnum {
> +downSampleType,

The first 

D9506: milti-screen sleep fix

[Oleg Solovyov]

McPain added a comment.


  I noticed that "shutdown" sequence is not the same as "standby" one.
  Shutdown for secondary:
  disconnect -> remove crtc -> connect -> assign crtc
  
  Standby for secondary:
  disconnect -> connect (why?) -> remove crtc -> assign crtc

REPOSITORY
  R110 KScreen Library

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

To: McPain, sebas, drosca
Cc: zerg, anthonyfieroni, ngraham, plasma-devel, ZrenBot, progwolff, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart