[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


Benjamin Poulain <benjamin.poulain at nokia.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #47622|0                           |1
        is obsolete|                            |
  Attachment #47622|commit-queue?               |
               Flag|                            |
  Attachment #47844|                            |review?, commit-queue?
               Flag|                            |




--- Comment #63 from Benjamin Poulain <benjamin.poulain at nokia.com>  2010-02-01 07:55:57 PST ---
Created an attachment (id=47844)
 --> (https://bugs.webkit.org/attachment.cgi?id=47844)
Repaint only the invalidated area after scrolling

Updated the patch thanks to Dave Hyatt's comments:

(In reply to comment #62)
> (1) You don't really need a hash of RenderObjects on FrameView.  A count is
> sufficient.  Then you can just walk the m_positionedObjects list of the
> RenderView and if the RenderBox is fixed positioned, it's one you care about.
> (This also bypasses the need to do any transform checking, since you'll only be
> in the RenderView list if your containing block was the RenderView).

I have changed the code to use the list contained in RenderBlock. 
I like that lot better than what I had done previously.
To access it from RenderView, I had to change: 
-    void insertPositionedObject(RenderBox*);
-    void removePositionedObject(RenderBox*);
+    virtual void insertPositionedObject(RenderBox*);
+    virtual void removePositionedObject(RenderBox*);
     void removePositionedObjects(RenderBlock*);
+    ListHashSet<RenderBox*>* positionedObjects() const { return
m_positionedObjects; }


> (2) Fixed positioned objects are always RenderBoxes, so you need to tighten up
> the code here. You can rename your methods to refer to use FixedPositionedBox
> rather than FixedPositionedObject.
> (3) You've duplicated code in RenderObject::destroy() and
> RenderWidget::destroy().  If you add a helper function to RenderBox, e.g.,
> unregisterFixedPositionedBox, then RenderWidget could call it in its destroy
> method.  The code in RenderObject should be moved to RenderBox.

I have moved the (un)registration to 
RenderView::insertPositionedObject()
RenderView::removePositionedObject()

So the problem of the transformations disappears.


> (3) All the code in RenderObject::styleWillChange is misplaced, since only
> RenderBoxes can be fixed positioned.  You should move the code into RenderBox.
> (Yes, some of the code in there was already misplaced, but by adding a big new
> pile of code, you've made the problem worse. :)

Right. Fixed :)

> (4) I dislike "scrollContentsFastPath" when we know what we mean is blitting.
> scrollContentsWithBlit would be my suggestion, although I don't like my own
> choice of name much better.

I have not changed the name yet. I don't know about putting "blit" in the name
because we don't blit if we reach the threshold.
What about scrollContentsWithBlitIfPossible(), tryScrollContentsWithBlit()? ;)

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