The current implementation of letter spacing, word spacing and justification calls GetWidth() and then DrawString() for each character separately, which gives very inaccurate results in complex text. I know that we plan to have a more comprehensive solution in Thebes, but in the meantime rendering would get a lot better (and I expect that performance would improve also) if we could measure the whole text in one call to an API which returned an array of glyph widths mapped to the code points of the input string and then added the extra spacing in a script-sensitive fashion. It turns out that on Windows XP this can be done with very little effort. I am still investigating possibilities for other platforms.

This is not remotely near finished, or worth anyone's time to look at. I'm just attaching it as a checkpoint.

I don't want to start marking dependencies just yet, but here are a few bugs that are fixed by this (on XP with complex text support): Bug 60546 Bug 127661 Bug 185600 Bug 202351 Bug 240914 Bug 248361 Bug 270012

Sorry, bug 127661 doesn't belong in that list.

This doesn't really block bug 157967. The work here can probably be leveraged into future support for Uniscribe etc., but the object is to achieve a better rendering with vanilla Win32 API, and the equivalent on other platforms if possible, without explicitly using the Uniscribe API.

Simon - I am now aware that this probably duplicates some of your work, but here it is to get it out there. It fixes the worst of my Hebrew rendering woes. Your GetWidth()/GetWidthArray() is the same as my GetCharacterSpacing(), and you have implemented it on Windows, while I have implemented it in GTK/Pango. While you are on vacation, I will work on a new patch that combines the best of our two. The changes in my patch are: * Fix nsTextFrame::RenderString() so it does not assume the width of a string equals the sum of its component characters (which assumption does not hold true for Hebrew+vowels). * Add a new optionally-implemented GetCharacterSpacing() function to the gfx layer that makes it return the horizontal spacing for each character that the gfx would use in the absence of justification. * Fix the layout of RTL text on Windows, which was not right. * Add NS_RENDERING_HINT_REORDER_SPACED_TEXT on Windows which should be there.

This new version of the patch combines my work with Simon's. It resolves the worst of the Hebrew rendering issues. I discuss this more on my website http://blacksapphire.com/firefox-rtl/ I've tested it on Linux and Windows. GetWidthArray is now implemented on both Pango and Windows. I have carefully written it to make sure it doesn't break anything it doesn't affect, and other than a little more general testing (particularly on Windows), I would be happy to have this patch committed to CVS. Simon - please review.

Version 4.9 patch: This fixes the brokenness on Windows 98 by using a more primitive method for DrawString and for GetWidthArray.

Fixes a problem that was occurring on Linux with Arabic. Arabic works nicely now.

Fixes a problem with Indic languages on Linux (i.e. GTK/Pango).

I have to say thank you for this patch. It's made a significant difference to Indic language support on Firefox. I've tested it on the latest Deer Park release and it works well. Originally in Firefox 1.0, Indic text would break up when selected with the cursor. This was subsequently fixed in DP although not perfect yet because for Indic text the syllables should be selected, not the individual text components. However with this patch, non-Justified text is fine when selecting, but Justified text still breaks up into pieces. Although I must say that this is considerably better than the current situation where justified text is always broken up and thus unreadable!

It's worth thinking about how this change interacts with the work on switching GFX to cairo.

The current patch doesn't apply cleanly to trunk or branch.

*** Bug 309101 has been marked as a duplicate of this bug. ***

(In reply to comment #14) > The current patch doesn't apply cleanly to trunk or branch. FWIW, I think this patch will help quite a bit but we also need bug 309099. There was a big problem even when text wasn't painted char by char (when using word-spacing). We shouldn't be repainting the entire screen every time an outline is painted.

(In reply to comment #16) > FWIW, I think this patch will help quite a bit (I was referring to the major screen reader perfomance drag in bug 307545)

For what its worth, all the code in that patch is thrown out for 1.9. We'll be providing an interface for getting an array of glyph advances and indexes, but it will be pretty different than this.

the gfx side of things, that is.

This fixes 95% of the screen reader slowness issue. All the real pages with the problem behave well enough with this patch. I have a torture test in bug 309099 which is still not perfect, but for the release of 1.5 fixing this would be good enough.

This greatly improves bug 307545 which is blocking 1.8b5+

Is this patch's risk is low for landing on 1.8? I don't think so...

(In reply to comment #22) > Is this patch's risk is low for landing on 1.8? I don't think so... I don't think so either but Roc asked me to find out if this fixes the problem, which we do need to deal with for 1.8.

I think that this patch's value is very big for i18n. But, this patch is very large for after beta release. If we find any regressions, we should block the release for the regression(s). (And if find after release, we should fix on 1.8 branch for minor release.) If it is guaranteed, I don't have dissenting opinion.

What impact (if any) does this patch have on platforms that don't implement the nsRenderingContext::GetWidth() stuff?

Patch is too large, so removing blocking request.

(In reply to comment #25) > What impact (if any) does this patch have on platforms that don't implement the > nsRenderingContext::GetWidth() stuff? I've worked hard to make sure there isn't a performance penalty in that situation. Take a look at nsTextFrame_MeasureWalker::visit(). You'll see that it asks nsRenderingContext::getWidthArray(), and if it isn't implemented, it uses a method that works the same as the old one, except that it is modified slightly in that it will behave more intelligently if it sees any characters it suspects might be complex layout. So for Latin text, the performance should be pretty much the same as before. Aaron - thanks for the cleanup of the patch. I cleaned it up myself for the 4.12 version, but the merge was broken within a week! I am continuing to work on this patch - see http://blacksapphire.com/firefox-rtl/ I hope all my work isn't thrown out with the new 1.9 code! Would it be a better use of my time to look at the new code? If so, how? I can certainly help with testing. Sukh - can you provide a URL and screenshots for the issues you mention, and instructions for someone who doesn't read Indic languages?

Talk to pavlov, he's working on this. Find him on IRC if necessary. Hopefully we can fold this into the new font code.

Try the IRC #gfx channel. The basic plan is to use Uniscribe on Windows, Pango on Linux and ATSUI on Mac to turn DOM text into glyph indices and coordinates, and then render the glyphs through cairo. I guess it will be similar in flavour to this patch. I'm sure we'd appreciate your help defining the API and then massaging this patch to use that API.

Comment on attachment 197201 [details] [diff] [review] Same stuff, but applies cleanly to MOZILLA_1_8_BRANCH I don't think we're taking this for branch.

I seriously do hope this patch makes it into Firefox 1.5. At the moment, Firefox is very poor at rendering Indic text on anything that is spaced text - to the extent that it is unusable and unreadable. Stephen: Visit http://tdil.mit.gov.in/hindi_site/ach-mat.htm and hopefully with the patch applied the page will look okay. Ensure your computer system has complex text rendering support for Indic scripts and that you have a Devanagari font installed. Now attempt to select some text. You'll notice that as you select the text, it breaks up into pieces. Now look at http://www.bbc.co.uk/hindi/sport/story/2005/10/051007_sania_qf.shtml and try to select the text. As the text is NOT justified, the selecting the text does not break it up into pieces. This is the desired form of selecting text (technically the best option would be selecting syllables, but that's the least of our worries in terms of Indic support on Firefox).

This is superseded by the new textframe.