Bug 40139 – [GTK] Widgets do not support CSS transformations

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Martin Robinson]

GTK+ themes require painting onto a GdkDrawable. In the common case this is the GdkPixmap of the widget window. Because this happens at a lower level than the GraphicsContext, nothing except the widget text honors the transformation matrix.

One approach to fixing this is to detect situations in which the GraphicsContext has a rotational transformation matrix and, in those cases, render onto an intermediary GdxPixbuf surface and then copy that to the graphics context.

[Martin Robinson]

Created attachment 70805 [details]
Patch for this issue

[Martin Robinson]

Comment on attachment 70805 [details]
Patch for this issue

Clearing flags, as I should have another patch coming which takes this idea a bit further.

[Martin Robinson]

Created attachment 71056 [details]
Updated patch for this issue

[WebKit Review Bot]

Attachment 71056 [details] did not build on gtk:
Build output: http://queues.webkit.org/results/4388070

[Martin Robinson]

Created attachment 71083 [details]
Patch against ToT (reason for build failure)

[Alejandro G. Castro]

Comment on attachment 71083 [details]
Patch against ToT (reason for build failure)

Patch looks like a very good idea :).

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

> LayoutTests/ChangeLog:14
> +        * platform/gtk/fast/forms/slider-transformed-expected.txt: Added.

I think this file is not in the patch.

> WebCore/platform/gtk/RenderThemeGtk.cpp:169
> +    , m_themeParts(g_new0(GtkThemeParts, 1))
> +#ifdef GTK_API_VERSION_2
> +    , m_themePartsHaveRGBAColormap(true)
> +#endif
>  {
> -    if (!mozGtkRefCount) {
> -        moz_gtk_init();
>  
> -        // Use the theme parts for the default drawable.
> -        moz_gtk_use_theme_parts(partsForDrawable(0));
> +#ifdef GTK_API_VERSION_2
> +    GdkColormap* colormap = gdk_screen_get_rgba_colormap(gdk_screen_get_default());
> +    if (!colormap) {
> +        m_themePartsHaveRGBAColormap = false;
> +        colormap = gdk_screen_get_default_colormap(gdk_screen_get_default());
>      }
> +    m_themeParts->colormap = colormap;
> +#endif

I was wondering if we could move this code and m_theme attributes to the WidgetRenderingContext object or to the gtkxdrawing. In any case we could leave it for future patches.

> WebCore/platform/gtk/WidgetRenderingContextGtk2.cpp:149
> +    // The origin of the blit is the original target rectangle adjusted for any extra space.
> +    IntPoint targetOrigin(m_targetRect.x() - m_extraSpace.width(),
> +                          m_targetRect.y() - m_extraSpace.height());

I think we can use IntRect and inflate to simplify this code.

> WebCore/platform/gtk/WidgetRenderingContextGtk3.cpp:51
>  

I needed these modifications to compile WidgetRenderingContextGtk3.cpp:


diff --git a/WebCore/platform/gtk/WidgetRenderingContextGtk3.cpp b/WebCore/platform/gtk/WidgetRenderingContextGtk3.cpp
index 693c40e..bd826d9 100644
--- a/WebCore/platform/gtk/WidgetRenderingContextGtk3.cpp
+++ b/WebCore/platform/gtk/WidgetRenderingContextGtk3.cpp
@@ -32,12 +32,12 @@ WidgetRenderingContext::WidgetRenderingContext(GraphicsContext* graphicsContext,
     : m_graphicsContext(graphicsContext)
     , m_targetRect(targetRect)
     , m_paintRect(targetRect)
-    , m_target(context->platformContext())
     , m_hadError(false)
+    , m_target(graphicsContext->platformContext())
 {
 }

-~WidgetRenderingContext::WidgetRenderingContext()
+WidgetRenderingContext::~WidgetRenderingContext()
 {
 }

@@ -47,4 +47,6 @@ bool WidgetRenderingContext::paintMozillaWidget(GtkThemeWidgetType type, GtkWidg
     return !m_hadError;
 }

+}
+
 #endif // !GTK_API_VERSION_2

[Martin Robinson]

Comment on attachment 71083 [details]
Patch against ToT (reason for build failure)

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

>> LayoutTests/ChangeLog:14
>> +        * platform/gtk/fast/forms/slider-transformed-expected.txt: Added.
> 
> I think this file is not in the patch.

It's the third one down on the pretty-diff. :)

>> WebCore/platform/gtk/RenderThemeGtk.cpp:169
>> +#endif
> 
> I was wondering if we could move this code and m_theme attributes to the WidgetRenderingContext object or to the gtkxdrawing. In any case we could leave it for future patches.

It needs to stay here as long as RenderThemeGtk manages widgets itself. Perhaps if we eventually do away with WidgetRenderingContext (when the style changes land in GTK+ for instance) it might make sense to move it.

>> WebCore/platform/gtk/WidgetRenderingContextGtk2.cpp:149
>> +                          m_targetRect.y() - m_extraSpace.height());
> 
> I think we can use IntRect and inflate to simplify this code.

Hrm. Maybe IntPoint targetOrigin = m_targetRect.topLeft - toPoint(m_extraSpace);

>> WebCore/platform/gtk/WidgetRenderingContextGtk3.cpp:51
>>  
> 
> I needed these modifications to compile WidgetRenderingContextGtk3.cpp:
> 
> 
> diff --git a/WebCore/platform/gtk/WidgetRenderingContextGtk3.cpp b/WebCore/platform/gtk/WidgetRenderingContextGtk3.cpp
> index 693c40e..bd826d9 100644
> --- a/WebCore/platform/gtk/WidgetRenderingContextGtk3.cpp
> +++ b/WebCore/platform/gtk/WidgetRenderingContextGtk3.cpp
> @@ -32,12 +32,12 @@ WidgetRenderingContext::WidgetRenderingContext(GraphicsContext* graphicsContext,
>      : m_graphicsContext(graphicsContext)
>      , m_targetRect(targetRect)
>      , m_paintRect(targetRect)
> -    , m_target(context->platformContext())
>      , m_hadError(false)
> +    , m_target(graphicsContext->platformContext())
>  {
>  }
> 
> -~WidgetRenderingContext::WidgetRenderingContext()
> +WidgetRenderingContext::~WidgetRenderingContext()
>  {
>  }
> 
> @@ -47,4 +47,6 @@ bool WidgetRenderingContext::paintMozillaWidget(GtkThemeWidgetType type, GtkWidg
>      return !m_hadError;
>  }
> 
> +}
> +
>  #endif // !GTK_API_VERSION_2

Thanks. I'll post a new patch with these fixes.

[Martin Robinson]

Created attachment 71183 [details]
Patch updated with Alex's suggestions

[Martin Robinson]

Created attachment 72362 [details]
Patch fixing scrollbar rendering issue

[Martin Robinson]

Created attachment 72788 [details]
Patch with slightly updated design

[Martin Robinson]

Created attachment 73519 [details]
Updated Patch

[Martin Robinson]

Based on Xan's comments I've updated this patch:

1. It no longer includes the next patch in the series.
2. m_themeParts is now an inline array of the RenderThemeGtk objects and not a pointer.
3. Small fix for a CSS rotation issue.

[Xan Lopez]

Comment on attachment 73519 [details]
Updated Patch

r=me

[Martin Robinson]

Committed r71791: <http://trac.webkit.org/changeset/71791>

[WebKit Review Bot]

http://trac.webkit.org/changeset/71791 might have broken GTK Linux 64-bit Debug