Closed Bug 327522 Opened 18 years ago Closed 17 years ago

Mac SVG implementation ignores font-weight, font-style on <text>

Categories

(Core :: SVG, defect)

PowerPC
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: pipian, Assigned: brian.ewins)

References

()

Details

Attachments

(7 files, 1 obsolete file)

User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X; en) AppleWebKit/417.9 (KHTML, like Gecko) Safari/417.8
Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8.0.1) Gecko/20060111 Firefox/1.5.0.1

May be related to Bug #311949.  Firefox 1.5.0.1 refuses to display SVG text properly on Mac, consistently bolding and italicising the entire text element, which should only have the tspan element bolded, and nothing italicised (as per SVG spec).

Page renders as expected with Cairo compile of Gentoo ebuild mozilla-firefox-1.5.0.1.

Trunk (as of last nightly) also displays incorrect kerning, in addition to ignoring font-weight and font-style.

Reproducible: Always
Attached image Test-case with trunk as of 20070105 (obsolete) —
Still happening, in an impressively wrong way, with a recent trunk build.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Attachment #251511 - Attachment is obsolete: true
Note that if the  width="10cm" height="3cm" viewBox="0 0 1000 300" attributes are removed from the <svg> element, then the test case renders much better: the only problem is the (unrequested) italic font style, which should probably be broken out into a seperate bug.
The test case I originally uploaded renders correctly using Gran Paradiso Alpha 1 on Windows XP.  However, using build "Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.9a2pre) Gecko/20070122 Minefield/3.0a2pre" the test case I originally uploaded renders the same as it did in Gran Paradiso Alpha 1 (Mac OS X).
Apologies, wrong bug.
With today's trunk, with or without the view box, this is considerably worse.
The 'unrequested italic' should be fixed in cairo 1.3.12:
http://gitweb.freedesktop.org/?p=cairo;a=commit;h=58265f3508959298eabab55ec28dc6d9516eedc3

The other half of the bug is clearly a scaling problem - I can see that the letters are being drawn with the font scaled but not the glyph extents, which is why the glyphs overlap. Thats this bug (with patch):
http://bugs.freedesktop.org/show_bug.cgi?id=9568

That'll be in 1.3.13. I'll take a look into the regression over the weekend.



Just to clarify, with the Mozilla trunk, which includes cairo 1.3.12, the italic problem *is* fixed, as you say. But the glyph extents problem now happens all the time - previously remove the SVG viewBox attribute from the <svg> element of the test case produced a string which was italic but with correct glyph positions.

I'm going to (or you can) apply the patch from Cairo bug 9568 to my local tree, and see what happens, fingers crossed everything will start working.
That might not fix it. I had a look at this morning using the inkscape test pages:
http://tavmjong.free.fr/INKSCAPE/MANUAL/images/TEXT/index.xml

