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

[bugzilla-daemon at webkit.org]

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


zaheer <zaheer.mot at gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |zaheer.mot at gmail.com




--- Comment #5 from zaheer <zaheer.mot at gmail.com>  2010-09-22 06:59:45 PST ---
I work with julien, providing the updated patch and comments.

(In reply to comment #4)
> (From update of attachment 68178 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=68178&action=review
> 
> This patch looks good, in general, but I think I'd rather see this implemented in terms of the Cairo API as much as possible. We really want to avoid GdkPixmap/GdkPixbuf as much as possible and especially deprecated APIs like GdkRegion.
> 
> > WebCore/platform/graphics/Tile.h:82
> > +    GdkPixmap* m_buffer;
> > +    GdkRegion* m_dirtyRegion;
> 
> Instead of using GdkPixmap, I'd like to see a GRefPtr<cairo_surface_t> here. GdkRegion is deprecated in recent GDK releases (replaced by cairo regions), so you'll either have to figure out some version independent method or use #ifdefs.
Done. used ifdefs for region (note cairo_region is not tested as its not available on my mc)
> 
> > WebCore/platform/graphics/gtk/TileGtk.cpp:61
> > +        Color color1(checkerColor1);
> > +        Color color2(checkerColor2);
> > +        float red1, green1, blue1, alpha1;
> > +        float red2, green2, blue2, alpha2;
> > +        color1.getRGBA(red1, green1, blue1, alpha1);
> > +        color2.getRGBA(red2, green2, blue2, alpha2);
> > +        for (unsigned y = 0; y < checkerSize; y += checkerSize / 2) {
> > +            bool alternate = y % checkerSize;
> > +            for (unsigned x = 0; x < checkerSize; x += checkerSize / 2) {
> > +                cairo_rectangle(cr, x, y, checkerSize / 2, checkerSize /2);
> > +                if (alternate)
> > +                    cairo_set_source_rgb(cr, red1, green1, blue1);
> > +                else
> > +                    cairo_set_source_rgb(cr, red2, green2, blue2);
> > +                cairo_fill(cr);
> > +                alternate = !alternate;
> 
> I think it might be cleaner to use the stippling approach that Benjamin Otte oulined here: http://blogs.gnome.org/otte/2010/07/27/rendering-cleanup/
Done. pixmap to cairo_pattern_t. iam not entirely sure if a client side image would be slower than the pixmap.
> 
> > WebCore/platform/graphics/gtk/TileGtk.cpp:119
> > +        if (!m_buffer) {
> > +            m_backBuffer = gdk_pixmap_new(0,
> > +                               m_backingStore->m_tileSize.width(),
> > +                               m_backingStore->m_tileSize.height(),
> > +                               gdk_visual_get_depth(gdk_visual_get_system()));
> > +        } else {
> 
> WebKit code style requires many block types with only one statement to omit the curly braces.
Removed the backbuffer as its not currently used.
> 
> > WebCore/platform/graphics/gtk/TileGtk.cpp:154
> > +    if (m_buffer)
> > +        g_object_unref(m_buffer);
> 
> The use of GRefPtr would elminate the need to do this.
Done
> 
> > WebCore/platform/graphics/gtk/TileGtk.cpp:170
> > +                   target.height());
> > +
> 
> Line lengths in the project can be ~120+ characters. So this should be:
>     IntRect source((target.x() - m_rect.x()), (target.y() - m_rect.y()), target.height());
Done
> 
> > WebCore/platform/graphics/gtk/TileGtk.cpp:178
> > +    cairo_t* cairoContextSource = gdk_cairo_create(m_buffer);
> > +    cairo_set_source_surface(
> > +        cairoContext,
> > +        cairo_get_target(cairoContextSource),
> > +        target.x() - source.x(),
> > +        target.y() - source.y());
> 
> Using a Cairo surface as the buffer will eliminate some of this and allow you do just do something like:
> cairo_set_source_surface(cairoContxt, m_buffer.get(), target.x() - source.x(), target.y() - source.y());
Done

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