[webkit-reviews] review denied: [Bug 11984] SVG <text> does not calculate the correct absoluteRepaintRect : [Attachment 12319] Final patch.

bugzilla-request-daemon at macosforge.org bugzilla-request-daemon at macosforge.org
Mon Jan 8 18:16:00 PST 2007


Eric Seidel <macdome at opendarwin.org> has denied Eric Seidel
<macdome at opendarwin.org>'s request for review:
Bug 11984: SVG <text> does not calculate the correct absoluteRepaintRect
http://bugs.webkit.org/show_bug.cgi?id=11984

Attachment 12319: Final patch.
http://bugs.webkit.org/attachment.cgi?id=12319&action=edit

------- Additional Comments from Eric Seidel <macdome at opendarwin.org>
This patch has some typos: "layouting" and "childre".

I'm not sure the removal of the hasPercentagValues() is wise.  We'll need to
add something like that back again.  It may be simpler to just fix the existing
optimization.  I'm still not sure what was wrong with it.

I'm confused why there are all these changes:
-      RenderSVGContainer {g} at (7,26) size 420x221
+      RenderSVGContainer {g} at (14,26) size 413x221

I assume they're related to text now having the correct bboxes?  Yet the
SVGText and SVGInlineTExt objects haven't chagned?

	 RenderSVGText {text} at (225,40) size 480x17
	   RenderSVGInlineText {#text} at (-197,-14) size 394x17


Also it looks like SVG text objects dump relative bboxes (relative to what?)
instead of absolute bboxes.  That seems wrong.

This is a big patch.  A good patch.  But being big, makes it a bit unwieldy...
I'm certain this patch would cause a performance regression for resizing the
window with SVGs (suddenly now SVGs will repaint themselves on any window
resize, even if not necessary).

Perhaps we can land a smaller piece of this patch first?  A piece where it's
easier to understand the layout test results changes?  Even something as simple
as just the RenderSVGImage changes might make an easy sub-patch to land first.



More information about the webkit-reviews mailing list