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

[bugzilla-daemon at webkit.org]

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





--- Comment #19 from Joone Hur <joone at kldp.org>  2010-12-28 19:56:29 PST ---
(In reply to comment #18)
> (From update of attachment 77469 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=77469&action=review
> 
> Looks good. There are a few issues that I can see though.
> 
> > WebCore/page/Frame.cpp:865
> > -    unsigned size = paintedArea.size();
> > +    int size = paintedArea.size();
> 
> Is this change related?

No, I just fixed a compiler warning:
../../WebCore/page/Frame.cpp:867: warning: comparison between signed and unsigned integer expressions

There are more compiler warnings in tiled backing store code, so it would be better to file another bug to fix them.

> > WebCore/platform/graphics/gtk/TileGtk.cpp:7
> > +   Redistribution and use in source and binary forms, with or without
> > +   modification, are permitted provided that the following conditions
> > +   are met:
> 
> Nit: do you mind adding the asterisks here so that this license block looks like others in this directory?

Of course not.

> > WebKit/gtk/WebCoreSupport/ChromeClientGtk.cpp:437
> > +    notImplemented();
> 
> Is this really notImplemented() or is it an empty implementation? There's a subtle difference.

QWebKit uses this method to emit the scrollRequested signal. http://trac.webkit.org/changeset/71352
I don't see any case that this callback is called during scrolling in WebKtGtk+, so we need more investigation why this callback is required.
> 
> > WebKit/gtk/webkit/webkitwebsettings.cpp:920
> > +    /**
> > +     * WebKitWebSettings: enable-tiled-backing-store:
> > +     *
> > +     * Enable or disable tiled backing store
> 
> This documentation should be expanded greatly. What is the tiled backing store and why, as an embedder, would I want to turn it on?

Okay, I will add more details.

> > WebKit/gtk/webkit/webkitwebview.cpp:721
> > +        if (frame->tiledBackingStore())
> > +                renderFromTiledBackingStore(frame, &context, clipRect);
> > +        else
> > +#endif
> >              frame->view()->paint(&context, rect);
> 
> The indentation seems incorrect here.

I'll fix it.

Thank you for the review.

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