Bug 14015 - SVG invalidates too much due to unclipped absoluteClippedOverflowRect values
Summary: SVG invalidates too much due to unclipped absoluteClippedOverflowRect values
Status: RESOLVED DUPLICATE of bug 41854
Alias: None
Product: WebKit
Classification: Unclassified
Component: SVG (show other bugs)
Version: 523.x (Safari 3)
Hardware: Mac OS X 10.4
: P2 Normal
Assignee: Nobody
URL: http://m8y.org/js/svgbounce2.xhtml
Keywords: SVGHitList
: 15387 (view as bug list)
Depends on: 23881
Blocks: 14019 15387 24627
  Show dependency treegraph
 
Reported: 2007-06-06 10:19 PDT by Eric Seidel (no email)
Modified: 2010-07-08 23:56 PDT (History)
6 users (show)

See Also:


Attachments
test case (46.07 KB, application/xhtml+xml)
2007-06-06 10:22 PDT, Eric Seidel (no email)
no flags Details
reduced test (3.78 KB, image/svg+xml)
2007-10-07 18:35 PDT, Eric Seidel (no email)
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Eric Seidel (no email) 2007-06-06 10:19:11 PDT
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?)
Comment 1 Eric Seidel (no email) 2007-06-06 10:22:24 PDT
Created attachment 14881 [details]
test case

attaching test case to bug so it doesn't disappear some day
Comment 2 Alp Toker 2007-06-06 10:41:43 PDT
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.
Comment 3 Eric Seidel (no email) 2007-10-06 20:22:50 PDT
Now we seem to be invalidating way too much.  This is even after my recent fixes for excess invalidation in Sun's Lively Kernel. :(
Comment 4 Eric Seidel (no email) 2007-10-07 18:35:06 PDT
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.
Comment 5 Eric Seidel (no email) 2007-10-07 18:43:57 PDT
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?
Comment 6 mitz 2007-10-08 00:34:37 PDT
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.
Comment 7 Eric Seidel (no email) 2007-10-09 00:24:13 PDT
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.
Comment 8 Eric Seidel (no email) 2007-10-12 02:18:42 PDT
*** Bug 15387 has been marked as a duplicate of this bug. ***
Comment 9 Eric Seidel (no email) 2007-10-12 02:22:02 PDT
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.
Comment 10 Eric Seidel (no email) 2007-12-15 04:06:51 PST
This is by far the worst bug affecting the Sun Labs Lively Kernel.
Comment 11 Eric Seidel (no email) 2009-10-07 13:34:29 PDT
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.
Comment 12 Nikolas Zimmermann 2010-07-08 23:56:42 PDT

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