Bug 47411 - [GTK] Implement subregion rendering in WebView when using gtk3
Summary: [GTK] Implement subregion rendering in WebView when using gtk3
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P2 Normal
Assignee: Nobody
Depends on:
Reported: 2010-10-08 05:32 PDT by Carlos Garcia Campos
Modified: 2010-10-09 11:36 PDT (History)
5 users (show)

See Also:

Patch to implement subregion rendering with gtk3 (5.71 KB, patch)
2010-10-08 05:37 PDT, Carlos Garcia Campos
mrobinson: review-
Details | Formatted Diff | Diff
Updated patch (9.60 KB, patch)
2010-10-09 09:29 PDT, Carlos Garcia Campos
no flags Details | Formatted Diff | Diff
New patch (9.97 KB, patch)
2010-10-09 11:02 PDT, Carlos Garcia Campos
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Carlos Garcia Campos 2010-10-08 05:32:17 PDT
It was removed with the port to gtk3.
Comment 1 Carlos Garcia Campos 2010-10-08 05:37:00 PDT
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
Comment 2 Martin Robinson 2010-10-08 08:53:23 PDT
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++)
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.
Comment 3 Martin Robinson 2010-10-08 08:54:12 PDT
I should also say that I love the approach this patch takes. :)
Comment 4 Carlos Garcia Campos 2010-10-09 09:29:12 PDT
Created attachment 70352 [details]
Updated patch
Comment 5 Carlos Garcia Campos 2010-10-09 11:02:05 PDT
Created attachment 70356 [details]
New patch

Updated patch, fixes build with gtk2.
Comment 6 Martin Robinson 2010-10-09 11:06:39 PDT
Comment on attachment 70356 [details]
New patch

Great. Thanks!
Comment 7 WebKit Review Bot 2010-10-09 11:18:55 PDT
Attachment 70352 [details] did not build on gtk:
Build output: http://queues.webkit.org/results/4325005
Comment 8 WebKit Commit Bot 2010-10-09 11:36:25 PDT
Comment on attachment 70356 [details]
New patch

Clearing flags on attachment: 70356

Committed r69452: <http://trac.webkit.org/changeset/69452>
Comment 9 WebKit Commit Bot 2010-10-09 11:36:30 PDT
All reviewed patches have been landed.  Closing bug.