D27669: [kstyle] Tools area

[Carson Black]

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 R31:ea978ea6eb36: [kstyle] Tools area (authored by cblack).

REPOSITORY
  R31 Breeze

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27669?vs=83075=83253

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

AFFECTED FILES
  kstyle/CMakeLists.txt
  kstyle/breeze.kcfg
  kstyle/breezehelper.cpp
  kstyle/breezehelper.h
  kstyle/breezestyle.cpp
  kstyle/breezestyle.h
  kstyle/breezetoolsareamanager.cpp
  kstyle/breezetoolsareamanager.h
  kstyle/config/breezestyleconfig.cpp
  kstyle/config/ui/breezestyleconfig.ui

To: cblack, #plasma, #breeze, #vdg, hpereiradacosta, davidre, ngraham
Cc: mart, maartens, abstractdevelop, IlyaBizyaev, davidre, davidedmundson, 
hpereiradacosta, ngraham, manueljlin, niccolove, ndavis, plasma-devel, Orage, 
LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, ragreen, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra

D27669: [kstyle] Tools area

[Carson Black]

cblack added a comment.


  In D27669#673112 , @ndavis wrote:
  
  > what are some ways the issue of toolbar colors not being perfectly in sync 
with titlebar colors could be fixed or worked around?
  
  
  When animating? Wayland.

REPOSITORY
  R31 Breeze

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

To: cblack, #plasma, #breeze, #vdg, hpereiradacosta, davidre, ngraham
Cc: mart, maartens, abstractdevelop, IlyaBizyaev, davidre, davidedmundson, 
hpereiradacosta, ngraham, manueljlin, niccolove, ndavis, plasma-devel, Orage, 
LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, ragreen, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra

D27669: [kstyle] Tools area

[Noah Davis]

ndavis added a comment.


  The line seems to stop just short of the sidebar: F8339738: 
Screenshot_20200520_001106.png 

REPOSITORY
  R31 Breeze

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

To: cblack, #plasma, #breeze, #vdg, hpereiradacosta, davidre, ngraham
Cc: mart, maartens, abstractdevelop, IlyaBizyaev, davidre, davidedmundson, 
hpereiradacosta, ngraham, manueljlin, niccolove, ndavis, plasma-devel, Orage, 
LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, ragreen, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra

D27669: [kstyle] Tools area

[Noah Davis]

ndavis added a comment.


  what are some ways the issue of toolbar colors not being perfectly in sync 
with titlebar colors could be fixed or worked around?

REPOSITORY
  R31 Breeze

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

To: cblack, #plasma, #breeze, #vdg, hpereiradacosta, davidre, ngraham
Cc: mart, maartens, abstractdevelop, IlyaBizyaev, davidre, davidedmundson, 
hpereiradacosta, ngraham, manueljlin, niccolove, ndavis, plasma-devel, Orage, 
LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, ragreen, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra

D27669: [kstyle] Tools area

[Nathaniel Graham]

ngraham added a comment.


  Now disabled toolbar items don't use the disabled text color when disabled.

REPOSITORY
  R31 Breeze

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

To: cblack, #plasma, #breeze, #vdg, hpereiradacosta, davidre, ngraham
Cc: mart, maartens, abstractdevelop, IlyaBizyaev, davidre, davidedmundson, 
hpereiradacosta, ngraham, manueljlin, niccolove, ndavis, plasma-devel, Orage, 
LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, ragreen, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra

D27669: [kstyle] Tools area

[Carson Black]

cblack updated this revision to Diff 83075.
cblack added a comment.


  Properly determine if tools area should be drawn on QDialogs with auto set

REPOSITORY
  R31 Breeze

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27669?vs=81137=83075

BRANCH
  arcpatch-D27669_2

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

AFFECTED FILES
  kstyle/CMakeLists.txt
  kstyle/breeze.kcfg
  kstyle/breezehelper.cpp
  kstyle/breezehelper.h
  kstyle/breezestyle.cpp
  kstyle/breezestyle.h
  kstyle/breezetoolsareamanager.cpp
  kstyle/breezetoolsareamanager.h
  kstyle/config/breezestyleconfig.cpp
  kstyle/config/ui/breezestyleconfig.ui

To: cblack, #plasma, #breeze, #vdg, hpereiradacosta, davidre, ngraham
Cc: mart, maartens, abstractdevelop, IlyaBizyaev, davidre, davidedmundson, 
hpereiradacosta, ngraham, manueljlin, niccolove, ndavis, plasma-devel, Orage, 
LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, ragreen, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra

D27669: [kstyle] Tools area

[Nathaniel Graham]

ngraham added a comment.


  I think this should land early in the 5.20 cycle so we have a lot of time for 
testing. However I think we need to land it alongside the new colorscheme so 
people don't get a bad first impression, or else re-work it to pull the color 
from a new "toolsarea" color set that we add to KColorScheme.

REPOSITORY
  R31 Breeze

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

To: cblack, #plasma, #breeze, #vdg, hpereiradacosta, davidre, ngraham
Cc: mart, maartens, abstractdevelop, IlyaBizyaev, davidre, davidedmundson, 
hpereiradacosta, ngraham, manueljlin, niccolove, ndavis, plasma-devel, Orage, 
LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, ragreen, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra

D27669: [kstyle] Tools area

[Nathaniel Graham]

ngraham added a comment.


  In D27669#664515 , @ndavis wrote:
  
  > Reposting here so that it won't be forgotten. Some windows are lacking the 
underline:
  >
  > F8288512: Screenshot_20200506_002747.png 

  >  F8288514: Screenshot_20200506_002908.png 

  
  
  Can confirm this with the current state of the patch.

REPOSITORY
  R31 Breeze

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

To: cblack, #plasma, #breeze, #vdg, hpereiradacosta, davidre, ngraham
Cc: mart, maartens, abstractdevelop, IlyaBizyaev, davidre, davidedmundson, 
hpereiradacosta, ngraham, manueljlin, niccolove, ndavis, plasma-devel, Orage, 
LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, ragreen, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra

D27669: [kstyle] Tools area

[Noah Davis]

ndavis added a comment.


  Reposting here so that it won't be forgotten. Some windows are lacking the 
underline:
  
  F8288512: Screenshot_20200506_002747.png 

  F8288514: Screenshot_20200506_002908.png 

  
  For some reason, the settings window for KDevelop takes roughly 4x longer to 
load with this patch. I'm not sure why that is and I don't see any suspicious 
messages that seem related to this patch in GDB.

REPOSITORY
  R31 Breeze

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

To: cblack, #plasma, #breeze, #vdg, hpereiradacosta, davidre, ngraham
Cc: mart, maartens, abstractdevelop, IlyaBizyaev, davidre, davidedmundson, 
hpereiradacosta, ngraham, manueljlin, niccolove, ndavis, plasma-devel, Orage, 
LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, ragreen, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra

D27669: [kstyle] Tools area

[Noah Davis]

ndavis added a comment.


  The only remaining problem I can find is this one with certain kinds of menu 
buttons:
  F8260819: vokoscreenNG-2020-04-26_00-43-00.mp4 


REPOSITORY
  R31 Breeze

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

To: cblack, #plasma, #breeze, #vdg, hpereiradacosta, davidre, ngraham
Cc: mart, maartens, abstractdevelop, IlyaBizyaev, davidre, davidedmundson, 
hpereiradacosta, ngraham, manueljlin, niccolove, ndavis, plasma-devel, Orage, 
LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, ragreen, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra

D27669: [kstyle] Tools area

[Carson Black]

cblack updated this revision to Diff 81137.
cblack added a comment.


  Re-add empty drawing for menu bar

REPOSITORY
  R31 Breeze

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27669?vs=81133=81137

BRANCH
  arcpatch-D27669_1

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

AFFECTED FILES
  kstyle/CMakeLists.txt
  kstyle/breeze.kcfg
  kstyle/breezehelper.cpp
  kstyle/breezehelper.h
  kstyle/breezestyle.cpp
  kstyle/breezestyle.h
  kstyle/breezetoolsareamanager.cpp
  kstyle/breezetoolsareamanager.h
  kstyle/config/breezestyleconfig.cpp
  kstyle/config/ui/breezestyleconfig.ui

To: cblack, #plasma, #breeze, #vdg, hpereiradacosta, davidre, ngraham
Cc: mart, maartens, abstractdevelop, IlyaBizyaev, davidre, davidedmundson, 
hpereiradacosta, ngraham, manueljlin, niccolove, ndavis, plasma-devel, Orage, 
LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, ragreen, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra

D27669: [kstyle] Tools area

[Nathaniel Graham]

ngraham added a comment.


  That fixes the crashes as well as the performance issues I was seeing. 
However every app with a menubar now shows a lighter-colored rectangle to the 
right of the menu area: F8258365: vokoscreenNG-2020-04-24_14-31-10.webm 


REPOSITORY
  R31 Breeze

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

To: cblack, #plasma, #breeze, #vdg, hpereiradacosta, davidre, ngraham
Cc: mart, maartens, abstractdevelop, IlyaBizyaev, davidre, davidedmundson, 
hpereiradacosta, ngraham, manueljlin, niccolove, ndavis, plasma-devel, Orage, 
LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, ragreen, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra

D27669: [kstyle] Tools area

[Carson Black]

cblack updated this revision to Diff 81133.
cblack added a comment.


  Disconnect signals on destruction

REPOSITORY
  R31 Breeze

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27669?vs=81123=81133

BRANCH
  arcpatch-D27669_1

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

AFFECTED FILES
  kstyle/CMakeLists.txt
  kstyle/breeze.kcfg
  kstyle/breezehelper.cpp
  kstyle/breezehelper.h
  kstyle/breezestyle.cpp
  kstyle/breezestyle.h
  kstyle/breezetoolsareamanager.cpp
  kstyle/breezetoolsareamanager.h
  kstyle/config/breezestyleconfig.cpp
  kstyle/config/ui/breezestyleconfig.ui

To: cblack, #plasma, #breeze, #vdg, hpereiradacosta, davidre, ngraham
Cc: mart, maartens, abstractdevelop, IlyaBizyaev, davidre, davidedmundson, 
hpereiradacosta, ngraham, manueljlin, niccolove, ndavis, plasma-devel, Orage, 
LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, ragreen, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra

D27669: [kstyle] Tools area

[Noah Davis]

ndavis added inline comments.

INLINE COMMENTS

