[Webkit-unassigned] [Bug 45092] [chromium] Accelerated Compositing: screen garbage when scrolling

[bugzilla-daemon at webkit.org]

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





--- Comment #9 from Vangelis Kokkevis <vangelis at chromium.org>  2010-09-03 19:03:22 PST ---
(From update of attachment 66535)
Looks real good!
A couple of high level comments/questions:
1. What is it that prevents us from moving all the dirty and scroll-rect logic down to the compositor?  Is it that the root layer doesn't know how to draw itself?  Because if that's the only reason, it can probably be fixed.
2. Remember that comments need to be complete sentences, starting with a capital letter and ending in a period. It takes some getting used to but really improves consistency in the file. 

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

> WebKit/chromium/src/WebPopupMenuImpl.cpp:158
> +void WebPopupMenuImpl::paint(WebCanvas* canvas, const WebRect& viewport)
nit: Is there a good reason to change the parameter name?  If yes, then you'll probably want to change in the header as well for consistency.

> WebKit/chromium/src/WebViewImpl.cpp:2189
> +// WebViewImpl method
nit: is this comment necessary?

> WebKit/chromium/src/WebViewImpl.cpp:2211
> +    // below emits damage in the space "before" scrolling
Add a period at the end of this sentence and the one below.

> WebKit/chromium/src/WebViewImpl.cpp:2212
> +    // Using layer space for damage rects allows us to intermix invalidates with scrolls
missing period

> WebKit/chromium/src/WebViewImpl.cpp:2243
> +    // TODO: add a smarter damage aggregation logic? Right now, LayerChromium does simple union-ing
nit: In WebKit TODO's are FIXME
Add a period at the end of the sentence.

> WebKit/chromium/src/WebViewImpl.cpp:2252
> +    // so that invalidations and scroll invalidations play well with one-another
Add a period at the end of the sentence.

> WebKit/chromium/src/WebViewImpl.cpp:2266
> +    // TODO: add a smarter damage aggregation logic? Right now, LayerChromium does simple union-ing
Add a period at the end of the sentence.

> WebKit/chromium/src/WebViewImpl.cpp:2307
> +    // Give the compoisitor a chance to setup/resize the root texture handle and perform scrolling
typo: compositor
Add a period at the end of the sentence.

> WebKit/chromium/src/WebViewImpl.cpp:2339
> +    // readback into the canvas, if requested
Capitalize and add a period at the end of the sentence.

> WebKit/chromium/src/WebViewImpl.cpp:2343
> +    // FIXME: handle finish flag
Capitalize and add a period at the end of the sentence.

> WebKit/chromium/src/WebViewImpl.cpp:2345
> +    // we're done... put result onscreen
Capitalize and add a period at the end of the sentence.

> WebKit/chromium/src/WebViewImpl.cpp:2348
> +    // tell the browser that the rendering is done ---
Capitalize and add a period at the end of the sentence.

> WebKit/chromium/src/WebViewImpl.cpp:2349
> +    // TODO: this is being called too early, we need to defer calling this 
TODO->FIXME

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