[webkit-reviews] review denied: [Bug 162673] [GTK] Cache RenderThemeGadget instances for rendering scrollbars for GTK+ 3.20+ : [Attachment 290082] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed May 24 09:49:38 PDT 2017


Carlos Garcia Campos <cgarcia at igalia.com> has denied Andre Moreira Magalhaes
<andrunko at gmail.com>'s request for review:
Bug 162673: [GTK] Cache RenderThemeGadget instances for rendering scrollbars
for GTK+ 3.20+
https://bugs.webkit.org/show_bug.cgi?id=162673

Attachment 290082: Patch

https://bugs.webkit.org/attachment.cgi?id=290082&action=review




--- Comment #7 from Carlos Garcia Campos <cgarcia at igalia.com> ---
Comment on attachment 290082
  --> https://bugs.webkit.org/attachment.cgi?id=290082
Patch

I've been looking at this in detail. It's tricky, as usual with GTK+ theming.
This approach doesn't really work, mainly because
gtk_style_context_save/restore breaks the gadgets hierarchy when using
GtkWidgetPath. This might not be a problem for some themes, or some widgets
like scrollbars, but will break for sure themes that rely on inheritance when
querying css properties, and styles using  information about siblings like the
combobox. Adding/removing classes to the GtkStyleContext doesn't work when
usingGtkWidgetPath either, classes needs to be present when building the
GtkWidgetPath. So, we can't cache style contexts or gadgets, we need to cache
hierarchies of gadgets with fixed style classes. Fortunately, setting the state
on a GtkStyleContext does work, so we can cache more generic hierarchies and
apply the state later. In the particular case of scrollbars we would cache
VerticalScrollbarRight, VerticalScrollbarLeft, HorizontalScrollbar,
VerticalScrollIndicatorRight, VerticalScrollIndicatorLeft and
HorizontalScrollIndicator. In practice, we will only have 4 of those at the
same time in the cache.


More information about the webkit-reviews mailing list