> breezetoolsareamanager.cpp:127
> +auto geoUpdate = [=]() {
> +_helper->_invalidateCachedRects = true;
> +emit toolbarUpdated();

I misclicked. It's this line with `_helper->_invalidateCachedRects = true;`

REPOSITORY
  R31 Breeze

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

To: cblack, #plasma, #breeze, #vdg, hpereiradacosta, davidre, ngraham
Cc: mart, maartens, abstractdevelop, IlyaBizyaev, davidre, davidedmundson, 
hpereiradacosta, ngraham, manueljlin, niccolove, ndavis, plasma-devel, Orage, 
LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, ragreen, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra

D27669: [kstyle] Tools area

[Noah Davis]

ndavis added inline comments.

INLINE COMMENTS

> ndavis wrote in breezetoolsareamanager.cpp:127
> When I run GDB on `oxygen-demo5`, sometimes it tells me that `oxygen-demo5` 
> crashes on this line. I'm not sure how to make it happen reliably, but it 
> usually involves changing QStyles with the Style combobox or color schemes 
> with the Color Scheme combobox.

Now I can consistently get oxygen-demo5 to crash on this line by switching to 
Fusion and then Oxygen.

REPOSITORY
  R31 Breeze

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

To: cblack, #plasma, #breeze, #vdg, hpereiradacosta, davidre, ngraham
Cc: mart, maartens, abstractdevelop, IlyaBizyaev, davidre, davidedmundson, 
hpereiradacosta, ngraham, manueljlin, niccolove, ndavis, plasma-devel, Orage, 
LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, ragreen, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra

D27669: [kstyle] Tools area

[Carson Black]

cblack updated this revision to Diff 81123.
cblack added a comment.


  Address tooling warnings

REPOSITORY
  R31 Breeze

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27669?vs=80819=81123

BRANCH
  arcpatch-D27669_1

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

AFFECTED FILES
  kstyle/CMakeLists.txt
  kstyle/breeze.kcfg
  kstyle/breezehelper.cpp
  kstyle/breezehelper.h
  kstyle/breezestyle.cpp
  kstyle/breezestyle.h
  kstyle/breezetoolsareamanager.cpp
  kstyle/breezetoolsareamanager.h
  kstyle/config/breezestyleconfig.cpp
  kstyle/config/ui/breezestyleconfig.ui

To: cblack, #plasma, #breeze, #vdg, hpereiradacosta, davidre, ngraham
Cc: mart, maartens, abstractdevelop, IlyaBizyaev, davidre, davidedmundson, 
hpereiradacosta, ngraham, manueljlin, niccolove, ndavis, plasma-devel, Orage, 
LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, ragreen, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra

D27669: [kstyle] Tools area

[Noah Davis]

ndavis added a comment.


  I'm seeing a lot of this message in GDB when I switch from a different QStyle 
such as Fusion back to Breeze:
  
QBackingStore::endPaint() called with active painter; did you forget to 
destroy it or call QPainter::end() on it?
  
  I also see this sometimes:
  
QWidget::repaint: Recursive repaint detected

INLINE COMMENTS

> breezetoolsareamanager.cpp:127
> +auto geoUpdate = [=]() {
> +_helper->_invalidateCachedRects = true;
> +emit toolbarUpdated();

When I run GDB on `oxygen-demo5`, sometimes it tells me that `oxygen-demo5` 
crashes on this line. I'm not sure how to make it happen reliably, but it 
usually involves changing QStyles with the Style combobox or color schemes with 
the Color Scheme combobox.

REPOSITORY
  R31 Breeze

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

To: cblack, #plasma, #breeze, #vdg, hpereiradacosta, davidre, ngraham
Cc: mart, maartens, abstractdevelop, IlyaBizyaev, davidre, davidedmundson, 
hpereiradacosta, ngraham, manueljlin, niccolove, ndavis, plasma-devel, Orage, 
LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, ragreen, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra

D27669: [kstyle] Tools area

[Marco Martin]

mart added a comment.


  Is on the right track but I think the technical approach needs more 
discussion, i think something needs to be encoded into KColorScheme, like a new 
color group (probably independent from titlebars actually)

REPOSITORY
  R31 Breeze

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

To: cblack, #plasma, #breeze, #vdg, hpereiradacosta, davidre, ngraham
Cc: mart, maartens, abstractdevelop, IlyaBizyaev, davidre, davidedmundson, 
hpereiradacosta, ngraham, manueljlin, niccolove, ndavis, plasma-devel, Orage, 
LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, ragreen, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra

D27669: [kstyle] Tools area

[Nathaniel Graham]

ngraham added a comment.


  Affected apps:
  
  - Spectacle: crashes as soon as it launches
  - Telegram: crashes when I click on a chat in the sidebar
  
  Other unusual behavior noticed:
  
  - Opening open/save dialogs and settings windows in Dolphin and Kate causes 
the parent apps to hang for a few seconds. Maybe an issue with QDialogs?

REPOSITORY
  R31 Breeze

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

To: cblack, #plasma, #breeze, #vdg, hpereiradacosta, davidre, ngraham
Cc: maartens, abstractdevelop, IlyaBizyaev, davidre, davidedmundson, 
hpereiradacosta, ngraham, manueljlin, niccolove, ndavis, plasma-devel, Orage, 
LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, ragreen, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra, mart

D27669: [kstyle] Tools area

[Nathaniel Graham]

ngraham added a comment.


  The crash is still happening for me. It's instantly reproducible with 
Spectacle. I open it and it immediately crashes.

REPOSITORY
  R31 Breeze

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

To: cblack, #plasma, #breeze, #vdg, hpereiradacosta, davidre, ngraham
Cc: maartens, abstractdevelop, IlyaBizyaev, davidre, davidedmundson, 
hpereiradacosta, ngraham, manueljlin, niccolove, ndavis, plasma-devel, Orage, 
LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, ragreen, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra, mart

D27669: [kstyle] Tools area

[Carson Black]

cblack updated this revision to Diff 80819.
cblack added a comment.


  Remove stray qDebug

REPOSITORY
  R31 Breeze

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27669?vs=80603=80819

BRANCH
  arcpatch-D27669_1

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

AFFECTED FILES
  kstyle/CMakeLists.txt
  kstyle/breeze.kcfg
  kstyle/breezehelper.cpp
  kstyle/breezehelper.h
  kstyle/breezestyle.cpp
  kstyle/breezestyle.h
  kstyle/breezetoolsareamanager.cpp
  kstyle/breezetoolsareamanager.h
  kstyle/config/breezestyleconfig.cpp
  kstyle/config/ui/breezestyleconfig.ui

To: cblack, #plasma, #breeze, #vdg, hpereiradacosta, davidre, ngraham
Cc: maartens, abstractdevelop, IlyaBizyaev, davidre, davidedmundson, 
hpereiradacosta, ngraham, manueljlin, niccolove, ndavis, plasma-devel, Orage, 
LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, ragreen, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra, mart

D27669: [kstyle] Tools area

[Nathaniel Graham]

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


  Now all of my QtWidgets apps are crashing with a backtrace like this one:
  
Application: Spectacle (spectacle), signal: Segmentation fault
Using host libthread_db library "/lib64/libthread_db.so.1".
29return SYSCALL_CANCEL (poll, fds, nfds, timeout);
[Current thread is 1 (Thread 0x7f4a76a76800 (LWP 22804))]

Thread 1 (Thread 0x7f4a76a76800 (LWP 22804)):
[KCrash Handler]
#6  QRasterPaintEngine::transformChanged (this=0x7f4a700076c0) at 
painting/qpaintengine_raster.cpp:941
#7  0x7f4a795f4e5a in QPainterPrivate::updateMatrix 
(this=0x7ffdb8f85530, this@entry=0x5628c5e23790) at painting/qpainter.cpp:664
#8  0x7f4a795f7e31 in QPainter::setWorldTransform 
(this=this@entry=0x7ffdb8f85660, matrix=..., combine=combine@entry=false) at 
painting/qpainter.cpp:8389
#9  0x7f4a79f6c7a6 in QGraphicsDropShadowEffect::draw (this=, painter=0x7ffdb8f85660) at effects/qgraphicseffect.cpp:1067
#10 0x7f4a79c09f8e in QWidgetPrivate::drawWidget (this=0x5628c5b48b60, 
pdev=0x5628c5cbb720, rgn=..., offset=..., flags=..., sharedPainter=0x0, 
repaintManager=0x5628c5c8dbb0) at kernel/qwidget.cpp:5301
#11 0x7f4a79c0a883 in QWidgetPrivate::paintSiblingsRecursive 
(this=0x5628c5ae4910, pdev=0x5628c5cbb720, siblings=..., index=, 
rgn=..., offset=..., flags=..., sharedPainter=0x0, 
repaintManager=0x5628c5c8dbb0) at 
../../include/QtCore/../../src/corelib/tools/qpoint.h:124
#12 0x7f4a79c0a766 in QWidgetPrivate::paintSiblingsRecursive 
(this=0x5628c5ae4910, pdev=0x5628c5cbb720, siblings=..., index=0, rgn=..., 
offset=..., flags=..., sharedPainter=0x0, repaintManager=0x5628c5c8dbb0) at 
kernel/qwidget.cpp:5589
#13 0x7f4a79c0a766 in QWidgetPrivate::paintSiblingsRecursive 
(this=0x5628c5ae4910, pdev=0x5628c5cbb720, siblings=..., index=1, rgn=..., 
offset=..., flags=..., sharedPainter=0x0, repaintManager=0x5628c5c8dbb0) at 
kernel/qwidget.cpp:5589
#14 0x7f4a79c0a766 in QWidgetPrivate::paintSiblingsRecursive 
(this=0x5628c5ae4910, pdev=0x5628c5cbb720, siblings=..., index=2, rgn=..., 
offset=..., flags=..., sharedPainter=0x0, repaintManager=0x5628c5c8dbb0) at 
kernel/qwidget.cpp:5589
#15 0x7f4a79c0a766 in QWidgetPrivate::paintSiblingsRecursive 
(this=0x5628c5ae4910, pdev=0x5628c5cbb720, siblings=..., index=3, rgn=..., 
offset=..., flags=..., sharedPainter=0x0, repaintManager=0x5628c5c8dbb0) at 
kernel/qwidget.cpp:5589
#16 0x7f4a79c0a766 in QWidgetPrivate::paintSiblingsRecursive 
(this=0x5628c5ae4910, pdev=0x5628c5cbb720, siblings=..., index=4, rgn=..., 
offset=..., flags=..., sharedPainter=0x0, repaintManager=0x5628c5c8dbb0) at 
kernel/qwidget.cpp:5589
#17 0x7f4a79c0a766 in QWidgetPrivate::paintSiblingsRecursive 
(this=0x5628c5ae4910, pdev=0x5628c5cbb720, siblings=..., index=5, rgn=..., 
offset=..., flags=..., sharedPainter=0x0, repaintManager=0x5628c5c8dbb0) at 
kernel/qwidget.cpp:5589
#18 0x7f4a79c0a766 in QWidgetPrivate::paintSiblingsRecursive 
(this=0x5628c5ae4910, pdev=0x5628c5cbb720, siblings=..., index=6, rgn=..., 
offset=..., flags=..., sharedPainter=0x0, repaintManager=0x5628c5c8dbb0) at 
kernel/qwidget.cpp:5589
#19 0x7f4a79c0a766 in QWidgetPrivate::paintSiblingsRecursive 
(this=0x5628c5ae4910, pdev=0x5628c5cbb720, siblings=..., index=7, rgn=..., 
offset=..., flags=..., sharedPainter=0x0, repaintManager=0x5628c5c8dbb0) at 
kernel/qwidget.cpp:5589
#20 0x7f4a79c0a766 in QWidgetPrivate::paintSiblingsRecursive 
(this=0x5628c5ae4910, pdev=0x5628c5cbb720, siblings=..., index=8, rgn=..., 
offset=..., flags=..., sharedPainter=0x0, repaintManager=0x5628c5c8dbb0) at 
kernel/qwidget.cpp:5589
#21 0x7f4a79c0a766 in QWidgetPrivate::paintSiblingsRecursive 
(this=0x5628c5ae4910, pdev=0x5628c5cbb720, siblings=..., index=11, rgn=..., 
offset=..., flags=..., sharedPainter=0x0, repaintManager=0x5628c5c8dbb0) at 
kernel/qwidget.cpp:5589
#22 0x7f4a79c0a766 in QWidgetPrivate::paintSiblingsRecursive 
(this=0x5628c5ae4910, pdev=0x5628c5cbb720, siblings=..., index=13, rgn=..., 
offset=..., flags=..., sharedPainter=0x0, repaintManager=0x5628c5c8dbb0) at 
kernel/qwidget.cpp:5589
#23 0x7f4a79c0a766 in QWidgetPrivate::paintSiblingsRecursive 
(this=this@entry=0x5628c5ae4910, pdev=pdev@entry=0x5628c5cbb720, siblings=..., 
index=14, rgn=..., offset=..., flags=..., sharedPainter=0x0, 
repaintManager=0x5628c5c8dbb0) at kernel/qwidget.cpp:5589
#24 0x7f4a79c0919c in QWidgetPrivate::drawWidget (this=0x5628c5ae4910, 
pdev=0x5628c5cbb720, rgn=..., offset=..., flags=..., sharedPainter=, repaintManager=) at 
../../include/QtCore/../../src/corelib/tools/qlist.h:176
#25 0x7f4a79c0a883 in QWidgetPrivate::paintSiblingsRecursive 
(this=0x5628c5b786d0, pdev=0x5628c5cbb720, siblings=..., index=, 
rgn=..., offset=..., flags=..., sharedPainter=0x0, 
repaintManager=0x5628c5c8dbb0) at 

