Re: [OpenJDK 2D-Dev] Font rendering issue

2010-06-16 Thread Phil Race
6961732: FontMetrics.getLeading() may be negative in freetype-based 
OpenJDK builds


-phil.

On 6/16/2010 8:43 AM, Mario Torre wrote:

Il giorno ven, 11/06/2010 alle 09.51 +0200, Mario Torre ha scritto:
   

Il giorno gio, 10/06/2010 alle 14.28 -0700, Phil Race ha scritto:
 

I've root-caused this although its still not clear what's the ideal answer,
the simplest and safest may be Mario's proposed fix here from 5/5/2010 :
   

I've applied the patch to the latest OpenJDK drop and it compiles and
work fine.

I'm now downloading the j2d repository, I think I need a bug id before
committing, can somebody please create one for me?

Cheers,
Mario
   




Re: [OpenJDK 2D-Dev] Font rendering issue

2010-06-16 Thread Mario Torre
Il giorno mer, 16/06/2010 alle 18.52 +0200, Mario Torre ha scritto:
 Il giorno mer, 16/06/2010 alle 09.37 -0700, Phil Race ha scritto:
  6961732: FontMetrics.getLeading() may be negative in freetype-based 
  OpenJDK builds
  
  -phil.
  

Hi Phil,

I need to fill the reviewer field, what is your nickname id for OpenJDK?

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/



Re: [OpenJDK 2D-Dev] Font rendering issue

2010-06-16 Thread Mario Torre
Il giorno mer, 16/06/2010 alle 19.40 +0200, Mario Torre ha scritto:
 Il giorno mer, 16/06/2010 alle 18.52 +0200, Mario Torre ha scritto:
  Il giorno mer, 16/06/2010 alle 09.37 -0700, Phil Race ha scritto:
   6961732: FontMetrics.getLeading() may be negative in freetype-based 
   OpenJDK builds
   
   -phil.
   

Hi Phil!

Just committed:

changeset:   2429:8b55669c7b7a
tag: tip
user:neugens
date:Wed Jun 16 20:46:10 2010 +0200
files:   src/share/native/sun/font/freetypeScaler.c
description:
6961732: FontMetrics.getLeading() may be negative in freetype-based
OpenJDK builds.
Summary: Fix premature integer roundings to preserve correct height,
width and descent values for fonts
Reviewed-by: prr

Thanks for the support!

Cheers,
Mario
-- 
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/



Re: [OpenJDK 2D-Dev] Font rendering issue

2010-06-12 Thread Phil Race

I've root-caused this although its still not clear what's the ideal answer,
the simplest and safest may be Mario's proposed fix here from 5/5/2010 :

http://cr.openjdk.java.net/~neugens/100134/webrev.01/

So, yes, I think as Mario suggested it starts with something in
freetype which has a #define GRID_FIT_METRICS that causes [premature] 
rounding of metrics to integer values :-


#ifdef GRID_FIT_METRICS
metrics-ascender= FT_PIX_CEIL( FT_MulFix( face-ascender,
   metrics-y_scale ) );

metrics-descender   = FT_PIX_FLOOR( FT_MulFix( face-descender,
metrics-y_scale ) );

metrics-height  = FT_PIX_ROUND( FT_MulFix( face-height,
metrics-y_scale ) );



So 
height is ROUNDED

ascent is CEIL'd
descent is FLOOR'd.

Mario's fix essentially removes this integer rounding.

The FLOOR is because descent is negative and so the apparent intent is
to ensure that the int ascent and descent are at least as great in
magnitude as the float versions. But this is at the cost of
ascent+descent == height.

Freetype extracts ascent and descent from the 'hhea' table (same
as T2K as it happens), and the values there in upem are scaled
to the 12 pt example in Mario's test program. As shown here.
Arithmetic is actually fixed point but FP values are clearer.
You can then see the effect of the conversion to int's

