[webkit-reviews] review denied: [Bug 75091] [SVG] Text does not show after scripted scaling. : [Attachment 126691] Use CSS smart font minimums for SVG font minimums

Mon Feb 13 08:56:43 PST 2012

Nikolas Zimmermann <zimmermann at kde.org> has denied  review:
Bug 75091: [SVG] Text does not show after scripted scaling.
https://bugs.webkit.org/show_bug.cgi?id=75091

Attachment 126691: Use CSS smart font minimums for SVG font minimums
https://bugs.webkit.org/attachment.cgi?id=126691&action=review

------- Additional Comments from Nikolas Zimmermann <zimmermann at kde.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=126691&action=review

> Source/WebCore/ChangeLog:9
> +	   computeNewScaledFontForStyle did not use smart minimums for fonts,
> +	   resulting in fonts being rendered with very small sizes (eg, 0.001).

Can you elaborate a bit? I'm not sure I understand fully yet.
RenderSVGInlineText should always request "absolute, scaled-up font sizes" in
the on screen coordinate system.
eg for <svg viewBox="0 0 1 1" width="400" height="400"><font size="0.5" it'll
request a font-size with 200px, not with 0.5.

I'm trying to understand why we still get into the case where we request tiny
font sizes like this.

> LayoutTests/svg/text/text-rescale.html:4
> +<!-- Test that a script can change the scale of text from very small to
something visible -->
> +<!-- https://bugs.webkit.org/show_bug.cgi?id=75091 -->

You should include fast/repaint/resources/repaint.js here, and use

> LayoutTests/svg/text/text-rescale.html:6
> +<body>

<body onload="runRepaintTest()"> to guarantee the initial painting happened.

> LayoutTests/svg/text/text-rescale.html:13
> +    function rescale() {

rename this to function repaintTest() { it will then get called by
runRepaintTest, once painting is done.

> LayoutTests/svg/text/text-rescale.html:23
> +	   if (window.layoutTestController)
> +	       layoutTestController.notifyDone();

This can be removed.

> LayoutTests/svg/text/text-rescale.html:31
> +	   layoutTestController.waitUntilDone();
> +    }
> +
> +    setTimeout(function() { rescale(); }, 1);

Ditto.