On Sun, 8 May 2022 06:31:49 GMT, Paul <d...@openjdk.java.net> 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:-
>> 
>> <img width="239" alt="Screenshot 2022-02-26 at 22 16 30" 
>> src="https://user-images.githubusercontent.com/437990/155880981-92163087-696b-4442-b047-845c276deb27.png";>
>> 
>> After this fix:-
>> 
>> <img width="218" alt="Screenshot 2022-02-26 at 22 18 17" 
>> 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

Reply via email to