Only thing to be concerned about is the copyright of the ttf file. If it is not self-generated or GPL licensed or we are not sure of of its origin(most of will be copyrighted to Adobe or such, which are not ok to be checked in), It's better to reuse the A.ttf file already present in the repo

Regards

Prasanta

On 22-Nov-19 1:52 PM, Jayathirth Rao wrote:
Hi Dmitry,

There are some test cases which use .ttf file for test case and they keep regression test and its corresponding .ttf file in same path.
So we can follow same approach.

Usage of hardcoded value seems reasonable here.

Thanks,
Jay

On 22-Nov-2019, at 12:56 PM, Dmitry Batrak <dmitry.bat...@jetbrains.com <mailto:dmitry.bat...@jetbrains.com>> wrote:

Hello Jay,

Thanks for looking into this!
Since JDK-8218854 JDK already hardcoded the value of FreeType's oblique modifier (to calculate max advance). After the proposed change, the hardcoded value will also be used for the actual transform applied to glyphs, making the code, in a way, more consistent (against the potential case of FreeType changing the oblique modifier at some point). As for creating a test for the fix, the test could verify that certain pixels are filled or not filled to confirm the correct slant direction. I think it's even possible to do without using Robot - by drawing into a BufferedImage. But to make the test more reliable, it should use a fixed font. Is it OK to add some font to JDK codebase along with test code? Or maybe A.ttf already used in some test cases can be reused? If the latter is acceptable, should it be copied to the location near the test's source code, or it can be loaded by a relative reference?

Best regards,
Dmitry Batrak

On Mon, Nov 18, 2019 at 1:29 PM Jayathirth Rao <jayathirth....@oracle.com <mailto:jayathirth....@oracle.com>> wrote:

    Hi Dmitry,

    Thanks for the patch.
    I can sponsor this.

    I went through the change and it looks okay.
    But I have a concern about using specific values for matrix based
    on Freetype version for Oblique type. I have less idea about that
    maybe Phil or others can clarify the same.

    Regarding adding regression test along with the patch, i think we
    can use AWT Robot to get pixel data to verify the patch.

    Thanks,
    Jay

    On 18-Nov-2019, at 2:49 PM, Dmitry Batrak
    <dmitry.bat...@jetbrains.com
    <mailto:dmitry.bat...@jetbrains.com>> wrote:

    Hello,

    Still trying.
    Any volunteers to sponsor/review?

    Best regards,
    Dmitry Batrak

    On Tue, Nov 5, 2019 at 11:27 AM Dmitry Batrak
    <dmitry.bat...@jetbrains.com
    <mailto:dmitry.bat...@jetbrains.com>> wrote:

        Hello,

        Let me repeat the request.
        Any volunteers to sponsor/review?

        Best regards,
        Dmitry Batrak

        ---------- Forwarded message ---------
        From: *Dmitry Batrak* <dmitry.bat...@jetbrains.com
        <mailto:dmitry.bat...@jetbrains.com>>
        Date: Thu, Aug 29, 2019 at 1:58 PM
        Subject: [PATCH] 8210058: Algorithmic Italic font leans
        opposite angle in Printing
        To: 2d-dev <2d-dev@openjdk.java.net
        <mailto:2d-dev@openjdk.java.net>>


        Hello,

        I'd like to submit a patch for JDK-8210058. I'm not a
        Committer, so I'll need someone to sponsor this change.

        The issue is related to the implementation of algorithmic
        italics in FreeType font scaler. At the moment it uses
        FT_GlyphSlot_Oblique, but its implementation doesn't take
        into account the glyph transform, previously set using
        FT_Set_Transform, and FreeType developers don't seem to have
        any interest in changing that (see
        https://savannah.nongnu.org/bugs/index.php?54565).
        The proposed solution is to include corresponding shear
        transform explicitly in matrix passed to FT_Set_Transform
        instead of using FT_GlyphSlot_Oblique.
        Proposed patch doesn't add any tests, as the change only
        impacts glyph rendering, and I couldn't think of a reliable
        way to test that automatically. Existing automated tests
        from OpenJDK pass after the fix.

        Issue: https://bugs.openjdk.java.net/browse/JDK-8210058
        Webrev: http://cr.openjdk.java.net/~dbatrak/8210058/webrev.00/

        Best regards,
        Dmitry Batrak





Reply via email to