Proposal to use a set of internal constants for all the button and icons
Open, Needs TriagePublic

Description

This is a proposal for adding a set of internal constant values that we should use for all the buttons and icons sizes inside Krita. After discussing it with Scott, we think it would be the best way to go...

Why:
-We need to be able to control the size of buttons independently from the theme selected.
For now we have random hardcoded values for almost each button, so setting some constants in a single place that we should reuse everywhere would be better for consistency and maintainability.

-We should probably not reuse Qt-provided constants, as those are theme dependant and not really possible to control as far as I could see.

-Having such constants will give us the possiblity in the long term to fix some scaling issues, and to make it easy for users to use a custom scale for buttons/icons. (example: currently, at least on Linux, when changing the display scaling, it doesn't change the size of all our hardcoded-values... So we could find a way to handle such scaling internally, or just provide a user setting to scale all the buttons/icons.)

How:
-define some constants in an easy-to-access place (KisIconUtils ?)

-replace all the hardcoded size values for buttons and icons with those constants. It could probably be possible to set them in a few strategical places for the different types of buttons we have, instead of setting it for each and every button like we do currently.

After analyzing our current buttons and icons with both Fusion and Breeze, and making a few tests, I propose to define the following set of values.

Buttons/Icons sizes (in px):
  • Default size: 32/22

(used in Toolbars, and everywhere else unless stated otherwise)

  • ToolBox: 26/16 (by default; + all the other optional icon sizes, with buttons size 10px bigger)
  • Small buttons: 24/16 (in places where it's good to save space, like the set of curve buttons in brush settings, or the buttons in the transform tool options...)
  • Dockers titlebar buttons: 18/16

For buttons on layers (visible, lock, alpha...), I suggest to use the Small buttons size for the width, and as minimum for the height; height is normally a bit higher anyway... (and even higher when increasing thumbnails size)

Icons-only (not directly on buttons):

-Big icons: 32 (like icons on the left of "Create new document", and left of "Configure Krita"...)

-Small icons: 16 (used at least for Layer types... currently it's 12, but we can probaly use 16 as minimum)

We should probably not reuse Qt-provided constants

Well, it'll mean that our widgets will look very different from the widgets from Qt. That doesn't look very good idea. I have a feeling that, if we go for constants for some custom widgets, we should still make these constants proportional to the ones returned from QStyle.

I first made this proposal after noticing that we already define our own sizes for buttons in several places, and so thought that it could make sense to do it everywhere the same way to be more consistent.

See for example, in kis_paintop_box.cc, we have a variable iconsize that is used to set the size of all the buttons inside it. Note that this variable name is also quite confusing, as far as I can see it's setting the size of the buttons (with -> setFixedSize), not the size of the icons.

I also tried to understand how the sizes of all the other ToolBar buttons are set (like New/Open/Save document, Undo/Redo, and all the other buttons that can be added to the toolbars), but could not figure it yet... I see that we do some stuff in ktoolbar.cpp which could be related but not sure yet. One thing for sure, they are not using style()->pixelMetric(QStyle::PM_ToolBarIconSize) , as I've tried to use this on the buttons in kis_paintop_box.cc and it gives a very different size.

I believe that in all the places where we use custom sizes (except of the few really custom widgets, like layers docker and onion skins widget), we should try to use values like style()->pixelMetric(QStyle::PM_ToolBarIconSize) instead. I didn't do any good research for it though. We used custom values because we were too lazy or didn't know about existence of QStyle-defined values. So I believe we can just replace them in most of the cases.

There might be some showstopper for such approach, but I don't know any atm.

While I agree that "technically" it would be nice to can use those QStyle values, in practice it doesn't seem very good...

I take again the example of our toolbar in details:

-Currently, all the buttons which are defined in kis_paintop_box.cc have a button size of 32x32 and icon size of 22x22, which I honestly think is the best size for those toolbar buttons.
All the other buttons ( File new/open/save, undo/redo, and other user-added buttons), with fusion style they have button size 30x29 and icon size 22x22, and with breeze they have button size 32x32 and icon size 22x22.

-If I change the buttons in kis_paintop_box.cc to use style()->pixelMetric(QStyle::PM_ToolBarIconSize) , the buttons become much smaller, which is not good in my opinion (icons become 16x16, and buttons are 24x23px with fusion, and 26x26 with breeze... and even worse, as you can see the icon of the brush editor becomes distorted with fusion, but not with breeze).


If I compare for reference with other KDE applications, like Dolphin, toolbar buttons have button size 32x32 and icon size 22x22, same as our kis_paintop_box.cc buttons and almost same as our other toolbar buttons with fusion style... And which is the size I would expect for those buttons.
So if we need to change something, I would say we need to make all our toolbar buttons 32x32 with icon size 22x22, which doesn't seem possible using the provided QStyle. Unless we create our own QStyle, but it's really not an easy task, I'm not sure it's something we want to do. So in the end I believe using our own set of constants where needed would be easier than creating a whole new QStyle.

Okay, I have spent some time on investigating into the code. It looks like for the toolbar we need to use values from KToolBar. It has a complicated system for determining the icon size. I still don't know how to fetch this value. (I haven't checked actually).

Basically, replacing widget->setFixedSize(32,32) to widget->setIconSize(22,22) fixes the issue for the buttons in the paintopbox. The only exception is the brush editor button, because it uses custom algorithm that doesn't care about this option (it is not QToolButton, but KisIconWidget).

So, I basically, for the toolbox buttons we can just reuse values from KToolBar. We don't need to define them anywhere separately.

timotheegiet added a comment.EditedFeb 10 2021, 2:46 PM

Thanks for the feedback :)
Though actually I would prefer to do the other way around, that is change KToolBar to use the same values as we have in the paintopbox, and in the paintopbox do both setFixedSize(32,32) and setIconSize(22,22). This looks much better I feel, as then the buttons are the same height as the combobox and as the sliders (else, with just setting the icon size, the buttons are a bit smaller than the sliders with fusion). But for this I need to dig more into KToolBar to see how to properly set the size of buttons, and the height of the toolbar itself (actually the height of the toolbar is adapted automatically, I was confused thinking it would not by setting its size by mistake in my tests...)

To complete my previous comment, I would add that there are also several other component on the toolbar that have a defined size that looks best if all the buttons are 32x32:
-The Gradient button and the Patterns button (which are 32x32)
-The Foreground/Background colors widget (this one is 28x28, but looks good when components around are 32x32)
-The blending mode combobox, and the sliders (they have a fixed height of 32)

So while the proposed solution to only use only setIconSize instead of setFixedSize for the buttons in the paintopbox would be simple, it would reduce a bit those buttons size, and would leave several other things not fixed...
Changing KToolBar to make sure that its buttons are always 32x32 is probably a bit more complicated, but it would make everything fit together at once in the toolbar.

Well, as I said on IRC I'm fine with making toolbar "an exception" and making all the widgets fixed size. But for other places in Krita, we would better use standard sizes.

I'm personally OK with using standard sizes in other places ;)

Now if only I could find how to change KToolBar to force 32x32 size for all the buttons... If you have some ideas, that would be awesome.
(my current search made me think we need to do something in the applyCurrentSetting function, that would find all the buttons in children of q , and use setFixedSize or setMinimumSize on those, but I'm a bit lost on how to do it for now...)

I finally gave up trying to set the size of all the buttons in KToolBar, there must be a way to do it but it's definitely beyond my very limited c++ skills...
I'll let someone more knowledgeable do it.

Note, I finally managed to harmonize all toolbar buttons and icons sizes, yay! :-D