Bug 14015 – SVG invalidates too much due to unclipped absoluteClippedOverflowRect values

• Status: RESOLVED
• Resolution: DUPLICATE
• Component: SVG

[Eric Seidel (no email)]

invalidation issue when moving SVG via js

Click "bounce", notice that the intial seal position is never re-drawn (or it's continuously redrawn in the wrong spot?)

[Eric Seidel (no email)]

Created attachment 14881 [details]
test case

attaching test case to bug so it doesn't disappear some day

[Alp Toker]

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.

[Eric Seidel (no email)]

Now we seem to be invalidating way too much.  This is even after my recent fixes for excess invalidation in Sun's Lively Kernel. :(

[Eric Seidel (no email)]

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.

[Eric Seidel (no email)]

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.

[Eric Seidel (no email)]

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.

[Eric Seidel (no email)]

*** Bug 15387 has been marked as a duplicate of this bug. ***

[Eric Seidel (no email)]

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.

[Eric Seidel (no email)]

This is by far the worst bug affecting the Sun Labs Lively Kernel.

[Eric Seidel (no email)]

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.

[Nikolas Zimmermann]

*** This bug has been marked as a duplicate of bug 41854 ***