[Webkit-unassigned] [Bug 33150] Do not render the full frame when there is some elements with fixed positioning

[bugzilla-daemon at webkit.org]

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





--- Comment #12 from Kenneth Rohde Christiansen <kenneth at webkit.org>  2010-01-06 20:56:20 PST ---
(From update of attachment 45996)

> +void FrameView::fastScrollContents(const IntSize& scrollDelta, const IntRect& rectToScroll, const IntRect& clipRect)

I dislike the method name. Shouldn't it always be fast? Does FrameView already
have a scrollContents method?

> +{
> +    const int fixedObjectNumberThreshold = 5;
> +
> +    if (m_fixedObjects.isEmpty())
> +        hostWindow()->scroll(scrollDelta, rectToScroll, clipRect);
> +    else {
> +        Vector<RenderObject*> objectsFixedInViewport;

We say fixedObjects elsewhere, right?

> +        objectsFixedInViewport.reserveCapacity(m_fixedObjects.size());
> +
> +        bool updateInvalidSubRect = true;
> +        // Get a list of fixed objects that are not in transformations
> +        HashSet<RenderObject*>::const_iterator end = m_fixedObjects.end();
> +        HashSet<RenderObject*>::const_iterator it = m_fixedObjects.begin();
> +        for (; it != end; ++it) {
> +            RenderObject* obj = *it;
> +            if (obj->containingBlock() == obj->view()) {
> +                objectsFixedInViewport.append(obj);
> +                if (objectsFixedInViewport.size() > fixedObjectNumberThreshold) {
> +                    updateInvalidSubRect = false;
> +                    break;
> +                }
> +            }
> +        }

Instead of the updateInvalidSubRect, couldn't you clear the
objectsFixedInViewport instead? and ask
if (!objectsFixedInViewport.isEmpty()) below? I find that less mysterious.

> +
> +        // scroll the content
> +        if (updateInvalidSubRect) {
> +            // 1) scroll
> +            hostWindow()->scroll(scrollDelta, rectToScroll, clipRect);
> +
> +            // 2) update the area of fixed objets that has been invalidated
> +            for (int i = 0; i < objectsFixedInViewport.size(); ++i) {
> +                IntRect topLevelRect;
> +                IntRect updateRect = (objectsFixedInViewport[i])->paintingRootRect(topLevelRect);

Are the () really needed?

> +                updateRect.move(-scrollX(), -scrollY());
> +                IntRect scrolledRect = updateRect;
> +                scrolledRect.move(scrollDelta);
> +                updateRect.unite(scrolledRect);
> +                updateRect.intersect(rectToScroll);
> +                hostWindow()->repaint(updateRect, true, false, true);
> +            }
> +        } else {
> +            // there are too many fixed objects, repaint everything might be easier

// the number of fixed objects exceed the threshold, so we repaint everything. 

> +            IntRect updateRect = clipRect;
> +            updateRect.intersect(rectToScroll);
> +            hostWindow()->repaint(updateRect, true, false, true);
> +        }
> +    }
> +}
> +

>      if (canBlitOnScroll() && !rootPreventsBlitting()) { // The main frame can just blit the WebView window
> -       // FIXME: Find a way to blit subframes without blitting overlapping content
> -       hostWindow()->scroll(-scrollDelta, scrollViewRect, clipRect);
> +        // FIXME: Find a way to blit subframes without blitting overlapping content
> +        fastScrollContents(-scrollDelta, scrollViewRect, clipRect);

Wrong indentation

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