I would prefer not systematically render the background, because it might break existing applications, like rendering a widget on top of a painted image. Also this goes against Qt's rendering model which does not require rendering widget background, relying on that of the parent widget or window itself. I would instead either do it in qqc, or detect if we are rendering a qqc control and in this case only render the background systematically. There are several places in the code that do that already. Look of "#if BREEZE_HAVE_QTQUICK" blocks.

Hugo

In D26572#592280, @hpereiradacosta wrote:

I would prefer not systematically render the background, because it might break existing applications, like rendering a widget on top of a painted image.

In fact that exact use case is already broken by the lack of a built-in background. See the wallpaper slideshow view:

Hugely improved visibility with this patch:

Since this improves checkbox visibility when rendered over images, should we or should we not do this? Is there a way to detect when a checkbox is rendered over an image? Is the added complexity of making all these exceptions better than always rendering the background?

Oh, I should have brought this up before landing, but any opinions about always using QPalette::Base (View Background) for the background color? I asked on the VDG channel and people seemed to be unanimously in favor of using View Background as the background color.

I would strongly object to that:
first that is a visual change unrelated to the issue this patch adress
second this is semantically wrong, and this will break on some color theme.
Things should be kept consistent. If you change the background role, you must also change the foreground role, and then you might really get funny results.

in more detail: imagine a color scheme where window background is black, window text is white, view background is white view text is black.
you will get a white square on a white background for unchecked checkbox ...
I would really revert this part of the change

In D26572#593379, @hpereiradacosta wrote:

in more detail: imagine a color scheme where window background is black, window text is white, view background is white view text is black.
you will get a white square on a white background for unchecked checkbox ...
I would really revert this part of the change

To be clear, that change hasn't been made.

You mean a white square on a black background? That could look odd if buttons aren't also normally white in that colorscheme, which might indicate that Button Background would be more semantically correct. By the way, the checkboxes already used View Background when selected unless they were drawn on top of a menu. In that case, they used Window Background when selected.

In D26572#593442, @ndavis wrote:

In D26572#593379, @hpereiradacosta wrote:

in more detail: imagine a color scheme where window background is black, window text is white, view background is white view text is black.
you will get a white square on a white background for unchecked checkbox ...
I would really revert this part of the change

To be clear, that change hasn't been made.

I'm confused: it has happened, no ? If I apply the patch I now get a white background (with normal breeze color scheme), behind checkboxes and radio button, always, which was not there before. This is , I think because of the change at line 3889.

You mean a white square on a black background?

No: if view color is white this would be white square (window text) on white background (view background).

That could look odd if buttons aren't also normally white in that colorscheme, which might indicate that Button Background would be more semantically correct. By the way, the checkboxes already used View Background when selected unless they were drawn on top of a menu.

No: in general background was not rendered. the View background was used for a special case of selection that happens only in views (if I remember the code right). With the standard breeze color scheme, a checkbox/radio button rendered in a window or a menu would never have a white background (from the view) drawn behind.
Unlike when applying the current patch.

In that case, they used Window Background when selected

What I meant is that I did not change the color of the checkbox background in this patch. I only made it so that the background would always be rendered.

It seems like it might be a good idea to detect whether or not the button is being rendered in a view area and then set it to View Background. Otherwise, it should be Window Background. Doing it that way would preserve the original look in most cases.

In D26572#593496, @ndavis wrote:

What I meant is that I did not change the color of the checkbox background in this patch. I only made it so that the background would always be rendered.

Clear enough. Indeed you did not change the colors, and it led to an unwanted visual change. Sorry for not having caught that up during review. (in fact it now leads to an inconsistency between checkboxes in menus and checkboxes in windows).

It seems like it might be a good idea to detect whether or not the button is being rendered in a view area and then set it to View Background.

yes

Otherwise, it should be Window Background. Doing it that way would preserve the original look in most cases.

This would lead to some regression (I think) for unchecked checkboxes in lists. (because of window background being used).