Summary: | SVG invalidates too much due to unclipped absoluteClippedOverflowRect values | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Eric Seidel (no email) <eric> | ||||||
Component: | SVG | Assignee: | Nobody <webkit-unassigned> | ||||||
Status: | RESOLVED DUPLICATE | ||||||||
Severity: | Normal | CC: | dino, emacemac7, krit, mitz, simon.fraser, zimmermann | ||||||
Priority: | P2 | Keywords: | SVGHitList | ||||||
Version: | 523.x (Safari 3) | ||||||||
Hardware: | Mac | ||||||||
OS: | OS X 10.4 | ||||||||
URL: | http://m8y.org/js/svgbounce2.xhtml | ||||||||
Bug Depends on: | 23881 | ||||||||
Bug Blocks: | 14019, 15387, 24627 | ||||||||
Attachments: |
|
Description
Eric Seidel (no email)
2007-06-06 10:19:11 PDT
Created attachment 14881 [details]
test case
attaching test case to bug so it doesn't disappear some day
The top-left most corner of the SVG is invalidated at each refresh. So, the size appears to be correct, but the offset is always 0,0. Now we seem to be invalidating way too much. This is even after my recent fixes for excess invalidation in Sun's Lively Kernel. :( Created attachment 16583 [details]
reduced test
I bet this is a bug in absoluteClippedOverflowRect. I bet it's supposed to to actually clip the return rect to the size of the viewport (size of the <svg>). Otherwise this is exactly the same as moving an absolutely positioned <div> around.
mitz would know the correct behavior of absoluteClippedOverflowRect. Currently RenderSVGRoot::absoluteClippedOverflowRect() returns the union of its kids bboxes. This union is not clipped (as from the method name I would assume it should be). I wonder though if absoluteClippedOverflowRect should ever return a rect smaller than the <svg>s bounds? An object's absoluteClippedOverflowRect should be the smallest rectangle, in absolute coordinates, containing all pixels the object paints into in its paint() method. For your typical RenderBlock it would be its overflow rect (which encompasses overhanging floats' overflow rects, because paint() paints the floats), intersected with the clip rect imposed by its ancestors, if any. In this example, the middle (olive) div's clipped overflow rect is a 100x75 rect: <div style="width: 100px; height: 100px; overflow: hidden;"> <div style="width: 150px; height: 50px; background: olive;"> <div style="width: 50px; height: 75px; background: silver;"> </div> </div> </div> LayoutState is an optional optimization that speeds up the computation of absolute clipped rects during layout. It is arguably easier to read the non-optimized branches of the code to see how things work. I tried just fixing absoluteClippedOverflowRect() on RenderSVGRoot, but that's insufficient. We have to fix it all the way up the chain. What is dirtying way too much is not the root, but rather the children. We need a way to access your parents clip rect as a child. Possibly a clipRect() function or an applyClipToRect(rect) function which can walk up the parent chain. We could also use/abuse the LayoutState optimization if we were careful. *** Bug 15387 has been marked as a duplicate of this bug. *** Ok, the correct fix for this is for SVG renderers to implement: computeAbsoluteRepaintRect And fix all of our uses (like in SVGRenderImage) of RenderObject::repaintRect to use renderer-relative offsets instead of absolute offsets! implementing computeAbsoluteRepaintRect correctly will also fix our partial-repaint issues with text: void RenderSVGText::layout() { ASSERT(needsLayout()); // FIXME: This is a hack to avoid the RenderBlock::layout() partial repainting code which is not (yet) SVG aware setNeedsLayout(true); It might also fix our repaint issues with text when switching from display:none to display: inline and back, not certain on that last one. This is by far the worst bug affecting the Sun Labs Lively Kernel. The original bug seems much better, probably from the rendering tree refactor this spring. We're still invalidating too much for the lively kernel, but I think we need better test cases to show that. *** This bug has been marked as a duplicate of bug 41854 *** |