Bug 47411 – [GTK] Implement subregion rendering in WebView when using gtk3

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

[Carlos Garcia Campos]

It was removed with the port to gtk3.

[Carlos Garcia Campos]

Created attachment 70230 [details]
Patch to implement subregion rendering with gtk3

We still can get the region in gtk3 with cairo_copy_clip_rectangle_list(). I've also moved the common code to a new method webkit_web_view_paint() used by both gtk2 and gtk3 code

[Martin Robinson]

Comment on attachment 70230 [details]
Patch to implement subregion rendering with gtk3

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

> WebKit/gtk/webkit/webkitwebview.cpp:495
> +static bool shouldCoalesce(GdkRectangle *rect, GdkRectangle* rects, int count)

Need to slide the asterisk on *rect over to GdkRectangle.

> WebKit/gtk/webkit/webkitwebview.cpp:515
> +static void webkit_web_view_paint(Frame* frame, gboolean transparent, GraphicsContext *ctx, GdkRectangle* clipRect, GdkRectangle* rects, int rectCount)

The method name should follow WebKit style, so camelCase. It's not a GTK+ method, so perhaps the verb should come first? paintWebView maybe.

Also need to fix the asterisk on GraphicsContext *ctx. In fact, it probably makes sense to just pass a reference here. const GraphicsContext& context. We try to avoid abbreviating context in new code.

> WebKit/gtk/webkit/webkitwebview.cpp:595
> +        if (!rectList->status && rectList->num_rectangles > 0) {
> +            GOwnPtr<GdkRectangle> rects(g_new(GdkRectangle, rectList->num_rectangles));
> +            for (int i = 0; i < rectList->num_rectangles; i++) {
> +                cairo_rectangle_t cairoRect = rectList->rectangles[i];
> +                rects.get()[i].x = static_cast<int>(cairoRect.x);
> +                rects.get()[i].y = static_cast<int>(cairoRect.y);
> +                rects.get()[i].width = static_cast<int>(cairoRect.width);
> +                rects.get()[i].height = static_cast<int>(cairoRect.height);
> +            }

Instead of converting to GdkRectangle and then paintWebView converting them to IntRects, why not just convert straight to IntRect and pass a Vector of them. The code would end up looking something like this:

Vector<IntRect> rects;
if (!rectList->status && rectList->num_rectangles > 0) {
    for (int = 0; i < rectList->num_rectangles; i++)
        rects.append(IntRect(rectList->rectangles[i]));
}
paintWebView(frame, priv->transparent, context, &clipRect, rects);

paintWebView would, of course, take a Vector<IntRect>& rects. You would need to update shouldCoalesce as well to take an IntRect and a Vector<IntRect>&. The Windows port actually does something very similar to this in getUpdateRects in WebKit/win/WebView.cpp. Maybe check that out.

[Martin Robinson]

I should also say that I love the approach this patch takes. :)

[Carlos Garcia Campos]

Created attachment 70352 [details]
Updated patch

[Carlos Garcia Campos]

Created attachment 70356 [details]
New patch

Updated patch, fixes build with gtk2.

[Martin Robinson]

Comment on attachment 70356 [details]
New patch

Great. Thanks!

[WebKit Review Bot]

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

[WebKit Commit Bot]

Comment on attachment 70356 [details]
New patch

Clearing flags on attachment: 70356

Committed r69452: <http://trac.webkit.org/changeset/69452>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.