D27669: [kstyle] Tools area

[David Redondo]

davidre added inline comments.

INLINE COMMENTS

> breezestyle.cpp:907
> +auto rect = _helper->toolsAreaToolbarsRect(widget);
> +qDebug() << rect;
> +

Stray qdebug

REPOSITORY
  R31 Breeze

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

To: cblack, #plasma, #breeze, #vdg, hpereiradacosta, davidre, ngraham
Cc: maartens, abstractdevelop, IlyaBizyaev, davidre, davidedmundson, 
hpereiradacosta, ngraham, manueljlin, niccolove, ndavis, plasma-devel, Orage, 
LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, ragreen, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra, mart

D27669: [kstyle] Tools area

[Nathaniel Graham]

ngraham accepted this revision.
ngraham added a comment.


  Much better, thanks.

REPOSITORY
  R31 Breeze

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

To: cblack, #plasma, #breeze, #vdg, hpereiradacosta, davidre, ngraham
Cc: maartens, abstractdevelop, IlyaBizyaev, davidre, davidedmundson, 
hpereiradacosta, ngraham, manueljlin, niccolove, ndavis, plasma-devel, Orage, 
LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, ragreen, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra, mart

D27669: [kstyle] Tools area

[Carson Black]

cblack updated this revision to Diff 80594.
cblack added a comment.


  Rebase on master

REPOSITORY
  R31 Breeze

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27669?vs=79010=80594

BRANCH
  arcpatch-D27669_1

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

AFFECTED FILES
  kstyle/CMakeLists.txt
  kstyle/breeze.kcfg
  kstyle/breezehelper.cpp
  kstyle/breezehelper.h
  kstyle/breezestyle.cpp
  kstyle/breezestyle.h
  kstyle/breezetoolsareamanager.cpp
  kstyle/breezetoolsareamanager.h
  kstyle/config/breezestyleconfig.cpp
  kstyle/config/ui/breezestyleconfig.ui

To: cblack, #plasma, #breeze, #vdg, hpereiradacosta, davidre, ngraham
Cc: maartens, abstractdevelop, IlyaBizyaev, davidre, davidedmundson, 
hpereiradacosta, ngraham, manueljlin, niccolove, ndavis, plasma-devel, Orage, 
LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, ragreen, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra, mart

D27669: [kstyle] Tools area

[Carson Black]

cblack updated this revision to Diff 80597.
cblack added a comment.


  Properly rebase

REPOSITORY
  R31 Breeze

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27669?vs=80594=80597

BRANCH
  arcpatch-D27669_1

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

AFFECTED FILES
  kstyle/CMakeLists.txt
  kstyle/breeze.kcfg
  kstyle/breezehelper.cpp
  kstyle/breezehelper.h
  kstyle/breezestyle.cpp
  kstyle/breezestyle.h
  kstyle/breezetoolsareamanager.cpp
  kstyle/breezetoolsareamanager.h
  kstyle/config/breezestyleconfig.cpp
  kstyle/config/ui/breezestyleconfig.ui

To: cblack, #plasma, #breeze, #vdg, hpereiradacosta, davidre, ngraham
Cc: maartens, abstractdevelop, IlyaBizyaev, davidre, davidedmundson, 
hpereiradacosta, ngraham, manueljlin, niccolove, ndavis, plasma-devel, Orage, 
LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, ragreen, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra, mart

D27669: [kstyle] Tools area

[Carson Black]

cblack updated this revision to Diff 80603.
cblack added a comment.


  Fix menubar quirks

REPOSITORY
  R31 Breeze

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27669?vs=80597=80603

BRANCH
  arcpatch-D27669_1

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

AFFECTED FILES
  kstyle/CMakeLists.txt
  kstyle/breeze.kcfg
  kstyle/breezehelper.cpp
  kstyle/breezehelper.h
  kstyle/breezestyle.cpp
  kstyle/breezestyle.h
  kstyle/breezetoolsareamanager.cpp
  kstyle/breezetoolsareamanager.h
  kstyle/config/breezestyleconfig.cpp
  kstyle/config/ui/breezestyleconfig.ui

To: cblack, #plasma, #breeze, #vdg, hpereiradacosta, davidre, ngraham
Cc: maartens, abstractdevelop, IlyaBizyaev, davidre, davidedmundson, 
hpereiradacosta, ngraham, manueljlin, niccolove, ndavis, plasma-devel, Orage, 
LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, ragreen, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra, mart

D27669: [kstyle] Tools area

[Nathaniel Graham]

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


  Found an interesting issue today: When you hide and show the menubar in 
Dolphin, this happens: F8247334: Screenshot_20200419_134038.png 
. It goes away if you interact with 
anything in the toolbar.
  
  Can reproduce in Kate too, and it goes away after interacting with the 
toolbar or switching tabs. Cannot reproduce in Konsole.

REPOSITORY
  R31 Breeze

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

To: cblack, #plasma, #breeze, #vdg, hpereiradacosta, davidre, ngraham
Cc: maartens, abstractdevelop, IlyaBizyaev, davidre, davidedmundson, 
hpereiradacosta, ngraham, manueljlin, niccolove, ndavis, plasma-devel, Orage, 
LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, ragreen, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra, mart

D27669: [kstyle] Tools area

[Nathaniel Graham]

ngraham added a comment.


  Needs a rebase

REPOSITORY
  R31 Breeze

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

To: cblack, #plasma, #breeze, #vdg, hpereiradacosta, davidre, ngraham
Cc: maartens, abstractdevelop, IlyaBizyaev, davidre, davidedmundson, 
hpereiradacosta, ngraham, manueljlin, niccolove, ndavis, plasma-devel, Orage, 
LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, ragreen, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra, mart

D27669: [kstyle] Tools area

[Nathaniel Graham]

ngraham accepted this revision.
ngraham added a comment.


  This is probably fine as-is, and we can fine-tune it later if need be.
  
  We should wait to land it until the companion patches for Breeze GTK 
(https://invent.kde.org/kde/breeze-gtk/-/merge_requests/2), Kirigami (TBD) and 
QQC2-desktop style (TBD) have also landed.

REPOSITORY
  R31 Breeze

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

To: cblack, #plasma, #breeze, #vdg, hpereiradacosta, davidre, ngraham
Cc: maartens, abstractdevelop, IlyaBizyaev, davidre, davidedmundson, 
hpereiradacosta, ngraham, manueljlin, niccolove, ndavis, plasma-devel, Orage, 
LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, ragreen, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra, mart

D27669: [kstyle] Tools area

[Nathaniel Graham]

ngraham added a comment.


  If we're going to consider the titlebar a part of the tools area, then it 
needs a separator line underneath it whether it's drawn on a Blender window or 
a Plasma config window The idea was always to draw a separator line under the 
bottom-most element of the Tools Area in all apps, whether that bottom-most 
element was a menubar, toolbar, or the SSD titlebar drawn by KWin.

REPOSITORY
  R31 Breeze

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

To: cblack, #plasma, #breeze, #vdg, hpereiradacosta, davidre
Cc: maartens, abstractdevelop, IlyaBizyaev, davidre, davidedmundson, 
hpereiradacosta, ngraham, manueljlin, niccolove, ndavis, plasma-devel, Orage, 
LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, 
ragreen, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, ahiemstra, mart

D27669: [kstyle] Tools area

[Carson Black]

cblack added a comment.


  In D27669#640491 , @ngraham wrote:
  
  > In D27669#640473 , @cblack wrote:
  >
  > > In D27669#639569 , @ngraham 
wrote:
  > >
  > > > Anyway I worked around that in D27669 
, and I guess it's not really a huge deal 
for toolbar icons.
  > > >
  > > > One thing I notice is that the bottom separator line for the titlebar 
isn't drawn for 3rd-party apps where there is no Tools Area. Maybe to do this, 
we could have the decoration theme draw the line by default unless specifically 
told not to by the widget theme, and then the widget theme could tell it not to 
because it will be drawing the separator underneath the menubar or toolbar (as 
appropriate).
  > > >
  > > > Thoughts?
  > >
  > >
  > > Apps liiike? The QStyle should draw a decoration line regardless of the 
presence of a toolbar/menubar area.
  >
  >
  > Like every 3rd-party app not using Qt or GTK. ...Which are principally 
Electron apps these days, I guess. But there are a handful of important apps 
not using Qt, GTK, or Electron, such as Blender. I've got a few others 
installed like ChiTuBox and SpiderOak One. Also Telegram, which is a Qt app, 
doesn't get a line below its titlebar.
  >
  > My idea to have the decoration theme draw the separarator line unless 
explicitly told not to because the QStyle or breeze-GTK theme is drawing it 
itself is intended to avoid having to play whack-a-mole with all the different 
3rd-party apps out there.
  
  
  I don't really consider it important to attempt to bring the Tools Area to 
third-party apps with their own strong branding

REPOSITORY
  R31 Breeze

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

To: cblack, #plasma, #breeze, #vdg, hpereiradacosta, davidre
Cc: maartens, abstractdevelop, IlyaBizyaev, davidre, davidedmundson, 
hpereiradacosta, ngraham, manueljlin, niccolove, ndavis, plasma-devel, Orage, 
LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, 
ragreen, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, ahiemstra, mart

D27669: [kstyle] Tools area

[Nathaniel Graham]

ngraham added a comment.


  In D27669#640473 , @cblack wrote:
  
  > In D27669#639569 , @ngraham 
wrote:
  >
  > > Anyway I worked around that in D27669 
, and I guess it's not really a huge deal 
for toolbar icons.
  > >
  > > One thing I notice is that the bottom separator line for the titlebar 
isn't drawn for 3rd-party apps where there is no Tools Area. Maybe to do this, 
we could have the decoration theme draw the line by default unless specifically 
told not to by the widget theme, and then the widget theme could tell it not to 
because it will be drawing the separator underneath the menubar or toolbar (as 
appropriate).
  > >
  > > Thoughts?
  >
  >
  > Apps liiike? The QStyle should draw a decoration line regardless of the 
presence of a toolbar/menubar area.
  
  
  Like every 3rd-party app not using Qt or GTK. ...Which are principally 
Electron apps these days, I guess. But there are a handful of important apps 
not using Qt, GTK, or Electron, such as Blender. I've got a few others 
installed like ChiTuBox and SpiderOak One. Also Telegram, which is a Qt app, 
doesn't get a line below its titlebar.
  
  My idea to have the decoration theme draw the separarator line unless 
explicitly told not to because the QStyle or breeze-GTK theme is drawing it 
itself is intended to avoid having to play whack-a-mole with all the different 
3rd-party apps out there.

REPOSITORY
  R31 Breeze

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

To: cblack, #plasma, #breeze, #vdg, hpereiradacosta, davidre
Cc: maartens, abstractdevelop, IlyaBizyaev, davidre, davidedmundson, 
hpereiradacosta, ngraham, manueljlin, niccolove, ndavis, plasma-devel, Orage, 
LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, 
ragreen, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, ahiemstra, mart

D27669: [kstyle] Tools area

[Carson Black]

cblack added a comment.


  In D27669#639569 , @ngraham wrote:
  
  > Anyway I worked around that in D27669 , 
and I guess it's not really a huge deal for toolbar icons.
  >
  > One thing I notice is that the bottom separator line for the titlebar isn't 
drawn for 3rd-party apps where there is no Tools Area. Maybe to do this, we 
could have the decoration theme draw the line by default unless specifically 
told not to by the widget theme, and then the widget theme could tell it not to 
because it will be drawing the separator underneath the menubar or toolbar (as 
appropriate).
  >
  > Thoughts?
  
  
  Apps liiike? The QStyle should draw a decoration line regardless of the 
presence of a toolbar/menubar area.

REPOSITORY
  R31 Breeze

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

To: cblack, #plasma, #breeze, #vdg, hpereiradacosta, davidre
Cc: maartens, abstractdevelop, IlyaBizyaev, davidre, davidedmundson, 
hpereiradacosta, ngraham, manueljlin, niccolove, ndavis, plasma-devel, Orage, 
LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, 
ragreen, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, ahiemstra, mart

D27669: [kstyle] Tools area

[Nathaniel Graham]

ngraham added a comment.


  Lightening the stuff on the Tools Area is a nice effect though, IMO. 
Implementation-wise, we could also use the Fade inactive window effect in the 
color scheme itself for this. However it's a bit buggy as various apps have 
widgets that don't respect it (I've filed bug reports and can dig them up if 
needed) and it affects more than just the tools area. Still, that might be the 
more technically correct way to lighten things for inactive windows.

