Bug 154007 - [GTK] Toggle buttons are blurry with GTK+ 3.19
Summary: [GTK] Toggle buttons are blurry with GTK+ 3.19
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: Other
Hardware: PC Linux
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-02-08 14:06 PST by Michael Catanzaro
Modified: 2016-02-09 22:58 PST (History)
8 users (show)

See Also:


Attachments
Patch (3.87 KB, patch)
2016-02-09 06:06 PST, Carlos Garcia Campos
mcatanzaro: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Catanzaro 2016-02-08 14:06:50 PST
Since 3.19.7 we can get at min-width and min-height as style properties:

https://git.gnome.org/browse/gtk+/commit/gtk/gtkcssstylepropertyimpl.c?id=ea69bf8c17dd5c9e7f76bf3cb4f56ec07b2e821f

According to Company, we need to use these by default, rather than indicator-size. If either one is zero, ignore them and fall back to indicator-size. That should fix our blurry toggle buttons (checkbuttons, radio buttons).

We should also audit all our other uses of style properties to make sure we're matching GTK+ 3.19.
Comment 1 Carlos Garcia Campos 2016-02-09 06:06:33 PST
Created attachment 270923 [details]
Patch
Comment 2 WebKit Commit Bot 2016-02-09 06:08:27 PST
Attachment 270923 [details] did not pass style-queue:


ERROR: Source/WebCore/rendering/RenderThemeGtk.cpp:551:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebCore/rendering/RenderThemeGtk.cpp:598:  Extra space before ( in function call  [whitespace/parens] [4]
Total errors found: 2 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Michael Catanzaro 2016-02-09 06:46:10 PST
Comment on attachment 270923 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=270923&action=review

Thanks, I presume you tested it.

> Source/WebCore/rendering/RenderThemeGtk.cpp:551
> +    gtk_style_context_get(childContext.get(), gtk_style_context_get_state (childContext.get()), "min-width", &minWidth, "min-height", &minHeight, nullptr);

Look what style checker caught: gtk_style_context_get_state (

> Source/WebCore/rendering/RenderThemeGtk.cpp:555
> +        minSize.setHeight(minHeight);

Company's suggestion was to do this:

if (minWidth && minHeight) {
    minSize.setWidth(minWidth);
    minSize.setHeight(minHeight);
}

Any particular reason for not following that?

> Source/WebCore/rendering/RenderThemeGtk.cpp:598
> +    gtk_style_context_get(context.get(), gtk_style_context_get_state (context.get()), "min-width", &minWidth, "min-height", &minHeight, nullptr);

Ditto

> Source/WebCore/rendering/RenderThemeGtk.cpp:602
> +        minSize.setHeight(minHeight);

Ditto
Comment 4 Benjamin Otte 2016-02-09 06:50:51 PST
(In reply to comment #3)
> Company's suggestion was to do this:
> 
> if (minWidth && minHeight) {
>     minSize.setWidth(minWidth);
>     minSize.setHeight(minHeight);
> }
> 
To be 100% absolutely exact, it's

if (minWidth)
  minSize.setWidth(minWidth);
if (minHeight)
  minSize.setHeight(minHeight);

because that's what the code in https://git.gnome.org/browse/gtk+/tree/gtk/gtkbuiltinicon.c#n56 essentially does.
Comment 5 Carlos Garcia Campos 2016-02-09 22:56:25 PST
(In reply to comment #4)
> (In reply to comment #3)
> > Company's suggestion was to do this:
> > 
> > if (minWidth && minHeight) {
> >     minSize.setWidth(minWidth);
> >     minSize.setHeight(minHeight);
> > }
> > 
> To be 100% absolutely exact, it's
> 
> if (minWidth)
>   minSize.setWidth(minWidth);
> if (minHeight)
>   minSize.setHeight(minHeight);
> 
> because that's what the code in
> https://git.gnome.org/browse/gtk+/tree/gtk/gtkbuiltinicon.c#n56 essentially
> does.

Which is exactly what the patch does.
Comment 6 Carlos Garcia Campos 2016-02-09 22:58:54 PST
Committed r196364: <http://trac.webkit.org/changeset/196364>