Bug 394963 - Spinboxes look flat
Summary: Spinboxes look flat
Status: RESOLVED UPSTREAM
Alias: None
Product: Breeze
Classification: Plasma
Component: QStyle (show other bugs)
Version: 5.12.5
Platform: Arch Linux Linux
: NOR normal
Target Milestone: ---
Assignee: Hugo Pereira Da Costa
URL:
Keywords:
: 395340 395382 (view as bug list)
Depends on:
Blocks:
 
Reported: 2018-06-02 18:07 UTC by Vlad Zahorodnii
Modified: 2018-06-14 19:19 UTC (History)
3 users (show)

See Also:
Latest Commit:
Version Fixed In:
Sentry Crash Report:


Attachments
The spinbox doesn't have outline and overall it looks super flat (3.02 KB, image/png)
2018-06-02 18:07 UTC, Vlad Zahorodnii
Details
Other controls (25.42 KB, image/png)
2018-06-02 18:09 UTC, Vlad Zahorodnii
Details
Oxygen controls (42.85 KB, image/png)
2018-06-02 19:47 UTC, Vlad Zahorodnii
Details
Editable combobox (35.27 KB, image/png)
2018-06-02 21:03 UTC, Vlad Zahorodnii
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Vlad Zahorodnii 2018-06-02 18:07:23 UTC
Created attachment 113020 [details]
The spinbox doesn't have outline and overall it looks super flat

System Info:
* Distro: Arch Linux
* Qt style: Breeze
* Qt version: 5.11
* KDE Plasma version: 5.12.5

breezerc:
```
[Common]
ShadowColor=0,0,0
ShadowSize=ShadowVeryLarge
ShadowStrength=222

[Windeco]
DrawBackgroundGradient=false
```

IIRC, I started noticing that spinboxes look really flat after
recent Qt update(from 5.10 to 5.11).
Comment 1 Vlad Zahorodnii 2018-06-02 18:09:41 UTC
Created attachment 113021 [details]
Other controls
Comment 2 Hugo Pereira Da Costa 2018-06-02 18:37:41 UTC
Hi Vlad,
Thanks for reporting. 
Is this the case for _all_ spinboxes ? Or only in some applications ? 
the code for rendering spinboxes have not changed in years, and if this is for all spinboxes, I cannot reproduce it with breeze from master here, with Qt-5.10
So that would point to some change in Qt-5.11 indeed. 
I might be able to investigate but might take some time to switch to Qt-5.11 on my side.
Now if you on the other hand, can run the same version of breeze against Qt-5.10 and Qt-5.11 and toggle the issue by switching version, then you might want to file a bug report against Qt. 
Thanks in advance,

Hugo
Comment 3 Vlad Zahorodnii 2018-06-02 19:46:47 UTC
(In reply to Hugo Pereira Da Costa from comment #2)
> Hi Vlad,

Hi, Hugo! :)

> Is this the case for _all_ spinboxes ? Or only in some applications ? 

No, that's the case for all QWidget apps. Kirigami apps seems don't have
such problem.

FWIW, that's also the case for Oxygen and MS Windows 9x. Spinboxes with
the Fusion widget style look OK.

> the code for rendering spinboxes have not changed in years, and if this is
> for all spinboxes, I cannot reproduce it with breeze from master here, with
> Qt-5.10
> So that would point to some change in Qt-5.11 indeed. 
> I might be able to investigate but might take some time to switch to Qt-5.11
> on my side.
> Now if you on the other hand, can run the same version of breeze against
> Qt-5.10 and Qt-5.11 and toggle the issue by switching version, then you
> might want to file a bug report against Qt. 

