RESOLVED FIXED 53480
REGRESSION (r76674): SVG + BiDi text broken (text-intro-05-t.svg)
https://bugs.webkit.org/show_bug.cgi?id=53480
Summary REGRESSION (r76674): SVG + BiDi text broken (text-intro-05-t.svg)
Nikolas Zimmermann
Reported 2011-02-01 00:33:57 PST
Between r76727 and r76640, LayoutTests/svg/W3C-SVG-1.1/text-intro-05-t.svg broke. Half of the text is not visible anymore. I suspect a problem in either FontComplexMac.mm or ComplexTextController.cpp. Some things have changed recently, and that probably broke it. Mitz can you have a look?
Attachments
Screenshot (31.03 KB, text/plain)
2011-02-01 00:52 PST, Nikolas Zimmermann
no flags
Use an offset relative to m_characters for m_indexEnd (2.17 KB, patch)
2011-02-07 15:51 PST, mitz
oliver: review+
mitz
Comment 1 2011-02-01 00:40:16 PST
With my build of r77063, run-webkit-tests --pixel svg/W3C-SVG-1.1/text-intro-05-t.svg reports success. I don’t see missing text nor do I see a difference between what the test looks like in Safari with that build and what it looks like in an older build. That is on Snow Leopard, but perhaps you are running the test on Leopard (the combination PC / Mac OS X 10.5 makes no sense). Can you attach screenshots showing the failure?
Nikolas Zimmermann
Comment 2 2011-02-01 00:52:48 PST
Created attachment 80726 [details] Screenshot
Nikolas Zimmermann
Comment 3 2011-02-01 00:53:26 PST
(In reply to comment #1) > With my build of r77063, run-webkit-tests --pixel svg/W3C-SVG-1.1/text-intro-05-t.svg reports success. I don’t see missing text nor do I see a difference between what the test looks like in Safari with that build and what it looks like in an older build. That is on Snow Leopard, but perhaps you are running the test on Leopard (the combination PC / Mac OS X 10.5 makes no sense). > > Can you attach screenshots showing the failure? Yes I'm testing on Leopard. I think 'startX' has changed in Font::drawComplexText, lemme trace a bit further.
Nikolas Zimmermann
Comment 4 2011-02-01 00:57:55 PST
Reduction: <?xml version="1.0" encoding="UTF-8"?> <svg xmlns="http://www.w3.org/2000/svg"> <text x="10" y="60" font-size="50" font-family="Andalus">لماذا لا يتكلمون اللّغة العربية فحسب؟</text> </svg> Font::drawComplexText() used to produce a 'startX' value of 10. <rect x="10" y="14" width="571" height="59" fill-opacity="0.3" fill="navy"/> 88 float startX = point.x() + getGlyphsAndAdvancesForComplexText(run, from, to, glyphBuffer); Value returned is $1 = 10 point.x() is 10. 94 // Draw the glyph buffer now at the starting point returned in startX. 95 FloatPoint startPoint(startX, point.y()); 96 drawGlyphBuffer(context, glyphBuffer, startPoint); (gdb) p startPoint $5 = { m_x = 267.5, m_y = 60 } It used to report just 10 here, IIRC. The correct bounds of this text are: x=10 y=14 w=571 h=59. startX is 267.5 now, this is the regression.
mitz
Comment 5 2011-02-01 01:01:24 PST
Possibly fallout from the ATSUI side of http://trac.webkit.org/changeset/76674
Nikolas Zimmermann
Comment 6 2011-02-01 01:08:27 PST
Hm, just noticed that the Chromium started skipping this test in r76678.
mitz
Comment 7 2011-02-01 08:44:31 PST
Please stop setting the platform to PC and the priority to P2.
Nikolas Zimmermann
Comment 8 2011-02-01 09:46:12 PST
(In reply to comment #7) > Please stop setting the platform to PC and the priority to P2. I'm not modifying the platform and priority? PC is the default for me, when reporting bugs - as I've just noticed. (see https://bugs.webkit.org/enter_bug.cgi?product=WebKit using a Leopard machine)
mitz
Comment 9 2011-02-01 10:02:51 PST
https://bugs.webkit.org/show_activity.cgi?id=53480 shows you changing it back to PC twice after I’ve changed it to Macintosh (and also resetting priority and removing keywords). Perhaps you were submitting a stale form, in which case Bugzilla should have warned you. If it didn’t, then it’s a Bugzilla bug.
WebKit Review Bot
Comment 10 2011-02-01 11:37:36 PST
Attachment 80726 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files']" exit_code: 1 Total errors found: 0 in 0 files If any of these errors are false positives, please file a bug against check-webkit-style.
mitz
Comment 11 2011-02-01 16:00:39 PST
mitz
Comment 12 2011-02-01 16:59:54 PST
This doesn’t look like it was caused by r76674.
Yair Yogev
Comment 13 2011-02-02 11:34:36 PST
is this related to the regression described here: http://code.google.com/p/chromium/issues/detail?id=71131 ? i was just about to post about it in BugZilla, i think that one regressed since http://trac.webkit.org/changeset/68976/
mitz
Comment 14 2011-02-07 15:51:46 PST
Created attachment 81544 [details] Use an offset relative to m_characters for m_indexEnd
Oliver Hunt
Comment 15 2011-02-07 15:59:47 PST
Comment on attachment 81544 [details] Use an offset relative to m_characters for m_indexEnd View in context: https://bugs.webkit.org/attachment.cgi?id=81544&action=review r=me, but remove the non-ascii character from the changelog > Source/WebCore/ChangeLog:11 > + offset into m_characters, not into the runâs characters. interesting non-ascii character?
mitz
Comment 16 2011-02-07 16:08:56 PST
Note You need to log in before you can comment on or make changes to this bug.