Re: RFR: 8181084: JavaFX show big icons in system menu on macOS with Retina display [v3]
On Sun, 8 May 2022 06:31:49 GMT, Paul wrote:
>> I hit on JDK-8181084 while making some changes to Scene Builder, so I
>> decided to investigate.
>>
>> Please note: I have not done any Objective-C or MacOS development before. So
>> I would really like some feedback from someone else who knows this stuff
>> better.
>>
>> Anyway, after some googling, I discovered that MacOS uses points values for
>> measurements and not pixels, so the actual fix for this issue was this block
>> in `GlassMenu.m`:-
>>
>>
>> if ((sx > 1) && (sy > 1) && (width > 1) && (height > 1)) {
>> NSSize imgSize = {width / sx, height / sy};
>> [image setSize: imgSize];
>> }
>>
>>
>> The rest of the changes were needed in order to get the `scaleX` and
>> `scaleY` values down from `PixelUtils.java` into `GlassMenu.m`.
>>
>> Before this fix:-
>>
>> > src="https://user-images.githubusercontent.com/437990/155880981-92163087-696b-4442-b047-845c276deb27.png";>
>>
>> After this fix:-
>>
>> > src="https://user-images.githubusercontent.com/437990/155880985-6bff0a06-9aee-4db2-b696-64730b0a6feb.png";>
>
> Paul has updated the pull request with a new target base due to a merge or a
> rebase. The incremental webrev excludes the unrelated changes brought in by
> the merge/rebase. The pull request contains five additional commits since the
> last revision:
>
> - Updated variable names to be a bit more consistent
> - Merge branch 'master' into JDK-8181084
> - Merge branch 'master' into JDK-8181084
> - Removing trailing whitespace.
> - Correctly scales hi-res icons in MacOS system menu items
I'll look at this in more detail later. You will likely need some error
checking when calling the JNI methods (even though they should not ever return
an error), and you might want to cache the class and field IDs it in `initIDs`.
This will need a testing on all platforms, since the change to create a Pixel
object using the scale is in shared code. Two things you could do to help with
this.
1. Enable GitHub actions runs in your personal fork of the jfx repo. You will
need to push some change (and empty commit is fine) to trigger it, once you do.
This will run the headless tests on all platforms, so is really just a "smoke
test".
2. Run the full set of tests on at least macOS by doing this:
gradle --continue --info -PFULL_TEST=true -PUSE_ROBOT=true test -x :web:test
-
PR: https://git.openjdk.java.net/jfx/pull/743
Re: RFR: 8286552: TextFormatter: UpdateValue/UpdateText is called, when no ValueConverter is set
On Tue, 10 May 2022 20:03:08 GMT, Marius Hanl wrote:
> A common reason for using the `TextFormatter` is the need for a `filter`.
> Therefore, the following constructor is typically used:
> `public TextFormatter(@NamedArg("filter") UnaryOperator filter) { ...
> }`
>
> With that, no `valueConverter` is set in the `TextFormatter`.
>
> When a `TextField` will commit his value, `TextFormatter.updateValue(...)` is
> called.
> Since `valueConverter` is null, an NPE is thrown and catched inside it and
> `TextFormatter.updateText()` is called (which will call
> `TextInputControl.updateText(...)`), which won't do anything either since
> `valueConverter` is still not set (=null).
>
> A minor improvement here is to just skip `TextFormatter.updateValue(...)` and
> therefore `TextFormatter.updateText()` (->
> `TextInputControl.updateText(...)`), since this methods won't do anything
> without a `valueConverter`.
> With that change, no exception (NPE) is thrown and catched, and therefore
> also exception breakpoints are not called (can be annoying ;)).
> This will also slightly increase the performance, as throwing exceptions is a
> (minor) bottleneck.
>
> Added tests for all `TextFormatter` functionalities, which pass before and
> after.
The fix and tests look good, with a couple requested changes.
modules/javafx.controls/src/main/java/javafx/scene/control/TextFormatter.java
line 40:
> 38: *
> 39: * A filter ({@link #getFilter()}) that can intercept and modify
> user input. This helps to keep the text
> 40: * in the desired format. A default text supplier can be used to
> provide the initial text.
I know this is a simple typo, but it is unrelated to your bug fix, and is in
public API docs, so I'd like to see it go in separately under a "Fix mistakes
in docs" bug. I filed
[JDK-8286678](https://bugs.openjdk.java.net/browse/JDK-8286678) to track this
and any other such issues that arise (as we've done for most recent releases).
modules/javafx.controls/src/main/java/javafx/scene/control/TextFormatter.java
line 202:
> 200:
> 201: void updateValue(String text) {
> 202: if (valueConverter != null &&!value.isBound()) {
Minor: please add a space between the `&&` and `!` operators.
-
PR: https://git.openjdk.java.net/jfx/pull/794
[jfx17u] Integrated: 8283218: Update GStreamer to 1.20.1
On Mon, 9 May 2022 21:07:39 GMT, Kevin Rushforth wrote: > Almost clean backport to `jfx17u`. Tested in connection with libffi update in > the `test-kcr-17.0.4` branch. > > The only difference from the mainline patch is that the following file is not > present in `jfx17u`, so that part of the patch was skipped: > > > modules/javafx.media/src/main/native/gstreamer/gstreamer-lite/gst-plugins-good/gst/audioparsers/gstaacparse.c This pull request has now been integrated. Changeset: 103cac04 Author:Kevin Rushforth URL: https://git.openjdk.java.net/jfx17u/commit/103cac04472d3eddb1f28a6a0b4f8d60f5ce3d7f Stats: 38921 lines in 412 files changed: 22007 ins; 5981 del; 10933 mod 8283218: Update GStreamer to 1.20.1 8283403: Update Glib to 2.72.0 Reviewed-by: jvos Backport-of: c4b1a72cc4d9253d1320d83281d50fb1f3bb6145 - PR: https://git.openjdk.java.net/jfx17u/pull/54
[jfx17u] Integrated: 8280840: Update libFFI to 3.4.2
On Mon, 9 May 2022 21:05:17 GMT, Kevin Rushforth wrote: > Clean backport to `jfx17u`. Tested in connection with gstreamer / glib update > in the `test-kcr-17.0.4` branch. This pull request has now been integrated. Changeset: 1c376ebd Author:Kevin Rushforth URL: https://git.openjdk.java.net/jfx17u/commit/1c376ebd51d59ae7b617cb84c5f1e04a80d87b0a Stats: 3475 lines in 34 files changed: 927 ins; 38 del; 2510 mod 8280840: Update libFFI to 3.4.2 Backport-of: 1beb3235223452929ec951ee18dd30a5345307cf - PR: https://git.openjdk.java.net/jfx17u/pull/53
