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 [v5]
On Fri, 13 Aug 2021 12:27:51 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. Optimized imports in the test. > 2. Got rid of Font.decode(). Marked as reviewed by prr (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/4572
[OpenJDK 2D-Dev] Integrated: 8205138: Remove Applet references from Font2DTest
On Mon, 19 Jul 2021 20:50:14 GMT, Phil Race wrote: > Applet support was removed already but the .html file was left as well as > docs on applet issues > and a parameter only relevant to applets. This pull request has now been integrated. Changeset: 0af645aa Author:Phil Race URL: https://git.openjdk.java.net/jdk/commit/0af645aa4fd138861a51b58dec4182679640776a Stats: 106 lines in 3 files changed: 0 ins; 96 del; 10 mod 8205138: Remove Applet references from Font2DTest Reviewed-by: serb, psadhukhan - PR: https://git.openjdk.java.net/jdk/pull/4831
Re: [OpenJDK 2D-Dev] RFR: 8269951: [macos] Focus not painted in JButton when setBorderPainted(false) is invoked [v3]
On Fri, 13 Aug 2021 12:15:38 GMT, Prasanta Sadhukhan wrote: > UIManager.getColor(). should suffice Fixed. > But I am not sure with this hardcoded values..Can't we leverage viewRect or > textRect to get the required coordinates? viewRect is set to be exactly (0, 0, b.width, b.height). Insets are buried deep inside JRS classes and used as hardcoded valued, i can not get them from there and both textRect and iconRect are not representing what needs to be drawn to be consistent with the way OS draws focus. Plus they are being initialized only later down the code and at the time of this call they are empty. Not that it would matter. - PR: https://git.openjdk.java.net/jdk/pull/5082
Re: [OpenJDK 2D-Dev] RFR: 8269951: [macos] Focus not painted in JButton when setBorderPainted(false) is invoked [v4]
> Initial implementation and a test case. > > The problem is that Aqua LaF shows the focused component with the glow on the > border, hence when the border is not painted the foxus is not displayed. The > idea is to paint the glowing border on the focused component anyways. Alexander Zuev has updated the pull request incrementally with one additional commit since the last revision: Remove getLookAndFeelDefaults() call - Changes: - all: https://git.openjdk.java.net/jdk/pull/5082/files - new: https://git.openjdk.java.net/jdk/pull/5082/files/fefcd37d..df34ab0b Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=5082&range=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=5082&range=02-03 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/5082.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5082/head:pull/5082 PR: https://git.openjdk.java.net/jdk/pull/5082
Re: [OpenJDK 2D-Dev] RFR: 8269223: -Xcheck:jni WARNINGs working with fonts on Linux [v3]
On Fri, 13 Aug 2021 12:11:15 GMT, Maxim Kartashev wrote: >> Does it actually suppress the "Xcheck:jni" or it clears a raised exception? >> If an exception is still "raised" after this call we should do some >> additional steps to log/clean it. > > Yes, the exception will still be "raised" after this call. Since there are no > JNI calls (at least those that are forbidden to make with an exception > pending) between here and return back to Java, I believe no additional steps > are necessary. Then I suggest logging them via some of the trace macros. - PR: https://git.openjdk.java.net/jdk/pull/4572
Re: [OpenJDK 2D-Dev] RFR: 8266079: Lanai: AlphaComposite shows differences on Metal compared to OpenGL [v4]
On Fri, 13 Aug 2021 14:30:58 GMT, Alexey Ushakov wrote: >> Keep MTLLayer opacity in sync with window content view >> Keep layer translucent for translucent windows > > Alexey Ushakov has updated the pull request incrementally with one additional > commit since the last revision: > > 8266079: Lanai: AlphaComposite shows differences on Metal compared to OpenGL > > Removed unnecessary initialisation of opacity Marked as reviewed by serb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/4946
Re: [OpenJDK 2D-Dev] RFR: JDK-8272374: doclint should report missing "body" comments
On Fri, 13 Aug 2021 03:51:23 GMT, Jonathan Gibbons wrote: > Please review a relatively simple update to have doclnt check for empty > "descriptions" -- the body of a doc comment, before the block tags. > > It is already the case that doclint checks for missing/empty descriptions in > block tags, like @param, @return, etc. > > There are three cases to consider: > > * The comment itself is missing: this was already checked and reported as > "missing comment". > * The comment is present, but is empty ... this seems relatively unlikely, > but is nevertheless checked and reported as "empty comment". > * The comment is present but only has block tags. This is not always a > problem, since the description may be inherited, for example, in an > overriding method, but when it is an issue, it is reported as "no initial > description". > > No diagnostic is reported if the description is missing but the first tag is > `@deprecated`, because in this case, javadoc will use the body of the > `@deprecated` tag for the summary. See > [`Character.UnicodeBlock#SURROGATES_AREA`](https://docs.oracle.com/en/java/javase/15/docs/api/java.base/java/lang/Character.UnicodeBlock.html#SURROGATES_AREA) > and the corresponding entry in the summary table to see an example of this > situation. > > Diagnostics are reported if the declaration is not an overriding method and > does not begin with `@deprecated`. This occurs in a number of places in the > `java.desktop` module, often where the doc comment is of the form `/** > @return _description_ */`. To suppress those warnings for now, the > `-missing` option is added to `DOCLINT_OPTIONS` for the `java.desktop` > module. To see the effects of this anti-pattern, look at the empty > descriptions for > [`javax.swing.text.html.parser.AttributeList`](https://docs.oracle.com/en/java/javase/15/docs/api/java.desktop/javax/swing/text/html/parser/AttributeList.html#method.summary) > > Many of the doclint tests needed to be updated, because of their > over-simplistic minimal comments. It was not possible, in general, to avoid > updating the source code while preserving line numbers, so in many cases, the > golden `*.out` files had to be updated as well. > > A new test is added, focussing on the different forms of empty/missing > descriptions, as described above. I tested this by using it to generate the JavaFX docs. We have 62 new warnings for methods with empty descriptions that we otherwise would not have easily found. - Marked as reviewed by kcr (Author). PR: https://git.openjdk.java.net/jdk/pull/5106
Re: [OpenJDK 2D-Dev] RFR: 8266079: Lanai: AlphaComposite shows differences on Metal compared to OpenGL [v4]
On Fri, 13 Aug 2021 01:46:20 GMT, Sergey Bylokhov wrote: >> Did you mean to make MTLLayer opaque by default? Yes, I did it in the latest >> version. > > Then why do you need " platformWindow.setOpaque(!isTranslucent());" above? Ah, I see. Updated. - PR: https://git.openjdk.java.net/jdk/pull/4946
Re: [OpenJDK 2D-Dev] RFR: 8266079: Lanai: AlphaComposite shows differences on Metal compared to OpenGL [v4]
> Keep MTLLayer opacity in sync with window content view > Keep layer translucent for translucent windows Alexey Ushakov has updated the pull request incrementally with one additional commit since the last revision: 8266079: Lanai: AlphaComposite shows differences on Metal compared to OpenGL Removed unnecessary initialisation of opacity - Changes: - all: https://git.openjdk.java.net/jdk/pull/4946/files - new: https://git.openjdk.java.net/jdk/pull/4946/files/f7a003af..02056138 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=4946&range=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=4946&range=02-03 Stats: 1 line in 1 file changed: 0 ins; 1 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/4946.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4946/head:pull/4946 PR: https://git.openjdk.java.net/jdk/pull/4946
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 [v5]
> 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. Optimized imports in the test. 2. Got rid of Font.decode(). - Changes: - all: https://git.openjdk.java.net/jdk/pull/4572/files - new: https://git.openjdk.java.net/jdk/pull/4572/files/050c2e97..13b08686 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=4572&range=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=4572&range=03-04 Stats: 7 lines in 1 file changed: 2 ins; 0 del; 5 mod Patch: https://git.openjdk.java.net/jdk/pull/4572.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4572/head:pull/4572 PR: https://git.openjdk.java.net/jdk/pull/4572
Re: [OpenJDK 2D-Dev] RFR: 8269951: [macos] Focus not painted in JButton when setBorderPainted(false) is invoked [v3]
On Fri, 13 Aug 2021 06:18:49 GMT, Alexander Zuev wrote:
>> Initial implementation and a test case.
>>
>> The problem is that Aqua LaF shows the focused component with the glow on
>> the border, hence when the border is not painted the foxus is not displayed.
>> The idea is to paint the glowing border on the focused component anyways.
>
> Alexander Zuev has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Update copyright year
src/java.desktop/macosx/classes/com/apple/laf/AquaButtonUI.java line 338:
> 336:
> 337: }
> 338: Color ringColor =
> UIManager.getLookAndFeelDefaults().getColor("Focus.color");
I guess we normally call like this in Basic L&F which is extended by different
L&Fs so that it will pick up the defaults from the particular L&F in question,
otherwise UIManager.getColor(). should suffice as Focus.color is defined in
AquaLookAndFeel.
But I am not sure with this hardcoded values..Can't we leverage viewRect or
textRect to get the required coordinates?
-
PR: https://git.openjdk.java.net/jdk/pull/5082
Re: [OpenJDK 2D-Dev] RFR: 8269223: -Xcheck:jni WARNINGs working with fonts on Linux [v3]
On Thu, 12 Aug 2021 21:57:58 GMT, Sergey Bylokhov wrote: >> Yes, the `ExceptionCheck()` call will silence the warnings from >> `-Xcheck:jni`. > > Does it actually suppress the "Xcheck:jni" or it clears a raised exception? > If an exception is still "raised" after this call we should do some > additional steps to log/clean it. Yes, the exception will still be "raised" after this call. Since there are no JNI calls (at least those that are forbidden to make with an exception pending) between here and return back to Java, I believe no additional steps are necessary. - PR: https://git.openjdk.java.net/jdk/pull/4572
