This looks good to me, so long as this is not actually moving the A.ttf file
and instead just making a new copy - despite how it appears here :-

|------ ------ ------ ------ --- New <http://cr.openjdk.java.net/~dbatrak/8210058/webrev.01/test/jdk/java/awt/font/Rotate/A.ttf.html> Patch <http://cr.openjdk.java.net/~dbatrak/8210058/webrev.01/test/jdk/java/awt/font/Rotate/A.ttf.patch> Raw <http://cr.openjdk.java.net/~dbatrak/8210058/webrev.01/raw_files/new/test/jdk/java/awt/font/Rotate/A.ttf> | *test/jdk/java/awt/font/Rotate/A.ttf* /(was test/jdk/java/awt/FontClass/CreateFont/A.ttf)/


-phil.

On 11/25/19 1:43 AM, Dmitry Batrak wrote:
Please find the link to the updated webrev below. A test was added, which verifies the change.

http://cr.openjdk.java.net/~dbatrak/8210058/webrev.01/

Best regards,
Dmitry Batrak

On Fri, Nov 22, 2019 at 11:48 AM Prasanta Sadhukhan <prasanta.sadhuk...@oracle.com <mailto:prasanta.sadhuk...@oracle.com>> wrote:

    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