[Webkit-unassigned] [Bug 45423] [Gtk] Port tiled backing store

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=45423





--- Comment #37 from Joone Hur <joone at kldp.org>  2011-01-29 00:15:12 PST ---
(In reply to comment #33)
> (From update of attachment 80038 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=80038&action=review
>
> Looking good! I think this should go one more round, but it's very close. We'll need another r+ before it can land though (since it includes an API change).

Thanks, Martin for the review. I am applying your suggestions to the patch.
>
> > Source/WebCore/platform/graphics/cairo/OwnPtrCairo.cpp:56
> > +template <> void deleteOwnedPtr<cairo_region_t>(cairo_region_t* ptr)
> > +{
> > +    if (ptr)
> > +        cairo_region_destroy(ptr);
> > +}
>
> Since cairo_region_t is new in 1.10 this needs to be guarded by #if CAIRO_VERSION >= CAIRO_VERSION_ENCODE(1, 10, 0). You may need to provide dummy struct implementation to fulfill the typedef for older Cairo versions. Maybe not, because the template is never instantiated for old versions. I can test that if you need me to.
> 

Right, people can try to build WebKitGtk+ with old cairo and Gtk+2, so it would be great to test this patch with them.

How about checking GTK+ version instead of Cairo version? 
Because cairo_region_t is used for Gtk+3, which is dependent on Cairo 1.10.
http://library.gnome.org/devel/gtk/2.99/ch25s02.html

> > Source/WebCore/platform/graphics/gtk/TileGtk.cpp:52
> > +static cairo_surface_t* checkeredPixmap()
>
> You should change the name of this method now that it no longer returns a pixmap.
>
> > Source/WebCore/platform/graphics/gtk/TileGtk.cpp:149
> > +#ifdef GTK_API_VERSION_2
> > +    int rectCount;
> > +    GOwnPtr<GdkRectangle> rects;
> > +    gdk_region_get_rectangles(m_dirtyRegion.get(), &rects.outPtr(), &rectCount);
> > +    m_dirtyRegion.clear();
> > +    m_dirtyRegion.set(gdk_region_new());
> > +#else
> > +    int rectCount = cairo_region_num_rectangles(m_dirtyRegion.get());
> > +    GOwnPtr<GdkRectangle> rects(g_new(GdkRectangle, rectCount));
> > +    for (int i = 0; i < rectCount; i++)
> > +        cairo_region_get_rectangle(m_dirtyRegion.get(), i, rects.get()+i);
> > +    m_dirtyRegion.clear();
> > +    m_dirtyRegion.set(cairo_region_create());
> > +#endif
> > +
> > +    RefPtr<cairo_t> cr = adoptRef(cairo_create(m_buffer.get()));
> > +    GraphicsContext context(cr.get());
> > +    context.translate(-m_rect.x(), -m_rect.y());
>
> I think it would be better to use a vector of IntRects here. See how it's done in webkit_web_view_expose_event and webkit_web_view_draw. We probably want to isolate the code in the #ifdefs into a static function that just returns a Vector of IntRects.

I am updating the patch as you suggested. This makes the Tile::updateBackBuffer() method simple, but I'm not sure if isolating the code is efficient.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.