FIELD  UPEM  | FP 12pt | Integer Val
ascent  | 1901   | 11.14  |   12 (CEIL'd)
descent | -483   | -2.83  |   -3 (FLOOR'd)
height  | 2384   | 13.97  |   14 (ROUNDed)


Note that ceil of ascent greatly increases it by almost a pixel
from its true value

If GRID_FIT_METRICS is not defined then no rounding
occurs and JDK code sees the floating point values
and it rounds in its own preferred way, and then
the results are the same as in the freetype and t2k cases.

But where does freetype get height, since the font provides
ascent, descent and linegap? The answer is its calculated
from those three :
 
   root-ascender  = face-horizontal.Ascender;

   root-descender = face-horizontal.Descender;

   root-height= (FT_Short)( root-ascender - root-descender +
 face-horizontal.Line_Gap );

Line_Gap is not exposed freetype API, and so the leading value required
by Java API can only be derived by doing the reverse arithmetic.

However since freetype has already rounded for us, this information
is not reliable and we see the result as 14-(12+3) = -1 
JDK therefore cannot use freetype (via this exact API)

to faithfully report leading.

Regardless of how the rest of this works out, this part should be fixed.

Negative leading values are (surprisingly) legal but I think either
deprecated or extremely rare. So I think it reasonable that if we
see a negative leading we ignore it (ie clamp leading to zero).

Mario's proposed fix should have the same effect as it bypasses
the rounding and we will see the same raw values as in the T2K rasteriser

Whether its done like that or as part of something more complex
depends on the relationship of the asc+dsc+ldg to height.

Clamping to zero and summing gives us hgt= 12+3+0=15, whereas freetype reports
its actually 14 because the rounding happened to be almost completely
precise whereas the CEIL(ascent) added a whole pixel.

It could be even worse. Imagine if we had in floating point that descent=2.10
Then height would be rounded to 13, but ascent+descent would still be 15.

So should we ensure height = ascent + descent + leading ?
I consider this a grey area as it depends on how you interpret
ascent and descent and their relationship to height, but freetype clearly
derived height from these but doesn't see it as necessary for them to
add up.

JDK has in practice tried to maintain this equation but freetype clearly doesn't
regard it as invariant. In fact I updated the spec for FontMetrics.getHeight()
back in JDK 1.4 under bug 4455492 and tried to implement something like
this. This led directly to Swing problems listed in 4467709, so I
ended up having to go for the tamer solution today.
So if we allow for openjdk/freetype to return the height as 14, then
I'd still expect some Swing problems, although apparently some do exist
today. Basically, concurrent with the fix, someone would have to take
on everything Roman writes about here :

http://mail.openjdk.java.net/pipermail/swing-dev/2010-May/001034.html

and be on call to evaluate and fix up all the Swing bugs that are
reported. We'd also have to be ready in case anyone complained in
general about metrics changes breaking their UI layout. This has
caused HUGE customer complaints in the past. Technically it was
their own fault, but they still complained.

I also suspect AWT would have some issues, as all the metrics
come via the 2D path.

And to be able to do this, we'd really want to bypass all that
rounding done by freetype, else we'd have 

Re: [OpenJDK 2D-Dev] Font rendering issue

2010-06-11 Thread Mario Torre
Il giorno gio, 10/06/2010 alle 14.28 -0700, Phil Race ha scritto:
 I've root-caused this although its still not clear what's the ideal answer,
 the simplest and safest may be Mario's proposed fix here from 5/5/2010 :

 

Hi Phil!

Thanks for the long explanation, everything makes perfect sense to me
now.

 Mario's concern about hinting isn't founded. It doesn't matter.

I thought about that, but I didn't have so much experience with fonts to
be so sure. This makes perfectly sense to me as well now.

 So I think (bit of hand waving)
 1) Implement Mario's fix which will solve negative leading

Yeah! :)

I'm not at home now, I'll be back on Monday evening, I will make sure
the fix fits in the latest j2d code drop, then we can go through the
committing process again. Of course if there's a rush somebody else can
commit on my behalf.

I believe this specific part should also go to OpenJDK 6, what are your
thoughts about that (NetBeans looks really awful)?

 2) Separately consider the larger change to
a) tighten up the height we reportywith the consequence that its common 
 that
getAscent()+getDescent()+getLeading()  getHeight()
b) Fix up everywhere in the JDK that assumes otherwise ..
   (a) and (b) ought to be as much as possible in the same commit
which means its a bigger job but still with follow-up inevitably needed.

