[webkit-reviews] review requested: [Bug 33150] Do not render the full frame when there is some elements with fixed positioning : [Attachment 50386] Repaint only the invalidated area after scrolling

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Mar 10 02:45:57 PST 2010


Benjamin Poulain <benjamin.poulain at nokia.com> has asked  for review:
Bug 33150: Do not render the full frame when there is some elements with fixed
positioning
https://bugs.webkit.org/show_bug.cgi?id=33150

Attachment 50386: Repaint only the invalidated area after scrolling
https://bugs.webkit.org/attachment.cgi?id=50386&action=review

------- Additional Comments from Benjamin Poulain <benjamin.poulain at nokia.com>
> (From update of attachment 50355 [details])
> > +	     // 2) update the area of fixed objets that has been invalidated
> 
> Typo.  This should be 'objects'.

Fixed :)

> > +	     size_t fixObjectsCount = subRectToUpdate.size();
> > +	     for (size_t i = 0; i < fixObjectsCount; ++i) {
> > +		 IntRect updateRect = subRectToUpdate[i];
> > +		 IntRect scrolledRect = updateRect;
> > +		 scrolledRect.move(scrollDelta);
> > +		 updateRect.unite(scrolledRect);
> > +		 updateRect.intersect(rectToScroll);
> > +		 hostWindow()->invalidateContentsForSlowScroll(updateRect,
false);
> 
> I don't think you want to use 'invalidateContentsForSlowScroll' here. 
Rather,
> I think 'invalidateContentsAndWindow' is the appropriate method call. 
> Otherwise you need to do it synchronously.

I have to admit I don't get the difference between
invalidateContentsForSlowScroll() and invalidateContentsAndWindow(). It seems
specific to the way Windows handle scrolling, I trust you on that one.


> I don't like that 'invalidateContentsForSlowScroll' is being called from a
> method named 'scrollContentsFastPath.'  This is confusing and I wonder if it
> points to a larger issue.  How about if scrollContentsFastPath returned true
if
> it was possible, but false if not?  Then the latter could be invoked.

I agree, it is clearer if scrollContentsFastPath() never invoke the slow path.
I have made the modification you suggested.


More information about the webkit-reviews mailing list