Il giorno gio, 06/05/2010 alle 00.18 +0200, Mario Torre ha scritto:
Hi all,
Crossposting, because this is related to what Roman just reported on the
Swing-dev mailing list, the original thread was on 2d-dev with the same
subject as this mail.
> When printing the values, the thing that came immediately to my eyes is
> that the leading is -1, which as far as I understand is an invalid
> value.
I passed a bit of time on that again yesterday, and now I think my last
proposed fix is not correct, because when we scale the values ourselves
we need to take into account if the font is hinted or not, something I
actually don't do in the previous patch. Also, the patch assumes that
the height of the font is incorrect as retuned by FT, and tries to fix
it (I took as a reference the values returned by the closed JDK,
assuming those are correct, which now I think it's not the case); I
believe that FT does the correct thing instead, and the bug is caused by
some issues with he closed JDK scaler that we carry on in the open
version to have a possibly common codebase.
Finally, I also think we have not just one, but 3 related bugs hiding
around. The first one is the leading itself, this guy is -1 (because of
rounding errors), and the leading can't be less than 0 as far as I
understood.
I did some little tests, because I am a bit worried by the case were
fonts are hinted vs. not hinted, we may result in ruining the hinter
work if we don't do correct rounding, and I found out that this may the
case with some fonts.
I now think my original patch was correct (not the one submitted in
webrev, but the one proposed in the mailing list):
ly = (jfloat) ROUND(FT26Dot6ToFloat(
scalerInfo->face->size->metrics.height +
bmodifier) + ay - dy);
That's it, we just round up adding 0.5, which will give us the value to
the closest next integer when we use it in the Java code, perhaps not
the ideal solution, but looks correct enough to me.
This leaves us with another problem though, because this just fixes the
leading, and only by chance (two bugs adding up together) it fixes the
rendering problem I was originally tracking.
As Roman noted in the swing-dev mailing list, Swing uses all over the
place metrics.getHeight() as an indication for the line spacing, while
there's not guarantee of non overlapping.
One obvious approach is to fix swing anywhere it uses height as proposed
before. This fix is not enough alone, because we also need to fix the
height of the fonts, which is the third issue related to this chain of
bugs.
FT knows about the height of the font, so we could just keep this value
and pass it back as an argument to StrikeMetrics, and use it directly
instead of computing ourselves when asked for height.
I've done some tests, and using height from FT plus rounding the
leading, plus a "demo" fix for Swing produced expected results.
Of course, we need to discuss it a bit more together, I would really
like to have some feedback at this point.
For sake of perennial backward compatibility one could argue that those
changes are too aggressive, in this case I guess that rounding alone (or
perhaps checking if ly < 0 and returning 0 in this case, although I
believe this to be slightly less correct) is good enough, even if this
would just really add another workaround to the big list of workarounds
done in this specific piece of code.
What are your feelings?
Thanks,
Mario
--
Mario Torre, Software Developer, http://www.jroller.com/neugens/
aicas GmbH, Haid-und-Neu-Straße 18 * D-76131 Karlsruhe * Germany
http://www.aicas.com * Tel: +49-721-663 968-0
USt-Id: DE216375633, Handelsregister HRB 109481, AG Mannheim
Geschäftsführer: Dr. James J. Hunt
pgp key: http://subkeys.pgp.net/ PGP Key ID: 80F240CF
Fingerprint: BA39 9666 94EC 8B73 27FA FC7C 4086 63E3 80F2 40CF
Please, support open standards: http://endsoftpatents.org/
--
pgp key: http://subkeys.pgp.net/ PGP Key ID: 80F240CF
Fingerprint: BA39 9666 94EC 8B73 27FA FC7C 4086 63E3 80F2 40CF
Proud GNU Classpath developer: http://www.classpath.org/
Read About us at: http://planet.classpath.org
OpenJDK: http://openjdk.java.net/projects/caciocavallo/
Please, support open standards:
http://endsoftpatents.org/