REPOSITORY
  R31 Breeze

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

To: cblack, #plasma, #breeze, #vdg, hpereiradacosta, davidre
Cc: maartens, abstractdevelop, IlyaBizyaev, davidre, davidedmundson, 
hpereiradacosta, ngraham, manueljlin, niccolove, ndavis, plasma-devel, Orage, 
LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, 
ragreen, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, ahiemstra, mart

D27669: [kstyle] Tools area

[Noah Davis]

ndavis added a comment.


  In D27669#639519 , @ngraham wrote:
  
  > While working on D28317 , I noticed 
something: when a window becomes inactive, the icons change their colors, but 
only for the parts of the icon using the text color. Red icons, or parts of 
icons that are red, don't change at all which is a little weird-looking:
  
  
  Perhaps we shouldn't change the text color on inactive windows? We can 
already change the background color. It would only require changes to the 
default color schemes.
  
  > Do we need an inactive red color or something?
  
  It should be possible to make an inactive version of NegativeText based on 
the inactive color.

REPOSITORY
  R31 Breeze

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

To: cblack, #plasma, #breeze, #vdg, hpereiradacosta, davidre
Cc: maartens, abstractdevelop, IlyaBizyaev, davidre, davidedmundson, 
hpereiradacosta, ngraham, manueljlin, niccolove, ndavis, plasma-devel, Orage, 
LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, 
ragreen, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, ahiemstra, mart

D27669: [kstyle] Tools area

[Nathaniel Graham]

ngraham added a comment.


  Anyway I worked around that in D27669 , 
and I guess it's not really a huge deal for toolbar icons.
  
  One thing I notice is that the bottom separator line for the titlebar isn't 
drawn for 3rd-party apps where there is no Tools Area. Maybe to do this, we 
could have the decoration theme draw the line by default unless specifically 
told not to by the widget theme, and then the widget theme could tell it not to 
because it will be drawing the separator underneath the menubar or toolbar (as 
appropriate).
  
  Thoughts?

REPOSITORY
  R31 Breeze

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

To: cblack, #plasma, #breeze, #vdg, hpereiradacosta, davidre
Cc: maartens, abstractdevelop, IlyaBizyaev, davidre, davidedmundson, 
hpereiradacosta, ngraham, manueljlin, niccolove, ndavis, plasma-devel, Orage, 
LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, 
ragreen, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, ahiemstra, mart

D27669: [kstyle] Tools area

[Nathaniel Graham]

ngraham added a comment.


  While working on D28317 , I noticed 
something: when a window becomes inactive, the icons change their colors, but 
only for the parts of the icon using the text color. Red icons, or parts of 
icons that are red, don't change at all which is a little weird-looking:
  
  F8209743: vokoscreenNG-2020-04-01_10-39-43.webm 

  
  Do we need an inactive red color or something?

REPOSITORY
  R31 Breeze

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

To: cblack, #plasma, #breeze, #vdg, hpereiradacosta, davidre
Cc: maartens, abstractdevelop, IlyaBizyaev, davidre, davidedmundson, 
hpereiradacosta, ngraham, manueljlin, niccolove, ndavis, plasma-devel, Orage, 
LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, 
ragreen, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, ahiemstra, mart

D27669: [kstyle] Tools area

[Nathaniel Graham]

ngraham added a comment.


  Thanks, that helps a lot. It still feels a little bit slower than before, but 
it's at least much improved.

REPOSITORY
  R31 Breeze

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

To: cblack, #plasma, #breeze, #vdg, hpereiradacosta, davidre
Cc: maartens, abstractdevelop, IlyaBizyaev, davidre, davidedmundson, 
hpereiradacosta, ngraham, manueljlin, niccolove, ndavis, plasma-devel, Orage, 
LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, 
ragreen, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, ahiemstra, mart

D27669: [kstyle] Tools area

[Carson Black]

cblack updated this revision to Diff 79010.
cblack added a comment.


  More optimization

REPOSITORY
  R31 Breeze

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27669?vs=79006=79010

BRANCH
  cblack/toolsarea

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

AFFECTED FILES
  kstyle/CMakeLists.txt
  kstyle/breeze.kcfg
  kstyle/breezehelper.cpp
  kstyle/breezehelper.h
  kstyle/breezestyle.cpp
  kstyle/breezestyle.h
  kstyle/breezetoolsareamanager.cpp
  kstyle/breezetoolsareamanager.h
  kstyle/config/breezestyleconfig.cpp
  kstyle/config/ui/breezestyleconfig.ui

To: cblack, #plasma, #breeze, #vdg, hpereiradacosta, davidre
Cc: maartens, abstractdevelop, IlyaBizyaev, davidre, davidedmundson, 
hpereiradacosta, ngraham, manueljlin, niccolove, ndavis, plasma-devel, Orage, 
LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, 
ragreen, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, ahiemstra, mart

D27669: [kstyle] Tools area

[Carson Black]

cblack updated this revision to Diff 79006.
cblack added a comment.


  More optimization

REPOSITORY
  R31 Breeze

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27669?vs=78928=79006

BRANCH
  cblack/toolsarea

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

AFFECTED FILES
  kstyle/CMakeLists.txt
  kstyle/breeze.kcfg
  kstyle/breezehelper.cpp
  kstyle/breezehelper.h
  kstyle/breezestyle.cpp
  kstyle/breezestyle.h
  kstyle/breezetoolsareamanager.cpp
  kstyle/breezetoolsareamanager.h
  kstyle/config/breezestyleconfig.cpp
  kstyle/config/ui/breezestyleconfig.ui

To: cblack, #plasma, #breeze, #vdg, hpereiradacosta, davidre
Cc: maartens, abstractdevelop, IlyaBizyaev, davidre, davidedmundson, 
hpereiradacosta, ngraham, manueljlin, niccolove, ndavis, plasma-devel, Orage, 
LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, 
ragreen, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, ahiemstra, mart

D27669: [kstyle] Tools area

[David Redondo]

davidre added a comment.


  Hi, I made a comparison video performing the same task with oxygen, current 
breeze, this patch with the option enabled and disabled (in order of 
appearance). The task is opening kdevelop, opening the same two files, 
switching between them and then closing the files and the application. As you 
can see the startup time is much worse and even the disabled option seems to 
have some impact.
  F8208176: vokoscreen-2020-03-31_14-07-55.mkv 

  Saying this is the fault of applications is imo unacceptable as it worked 
fine before and in other styles and is caused directly by this. In my mind the 
style should not have such a big impact on performance .

REPOSITORY
  R31 Breeze

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

To: cblack, #plasma, #breeze, #vdg, hpereiradacosta, davidre
Cc: maartens, abstractdevelop, IlyaBizyaev, davidre, davidedmundson, 
hpereiradacosta, ngraham, manueljlin, niccolove, ndavis, plasma-devel, Orage, 
LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, 
ragreen, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, ahiemstra, mart

D27669: [kstyle] Tools area

[Hugo Pereira Da Costa]

hpereiradacosta added a comment.


  Hi 
  I don't think it is very satisfactory to say that the perforrmance hit it is 
"the apps fault" ... It is not there without the change, and with other widgets 
styles (or is it ? Did any one check, e.g. oxygen which is quite resource heavy 
because of gradients and so ?)
  You can't expect applcations dev to go optimize their app for the need of a 
given style. especially if it is not trivial. 
  I don't see another choice, if such popular apps as kate and kdevelop are 
affected, than to go do the optimization 'ourself', if indeed this is the issue.
  At the minimum someone should try to profile this, using e.g. callgrind, to 
see where the curlprit is ...

REPOSITORY
  R31 Breeze

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

To: cblack, #plasma, #breeze, #vdg, hpereiradacosta, davidre
Cc: maartens, abstractdevelop, IlyaBizyaev, davidre, davidedmundson, 
hpereiradacosta, ngraham, manueljlin, niccolove, ndavis, plasma-devel, Orage, 
LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, 
ragreen, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, ahiemstra, mart

D27669: [kstyle] Tools area

[Carson Black]

cblack added a comment.


  In D27669#638327 , @ngraham wrote:
  
  > Found an issue in Virtualbox: the menubar isn't considered a part of the 
Tools Area:
  >
  > F8206280: Screenshot_20200330_113752.png 

  
  
  I'm going to WONTFIX on this one-VirtualBox is simply a massive PITA with 
theming, and already breaks badly in many cases with many themes, including 
"official" ones provided by Qt. Frankly, it's a miracle that this doesn't 
result in titlebar color on titlebar color.

REPOSITORY
  R31 Breeze

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

To: cblack, #plasma, #breeze, #vdg, hpereiradacosta, davidre
Cc: abstractdevelop, IlyaBizyaev, davidre, davidedmundson, hpereiradacosta, 
ngraham, manueljlin, niccolove, ndavis, plasma-devel, Orage, LeGast00n, 
The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra, mart

D27669: [kstyle] Tools area

[Carson Black]

cblack added a comment.


  In D27669#638175 , @ngraham wrote:
  
  > Nice, that seems to have fixed the issue again.
  >
  > However I still see a significant performance regression though. Switching 
tabs in Kate is now quite slow, especially when there are many open and you use 
a keyboard shortcut to do so. I see the spinning busy cursor all the time. The 
Tools area also changes its vertical height a bit when changing tabs, which is 
odd.
  >
  > F8206041: vokoscreenNG-2020-03-30_08-55-52.webm 

  
  
  Any performance regressions when a kate widget is involved (Kate, KDevelop) 
is likely to due to the fact that tab switching results in a spam of repainting 
because of incessant toggling of a toolbar's visible property, which results in 
costly repainting for no reason. I've already optimized what I can from the 
QStyle side, and anything further will have to be fixed in the offending widget.

REPOSITORY
  R31 Breeze

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

To: cblack, #plasma, #breeze, #vdg, hpereiradacosta, davidre
Cc: abstractdevelop, IlyaBizyaev, davidre, davidedmundson, hpereiradacosta, 
ngraham, manueljlin, niccolove, ndavis, plasma-devel, Orage, LeGast00n, 
The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra, mart

D27669: [kstyle] Tools area

[Carson Black]

cblack updated this revision to Diff 78928.
cblack added a comment.


  Change how cache invalidation works

REPOSITORY
  R31 Breeze

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27669?vs=78813=78928

BRANCH
  cblack/toolsarea

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

AFFECTED FILES
  kstyle/CMakeLists.txt
  kstyle/breeze.kcfg
  kstyle/breezehelper.cpp
  kstyle/breezehelper.h
  kstyle/breezestyle.cpp
  kstyle/breezestyle.h
  kstyle/breezetoolsareamanager.cpp
  kstyle/breezetoolsareamanager.h
  kstyle/config/breezestyleconfig.cpp
  kstyle/config/ui/breezestyleconfig.ui

To: cblack, #plasma, #breeze, #vdg, hpereiradacosta, davidre
Cc: abstractdevelop, IlyaBizyaev, davidre, davidedmundson, hpereiradacosta, 
ngraham, manueljlin, niccolove, ndavis, plasma-devel, Orage, LeGast00n, 
The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra, mart

D27669: [kstyle] Tools area

[Carson Black]

cblack added a comment.


  In D27669#638328 , 
@abstractdevelop wrote:
  
  > Also should we implement this for GTK apps?
  
  
  Pretty much already there :P
  
  F8206301: image.png 
  
  But yes, I'm already investigating how to apply this to menubar+toolbar'd GTK 
apps.

REPOSITORY
  R31 Breeze

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

To: cblack, #plasma, #breeze, #vdg, hpereiradacosta, davidre
Cc: abstractdevelop, IlyaBizyaev, davidre, davidedmundson, hpereiradacosta, 
ngraham, manueljlin, niccolove, ndavis, plasma-devel, Orage, LeGast00n, 
The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra, mart

D27669: [kstyle] Tools area

[Abstract Software]

abstractdevelop added a comment.


  Also should we implement this for GTK apps?

REPOSITORY
  R31 Breeze

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

To: cblack, #plasma, #breeze, #vdg, hpereiradacosta, davidre
Cc: abstractdevelop, IlyaBizyaev, davidre, davidedmundson, hpereiradacosta, 
ngraham, manueljlin, niccolove, ndavis, plasma-devel, Orage, LeGast00n, 
The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra, mart

D27669: [kstyle] Tools area

[Nathaniel Graham]

ngraham added a comment.


  Found an issue in Virtualbox: the menubar isn't considered a part of the 
Tools Area:
  
  F8206280: Screenshot_20200330_113752.png 


REPOSITORY
  R31 Breeze

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

To: cblack, #plasma, #breeze, #vdg, hpereiradacosta, davidre
Cc: abstractdevelop, IlyaBizyaev, davidre, davidedmundson, hpereiradacosta, 
ngraham, manueljlin, niccolove, ndavis, plasma-devel, Orage, LeGast00n, 
The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra, mart

D27669: [kstyle] Tools area

[Nathaniel Graham]

ngraham added a comment.


  Nice, that seems to have fixed the issue again.
  
  However I still see a significant performance regression though. Switching 
tabs in Kate is now quite slow, especially when there are many open and you use 
a keyboard shortcut to do so. I see the spinning busy cursor all the time. The 
Tools area also changes its vertical height a bit when changing tabs, which is 
odd.
  
  F8206041: vokoscreenNG-2020-03-30_08-55-52.webm 


REPOSITORY
  R31 Breeze

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

To: cblack, #plasma, #breeze, #vdg, hpereiradacosta, davidre
Cc: abstractdevelop, IlyaBizyaev, davidre, davidedmundson, hpereiradacosta, 
ngraham, manueljlin, niccolove, ndavis, plasma-devel, Orage, LeGast00n, 
The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra, mart

D27669: [kstyle] Tools area

[Carson Black]

cblack updated this revision to Diff 78813.
cblack added a comment.


  Rerender toolbar when invalidating cached rects

REPOSITORY
  R31 Breeze

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27669?vs=78754=78813

BRANCH
  cblack/toolsarea

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

AFFECTED FILES
  kstyle/CMakeLists.txt
  kstyle/breeze.kcfg
  kstyle/breezehelper.cpp
  kstyle/breezehelper.h
  kstyle/breezestyle.cpp
  kstyle/breezestyle.h
  kstyle/breezetoolsareamanager.cpp
  kstyle/breezetoolsareamanager.h
  kstyle/config/breezestyleconfig.cpp
  kstyle/config/ui/breezestyleconfig.ui

To: cblack, #plasma, #breeze, #vdg, hpereiradacosta, davidre
Cc: abstractdevelop, IlyaBizyaev, davidre, davidedmundson, hpereiradacosta, 
ngraham, manueljlin, niccolove, ndavis, plasma-devel, Orage, LeGast00n, 
The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra, mart

D27669: [kstyle] Tools area

[Hugo Pereira Da Costa]

hpereiradacosta added a comment.


  In D27669#637164 , @ndavis wrote:
  
  > I've discovered another bug. When you move a window by dragging on an empty 
area, all hover effects stop working.
  
  
  This bug has been here since forever and is not related to the changes in 
this patch.
  It is due to X11 eating one of Qt's mouse release events when starting to 
drag the window from empty areas. 
  It was fixed at some point in the distant past, and the re-introduced ...

REPOSITORY
  R31 Breeze

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

To: cblack, #plasma, #breeze, #vdg, hpereiradacosta, davidre
Cc: abstractdevelop, IlyaBizyaev, davidre, davidedmundson, hpereiradacosta, 
ngraham, manueljlin, niccolove, ndavis, plasma-devel, Orage, LeGast00n, 
The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra, mart

D27669: [kstyle] Tools area

[Noah Davis]

ndavis added a comment.


  I've discovered another bug. When you move a window by dragging on an empty 
area, the hover effect on toolbar buttons stops working.

REPOSITORY
  R31 Breeze

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

To: cblack, #plasma, #breeze, #vdg, hpereiradacosta, davidre
Cc: abstractdevelop, IlyaBizyaev, davidre, davidedmundson, hpereiradacosta, 
ngraham, manueljlin, niccolove, ndavis, plasma-devel, Orage, LeGast00n, 
The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra, mart

D27669: [kstyle] Tools area

[Nathaniel Graham]

ngraham added a comment.


  I just found an interesting bug. KDE apps with menubars show a mis-colored 
area to the right of the menubar when maximized: F8203808: 
vokoscreenNG-2020-03-28_22-03-54.webm 

REPOSITORY
  R31 Breeze

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

To: cblack, #plasma, #breeze, #vdg, hpereiradacosta, davidre
Cc: abstractdevelop, IlyaBizyaev, davidre, davidedmundson, hpereiradacosta, 
ngraham, manueljlin, niccolove, ndavis, plasma-devel, Orage, LeGast00n, 
The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra, mart

D27669: [kstyle] Tools area

[Carson Black]

cblack updated this revision to Diff 78754.
cblack added a comment.


  Optimize toolbar rect function

REPOSITORY
  R31 Breeze

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27669?vs=78667=78754

BRANCH
  cblack/toolsarea

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

AFFECTED FILES
  kstyle/CMakeLists.txt
  kstyle/breeze.kcfg
  kstyle/breezehelper.cpp
  kstyle/breezehelper.h
  kstyle/breezestyle.cpp
  kstyle/breezestyle.h
  kstyle/breezetoolsareamanager.cpp
  kstyle/breezetoolsareamanager.h
  kstyle/config/breezestyleconfig.cpp
  kstyle/config/ui/breezestyleconfig.ui

To: cblack, #plasma, #breeze, #vdg, hpereiradacosta, davidre
Cc: abstractdevelop, IlyaBizyaev, davidre, davidedmundson, hpereiradacosta, 
ngraham, manueljlin, niccolove, ndavis, plasma-devel, Orage, LeGast00n, 
The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra, mart

D27669: [kstyle] Tools area

[Carson Black]

cblack updated this revision to Diff 78667.
cblack added a comment.


  Address issues

REPOSITORY
  R31 Breeze

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27669?vs=78664=78667

BRANCH
  cblack/toolsarea

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

AFFECTED FILES
  kstyle/CMakeLists.txt
  kstyle/breeze.kcfg
  kstyle/breezehelper.cpp
  kstyle/breezehelper.h
  kstyle/breezestyle.cpp
  kstyle/breezestyle.h
  kstyle/breezetoolsareamanager.cpp
  kstyle/breezetoolsareamanager.h
  kstyle/config/breezestyleconfig.cpp
  kstyle/config/ui/breezestyleconfig.ui

To: cblack, #plasma, #breeze, #vdg, hpereiradacosta, davidre
Cc: abstractdevelop, IlyaBizyaev, davidre, davidedmundson, hpereiradacosta, 
ngraham, manueljlin, niccolove, ndavis, plasma-devel, Orage, LeGast00n, 
The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra, mart

D27669: [kstyle] Tools area

[Niccolò Venerandi]

niccolove added a comment.


  Couple of things:
  
  - There's strong consensus in the VDG that we should go with a standard 
separator line for now, and eventually make it consistently darker if we decide 
it's not dark enough.
  - The above mentioned colorscheme with transparent titlebar still does not 
work; this is not a blocker and I've already contacted @cblack who told me they 
couldn't do it, but I'll still mention it in case somebody else knows why.

REPOSITORY
  R31 Breeze

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

To: cblack, #plasma, #breeze, #vdg, hpereiradacosta, davidre
Cc: abstractdevelop, IlyaBizyaev, davidre, davidedmundson, hpereiradacosta, 
ngraham, manueljlin, niccolove, ndavis, plasma-devel, Orage, LeGast00n, 
The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra, mart

D27669: [kstyle] Tools area

[Carson Black]

cblack updated this revision to Diff 78664.
cblack added a comment.


  Fix bad toolbar heuristic

REPOSITORY
  R31 Breeze

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27669?vs=78577=78664

BRANCH
  cblack/toolsarea

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

AFFECTED FILES
  kstyle/CMakeLists.txt
  kstyle/breeze.kcfg
  kstyle/breezehelper.cpp
  kstyle/breezehelper.h
  kstyle/breezestyle.cpp
  kstyle/breezestyle.h
  kstyle/breezetoolsareamanager.cpp
  kstyle/breezetoolsareamanager.h
  kstyle/config/breezestyleconfig.cpp
  kstyle/config/ui/breezestyleconfig.ui

To: cblack, #plasma, #breeze, #vdg, hpereiradacosta, davidre
Cc: abstractdevelop, IlyaBizyaev, davidre, davidedmundson, hpereiradacosta, 
ngraham, manueljlin, niccolove, ndavis, plasma-devel, Orage, LeGast00n, 
The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra, mart

D27669: [kstyle] Tools area

[Nathaniel Graham]

ngraham added a comment.


  Can can confirm. Toolbars are no longer considered to be a part of the Tools 
area in all apps with the latest version of the patch.

REPOSITORY
  R31 Breeze

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

To: cblack, #plasma, #breeze, #vdg, hpereiradacosta, davidre
Cc: abstractdevelop, IlyaBizyaev, davidre, davidedmundson, hpereiradacosta, 
ngraham, manueljlin, niccolove, ndavis, plasma-devel, Orage, LeGast00n, 
The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra, mart

D27669: [kstyle] Tools area

[Abstract Software]

abstractdevelop added a comment.


  For me it only merges the menus with the window decoration, not the toolbar :/
  F8200218: QtAssistantToolbars.png 

REPOSITORY
  R31 Breeze

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

To: cblack, #plasma, #breeze, #vdg, hpereiradacosta, davidre
Cc: abstractdevelop, IlyaBizyaev, davidre, davidedmundson, hpereiradacosta, 
ngraham, manueljlin, niccolove, ndavis, plasma-devel, Orage, LeGast00n, 
The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra, mart

D27669: [kstyle] Tools area

[Carson Black]

cblack updated this revision to Diff 78577.
cblack added a comment.


  Don't include toolbars in MDI/dock widgets for the tools area calculation

REPOSITORY
  R31 Breeze

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27669?vs=78572=78577

BRANCH
  cblack/toolsarea

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

AFFECTED FILES
  kstyle/CMakeLists.txt
  kstyle/breeze.kcfg
  kstyle/breezehelper.cpp
  kstyle/breezehelper.h
  kstyle/breezestyle.cpp
  kstyle/breezestyle.h
  kstyle/breezetoolsareamanager.cpp
  kstyle/breezetoolsareamanager.h
  kstyle/config/breezestyleconfig.cpp
  kstyle/config/ui/breezestyleconfig.ui

To: cblack, #plasma, #breeze, #vdg, hpereiradacosta, davidre
Cc: IlyaBizyaev, davidre, davidedmundson, hpereiradacosta, ngraham, manueljlin, 
niccolove, ndavis, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27669: [kstyle] Tools area

[David Redondo]

davidre added a comment.


  two videos showing the performance regression (on the left current breeze, on 
the right with this patch applied)
  F8200103: breeze.webm  F8200105: 
toolarea.webm 

REPOSITORY
  R31 Breeze

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

To: cblack, #plasma, #breeze, #vdg, hpereiradacosta, davidre
Cc: IlyaBizyaev, davidre, davidedmundson, hpereiradacosta, ngraham, manueljlin, 
niccolove, ndavis, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27669: [kstyle] Tools area

[Nathaniel Graham]

ngraham added a comment.


  With the latest version of the patch, I still see this hilarious visual bug: 
F8200104: vokoscreenNG-2020-03-26_10-37-25.webm 


REPOSITORY
  R31 Breeze

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

To: cblack, #plasma, #breeze, #vdg, hpereiradacosta, davidre
Cc: IlyaBizyaev, davidre, davidedmundson, hpereiradacosta, ngraham, manueljlin, 
niccolove, ndavis, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27669: [kstyle] Tools area

[Nathaniel Graham]

ngraham added a comment.


  Yes, the decoration theme determines its own border value. For Breeze it's 
"No borders", but we can't assume that.

REPOSITORY
  R31 Breeze

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

To: cblack, #plasma, #breeze, #vdg, hpereiradacosta, davidre
Cc: IlyaBizyaev, davidre, davidedmundson, hpereiradacosta, ngraham, manueljlin, 
niccolove, ndavis, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27669: [kstyle] Tools area

[Carson Black]

cblack added a comment.


  In D27669#635285 , @davidre wrote:
  
  > In D27669#635243 , @cblack wrote:
  >
  > > Assume there's no borders when there's not a BorderSize entry
  >
  >
  > That's totally not correct. The default value when there's no BorderSize 
entry is "Normal" . This patch is now also enabled when the user manually 
selects "Normal". The correct thing is to check  first whether `BorderSizeAuto` 
is enabled (default true) . If it's not enabled you can read `BorderSize` 
(default "Normal") otherwise check what the preference the decoration sets. I 
didn't check yet where that value is defined
  
  
  Took a look: it's not defined anywhere; it's up to the decorations to do 
whatever they feel like.

REPOSITORY
  R31 Breeze

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

To: cblack, #plasma, #breeze, #vdg, hpereiradacosta, davidre
Cc: IlyaBizyaev, davidre, davidedmundson, hpereiradacosta, ngraham, manueljlin, 
niccolove, ndavis, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27669: [kstyle] Tools area

[David Redondo]

davidre added a comment.


  In D27669#635243 , @cblack wrote:
  
  > Assume there's no borders when there's not a BorderSize entry
  
  
  That's totally not correct. The default value when there's no BorderSize 
entry is "Normal" . This patch is now also enabled when the user manually 
selects "Normal". The correct thing is to check  first whether `BorderSizeAuto` 
is enabled (default true) . If it's not enabled you can read `BorderSize` 
(default "Normal") otherwise check what the preference the decoration sets. I 
didn't check yet where that value is defined

REPOSITORY
  R31 Breeze

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

To: cblack, #plasma, #breeze, #vdg, hpereiradacosta, davidre
Cc: IlyaBizyaev, davidre, davidedmundson, hpereiradacosta, ngraham, manueljlin, 
niccolove, ndavis, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27669: [kstyle] Tools area

[Carson Black]

cblack updated this revision to Diff 78572.
cblack marked an inline comment as done.
cblack added a comment.


  Fix error

REPOSITORY
  R31 Breeze

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27669?vs=78567=78572

BRANCH
  cblack/toolsarea

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

AFFECTED FILES
  kstyle/CMakeLists.txt
  kstyle/breeze.kcfg
  kstyle/breezehelper.cpp
  kstyle/breezehelper.h
  kstyle/breezestyle.cpp
  kstyle/breezestyle.h
  kstyle/breezetoolsareamanager.cpp
  kstyle/breezetoolsareamanager.h
  kstyle/config/breezestyleconfig.cpp
  kstyle/config/ui/breezestyleconfig.ui

To: cblack, #plasma, #breeze, #vdg, hpereiradacosta, davidre
Cc: IlyaBizyaev, davidre, davidedmundson, hpereiradacosta, ngraham, manueljlin, 
niccolove, ndavis, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27669: [kstyle] Tools area

[Hugo Pereira Da Costa]

hpereiradacosta added inline comments.

INLINE COMMENTS

> breezehelper.cpp:1687
> +while (parent != nullptr) {
> +if (qobject_cast(widget) || qobject_cast QDockWidget*>(widget)) {
> +return false;

mmm shouldn't you test on parent rather than widget here ?

REPOSITORY
  R31 Breeze

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

To: cblack, #plasma, #breeze, #vdg, hpereiradacosta, davidre
Cc: IlyaBizyaev, davidre, davidedmundson, hpereiradacosta, ngraham, manueljlin, 
niccolove, ndavis, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27669: [kstyle] Tools area

[Carson Black]

cblack updated this revision to Diff 78567.
cblack added a comment.


  Assume there's no borders when there's not a BorderSize entry

REPOSITORY
  R31 Breeze

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27669?vs=78565=78567

BRANCH
  cblack/toolsarea

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

AFFECTED FILES
  kstyle/CMakeLists.txt
  kstyle/breeze.kcfg
  kstyle/breezehelper.cpp
  kstyle/breezehelper.h
  kstyle/breezestyle.cpp
  kstyle/breezestyle.h
  kstyle/breezetoolsareamanager.cpp
  kstyle/breezetoolsareamanager.h
  kstyle/config/breezestyleconfig.cpp
  kstyle/config/ui/breezestyleconfig.ui

To: cblack, #plasma, #breeze, #vdg, hpereiradacosta, davidre
Cc: IlyaBizyaev, davidre, davidedmundson, hpereiradacosta, ngraham, manueljlin, 
niccolove, ndavis, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27669: [kstyle] Tools area

[Carson Black]

cblack updated this revision to Diff 78565.
cblack added a comment.


  Account for the applications that think it's semantically correct to put a 
main window in a dock widget

REPOSITORY
  R31 Breeze

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27669?vs=78505=78565

BRANCH
  cblack/toolsarea

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

AFFECTED FILES
  kstyle/CMakeLists.txt
  kstyle/breeze.kcfg
  kstyle/breezehelper.cpp
  kstyle/breezehelper.h
  kstyle/breezestyle.cpp
  kstyle/breezestyle.h
  kstyle/breezetoolsareamanager.cpp
  kstyle/breezetoolsareamanager.h
  kstyle/config/breezestyleconfig.cpp
  kstyle/config/ui/breezestyleconfig.ui

To: cblack, #plasma, #breeze, #vdg, hpereiradacosta, davidre
Cc: IlyaBizyaev, davidre, davidedmundson, hpereiradacosta, ngraham, manueljlin, 
niccolove, ndavis, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27669: [kstyle] Tools area

[Nathaniel Graham]

ngraham added a comment.


  In D27669#634903 , @davidre wrote:
  
  > - Why are we drawing a separator if the window has no toolbar?
  
  
  Because this looks really good :)
  
  F821: vokoscreenNG-2020-03-26_09-46-28.webm 


REPOSITORY
  R31 Breeze

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

To: cblack, #plasma, #breeze, #vdg, hpereiradacosta, davidre
Cc: IlyaBizyaev, davidre, davidedmundson, hpereiradacosta, ngraham, manueljlin, 
niccolove, ndavis, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27669: [kstyle] Tools area

[Nathaniel Graham]

ngraham added a comment.


  Can confirm, if I override the default border setting and change it to "no 
borders", the appearance looks right again. So I guess we need to handle cases 
where the window decoration theme's default border setting is "no borders".
  
  It might also be nice to mention in the config UI that the appearance change 
only takes effect when using No Borders. The text could be something like 
"Visually merge toolbars and menubars into window titlebar when using no window 
borders"

REPOSITORY
  R31 Breeze

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

To: cblack, #plasma, #breeze, #vdg, hpereiradacosta, davidre
Cc: IlyaBizyaev, davidre, davidedmundson, hpereiradacosta, ngraham, manueljlin, 
niccolove, ndavis, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27669: [kstyle] Tools area

[David Redondo]

davidre added a comment.


  Some observation:
  
- Performance in KDevelop is horrible (First startup, opening files, 
switching between file tabs...)
- It also colors the toolbars of mdi windows F8199474: 
Screenshot_20200326_101026.png 
  - It also draws the line around some views double, see above screenshot and 
F8199478: Screenshot_20200326_101134.png 
  - Why are we drawing a separator if the window has no toolbar?

REPOSITORY
  R31 Breeze

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

To: cblack, #plasma, #breeze, #vdg, hpereiradacosta, davidre
Cc: IlyaBizyaev, davidre, davidedmundson, hpereiradacosta, ngraham, manueljlin, 
niccolove, ndavis, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27669: [kstyle] Tools area

[David Redondo]

davidre requested changes to this revision.
davidre added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> breezehelper.cpp:1782
> +auto borderSize = kdecorationGroup.readEntry("BorderSize", 
> "Invalid");
> +if (borderSize != "None" && borderSize != "NoSides") {
> +return false;

This check is the problem. If "Use Theme's default border size" is checked (the 
default) then no entry will be written in the config file because the defaults 
are used. This is for`BorderSizeAuto` "true" and for `BorderSize` "Normal" 
(Note however that because of the the BorderSizeAuto the default borders of the 
deco are used and not normal sized borders)

REPOSITORY
  R31 Breeze

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

To: cblack, #plasma, #breeze, #vdg, hpereiradacosta, davidre
Cc: IlyaBizyaev, davidre, davidedmundson, hpereiradacosta, ngraham, manueljlin, 
niccolove, ndavis, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27669: [kstyle] Tools area

[Noah Davis]

ndavis added a comment.


  I use a kdesrc-build environment. My process was this:
  
cd breeze
arc patch D27669
ninja -C "$( pwd | sed 's:/src/:/build/:' )" install  #I use this through a 
script named kdesrc-ninja and it works well
source ~/kde/.setup-env  #This makes the terminal session use the 
kdesrc-build environment
dolphin
  
  
  
  In D27669#634824 , 
@hpereiradacosta wrote:
  
  > F8199260: Screenshot_20200325_210812.png 

  
  
  That white combo box looks a little nasty against the dark gray background. I 
wonder if there's anything that can be done about it.

REPOSITORY
  R31 Breeze

BRANCH
  cblack/toolsarea

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

To: cblack, #plasma, #breeze, #vdg, hpereiradacosta
Cc: IlyaBizyaev, davidre, davidedmundson, hpereiradacosta, ngraham, manueljlin, 
niccolove, ndavis, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27669: [kstyle] Tools area

[Hugo Pereira Da Costa]

hpereiradacosta added a comment.


  In D27669#634758 , @ngraham wrote:
  
  > So I'm still having an issue with the feature um, not working. :( I have it 
turned on in the Breeze settings but it behaves as if off; that is to say, I 
see no different appearance compared to the status quo.
  >
  > Is anyone else experiencing this, or just me?
  
  
  ... it certainly is working here. 
  F8199260: Screenshot_20200325_210812.png 

  
  So ... can't help you (not to say that there isn't any problem. just that it 
is not completely trivial to find)

REPOSITORY
  R31 Breeze

BRANCH
  cblack/toolsarea

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

To: cblack, #plasma, #breeze, #vdg, hpereiradacosta
Cc: IlyaBizyaev, davidre, davidedmundson, hpereiradacosta, ngraham, manueljlin, 
niccolove, ndavis, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27669: [kstyle] Tools area

[Noah Davis]

ndavis added a comment.


  In D27669#634758 , @ngraham wrote:
  
  > So I'm still having an issue with the feature um, not working. :( I have it 
turned on in the Breeze settings but it behaves as if off; that is to say, I 
see no different appearance compared to the status quo.
  >
  > Is anyone else experiencing this, or just me?
  
  
  It doesn't work for me either. I tested with Dolphin and Kate.

REPOSITORY
  R31 Breeze

BRANCH
  cblack/toolsarea

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

To: cblack, #plasma, #breeze, #vdg, hpereiradacosta
Cc: IlyaBizyaev, davidre, davidedmundson, hpereiradacosta, ngraham, manueljlin, 
niccolove, ndavis, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27669: [kstyle] Tools area

[Nathaniel Graham]

ngraham added a comment.


  So I'm still having an issue with the feature um, not working. :( I have it 
turned on in the Breeze settings but it behaves as if off; that is to say, I 
see no different appearance compared to the status quo.
  
  Is anyone else experiencing this, or just me?

REPOSITORY
  R31 Breeze

BRANCH
  cblack/toolsarea

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

To: cblack, #plasma, #breeze, #vdg, hpereiradacosta
Cc: IlyaBizyaev, davidre, davidedmundson, hpereiradacosta, ngraham, manueljlin, 
niccolove, ndavis, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27669: [kstyle] Tools area

[Hugo Pereira Da Costa]

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


  All good, as far as I am concerned. Thanks !

REPOSITORY
  R31 Breeze

BRANCH
  cblack/toolsarea

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

To: cblack, #plasma, #breeze, #vdg, hpereiradacosta
Cc: IlyaBizyaev, davidre, davidedmundson, hpereiradacosta, ngraham, manueljlin, 
niccolove, ndavis, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27669: [kstyle] Tools area

[Carson Black]

cblack updated this revision to Diff 78505.
cblack added a comment.


  Address feedback

REPOSITORY
  R31 Breeze

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27669?vs=78306=78505

BRANCH
  cblack/toolsarea

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

AFFECTED FILES
  kstyle/CMakeLists.txt
  kstyle/breeze.kcfg
  kstyle/breezehelper.cpp
  kstyle/breezehelper.h
  kstyle/breezestyle.cpp
  kstyle/breezestyle.h
  kstyle/breezetoolsareamanager.cpp
  kstyle/breezetoolsareamanager.h
  kstyle/config/breezestyleconfig.cpp
  kstyle/config/ui/breezestyleconfig.ui

To: cblack, #plasma, #breeze, #vdg, hpereiradacosta
Cc: IlyaBizyaev, davidre, davidedmundson, hpereiradacosta, ngraham, manueljlin, 
niccolove, ndavis, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27669: [kstyle] Tools area

[Hugo Pereira Da Costa]

hpereiradacosta added a comment.


  I finally got some time to look at the code. Some minor comments below 
(compiler warnings)
  Also there is a problem with menubar text color when option is disabled. Here 
at least it is still set to the decoration color, leading to invisible text 
with default breeze color scheme.
  See: 
  F8199046: Screenshot_20200325_155010.png 

  F8199048: Screenshot_20200325_155034.png 


INLINE COMMENTS

> breezehelper.cpp:50
> +_config( std::move( config ) ),
> +_decorationConfig( new InternalSettings() ),
> +_kwinConfig( KSharedConfig::openConfig("kwinrc") ),

compiler complains about wrong initialization order

> breezestyle.cpp:4935
> +
> +bool Style::drawMenuBarEmptyAreaControl( const QStyleOption* option, 
> QPainter* painter, const QWidget* widget ) const
> +{

same remark about option being unused

REPOSITORY
  R31 Breeze

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

To: cblack, #plasma, #breeze, #vdg, hpereiradacosta
Cc: IlyaBizyaev, davidre, davidedmundson, hpereiradacosta, ngraham, manueljlin, 
niccolove, ndavis, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27669: [kstyle] Tools area

[Hugo Pereira Da Costa]

hpereiradacosta added a comment.


  Hi Carson, 
  Thanks for the update. At first glance this all look pretty good. I should be 
able to do some more in depth testing and code review by the end of tomorrow. 
  Hugo

REPOSITORY
  R31 Breeze

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

To: cblack, #plasma, #breeze, #vdg
Cc: IlyaBizyaev, davidre, davidedmundson, hpereiradacosta, ngraham, manueljlin, 
niccolove, ndavis, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27669: [kstyle] Tools area

[Carson Black]

cblack updated this revision to Diff 78306.
cblack added a comment.


  Use explicit setting instead of colour palette heuristics

REPOSITORY
  R31 Breeze

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27669?vs=78303=78306

BRANCH
  cblack/toolsarea

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

AFFECTED FILES
  kstyle/CMakeLists.txt
  kstyle/breeze.kcfg
  kstyle/breezehelper.cpp
  kstyle/breezehelper.h
  kstyle/breezestyle.cpp
  kstyle/breezestyle.h
  kstyle/breezetoolsareamanager.cpp
  kstyle/breezetoolsareamanager.h
  kstyle/config/breezestyleconfig.cpp
  kstyle/config/ui/breezestyleconfig.ui

To: cblack, #plasma, #breeze, #vdg
Cc: IlyaBizyaev, davidre, davidedmundson, hpereiradacosta, ngraham, manueljlin, 
niccolove, ndavis, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27669: [kstyle] Tools area

[Carson Black]

cblack updated this revision to Diff 78303.
cblack marked an inline comment as done.
cblack added a comment.


  Drop giant if

REPOSITORY
  R31 Breeze

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27669?vs=78199=78303

BRANCH
  cblack/toolsarea

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

AFFECTED FILES
  kstyle/CMakeLists.txt
  kstyle/breezehelper.cpp
  kstyle/breezehelper.h
  kstyle/breezestyle.cpp
  kstyle/breezestyle.h
  kstyle/breezetoolsareamanager.cpp
  kstyle/breezetoolsareamanager.h

To: cblack, #plasma, #breeze, #vdg
Cc: IlyaBizyaev, davidre, davidedmundson, hpereiradacosta, ngraham, manueljlin, 
niccolove, ndavis, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27669: [kstyle] Tools area

[David Redondo]

davidre added inline comments.

INLINE COMMENTS

> breezehelper.cpp:90
> +_decorationConfig->load();
> +if (qApp && qApp->property("KDE_COLOR_SCHEME_PATH").isValid()) {
> +const auto path = 
> qApp->property("KDE_COLOR_SCHEME_PATH").toString();

I don't think you need such a big if. Loading a kconfig with empty string will 
load kdeglobals

REPOSITORY
  R31 Breeze

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

To: cblack, #plasma, #breeze, #vdg
Cc: IlyaBizyaev, davidre, davidedmundson, hpereiradacosta, ngraham, manueljlin, 
niccolove, ndavis, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27669: [kstyle] Tools area

[Carson Black]

cblack updated this revision to Diff 78199.
cblack added a comment.


  Address some feedback and fix issues

REPOSITORY
  R31 Breeze

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27669?vs=78198=78199

BRANCH
  cblack/toolsarea

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

AFFECTED FILES
  kstyle/CMakeLists.txt
  kstyle/breezehelper.cpp
  kstyle/breezehelper.h
  kstyle/breezestyle.cpp
  kstyle/breezestyle.h
  kstyle/breezetoolsareamanager.cpp
  kstyle/breezetoolsareamanager.h

To: cblack, #plasma, #breeze, #vdg
Cc: IlyaBizyaev, davidre, davidedmundson, hpereiradacosta, ngraham, manueljlin, 
niccolove, ndavis, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27669: [kstyle] Tools area

[Carson Black]

cblack updated this revision to Diff 78198.
cblack marked 8 inline comments as done.
cblack added a comment.


  Specifically set bottom margin on bottommost toolbar instead of using a metric

REPOSITORY
  R31 Breeze

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27669?vs=78196=78198

BRANCH
  cblack/toolsarea

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

AFFECTED FILES
  kstyle/CMakeLists.txt
  kstyle/breezehelper.cpp
  kstyle/breezehelper.h
  kstyle/breezestyle.cpp
  kstyle/breezestyle.h
  kstyle/breezetoolsareamanager.cpp
  kstyle/breezetoolsareamanager.h

To: cblack, #plasma, #breeze, #vdg
Cc: IlyaBizyaev, davidre, davidedmundson, hpereiradacosta, ngraham, manueljlin, 
niccolove, ndavis, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27669: [kstyle] Tools area

[Carson Black]

cblack updated this revision to Diff 78196.
cblack added a comment.


  Check for presence of KDE_COLOR_SCHEME_PATH and utilize it if set

REPOSITORY
  R31 Breeze

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27669?vs=78194=78196

BRANCH
  cblack/toolsarea

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

AFFECTED FILES
  kstyle/CMakeLists.txt
  kstyle/breeze.h
  kstyle/breezehelper.cpp
  kstyle/breezehelper.h
  kstyle/breezestyle.cpp
  kstyle/breezestyle.h
  kstyle/breezetoolsareamanager.cpp
  kstyle/breezetoolsareamanager.h

To: cblack, #plasma, #breeze, #vdg
Cc: IlyaBizyaev, davidre, davidedmundson, hpereiradacosta, ngraham, manueljlin, 
niccolove, ndavis, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27669: [kstyle] Tools area

[Carson Black]

cblack updated this revision to Diff 78194.
cblack added a comment.


  Render tools area background in drawWidgetPrimitive

REPOSITORY
  R31 Breeze

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27669?vs=78065=78194

BRANCH
  cblack/toolsarea

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

AFFECTED FILES
  kstyle/CMakeLists.txt
  kstyle/breeze.h
  kstyle/breezehelper.cpp
  kstyle/breezehelper.h
  kstyle/breezestyle.cpp
  kstyle/breezestyle.h
  kstyle/breezetoolsareamanager.cpp
  kstyle/breezetoolsareamanager.h

To: cblack, #plasma, #breeze, #vdg
Cc: IlyaBizyaev, davidre, davidedmundson, hpereiradacosta, ngraham, manueljlin, 
niccolove, ndavis, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27669: [kstyle] Tools area

[Nathaniel Graham]

ngraham added a comment.


  Regarding the Tools Area separator color being different:
  
  I agree with Hugo, and have voiced this concern before. It's also not 
consistent with the mockups and IMO the presented justification demonstrates 
why the different color isn't needed: the Tools Area's background color is 
already different, which aids in the visual separation. IMO the separator color 
doesn't need to be changed.

REPOSITORY
  R31 Breeze

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

To: cblack, #plasma, #breeze, #vdg
Cc: IlyaBizyaev, davidre, davidedmundson, hpereiradacosta, ngraham, manueljlin, 
niccolove, ndavis, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27669: [kstyle] Tools area

[Hugo Pereira Da Costa]

hpereiradacosta added inline comments.

INLINE COMMENTS

> cblack wrote in breeze.h:104
> The tools area's separator has to separate two large areas of the window and 
> thus should be stronger than the separators that only have to separate 
> borderless and backgroundless buttons.

As I said, to me at least it does not look so good in the two screenshots you 
posted above. 
In general I think it is not a good idea to arbitrarily multiply the various 
shades/colors used in a single ui, to fullfil the same role. Beside, the two 
areas in questions have a different background already which in itself 
constitutes a strong separator. 
Anyway I'll let other VDG members comments/decide on this. 
Just posting my opinion.
Not a show stopper on my side

REPOSITORY
  R31 Breeze

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

To: cblack, #plasma, #breeze, #vdg
Cc: IlyaBizyaev, davidre, davidedmundson, hpereiradacosta, ngraham, manueljlin, 
niccolove, ndavis, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27669: [kstyle] Tools area

[Carson Black]

cblack added inline comments.

INLINE COMMENTS

> hpereiradacosta wrote in breeze.h:104
> Sorry for the many postings, I had another unrelated comment on these 
> screeshot: 
> I find it strange that the toolarea separator color in this screenshot is 
> brighter than the toolbar separators (the vertical lines between tool 
> buttons). 
> IMO it would make more sense to have both have the exact same color, since 
> they fullfil the same role (separating ...)

The tools area's separator has to separate two large areas of the window and 
thus should be stronger than the separators that only have to separate 
borderless and backgroundless buttons.

REPOSITORY
  R31 Breeze

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

To: cblack, #plasma, #breeze, #vdg
Cc: IlyaBizyaev, davidre, davidedmundson, hpereiradacosta, ngraham, manueljlin, 
niccolove, ndavis, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27669: [kstyle] Tools area

[Hugo Pereira Da Costa]

hpereiradacosta added inline comments.

INLINE COMMENTS

> cblack wrote in breeze.h:104
> With: F8185479: image.png 
> Without: F8185480: image.png 

Sorry for the many postings, I had another unrelated comment on these 
screeshot: 
I find it strange that the toolarea separator color in this screenshot is 
brighter than the toolbar separators (the vertical lines between tool buttons). 
IMO it would make more sense to have both have the exact same color, since they 
fullfil the same role (separating ...)

REPOSITORY
  R31 Breeze

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

To: cblack, #plasma, #breeze, #vdg
Cc: IlyaBizyaev, davidre, davidedmundson, hpereiradacosta, ngraham, manueljlin, 
niccolove, ndavis, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27669: [kstyle] Tools area

[Hugo Pereira Da Costa]

hpereiradacosta added inline comments.

INLINE COMMENTS

> cblack wrote in breezetoolsareamanager.cpp:133
> This only triggers when the window has no items in the tools area—it's used 
> in conjunction with line 878 of breezestyle.cpp in order to render a border 
> at the top of the window.

Clear enough ! Sorry for the noise ! (and not having read the code with enough 
attention).

REPOSITORY
  R31 Breeze

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

To: cblack, #plasma, #breeze, #vdg
Cc: IlyaBizyaev, davidre, davidedmundson, hpereiradacosta, ngraham, manueljlin, 
niccolove, ndavis, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27669: [kstyle] Tools area

[Carson Black]

cblack added inline comments.

INLINE COMMENTS

> hpereiradacosta wrote in breezetoolsareamanager.cpp:133
> Did you check that this chunk of code (changing the contents margins of the 
> QMainWindow) has any effect ?  I had my doubt so I changed the margin to 100 
> instead of 1, added a printout to make sure that this line of code is called, 
> then ran dolphin and 
> 1/ the code is indeed called
> 2/ it changes nothing, on dolphin, okular, ...
> if you can confirm, then just drop it.
> 
> (in fact, I don't understand the login in the first place: why would you need 
> to add one pixel on top of everything in the QMainWindow ?)

This only triggers when the window has no items in the tools area—it's used in 
conjunction with line 878 of breezestyle.cpp in order to render a border at the 
top of the window.

REPOSITORY
  R31 Breeze

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

To: cblack, #plasma, #breeze, #vdg
Cc: IlyaBizyaev, davidre, davidedmundson, hpereiradacosta, ngraham, manueljlin, 
niccolove, ndavis, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27669: [kstyle] Tools area

[Carson Black]

cblack added a comment.


  In D27669#632147 , 
@hpereiradacosta wrote:
  
  > In D27669#630974 , @ngraham 
wrote:
  >
  > > Nice, the colors are better now. I still see a difference in animation 
speed when the titlebar and toolbar change color though. It's especially 
visible with the current Breeze color scheme.
  > >
  > > F8185590: vokoscreenNG-2020-03-19_20-53-04.webm 

  >
  >
  > Here at least (also X11) not only is the window decoration out of sync with 
the toolarea, but when you have both a menubar and a toolbar (as in e.g. 
okular), both are also out of sync. Can you double-check, confirm ? 
  >  I think it is because rendering the background is expensive (especially in 
case of animation) and both widgets are not rendered at the exact same time. 
  >  one solution could be to render the background directly on the mainwindow 
(leaving the toolbar and the menubars transparent, as they were before). I have 
not investigated further how difficult this would be to implement though (if 
even possible)
  
  
  I could move the rendering of the background and border of the tools area 
into the window's background that's currently only utilised for drawing a 
border without tools area.

REPOSITORY
  R31 Breeze

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

To: cblack, #plasma, #breeze, #vdg
Cc: IlyaBizyaev, davidre, davidedmundson, hpereiradacosta, ngraham, manueljlin, 
niccolove, ndavis, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27669: [kstyle] Tools area

[Hugo Pereira Da Costa]

hpereiradacosta added a comment.


  In D27669#630974 , @ngraham wrote:
  
  > Nice, the colors are better now. I still see a difference in animation 
speed when the titlebar and toolbar change color though. It's especially 
visible with the current Breeze color scheme.
  >
  > F8185590: vokoscreenNG-2020-03-19_20-53-04.webm 

  
  
  Here at least (also X11) not only is the window decoration out of sync with 
the toolarea, but when you have both a menubar and a toolbar (as in e.g. 
okular), both are also out of sync. I think it is because rendering the 
background is expensive and both widgets are not rendered at the exact same 
time. 
  one solution could be to render the background directly on the mainwindow 
(leaving the toolbar and the menubars transparent, as they were before). I have 
not investigated further how difficult this would be to implement though (if 
even possible)

REPOSITORY
  R31 Breeze

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

To: cblack, #plasma, #breeze, #vdg
Cc: IlyaBizyaev, davidre, davidedmundson, hpereiradacosta, ngraham, manueljlin, 
niccolove, ndavis, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27669: [kstyle] Tools area

[Hugo Pereira Da Costa]

hpereiradacosta added a comment.


  Hi Carlston, 
  Thanks for the updated patch and screenshot. 
  First I agree that the new (latest) checks on whether the toolbar palette was 
changed or not are much more elegant and just as efficient as the previous 
implementation.
  Second, some more comments below.

INLINE COMMENTS

> cblack wrote in breeze.h:104
> With: F8185479: image.png 
> Without: F8185480: image.png 

Hi,
thanks for posting the screenshots. If I understand right this is really an 
issue with the bottom margin only, and in particular with respect to the 
separator. In that case I agree this is part of this patch. 
I don't think the solution is the right one though. First, changing the metric 
will also affect widget rendering when

- the effect is disabled (and I don't think it should in that case)
- when there is no separator line (and IMO I don't think the extra margin is 
needed in that case either).
- when toolbars are located elsewhere (on the sides of the main window for 
instance).

Second, it affects all sides, whereas only the bottom one needs changed. 
i would rather change the "contentsmargin" of the toolbars when necessary, 
while leaving the metrics unchanged. This could be done whenever you check that 
a given toolbar is in the toolarea.

If you insist on changing the margins on all toolbars, all sides, disregarding 
their location and disregarding whether the effect is enabled or not, then this 
is a change orthogonal (and of much broader scope) to this patch and must go to 
a different commit.

> breezetoolsareamanager.cpp:133
> +window->setContentsMargins(0,1,0,0);
> +}
> +});

Did you check that this chunk of code (changing the contents margins of the 
QMainWindow) has any effect ?  I had my doubt so I changed the margin to 100 
instead of 1, added a printout to make sure that this line of code is called, 
then ran dolphin and 
1/ the code is indeed called
2/ it changes nothing, on dolphin, okular, ...
if you can confirm, then just drop it.

(in fact, I don't understand the login in the first place: why would you need 
to add one pixel on top of everything in the QMainWindow ?)

REPOSITORY
  R31 Breeze

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

To: cblack, #plasma, #breeze, #vdg
Cc: IlyaBizyaev, davidre, davidedmundson, hpereiradacosta, ngraham, manueljlin, 
niccolove, ndavis, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27669: [kstyle] Tools area

[Niccolò Venerandi]

niccolove added a comment.


  If possible, I think that supporting toparea transparency with those 
colorscheme would be a great feature for people who would like plasma to look 
like this (see below), as they would only need to change colorscheme.
  F8185993: image.png 
  My mockup for a slighly transparent toolsarea (on the right):
  F8185995: image.png 

REPOSITORY
  R31 Breeze

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

To: cblack, #plasma, #breeze, #vdg
Cc: IlyaBizyaev, davidre, davidedmundson, hpereiradacosta, gvgeo, ngraham, 
manueljlin, niccolove, ndavis, plasma-devel, Orage, LeGast00n, 
The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra, mart

D27669: [kstyle] Tools area

[David Redondo]

davidre added a comment.


  Some colorschemes do have transparent titlebars, do you plan to support 
transparency or leave it as is? F8185862: Screenshot_20200320_090510.PNG 

  
  To support apps that have another color schemes than the global one look at 
the property KColoSchemeManager sets: 
https://cgit.kde.org/kconfigwidgets.git/tree/src/kcolorschememanager.cpp#n42
  Otherwise it looks like this:
  F8185866: Screenshot_20200320_090912.PNG 


REPOSITORY
  R31 Breeze

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

To: cblack, #plasma, #breeze, #vdg
Cc: IlyaBizyaev, davidre, davidedmundson, hpereiradacosta, gvgeo, ngraham, 
manueljlin, niccolove, ndavis, plasma-devel, Orage, LeGast00n, 
The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra, mart