|Summary:||small text which is scaled to be large renders pixelated|
|Product:||WebKit||Reporter:||Andreas Neumann <a.neumann>|
|Component:||SVG||Assignee:||Nikolas Zimmermann <zimmermann>|
|Severity:||Normal||CC:||abarth, ap, aroben, baker, bugzilla, ctruta, dglazkov, edoardo, eric, jeffschiller, krit, marcus.erronius, mitz, nickshanks, ossy, rhodovan.u-szeged, simon.fraser, webkit-ews, webkit.review.bot, zherczeg, zimmermann|
|OS:||OS X 10.4|
Description Andreas Neumann 2007-01-28 04:38:24 PST
In the provided example the transformed text looks very ugly. It seems like no or ugly antialiasing is used. Here is the same example but with animation (scripting): http://www.carto.net/neumann/webkitsvgbugs/Richard4Suzanne.svg This renders fine in Batik and Opera. Firefox seems to have problems as well. Thanks for checking on this!
Comment 1 Eric Seidel (no email) 2007-01-30 22:39:35 PST
I wonder if CG is doing this automatically.
Comment 2 Eric Seidel (no email) 2007-01-30 22:44:22 PST
Mac OS X will turn off antialiasing in text which is below a specific size. In this case, font-size='4' on the heart text is causing the text to lose its antialiasing. I'm not sure how we'd fix this. You really want that system setting to affect the final rendered size, not the intermediate size.
Comment 4 Andreas Neumann 2007-02-05 01:27:30 PST
(In reply to comment #2) > Mac OS X will turn off antialiasing in text which is below a specific size. In > this case, font-size='4' on the heart text is causing the text to lose its > antialiasing. > > I'm not sure how we'd fix this. You really want that system setting to affect > the final rendered size, not the intermediate size. > ah, that explains it. So we need some extra intelligence in text rendering, checking the final rendered size in device pixels ...
Comment 5 Eric Seidel (no email) 2007-06-12 10:14:38 PDT
One could argue this is a CG bug. CG could be smarter about the "final rendered size". Maybe it's possible for us to add those smarts to the way we call into CG text rendering.
Comment 6 Mark Hubbart 2007-07-20 01:30:25 PDT
(In reply to comment #3) > See also: bug 4842 (duplicate?). This appears to be somewhat linked to bug 14242 as far as I can tell. There may be one fix for the two issues here: 1. Anti-aliasing is based on the specified point size, rather than the final display size; 2. Point size is rounded to the nearest integer value, regardless of possible later scaling. I'll upload a testcase...
Comment 7 Mark Hubbart 2007-07-20 01:47:16 PDT
Created attachment 15595 [details] Both lines of text should render exactly the same. In this test, both lines of text should be rendered exactly the same. The SVG markup is pretty self explanatory; line one starts at 10pt text with no scaling, and line two starts with 1pt text with 10x scaling.
Comment 8 Nicholas Shanks 2008-03-25 05:27:42 PDT
I hit both of the bugs Mark refers to in comment #6 in my own SVG work. It's very frustrating. Scaling can also expose these problems when zooming the document (Command-Plus/Minus).
Comment 10 Renata Hodovan 2010-09-02 02:58:29 PDT
Created attachment 66342 [details] Better scaling for small fonts
Comment 11 Eric Seidel (no email) 2010-09-15 18:23:16 PDT
Comment on attachment 66342 [details] Better scaling for small fonts View in context: https://bugs.webkit.org/attachment.cgi?id=66342&action=prettypatch I'm confused why Qt doesn't do this all for you. How do other platforms avoid needing this code? Why is CoreGraphics (mac) smart enough not to need it? Or does it have it and I just don't realize? > WebCore/ChangeLog:48887 > - (WebCore::HTMLTreeBuilder::insertComment): > + (WebCore::HTMLTreeBuilder::insertComment): > - Eliminate one unnecessary ref/deref pair. Huh? > WebCore/platform/graphics/qt/FontQt.cpp:223 > + scale = sqrtf(trans.c() * trans.c() + trans.d() * trans.d()); I guess no inline sqr() function. ;) Does this operation have a name? One which AffineTransform should support for you?
Comment 12 Alexey Proskuryakov 2010-09-16 08:20:55 PDT
This was originally reported about Mac OS X. We shouldn't making a Qt-only fix in this bug.
Comment 13 Nikolas Zimmermann 2010-09-16 12:21:02 PDT
Comment on attachment 66342 [details] Better scaling for small fonts I've discussed this with Reni already, forgot to mark the patch r-. It needs to be solved cross-platform, but it's very hard to do that with the current design of the SVG text layout engine. I'm rewriting it at the moment, please stay tuned.
Comment 14 Nikolas Zimmermann 2011-02-01 02:30:23 PST
New patch coming soon.
Comment 15 Nikolas Zimmermann 2011-02-01 04:02:29 PST
Created attachment 80736 [details] Patch v1 Incorporated the nice testcase in my patch. This should finally fix all text problem with small font sizes on all platforms, at least that's the goal.
Comment 16 Nikolas Zimmermann 2011-02-01 04:02:33 PST
*** Bug 14242 has been marked as a duplicate of this bug. ***
Comment 17 Nikolas Zimmermann 2011-02-01 04:02:35 PST
*** Bug 17053 has been marked as a duplicate of this bug. ***
Comment 18 Nikolas Zimmermann 2011-02-01 04:02:38 PST
*** Bug 19393 has been marked as a duplicate of this bug. ***
Comment 19 Nikolas Zimmermann 2011-02-01 04:02:40 PST
*** Bug 20192 has been marked as a duplicate of this bug. ***
Comment 20 Nikolas Zimmermann 2011-02-01 04:02:43 PST
*** Bug 21774 has been marked as a duplicate of this bug. ***
Comment 21 Nikolas Zimmermann 2011-02-01 04:02:48 PST
*** Bug 49846 has been marked as a duplicate of this bug. ***
Comment 22 Nikolas Zimmermann 2011-02-01 04:02:49 PST
*** Bug 50313 has been marked as a duplicate of this bug. ***
Comment 23 Nikolas Zimmermann 2011-02-01 04:02:52 PST
*** Bug 50528 has been marked as a duplicate of this bug. ***
Comment 24 Nikolas Zimmermann 2011-02-01 04:03:52 PST
Comment on attachment 80736 [details] Patch v1 Forgot to set r?
Comment 25 Nikolas Zimmermann 2011-02-01 04:05:56 PST
I left out all LayoutTests/ changes, as it rebases most pixel test results with text, which is huge.
Comment 26 WebKit Review Bot 2011-02-01 04:12:16 PST
Attachment 80736 [details] did not build on chromium: Build output: http://queues.webkit.org/results/7682620
Comment 27 Early Warning System Bot 2011-02-01 04:19:51 PST
Attachment 80736 [details] did not build on qt: Build output: http://queues.webkit.org/results/7500403
Comment 28 Dirk Schulze 2011-02-01 04:41:31 PST
Comment on attachment 80736 [details] Patch v1 View in context: https://bugs.webkit.org/attachment.cgi?id=80736&action=review The patch itself looks good. Please fix the builds as well as the noted issues and upload at least the pixel/DRT tests + results of the new tests. > Source/WebCore/rendering/svg/SVGInlineTextBox.cpp:153 > + // If the absolute font size on screen is below x=0.5, 't render anything. s/'t/don't/ > Source/WebCore/svg/SVGFont.cpp:258 > + ASSERT(renderObject->parent()); You call renderObject->parent() multiple times (you use it at least 3 times for debug build) can save the pointer please? > Source/WebCore/svg/SVGFont.cpp:412 > + ASSERT(renderObject->parent()); ditto. > Source/WebCore/svg/SVGFont.cpp:502 > + ASSERT(renderObject->parent()); ditto. > Source/WebCore/svg/SVGFont.cpp:547 > + ASSERT(renderObject); you asserted this at the beginning already.It might be unnecessary if you store the pointer in a local variable before entering this condition.
Comment 29 WebKit Review Bot 2011-02-01 04:42:16 PST
Attachment 80736 [details] did not build on chromium: Build output: http://queues.webkit.org/results/7685531
Comment 30 Nikolas Zimmermann 2011-02-01 05:04:59 PST
Created attachment 80741 [details] Patch v2 Fix Dirks issues, fix release builds, include some of the layout tests changes (already almost 2mb).
Comment 31 Dirk Schulze 2011-02-01 05:35:13 PST
Comment on attachment 80741 [details] Patch v2 View in context: https://bugs.webkit.org/attachment.cgi?id=80741&action=review > LayoutTests/ChangeLog:18 > + Since we switched to use floating-point precision metrics, the actual drawn selection may exceed the red rectangle to up to 1px. a 'to' too much > LayoutTests/ChangeLog:21 > + In theory the red re sentence is incomplete. > LayoutTests/ChangeLog:22 > + The red rectangle represents the results of the text query, using getExtentOfChar(), which gives the precise text metrics using floating-point accurancy. > + In theory the red re > + The selection rects are aligned on integer boundaries. remove these lines completely > LayoutTests/ChangeLog:27 > + - svg/text/selection-(doubleclick|tripleclick).svg: > + These tests use DRTs dumpSelectionRect() which draws a red rectangle, highlighting the SelectionController::bounds() obtained through RenderView. > + In theory this red rectangle, should be equal to the drawn blue selection rect. Again it isn't, as the bounds() methods uses selectionRectForRepaint() > + of RenderText, which returns an enclosingIntRect of the actual selection. The drawn selection starts at y=0.93 in the first testcase, the selectionRectForRepaint() You should concentrate on the facts, you already wrote about the red rects before :-/ > LayoutTests/ChangeLog:33 > + the integer variants ascent/descent/height - hopefully we'll see consistent results across platforms for these tests now, if not please comment in the bug report. There are some minor misspellings, talked on IRC about it. Won't mention these mistakes here. You're talking about a difference of <1px, but we can see more pixel differences in some cases. We talked about the reason on IRC. But please add a short note about the scaling of hinting that is responsible for this as well.
Comment 32 WebKit Review Bot 2011-02-01 11:41:39 PST
Attachment 80736 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1 Source/WebCore/rendering/svg/RenderSVGInlineText.cpp:36: Alphabetical sorting problem. [build/include_order]  Total errors found: 1 in 12 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 33 WebKit Review Bot 2011-02-01 11:45:06 PST
Attachment 80741 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/css3..." exit_code: 1 Source/WebCore/rendering/svg/RenderSVGInlineText.cpp:36: Alphabetical sorting problem. [build/include_order]  Total errors found: 1 in 127 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 34 Nikolas Zimmermann 2011-02-03 06:48:41 PST
Landed in r77485, pixel tests still to be updated.
Comment 35 Andreas Neumann 2011-02-03 06:55:17 PST
Hey - congratulations on fixing this bug - right in time for Valentines day ;-) The bug report was made a long time ago (2007). Good to see it wasn't buried and is fixed now! Andreas
Comment 36 WebKit Review Bot 2011-02-03 07:28:06 PST
http://trac.webkit.org/changeset/77485 might have broken Qt Linux Release
Comment 37 Nikolas Zimmermann 2011-02-03 08:59:32 PST
Sorry, it's taking ages, I'm trying to land mac pixel test results since a while, but it keeps gettin stuck, I'm splitting up in even smaller chunks at the moment.
Comment 38 Nikolas Zimmermann 2011-02-03 09:43:46 PST
Comment 39 Csaba Osztrogonác 2011-02-03 09:56:26 PST
Unfortunately we have 2 regressions on Qt: http://build.webkit.org/results/Qt%20Linux%20Release/r77501%20(27628)/css3/zoom-coords-pretty-diff.html http://build.webkit.org/results/Qt%20Linux%20Release/r77501%20(27628)/svg/zoom/page/zoom-zoom-coords-pretty-diff.html Could you check it please?
Comment 40 Csaba Osztrogonác 2011-02-03 15:54:55 PST
(In reply to comment #39) > Unfortunately we have 2 regressions on Qt: > http://build.webkit.org/results/Qt%20Linux%20Release/r77501%20(27628)/css3/zoom-coords-pretty-diff.html > http://build.webkit.org/results/Qt%20Linux%20Release/r77501%20(27628)/svg/zoom/page/zoom-zoom-coords-pretty-diff.html > > Could you check it please? I skipped them until fix to make buildbot happy: http://trac.webkit.org/changeset/77547
Comment 41 Nikolas Zimmermann 2011-02-04 02:32:08 PST
Dimitri, thanks a lot for the help with the rebaselines! Unfortunately, I couldn't land any chunk with more than 10+ pngs at once, even that sometimes got stuck - I guess my internet connection had problems yesterday, appologizes again, that it took so long to update the pixel baseline :/
Comment 42 WebKit Review Bot 2011-02-07 05:25:34 PST
http://trac.webkit.org/changeset/77804 might have broken Qt Linux Release
Comment 43 Csaba Osztrogonác 2011-02-07 05:32:57 PST
(In reply to comment #42) > http://trac.webkit.org/changeset/77804 might have broken Qt Linux Release It is a flakey crash, sorry for the SPAM.
Comment 44 WebKit Review Bot 2011-02-07 05:34:08 PST
http://trac.webkit.org/changeset/77805 might have broken Qt Linux Release
Comment 45 WebKit Review Bot 2011-02-07 05:34:14 PST
http://trac.webkit.org/changeset/77806 might have broken Qt Linux Release
Comment 46 Eric Seidel (no email) 2011-02-07 12:45:44 PST
@ossy: Do we have a bug on the flaky crash?
Comment 47 Csaba Osztrogonác 2011-02-07 14:41:19 PST
(In reply to comment #46) > @ossy: Do we have a bug on the flaky crash? not yet :( I haven't found the exact root of the problem, but I suspect this flakiness was caused by GC related changes at last week. I'm going to file a bug about it as soon as possible.
Comment 48 Csaba Osztrogonác 2011-02-07 14:50:32 PST
(In reply to comment #46) > @ossy: Do we have a bug on the flaky crash? It's too late for me today. :( We do have bug for it: https://bugs.webkit.org/show_bug.cgi?id=53912 I'll add some usable comment and try to make buildbot happier.
Comment 49 Simon Fraser (smfr) 2011-02-09 08:36:21 PST
svg/text/font-size-below-point-five.svg contains bad English: "Font size should decrease monotonic". It's also failing on Leopard: http://build.webkit.org/results/Leopard%20Intel%20Debug%20(Tests)/r78055%20(27056)/results.html
Comment 50 Dominik Röttsches (drott) 2012-06-06 02:51:52 PDT
Comment on attachment 80741 [details] Patch v2 View in context: https://bugs.webkit.org/attachment.cgi?id=80741&action=review > Source/WebCore/rendering/svg/SVGInlineTextBox.cpp:593 > + context->scale(FloatSize(1 / scalingFactor, 1 / scalingFactor)); Unfortunately, temporarily changing the scale/transform of the context breaks using gradient fills for text. The gradients turn out incorrectly scaled. Ideas how to fix this are welcome on bug 64966.