Hi Phil, Thanks for the detailed evaluation in bug and here. Change looks good to me.
Newly added file has copyright year 2018. Please update it before pushing. I ran smaller set of rotate tests for font and they all pass with this change. Does this new change handle non-bold use cases also as mentioned in bug description “However, if we use "MS P Gothic" as a font, even normal alphabet characters are drawn at wrong positions.” ? Thanks, Jay > On 16-Apr-2020, at 2:30 AM, Philip Race <philip.r...@oracle.com> 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