Re: RFR: 8277785: ListView scrollTo jumps to wrong location when CellHeight is changed [v4]
On Mon, 25 Apr 2022 09:00:54 GMT, Florian Kirmaier wrote: >> I agree with that observation. The mathematical perfect situation would be >> to pre-calculate the height of all items, so that the scrolbar position can >> be exact, and the content placing can be exact as well. That would be at the >> price of a major performance overhead for large lists. For small lists, this >> overhead is more acceptable. >> >> I agree that this is something that could be rather application-defined >> instead of having heuristics in the ListView. >> >> As a historical note, before >> https://bugs.openjdk.java.net/browse/JDK-8089589 was fixed, all items were >> considered of equali height for the position of the scrollbar. In the >> current case, if that precondition holds, the estimated size will be the >> real size, so in that case there is no need to calculate the height of every >> item before getting the correct total size. >> This is again something the application knows best -- even with non-fixed >> cell heights, the expected variations might be small enough and if they are >> randomly spread, the estimate will soon be "good enough". > > @johanvos > As requested, we've made a unit test, which tests this bug. > It's based on your test and our original test class. It can be added to the > ListViewTest. > You can find it, in the JDK ticket. > > Btw, adding the cells incrementally seems to make a difference - which is why > the new test class tests both cases. > > It accepts various inputs of cell sizes - and should work with any inputs. > It should catch all the cases from the original test class. > Because it works with all possible inputs - one could even make a random > test-cases generator to make sure it works in every case. > > It basically only works well, when the sizes are only 20s - in most other > cases it fails. The later reported issues were due to the fact that the estimatedSize got updated during the layoutChildren call. That makes things much more complicated and leads to discrepancies. I disabled the updates to the estimated size during the layoutChildren call, so that all operations that are happening in that call are using a consistent value of the estimated size. We now pass all the tests added by @FlorianKirmaier on the JBS issue . Those tests are great regression tests. - PR: https://git.openjdk.java.net/jfx/pull/712
Re: RFR: 8181084: JavaFX show big icons in system menu on macOS with Retina display [v3]
> 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
-
Changes:
- all: https://git.openjdk.java.net/jfx/pull/743/files
- new: https://git.openjdk.java.net/jfx/pull/743/files/3057f683..bb6d34fe
Webrevs:
- full: https://webrevs.openjdk.java.net/?repo=jfx=743=02
- incr: https://webrevs.openjdk.java.net/?repo=jfx=743=01-02
Stats: 40324 lines in 468 files changed: 22990 ins; 6262 del; 11072 mod
Patch: https://git.openjdk.java.net/jfx/pull/743.diff
Fetch: git fetch https://git.openjdk.java.net/jfx pull/743/head:pull/743
PR: https://git.openjdk.java.net/jfx/pull/743
Re: RFR: 8181084: JavaFX show big icons in system menu on macOS with Retina display [v2]
On Sat, 2 Apr 2022 10:23:23 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 three additional commits since
> the last revision:
>
> - Merge branch 'master' into JDK-8181084
> - Removing trailing whitespace.
> - Correctly scales hi-res icons in MacOS system menu items
Made a minor change to some variable names to be more consistent with how those
values are referenced elsewhere and pulled in the latest upstream changes.
Can anyone help me out with pushing this little tweak over the finishing line?
-
PR: https://git.openjdk.java.net/jfx/pull/743