Not sure if I can help with it, it takes eternity to compile Qt on my laptop.
Comment 4 Vlad Zahorodnii 2018-06-02 19:47:22 UTC
Created attachment 113022 [details]
Oxygen controls
Comment 5 Hugo Pereira Da Costa 2018-06-02 20:01:00 UTC
(In reply to Vlad Zagorodniy from comment #3)
> (In reply to Hugo Pereira Da Costa from comment #2)
> > Hi Vlad,
> 
> Hi, Hugo! :)
> 
> > Is this the case for _all_ spinboxes ? Or only in some applications ? 
> 
> No, that's the case for all QWidget apps. Kirigami apps seems don't have
> such problem.
> 
> FWIW, that's also the case for Oxygen and MS Windows 9x. Spinboxes with
> the Fusion widget style look OK.
> 

Thanks. That's useful. I'll have a look, and if you by chance can compile breeze locally, I'll probably have some patches for you to test.
There is a flag "isFlat" for spinboxes, to allow for rendering frameless spinboxes in e.g. listviews. It seems that for some reason, and some widget styles, this flag is set to true in cases where it should not. That's what I'll investigate. 
I'd be curious to know if "editable" comboboxes show the same issue. 
Example of editable comboboxes can be found in "oxygen-demo5" or in any "save" dialog. Can you check ?
Comment 6 Vlad Zahorodnii 2018-06-02 20:57:48 UTC
After taking a look at Fusion widget style source code, here's a patch
to "fix" this bug in some part:

```
From 8a1b11a4ed6fdd639259d7eea1c04bc926b78483 Mon Sep 17 00:00:00 2001
From: Vlad Zagorodniy <vladzzag@gmail.com>
Date: Sat, 2 Jun 2018 23:42:38 +0300
Subject: [PATCH] Fix SpinBox frame rendering with Qt 5.11

---
 kstyle/breezestyle.cpp | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/kstyle/breezestyle.cpp b/kstyle/breezestyle.cpp
index 28746216..88042362 100644
--- a/kstyle/breezestyle.cpp
+++ b/kstyle/breezestyle.cpp
@@ -6200,13 +6200,11 @@ namespace Breeze
         const auto& palette( option->palette );
         const auto& rect( option->rect );
 
-
-        if( option->subControls & SC_SpinBoxFrame )
+        if( spinBoxOption->frame )
         {
 
             // detect flat spinboxes
-            bool flat( !spinBoxOption->frame );
-            flat |= ( rect.height() < 2*Metrics::Frame_FrameWidth + Metrics::SpinBox_ArrowButtonWidth );
+            const bool flat = ( rect.height() < 2*Metrics::Frame_FrameWidth + Metrics::SpinBox_ArrowButtonWidth );
             if( flat )
             {
 
-- 
2.17.1

```

Also, Fusion checks whether it should draw arrows or plus/minus as follows:
if (spinBox->buttonSymbols == QAbstractSpinBox::PlusMinus) { ... }
else if (spinBox->buttonSymbols == QAbstractSpinBox::UpDownArrows) { ... }

I hope this will help you.
Comment 8 Vlad Zahorodnii 2018-06-02 21:03:19 UTC
(In reply to Hugo Pereira Da Costa from comment #5)
> (In reply to Vlad Zagorodniy from comment #3)
> > (In reply to Hugo Pereira Da Costa from comment #2)
> > > Hi Vlad,
> > 
> > Hi, Hugo! :)
> > 
> > > Is this the case for _all_ spinboxes ? Or only in some applications ? 
> > 
> > No, that's the case for all QWidget apps. Kirigami apps seems don't have
> > such problem.
> > 
> > FWIW, that's also the case for Oxygen and MS Windows 9x. Spinboxes with
> > the Fusion widget style look OK.
> > 
> 
> Thanks. That's useful. I'll have a look, and if you by chance can compile
> breeze locally, I'll probably have some patches for you to test.

Sure, I can.

> There is a flag "isFlat" for spinboxes, to allow for rendering frameless
> spinboxes in e.g. listviews. It seems that for some reason, and some widget
> styles, this flag is set to true in cases where it should not. That's what
> I'll investigate. 
> I'd be curious to know if "editable" comboboxes show the same issue. 
> Example of editable comboboxes can be found in "oxygen-demo5" or in any
> "save" dialog. Can you check ?

Everything is okay.
Comment 9 Vlad Zahorodnii 2018-06-02 21:03:53 UTC
Created attachment 113023 [details]
Editable combobox
Comment 10 Hugo Pereira Da Costa 2018-06-02 22:00:19 UTC
(In reply to Vlad Zagorodniy from comment #6)
> After taking a look at Fusion widget style source code, here's a patch
> to "fix" this bug in some part:
> 
> ```
> From 8a1b11a4ed6fdd639259d7eea1c04bc926b78483 Mon Sep 17 00:00:00 2001
> From: Vlad Zagorodniy <vladzzag@gmail.com>
> Date: Sat, 2 Jun 2018 23:42:38 +0300
> Subject: [PATCH] Fix SpinBox frame rendering with Qt 5.11
> 
> ---
>  kstyle/breezestyle.cpp | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/kstyle/breezestyle.cpp b/kstyle/breezestyle.cpp
> index 28746216..88042362 100644
> --- a/kstyle/breezestyle.cpp
> +++ b/kstyle/breezestyle.cpp
> @@ -6200,13 +6200,11 @@ namespace Breeze
>          const auto& palette( option->palette );
>          const auto& rect( option->rect );
>  
> -
> -        if( option->subControls & SC_SpinBoxFrame )
> +        if( spinBoxOption->frame )
>          {
>  
>              // detect flat spinboxes
> -            bool flat( !spinBoxOption->frame );
> -            flat |= ( rect.height() < 2*Metrics::Frame_FrameWidth +
> Metrics::SpinBox_ArrowButtonWidth );
> +            const bool flat = ( rect.height() < 2*Metrics::Frame_FrameWidth
> + Metrics::SpinBox_ArrowButtonWidth );
>              if( flat )
>              {
>  

Ok. Fix looks "sensible", but if I read the patch right, it would indicate  that SC_SpinBoxFrame has become deprecated, in the sense that it is never set as option subcontrols anymore. Correct ? 

If so, I must still check that it does not break things agains past version of Qt (5.10 or even 4.xx), or add #ifdefs. 

Concerning arrows vs plus-minus signs, well right now we just ignore this setting (of which I was not aware), but thats a completely different issue :)

Thanks for the investigation  !

> ```
> 
> Also, Fusion checks whether it should draw arrows or plus/minus as follows:
> if (spinBox->buttonSymbols == QAbstractSpinBox::PlusMinus) { ... }
> else if (spinBox->buttonSymbols == QAbstractSpinBox::UpDownArrows) { ... }
> 
> I hope this will help you.
Comment 11 Vlad Zahorodnii 2018-06-02 22:26:34 UTC
(In reply to Hugo Pereira Da Costa from comment #10)
> Ok. Fix looks "sensible", but if I read the patch right, it would indicate 
> that SC_SpinBoxFrame has become deprecated, in the sense that it is never
> set as option subcontrols anymore. Correct ? 

I'm not sure about that. I suggest to ask Qt folks first. Maybe it's not deprecated
and what we are facing is a bug in Qt.
Comment 12 Christoph Feck 2018-06-03 03:27:03 UTC
https://bugreports.qt.io/browse/QTBUG-68238
Comment 13 Christoph Feck 2018-06-14 09:19:00 UTC
*** Bug 395340 has been marked as a duplicate of this bug. ***
Comment 14 Matej Mrenica 2018-06-14 19:19:39 UTC
*** Bug 395382 has been marked as a duplicate of this bug. ***