I'll go through some of those myself in the next days. Of course, I
can't help on Sun/Oracle customers specific problems, but I can surely
help on the OpenJDK ones. Perhaps this is a bit too much risk for jdk 6
anyway, but it can be surely done for 7.

Cheers and thanks again for looking into it!
Mario
-- 
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/



Re: [OpenJDK 2D-Dev] Font rendering issue

2010-06-08 Thread Mario Torre
Il giorno ven, 28/05/2010 alle 20.25 +0200, Mario Torre ha scritto:

 Ok, I tried to send once, but the message is moderated as it's too big.
 
 I've uploaded the package here:
 
 http://www.limasoftware.net/neugens/downloads/stuff/font2dbug/FontTest.tar.gz

Hi all,

Is there still any interest in this?

Cheers,
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/



Re: [OpenJDK 2D-Dev] Font rendering issue

2010-05-28 Thread Mario Torre
Il giorno ven, 28/05/2010 alle 19.11 +0200, Roman Kennke ha scritto:
 Hi Mario,

 You could bundle a specific font with the testcase and use it, so it's
 no more system specific.
 
 /Roman

Ok, I tried to send once, but the message is moderated as it's too big.

I've uploaded the package here:

http://www.limasoftware.net/neugens/downloads/stuff/font2dbug/FontTest.tar.gz