(the original test case isn't atttached here) Something that's obvious from those pages is that rotated text has also regressed. That's a bug in the new nquartz text layout (which I also have a patch for), which bypasses the cairo-atsui code now. The quickest way to get it working should be to apply that patch and change _cairo_nquartz_surface_show_glyphs (in cairo-nquartz-surface.c) to return CAIRO_INT_STATUS_UNSUPPORTED immediately. That should fall back to the atsui code.

That's just a band aid, I'll need to fix this properly in nquartz. 

Just to confuse things further, the title of this bug - (ignoring font properties) - is still a bug. Its a limitation of the toy font API. moz-svg needs to select fonts for itself and use cairo_set_font_face, not cairo_select_font_face, to avoid this problem. I understand that the new text apis do this (but I havent looked into that too much) 
Okay, having tested with the merged mozilla + cairo 1.3.12, I can confirm the patch on freedesktop.org bug 9569 doesn't seem to affect the problem at all - I still see glyphs which basically look like they are being computed with zero x-advance, because no advance is taking place at all. (though each glyph is being rendered okay, so presumably the individual widths are okay?).

I tried this at the weekend and found that I got different results on PPC and Intel. Where I got to is that the cairo_glyph_t structure, which contains the glyphid, x, y, ends up with nonsensical position values on intel (eg 3E-315), and so on. On PPC, things work correctly. The glyphid is unaffected, which suggest that its an endianness problem, not memory scribbling. The x,y values are calculated correctly in the atsui code, the junk is retrieved later the glyph cache, with or without the fix (which is even more bizarre since x/y should not be set again). The regression affects both the image and nquartz surfaces, so it's not just an nquartz regression either.

I'm a bit puzzled as to what could have caused this, unfortunately the test I have that is broken was broken differently in all previous PPC versions - I need to find a test that is broken now but worked in (say) the 1.2 timeframe.

James, it would really help if you posted the svg you're using for this test and the results you're seeing with the current build, I need to confirm that this isn't some artifact I'm getting from recently transferring my PPC profile to Intel. 
Easiest test-case is the W3C example listed in this bug's URL field - I'll attach a screenshot in a bit that shows how it appears with current Mozilla trunk.
The endianess issue might be bug 365339, though I would have expected big endianess platforms to have problems.

Just verified that the 'fix' for 365339 makes no difference to this defect, ick.
Following on from Brian's investigation, I started looking at the rendering code. With some crude instrumentation in _cairo_nquartz_surface_show_glyphs, I looked at the advance values, and they seem plausible to me, on an Intel machine. Possibly somewhat small, but I don't know enough about this code to be aware of the units and other scaling that is going on. Crucially, I'm not seeing any negative / huge / otherwise bogus advance values, which makes me think it's not an endian problem.

Could it actually be a scaling problem in the advance values? For the tspan01.svg example, the 'raw' advance values passed to CGContextShowGlyphsWithAdvances are all in the 3.0 to 7.9 range, but as I said, I have no idea how those related to pixels or any other unit.
Attached image testcase simplified
I stole a mac for five minutes to take a look at the testcase. I've simplified it and here's the result. The interesting observation I've made is that a font-size of 15.4 or less seems to work fine. Increase that to 15.5 or more and suddenly all the letters collapse down so that there's barely any advance between glyphs.

Note that it doesn't seem to matter what the font-size units are, but rather what matters is the actual size the glyphs end up being rendered at. For example the same sudden change exists when changing the font-size from 0.96em to 0.97em.
Looking at the Mozilla archives the build right before the cairo update does not have any problem with the simplified testcase, whereas the build from right after does. I don't see any other checkins in the time between the two builds that could have caused this, so it definitely seems to be a new cairo bug. Hope that helps.
ok, #19 is interesting: I can now reproduce the bug by switching the font size in cairo's text-antialias-none test; the bug happens when you switch up from 15pt to 16pt. Better still, it happens on the image surface, so its not from the quartz->nquartz switch. Its also not endianness, James was right, that turned out to be my mistake.

Should be easier to track down now, this test has always worked on this backend, so I can git-bisect to see where it changed.
Brian, that's great news - let me know if there's anything I can do to help / test.
git-bisect says:

d30b1bf157126668c4309731022b2ded2ad09571 is first bad commit
commit d30b1bf157126668c4309731022b2ded2ad09571
Author: Brian Ewins <Brian.Ewins@gmail.com>
Date:   Thu Jan 4 01:36:32 2007 +0000

Oops, that's me. Before this patch, the code scaled a (1,1) vector by the font
scale, and used the y component of the result as the font size (which makes no
geometric sense). After the patch, the font size is always 1.0 and the real x,y
scale factors were used in the font matrix. The two things that seem most
suspicious are the use of sqrt and the 1.0 font size, but I should be able to
figure this out now.
Sorry this is just a patch against cairo at the moment. If you want a patch for gecko directly, I'd be happy to work one up, but its late right now :), just holler. anyway, this fixes the overlapping text bug on the image backend, and doesnt break any of the other tests.

There are at least 2 other places where the 'font size 1.0' trick gets used (one in nquartz), I'll check those too. But a cleaned up version of this fix will be in cairo trunk tomorrow, and in the 1.4 release.
Brian, fantastic, thanks very much! I'm told by one of my colleges that it works great. Patches against cairo are absolutely fine. In general I think it's best to feed cairo patches into cairo git and have them trickle down to Mozilla. CC'ing vlad and pav in case this fix is of interest to them and they want it now.
Assignee: general → brian.ewins
Attached image 1-4ems
Attached image 1-4emsJpg-Grab
Ems are recommended for accessibility.
the attached file and screen grab demonstrate the effect of changing the font-size.
letter-spacing is the attribute that's awry.
please let me know if this is a separate bug.

cheers
This was fixed by the cairo update in bug 374462. Confirmed fixed in
Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.9a5pre) Gecko/20070527 Minefield/3.0a5pre

Jonathan (#26 #27 #28), that may have been the same bug or a different one, but there was no html attached?
Status: NEW → RESOLVED
Closed: 17 years ago
Depends on: 374462
Resolution: --- → FIXED
#29

Brian, attachment in comment #26 has SVG, #27 is screenshot from earlier version...

ayscs fixed, please keep up the great work! ~:"
Flags: in-testsuite?
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: