[OpenJDK 2D-Dev] RFR: 8233006: freetype incorrectly adjusts advances when emboldening rotated glyphs

[Sergey Bylokhov]

Hi, Phil.
I have only the question about the new comment:

340     // Let's not adjust the metrics of any glyph that is zero advance.
341     if (slot->linearHoriAdvance == 0) {
342         return;
343     }

The comments said that we do not want to adjust the metrics and return, but we already adjusted it a little bit before:
335     slot->metrics.width        += extra;
336     slot->metrics.height       += extra;

I do not know the exact reason to check linearHoriAdvance at line 341, but then why we skip the check of "linearVertAdvance"?


On 4/15/20 2:00 pm, Philip Race wrote:
> Bug : https://bugs.openjdk.java.net/browse/JDK-8233006
> Webrev : http://cr.openjdk.java.net/~prr/8233006
> 
> The bug here is that the freetype function for synthesising bold is not ready to handle rotation.
> 
> In the process I noticed it did not adjust the advance used by the fractional metrics case,
> even though the outline is bolded.
> 
> Also, in what seems to be a completely wrong thing to do, freetype would
> widen the advance of glyphs which have zero advance.
> 
> So I decided that the best thing to do was to write our own.
> A chunk of the heavy lifting - widening the outline - is still done by freetype
> but there were a lot of details to get right and test.
> 
> I wrote a test to visualise the problem but the actual test checks by looking
> at the bounding rectangle of the drawn pixels and compares its height to
> the declared metrics of the font, failing if they disagree by too much.
> 
> Note that the code path is only exercised when synthetic bolding is needed.
> So real bold fonts don't test this code.
> Since there's not an easy way to say which fonts have real bold, I decided the
> test should use a BOLD version of every font on the system, which on almost
> all systems will test some significant number of such cases.
> I kept the UI for visualising as it will be useful for later debugging of failures.
> 
> Also it made me notice that the case where the text was not rotated at all was
> drawing shorter than all the other cases.
> I traced this back to the fix for 8203485 which added a macro FT26Dot6ToInt
> and used it to get the integer advance in the unrotated, integer metrics case.
> The idea there wasn't completely wrong, but I don't think it was completely right either.
> I got rid of the macro and instead used the same FT26Dot6ToFloat macro as used
> in the rotation cases. So we now return the exact floating point value to the calling
> Java code. That then can round appropriately as it needs to. This fixed the inconsistency
> and the test for 8203485 still passes as do all other tests.
> This change will likely lead to some cases where unrotated advances now round up one pixel wider,
> but so far it looks correct to me. They'll be restored to something more like what they were
> before 8203485, since that removed rounding and added truncation instead to fix a problem
> with the rounding being incorrect for rotations because it could round down when it should round up.
> Now we just let the Java code handle it.
> 
> I've run these tests on all platforms and they pass. Mac isn't using this freetype path so it is not affected
> but it is still good to know the tests pass there ...
> 
> -phil


-- 
Best regards, Sergey.