This contains the font (note, it's not the only font that shows the issue, but 
it's the default font for Fedora).

I would like to put this test into the bug entry, but the bug database is down, 
can someone please look at it?

Thanks,
Mario
-- 
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/




Re: [OpenJDK 2D-Dev] Font rendering issue

2010-05-19 Thread Roman Kennke
Hi Phil,

 - Swing understands that there's no guarantee that all the pixels fit
in the reported height of the line. I don't think you want to space
out the text so much that you guarantee no glyph overlap.

No, that's not really the point. Swing assumes wrongly that all pixels
*do* fit into 'height'. This leads to (e.g.) '_' beeing cut off.

  Actually
due to some perhaps questionable choices of the fields in the font
which should be used AND the way that the height of logical
fonts is assembled (the maximum leading of all components +
the maximum descent of all components + the maximum ascent of all 
 components)
its higher than I'd like. Much as I'd love to fix this its going to 
 get someone upset,
so I've steered well clear. This goes all the way back to 1.1

Yeah.

   We have less compatibility history to maintain (in the behavioural 
 sense) in the freetype code
so that's easier to change.

Yeah, but do we really want to bend what freetype reports correctly,
only to make it fit with existing code? I.e. introduce workarounds at
the low level to fix other (multiple levels) of workarounds and bugs at
higher levels? I don't know. I'd rather get text rendering correct even
if it makes somebody upset. (But that's just me..)

   Anything in the shared code, I'd want to actually try out. Any claimed 
 errors
   in the closed code, I'd want to track down and see if its actually so and
   what we can/should do about it.

You can easily try out the reported problems. For example, fire up
Netbeans with OpenJDK on Ubuntu or Fedora, and you'll see that the '_'
is not rendered in many cases, which is *very* irritating because in
code, you really want to see '_'.

   If I have things right, the most obvious problem Mario saw is a negative
   value for leading. That could be an incorrect interpretation of sign 
 somewhere?

I think this comes from height, ascent and descent not beeing
'consistent' in the sense that height-ascent-descent=leading. In this
particular case, ascent+descent is one pixel more than height, and
leading ends up at -1, even though in the font, you have an (unscaled)
leading of 0. This is a problem introduced by the grid fitting, all of
ascent, descent and height are adjusted for grid-fitting, but leading
not (actually, freetype does not report a scaled linegap/leading and we
calculate it from the other 3 values).

 Seems like rounding up to get rid of it isn't addressing the real problem

I believe that we should take the unscaled leading value from the font
and apply the scaling transform ourselves. That's the best that we can
do I think.

/Roman




Re: [OpenJDK 2D-Dev] Font rendering issue

2010-05-19 Thread Mario Torre
Il giorno mar, 18/05/2010 alle 15.06 -0700, Jim Graham ha scritto:
 The thing that bothers me about this fix is that the value being 
 returned here is the raw computed value.  All of the values in this 
 routine are being returned in floating point sub-pixel maximum 
 accuracy.  I don't see why *this* code needs to round this value.  If 
 something that uses the data returned from this method needs an integer 
 then it should be up to that code to do whatever rounding is 
 appropriate, but rounding at the most primitive level to fix a bug at a 
 higher level is premature (IMHO)...
 
   ...jim

Hi Jim,

I agree with that, but rounding up in the Java code (where this float
gets converted to integer) was rejected in the first place.

Cheers,
Mario
-- 
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/



Re: [OpenJDK 2D-Dev] Font rendering issue

2010-05-19 Thread Phil Race

Roman Kennke wrote:

Hi Phil,

  

- Swing understands that there's no guarantee that all the pixels fit
   in the reported height of the line. I don't think you want to space
   out the text so much that you guarantee no glyph overlap.



No, that's not really the point. Swing assumes wrongly that all pixels
*do* fit into 'height'. This leads to (e.g.) '_' beeing cut off.
  


What I was saying is that they know problems can arise but its a compromise.
I recall there were attempts in the past to ensure mnemonics were properly
positioned and I would suppose, visible.
You need to use logical metrics, not the glyph's visual bounds to position
an underline otherwise it'll be like a wandering baseline
However you don't know where all the pixels fall from font-wide metrics, 
and glyphs

with diacritics have been known to be placed above the supposed max ascent.

BTW Its not just ascent and descent, there's problems on the left and 
right sometimes

as logical left and advance don't necessarily encompass all the pixels.

So maybe Swing is culpable here. Its also possible that
something isn't ideal in the delicate task of making choices about how
to round up several inter-related floating point numbers
What I'd like to have is a small test case that demonstrates exactly 
this so I can follow the chain.
Mario sounds to  have  that, right down to specific font and size and 
using the same mnemonic

positioning logic as swing.

  

 Actually
   due to some perhaps questionable choices of the fields in the font
   which should be used AND the way that the height of logical
   fonts is assembled (the maximum leading of all components +
   the maximum descent of all components + the maximum ascent of all 
components)
   its higher than I'd like. Much as I'd love to fix this its going to 
get someone upset,

   so I've steered well clear. This goes all the way back to 1.1



Yeah.

  
  We have less compatibility history to maintain (in the behavioural 
sense) in the freetype code

   so that's easier to change.



Yeah, but do we really want to bend what freetype reports correctly,
only to make it fit with existing code? I.e. introduce workarounds at
the low level to fix other (multiple levels) of workarounds and bugs at
higher levels? I don't know. I'd rather get text rendering correct even
if it makes somebody upset. (But that's just me..)

  

Leaving aside for a moment the issue of the rounding problem,
What is correct  metrics ?
T2K (the JDK  rasteriser) was from one of the original Apple truetype 
engineers

and Apple have always used the hhea table for ascent, descent, and linegap
and so does T2K

freetype, I don't know for sure sfnt_load_face has several options and
some of them are ifdefed out in the source I have for 2.3.5

The part that's not ifdefed out appear to use the same data as the JDK

   root-ascender  = face-horizontal.Ascender;
   root-descender = face-horizontal.Descender;

   root-height= (FT_Short)( root-ascender - root-descender +
 face-horizontal.Line_Gap );

The ifdefed out code gets it from the OS/2 table's  
typoAscender/typoDescender .
(BTW there's also usWinAscent/usWinDescent. My impression is that 
Windows GDI

has always used the latter, although maybe GDI+ and other APIs use the
former

None of these are wrong. They are just different.
So long as they are internally consistent this is fine.

BTW as MS note you can get clipping with usWin values too ..
http://www.microsoft.com/typography/otspec/os2.htm#wa

Since with T2K we are using data from the hhea, we are essentially getting
max ascent as ascent and max descent as descent.

http://bugs.sun.com/view_bug.do?bug_id=6623223
This is an unfortunate state of affairs but one that's risky to change.
Where I can imagine that this might cause problems is in code that got used
to ascent/descent being max values, and it doesn't work so well when they
are not. Perhaps some of the problems in Swing are related to that except
that if freetype is doing this too, I'm not sure we can blame that.


  Anything in the shared code, I'd want to actually try out. Any claimed 
errors

  in the closed code, I'd want to track down and see if its actually so and
  what we can/should do about it.



You can easily try out the reported problems. For example, fire up
Netbeans with OpenJDK on Ubuntu or Fedora, and you'll see that the '_'
is not rendered in many cases, which is *very* irritating because in
code, you really want to see '_'.

  

  If I have things right, the most obvious problem Mario saw is a negative
  value for leading. That could be an incorrect interpretation of sign 
somewhere?



I think this comes from height, ascent and descent not beeing
'consistent' in the sense that height-ascent-descent=leading. In this
particular case, ascent+descent is one pixel more than height, and
leading ends up at -1, even though in the font, you have an (unscaled)
leading of 0. 


Is 

Re: [OpenJDK 2D-Dev] Font rendering issue

2010-05-18 Thread Roman Kennke
Hi Mario,

   ly = (jfloat) ROUND(FT26Dot6ToFloat(
 scalerInfo-face-size-metrics.height +
 bmodifier) + ay - dy);
   
  
  And here is the proposed webrev:
  
  http://cr.openjdk.java.net/~neugens/100134/webrev.02/
  
  As noted, this doesn't really fix all the bugs, it just fixes the
  rounding for leading, which, by chance, workarounds the other issues and
  appear to fix the rendering as well.
  
  Cheers,
  Mario
 
 Any comment on that?

To be honest, I don't like that. It's (yet another) workaround for
problems in other areas of JDK. We already have plenty of workarounds
and adjustments on the metrics, and adding more of it doesn't seem
right.

I already started a 'discussion' (well, so far it's been a
monologue ;-) ) on the topic here:

http://mail.openjdk.java.net/pipermail/swing-dev/2010-May/001034.html

I believe that the metrics returned by the freetype scaler are (mostly)
correct. Mostly, because the leading is most likely broken, it should
not be -1. But the leading is rarely used and doesn't really have a
direct impact. What *does* matter is the line height. And this comes out
correctly, albeit only per accident. Why accident? Well, height is not
fetched directly from the font scaler (as it should IMO), but instead
calculated from ascent, descent and leading. And since we calculate
leading from height, ascent and descent as reported by the scaler, it
comes out right in the end. (Funny, eh?)

So what is the actual problem that leads to 'g' and '_' beeing cut off?
In my opinion, it is a problem in how Swing renders and lays out text.
In many places (for example, in PlainView) it assumes that font height
is the space that a single line of text needs to fully contain all its
glyphs. But if you take a look at the specification, that's not the
case: height is the distance from one baseline to the next. Why is that
different? Because two lines of text may have a slight overlap. For
example, the undershoot of 'g' (in certain fonts at certain sizes)
overlaps by one pixel into the next row of text. The only metrics that
guarantee to fully contain a line are maxAscent and maxDescent. So, for
a multiline textfield (just to pick an example) the total height would
be maxAscent + (n-1) * height + maxDescent. The total height for a
single line would be maxAscent + maxDescent (which is actually a
degenerated case of the other formula).

To really fix this problem (which I'd support, as opposed to add more
workarounds), we'd need to fix Swing's text rendering. Problem here is
that this would break 3rd party code, as can be witnessed already. For
example, Netbeans' text rendering is also broken, and it's not using
Swing for its text editors, which leads to '_' beeing invisible in many
cases, when running in OpenJDK. On the other hand, introducing more
workarounds would make OpenJDK remain 'compatible' in the sense that
existing apps would run as fine as with closed JDK, but by sacrificing
correct text rendering (as specified in the by the font designer).

I also blogged about it:

http://rkennke.wordpress.com/2010/05/09/subtleties-in-java-text-rendering/

So to summarize, I think fixing this involves the following:
- Fix Swing text rendering
- File bugs in important projects like Netbeans
- Fix metrics so that height is not calculated from ascent, descent and
leading, but instead fetched from the scaler directly.
- Review and get rid of workarounds that wrongly adjust the metrics

Not sure if it's likely to ever happen for compatibility reasons, I
think it would be nice, and would improve Java's font rendering subtly
but significantly.

Would like to hear other's (Phil, Igor,..) opinions on that, I might be
missing important points.

Cheers, Roman




Re: [OpenJDK 2D-Dev] Font rendering issue

2010-05-18 Thread Mario Torre
Il giorno mar, 18/05/2010 alle 21.07 +0200, Roman Kennke ha scritto:
 Hi Mario,
 
ly = (jfloat) ROUND(FT26Dot6ToFloat(
  scalerInfo-face-size-metrics.height +
  bmodifier) + ay - dy);



 To be honest, I don't like that. It's (yet another) workaround for
 problems in other areas of JDK. We already have plenty of workarounds
 and adjustments on the metrics, and adding more of it doesn't seem
 right.

Hi Roman,

Thanks for commenting.

I totally agree on that, I don't want to add other workarounds, but I
think this is still part of the fix (actually, it's a fix for a separate
issue than the one I opened initially, even if they are related).

 So what is the actual problem that leads to 'g' and '_' beeing cut off?
 In my opinion, it is a problem in how Swing renders and lays out text.
 In many places (for example, in PlainView) it assumes that font height
 is the space that a single line of text needs to fully contain all its
 glyphs. 

I've seen this code, and did some tests, my target component was the
text editor because this is what I though NetBeans was using, perhaps
with some fancy addition. It turned out that fixing it didn't have any
effect on NetBeans.

Anyway, I think it makes sense to fix our stuff and let other people fix
their own, and I agree that we should fix Swing now, because it's not so
complicated either. I prepared a patch to let pass the height of the
font from the scaler when it instantiates the StrikeMetrics, so that
this value is the correct one calculated by FreeType:

http://www.limasoftware.net/neugens/downloads/stuff/font2dbug/2010-05-18-fontscaler.patch

(need to prepare a webrev and double check the code, but you get the
idea).

I don't think it's a big issue for perennial backward compatibility
though, the closed JDK wrongly report a size one pixel higher, and this
accidentally hides the bug, everybody is happy there. If we fix the
FreeType scaler, we only fix the open code, which already render
incorrectly *every* application anyway, so it can't be worse than that.

 Would like to hear other's (Phil, Igor,..) opinions on that, I might be
 missing important points.

Yes, please!

Cheers,
Mario

-- 
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/



Re: [OpenJDK 2D-Dev] Font rendering issue

2010-05-18 Thread Jim Graham
The thing that bothers me about this fix is that the value being 
returned here is the raw computed value.  All of the values in this 
routine are being returned in floating point sub-pixel maximum 
accuracy.  I don't see why *this* code needs to round this value.  If 
something that uses the data returned from this method needs an integer 
then it should be up to that code to do whatever rounding is 
appropriate, but rounding at the most primitive level to fix a bug at a 
higher level is premature (IMHO)...


...jim

Mario Torre wrote:

Il giorno dom, 09/05/2010 alle 14.17 +0200, Mario Torre ha scritto:

Il giorno dom, 09/05/2010 alle 12.14 +0200, Mario Torre ha scritto:


ly = (jfloat) ROUND(FT26Dot6ToFloat(
  scalerInfo-face-size-metrics.height +
  bmodifier) + ay - dy);


And here is the proposed webrev:

http://cr.openjdk.java.net/~neugens/100134/webrev.02/

As noted, this doesn't really fix all the bugs, it just fixes the
rounding for leading, which, by chance, workarounds the other issues and
appear to fix the rendering as well.

Cheers,
Mario


Any comment on that?

Cheers,
Mario


Re: [OpenJDK 2D-Dev] Font rendering issue

2010-05-18 Thread Jim Graham
Phil's message brings up another issue for me with the patch.  Why use 
ROUND instead of a ceiling operation?  Do we know what the best option 
is for the code above?


Again, I would strongly favor leaving these base calculations in the 
scalar alone and focus more on making sure the proper operations are 
done in the code above (whether, contextually, they may be a round or a 
ceiling or working directly with the raw float value)...


...jim

Phil Race wrote:

Too many emails with too many comments for me to address them all.

- Swing understands that there's no guarantee that all the pixels fit
  in the reported height of the line. I don't think you want to space
  out the text so much that you guarantee no glyph overlap. Actually
  due to some perhaps questionable choices of the fields in the font
  which should be used AND the way that the height of logical
  fonts is assembled (the maximum leading of all components +
  the maximum descent of all components + the maximum ascent of all 
components)
  its higher than I'd like. Much as I'd love to fix this its going to 
get someone upset,

  so I've steered well clear. This goes all the way back to 1.1

 And I doubt we'll be able to go changing Swing either.

 Irritating as this may, where there are consequences for upstream code
  its not something I'd want to sign off on lightly unless its clearly 
fixing a blatant bug.
 We have less compatibility history to maintain (in the behavioural 
sense) in the freetype code

  so that's easier to change.

 Anything in the shared code, I'd want to actually try out. Any claimed 
errors

 in the closed code, I'd want to track down and see if its actually so and
 what we can/should do about it.

 If I have things right, the most obvious problem Mario saw is a negative
 value for leading. That could be an incorrect interpretation of sign 
somewhere?

Seems like rounding up to get rid of it isn't addressing the real problem

-phil.

On 5/18/2010 2:39 PM, Mario Torre wrote:

Il giorno mar, 18/05/2010 alle 23.33 +0200, Mario Torre ha scritto:
  

Il giorno mar, 18/05/2010 alle 21.07 +0200, Roman Kennke ha scritto:


Hi Mario,

  

ly = (jfloat) ROUND(FT26Dot6ToFloat(
   scalerInfo-face-size-metrics.height +
   bmodifier) + ay - dy);

 
 

Just one little note.

Because this is not necessarily just a rounding problem, but it's an
hinting problem, it would be possible to construct a case where this fix
is not enough either.

I'm not so deep into this code (this area in general) to know by
intuition if this is the case or not, or if there's an obvious
alternative, this is another reason I'm pushing a bit for a reply, I
would like to get some insight from the people who wrote this code in
the first place ;)

And yes, I want this fixed in OpenJDK as soon as possible, now that I
know the cause and a possible solution, editing in NetBeans is starting
to be so annoying, makes me feel really bad!!!

Thanks,
Mario

   




Re: [OpenJDK 2D-Dev] Font rendering issue

2010-05-09 Thread Mario Torre
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/



Re: [OpenJDK 2D-Dev] Font rendering issue

2010-05-09 Thread Mario Torre
Il giorno dom, 09/05/2010 alle 12.14 +0200, Mario Torre ha scritto:

 ly = (jfloat) ROUND(FT26Dot6ToFloat(
   scalerInfo-face-size-metrics.height +
   bmodifier) + ay - dy);
 

And here is the proposed webrev:

http://cr.openjdk.java.net/~neugens/100134/webrev.02/

As noted, this doesn't really fix all the bugs, it just fixes the
rounding for leading, which, by chance, workarounds the other issues and
appear to fix the rendering as well.

Cheers,
Mario
-- 
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/



Re: [OpenJDK 2D-Dev] Font rendering issue

2010-05-05 Thread Mario Torre
Il giorno mar, 23/03/2010 alle 13.10 -0700, Phil Race ha scritto:
 It could be the hinting that Mario referred to. There are some
 notes (not by me) that the T2K values produced in JDK 1.4 when
 hinting was available were more correct than previously - when
 there was no hinting.
 
 But Swing wanted values that looked right for some common cases
 and 1.0 was excessive. I really can't remember exactly what
 cases after 9 years. And I don't have the cycles to dig up
 and repeat this now.
 
 So all I can say is that looking at what freetype's results
 are and adjusting for whether or not its using hinting is
 probably what's needed.
 
 Perhaps if Swing placed the mnemonics based on the underline
 offset of the font it would be better .. but I think Swing
 may just use the height of the font as the height of the
 component and taking into account that underline offset
 would add more runtime work and metrics churn now. And
 of course that offset wasn't available in 1.1 days where
 Swing started out.
 
 -phil.

Hi Phil!

I'm sorry I disappeared, but my priorities got shifted to something
else :(

Finally, I could work on that a bit today, and I think that my initial
estimation was correct.

I've found some interesting things, but first of all, some little
background.

I wrote a small application that draws a character and the bounding box
on screen, as well as printing the various properties for the font, like
ascent, descent, etc..

Here is a screen shot of the application, on the left OpenJDK, on the
right the closed one (rendering the same font):

http://www.limasoftware.net/neugens/downloads/stuff/font2dbug/bounds_jdk.png

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.

Because the leading is calculated based on the values of ascending,
descending and the height, I printed the height of the font and here I
got that there is *always* one pixel less in the height as returned by
OpenJDK compared to the closed JDK.

That is, when a font has height of 14 for OpenJDK, it is 15 for the
closed one, etc...

If we print the unscaled values though, we see that they match, in
other words, there's something that gets lost when FreeType scales the
values from font coordinates to device coordinates, I believe this is
because the operations are done in fixed point maths.

I don't know if this is a bug in Freetype, but I can fix it at this
point doing the scaling myself.

In the proposed webrev I use FT26Dot6ToFloat in conjunction with
FT_MulFix instead of shifting the bits myself, actually I did this just
because the FT documentation says that FT_MulFix is optimised for 16.16
operations, and our scale factor is indeed expressed in 16.16, but I
don't know how this can be faster than height*scale16 :)

In any case, here's the webrev:

http://cr.openjdk.java.net/~neugens/100134/webrev.01/

I've not yet tested with the latest OpenJDK code, but before I update
the whole world, I would like some feedback.

What do you think? :)

Cheers,
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/



Re: [OpenJDK 2D-Dev] Font rendering issue

2010-03-23 Thread Roman Kennke
Phil, is it possible that these rounding effects are different in T2K
vs. FT? In this case, the rounding should probably be done in the font
backends altogether. However, I doubt I fully understand what exactly is
going on here. My understanding is that we need to round at least up to
the next full integer, because if height is reported as e.g. 13.1, and
we do normal float-int conversion, this would be treated as 13.0, i.e.
at least for some glyphs layout will end up too small. Infact, this
would even be true if height was reported as 13.9. In my understanding,
what needs to be done is to 'ceil' the value up to the next full integer
here. But the comment you reference below seems to indicate otherwise.
ceil() would have the same effect as using 1.0 as rounding value, but as
you said, you found that 0.95 is better for some reason. I wonder why?
Wouldn't this result in bad behaviour in some rare cases, e.g. for fonts
that have a very small (or none) leading? OTOH, choosing the value a
little higher (1.0) does what kind of damage? Adding one pixel too much
for some fonts? Is this so bad?

I also wonder why this doesn't happen with other applications that use
Freetype?

Mario, it would be helpful if you made this a smaller testcase. It would
probably render some difficult glyphs (gq_, etc) with clipping set to
the font's reported height in different font sizes. And it should set a
specific font, not simply use what's Swing's default, because those
differ on FT vs. T2K. This way we could compare the FT vs. the T2K
implementation. Maybe this gives some hints what goes wrong.

 So a freetype specific fix is preferable here.

The rounding should always be done when the value needs to be converted
to int IMO, not earlier (i.e. in the freetype driving code). However,
right now the rounding is in shared code. If we need different rounding
values for FT vs. T2K, we need to pull this stuff into the FontScaler,
which might be a lot of work. I took a look at how convoluted this font
metrics code is, my god, what a mess! ;-) We have at least 6 different
*Metrics classes, most of which seem to only carry values to another, it
looks quite messy. Also, there's stuff like X11FontMetrics, which looks
as if it's dead code.

Thanks, Roman

 
 -phil.
 
 On 3/23/2010 8:28 AM, Mario Torre wrote:
  Il giorno lun, 22/03/2010 alle 23.27 +0100, Mario Torre ha scritto:
 
  I started tracking this because anything I write something with an
  underscore in NetBeans I don't see the underscore, and that drives me
  crazy (ok, it also affect JamaicaVM, but I fixed it because I type my
  code in NetBeans ;)
 
  ...
 
  [2]
  http://www.limasoftware.net/neugens/downloads/stuff/font2dbug/possible_fix.png
 
  So, I can confirm that this also fixes my NetBeans bug.
 
  I created a bug entry:
 
  https://bugs.openjdk.java.net/show_bug.cgi?id=100134
 
  webrev at
 
  http://cr.openjdk.java.net/~neugens/100134/webrev.00/
 
  Cheers,
  Mario