D27669: [kstyle] Tools area

[Hugo Pereira Da Costa]

hpereiradacosta added inline comments.

INLINE COMMENTS

> breezestyle.cpp:4382
> +            QPixmap pixmap = toolButtonOption->icon.pixmap( iconSize, iconMode, iconState );
> +            if (_helper->isInToolsArea(widget)) {
> +                KIconLoader::global()->setCustomPalette(widget->palette());

updating the palette in every paint event (and calling resetPalette()) will be very innefficient. You need to track whether the palette actually needs change before calling these.

> breezestyle.cpp:4894
> +            toolbar->setPalette(toolbar->parentWidget()->palette());
> +            return true;
> +        }

Same remark. This is very innefficient. 
You need to track whether the palette actually need change.
Ideally it should be enough to set the right palette in polish, and then to update on toolbar position change (can be achieved by catching moveEvent with an event filter on the toolbar)

> breezestyle.cpp:4902
> +
> +        toolbar->setPalette(palette);
> +

same remark

> breezetoolsareamanager.cpp:120
> +                    this, [this]() {
> +                        emit this->toolbarUpdated();
> +                    });

the this-> here are all unnecessary.

> cblack wrote in breezetoolsareamanager.cpp:108
> This is intentional—if a window's titlebar and content colours are the same, the window is drawn as a homogeneous window instead of a window with a tools area.

I was not refering to the rendering of the separator, but rather to the fact that the widgets margin change between active and inactive. This should not happen (disregarding the color scheme). 
All in all, whether you need to draw the separator or not you need to add the margin always (or follow David's suggestion)
Unless I misunderstood the actual issue reported by Nathan of course.

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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20200310/a0aae4ba/attachment.html>