Re: [OpenJDK 2D-Dev] RFR: 8269223: -Xcheck:jni WARNINGs working with fonts on Linux [v4]
On Fri, 13 Aug 2021 14:28:53 GMT, Phil Race wrote: >> But `getFontMetrics()` is the primary "entry point" that generated all the >> JNI warnings in the first place. And I'm also not sure that we could've >> gotten all the warnings on Windows without `JFrame`. > > So what's wrong with > g2d.setFont(font); > FontMetrics metrics = g2d.getFontMetrics(font); > > ? > > No frame needed. Indeed. Changed the test per your suggestions. - PR: https://git.openjdk.java.net/jdk/pull/4572
Re: [OpenJDK 2D-Dev] RFR: 8269223: -Xcheck:jni WARNINGs working with fonts on Linux [v4]
On Thu, 12 Aug 2021 21:13:04 GMT, Phil Race wrote: > passes Looks good. I tried the test as headless and headful which happens to then exercise a broader set of distros than only one or the other the way our systems are set up. - PR: https://git.openjdk.java.net/jdk/pull/4572
Re: [OpenJDK 2D-Dev] RFR: 8269223: -Xcheck:jni WARNINGs working with fonts on Linux [v4]
On Fri, 13 Aug 2021 12:29:06 GMT, Maxim Kartashev wrote: >> oh you are getting metrics for a JFrame ? That's not going to exercise any >> new font code so is pointless except to make it so the test has to be >> headful. > > But `getFontMetrics()` is the primary "entry point" that generated all the > JNI warnings in the first place. And I'm also not sure that we could've > gotten all the warnings on Windows without `JFrame`. So what's wrong with g2d.setFont(font); FontMetrics metrics = g2d.getFontMetrics(font); ? No frame needed. - PR: https://git.openjdk.java.net/jdk/pull/4572
Re: [OpenJDK 2D-Dev] RFR: 8269223: -Xcheck:jni WARNINGs working with fonts on Linux [v4]
On Fri, 13 Aug 2021 00:07:56 GMT, Phil Race wrote: >> test/jdk/java/awt/font/JNICheck/FreeTypeScalerJNICheck.java line 29: >> >>> 27: * @summary Verifies that -Xcheck:jni issues no warnings from >>> freetypeScaler.c >>> 28: * @library /test/lib >>> 29: * @key headful >> >> What about this test is headful ? > > oh you are getting metrics for a JFrame ? That's not going to exercise any > new font code so is pointless except to make it so the test has to be headful. But `getFontMetrics()` is the primary "entry point" that generated all the JNI warnings in the first place. And I'm also not sure that we could've gotten all the warnings on Windows without `JFrame`. - PR: https://git.openjdk.java.net/jdk/pull/4572
Re: [OpenJDK 2D-Dev] RFR: 8269223: -Xcheck:jni WARNINGs working with fonts on Linux [v4]
On Thu, 12 Aug 2021 21:57:37 GMT, Phil Race wrote:
>> Maxim Kartashev has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Addressed PR comments
>>
>> 1. Added CHECK_NULL() to awt_Component.cpp
>
> test/jdk/java/awt/font/JNICheck/FreeTypeScalerJNICheck.java line 36:
>
>> 34: import java.awt.geom.Rectangle2D;
>> 35: import java.awt.image.*;
>> 36: import java.io.*;
>
> Can we get rid of all these wild card imports ?
Sure, replaced with single-class imports.
> test/jdk/java/awt/font/JNICheck/FreeTypeScalerJNICheck.java line 59:
>
>> 57: for (String ff : families)
>> 58: {
>> 59: Font font = Font.decode(ff);
>
> Gosh, does anyone still use decode() ? I keep forgetting it exists.
> You have all the family names, why not just new Font(ff, Font.PLAIN, 12) ?
OK, changed to `new Font(...)`.
-
PR: https://git.openjdk.java.net/jdk/pull/4572
Re: [OpenJDK 2D-Dev] RFR: 8269223: -Xcheck:jni WARNINGs working with fonts on Linux [v4]
On Thu, 12 Aug 2021 21:58:07 GMT, Phil Race wrote: >> Maxim Kartashev has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Addressed PR comments >> >> 1. Added CHECK_NULL() to awt_Component.cpp > > test/jdk/java/awt/font/JNICheck/FreeTypeScalerJNICheck.java line 29: > >> 27: * @summary Verifies that -Xcheck:jni issues no warnings from >> freetypeScaler.c >> 28: * @library /test/lib >> 29: * @key headful > > What about this test is headful ? oh you are getting metrics for a JFrame ? That's not going to exercise any new font code so is pointless except to make it so the test has to be headful. - PR: https://git.openjdk.java.net/jdk/pull/4572
Re: [OpenJDK 2D-Dev] RFR: 8269223: -Xcheck:jni WARNINGs working with fonts on Linux [v4]
On Mon, 19 Jul 2021 09:38:27 GMT, Maxim Kartashev
wrote:
>> Added an `ExceptionCheck()` followed by `ExceptionDescribe()` and
>> `ExceptionClear()` immediately after the Java calls made from the callback
>> function `ReadTTFontFileFunc()` in `freetypeScaler.c`.
>>
>> The exception(s) need to be cleared because we're not returning immediately
>> to Java that would've been able to handle them gracefully. And in order not
>> to loose the exception entirely (even though the return value would also
>> indicate an error condition), print out the exception with
>> `ExceptionDescribe()` to aid in debugging.
>
> Maxim Kartashev has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Addressed PR comments
>
> 1. Added CHECK_NULL() to awt_Component.cpp
test/jdk/java/awt/font/JNICheck/FreeTypeScalerJNICheck.java line 29:
> 27: * @summary Verifies that -Xcheck:jni issues no warnings from
> freetypeScaler.c
> 28: * @library /test/lib
> 29: * @key headful
What about this test is headful ?
test/jdk/java/awt/font/JNICheck/FreeTypeScalerJNICheck.java line 36:
> 34: import java.awt.geom.Rectangle2D;
> 35: import java.awt.image.*;
> 36: import java.io.*;
Can we get rid of all these wild card imports ?
test/jdk/java/awt/font/JNICheck/FreeTypeScalerJNICheck.java line 59:
> 57: for (String ff : families)
> 58: {
> 59: Font font = Font.decode(ff);
Gosh, does anyone still use decode() ? I keep forgetting it exists.
You have all the family names, why not just new Font(ff, Font.PLAIN, 12) ?
-
PR: https://git.openjdk.java.net/jdk/pull/4572
Re: [OpenJDK 2D-Dev] RFR: 8269223: -Xcheck:jni WARNINGs working with fonts on Linux [v4]
On Mon, 19 Jul 2021 09:38:27 GMT, Maxim Kartashev wrote: >> Added an `ExceptionCheck()` followed by `ExceptionDescribe()` and >> `ExceptionClear()` immediately after the Java calls made from the callback >> function `ReadTTFontFileFunc()` in `freetypeScaler.c`. >> >> The exception(s) need to be cleared because we're not returning immediately >> to Java that would've been able to handle them gracefully. And in order not >> to loose the exception entirely (even though the return value would also >> indicate an error condition), print out the exception with >> `ExceptionDescribe()` to aid in debugging. > > Maxim Kartashev has updated the pull request incrementally with one > additional commit since the last revision: > > Addressed PR comments > > 1. Added CHECK_NULL() to awt_Component.cpp I'm going to see if the test passes on the internal systems we use which I think will be mostly variout RH/OL systems for this headless test for Linux - and it also will get run on Windows Server of some kind .. I don't think macOS will hit this code path at all for this test. - PR: https://git.openjdk.java.net/jdk/pull/4572
Re: [OpenJDK 2D-Dev] RFR: 8269223: -Xcheck:jni WARNINGs working with fonts on Linux [v4]
On Mon, 19 Jul 2021 09:38:27 GMT, Maxim Kartashev wrote: >> Added an `ExceptionCheck()` followed by `ExceptionDescribe()` and >> `ExceptionClear()` immediately after the Java calls made from the callback >> function `ReadTTFontFileFunc()` in `freetypeScaler.c`. >> >> The exception(s) need to be cleared because we're not returning immediately >> to Java that would've been able to handle them gracefully. And in order not >> to loose the exception entirely (even though the return value would also >> indicate an error condition), print out the exception with >> `ExceptionDescribe()` to aid in debugging. > > Maxim Kartashev has updated the pull request incrementally with one > additional commit since the last revision: > > Addressed PR comments > > 1. Added CHECK_NULL() to awt_Component.cpp Ping. - PR: https://git.openjdk.java.net/jdk/pull/4572
