Re: RFR: 8228363: ContextMenu.show with side=TOP does not work the first time in the presence of CSS
On Wed, 20 Jan 2021 16:37:04 GMT, Kevin Rushforth wrote: >> By using the anchor location facility of PopupWindows we can avoid >> miscalculation of the >> menu's height entirely. >> This fix also cleans up some documentation issues. >> This fix introduces tests that check the correct positioning >> (test_position_*) >> test_position_withCSS reproduces the problem that is fixed with this patch. >> The other test_position_* cases serve as "proof" that no regressions are >> introduces. >> They work before and after the fix is introduced. > > This changes the specification in a way that will require prior discussion,. > It also will need a CSR. I recommend that you follow the instructions in the earlier comment about pushing these changes to a new branch, resetting your master branch, and creating a new PR from your new branch. - PR: https://git.openjdk.java.net/jfx/pull/381
Re: RFR: 8228363: ContextMenu.show with side=TOP does not work the first time in the presence of CSS
On Wed, 20 Jan 2021 16:39:45 GMT, Kevin Rushforth wrote: > I recommend that you follow the instructions in the earlier comment about > pushing these changes to a new branch, resetting your master branch, and > creating a new PR from your new branch. Ah yes, will try to do so tomorrow. - PR: https://git.openjdk.java.net/jfx/pull/381
Re: RFR: JDK-8259718 Remove the Marlin rasterizer (single-precision) [v3]
On Tue, 19 Jan 2021 15:06:00 GMT, Laurent Bourgès wrote: >> Changes: >> - Removed single-precision Marlin renderer >> - class name refactoring >> - fix prism >> - copyright year > > Laurent Bourgès has updated the pull request incrementally with one > additional commit since the last revision: > > updated Version Looks good. As a sanity check to make reviewing the renamed files easier, I split the patch locally into two commits -- one to remove the old single-precision classes and one with the rest of the changes, including the refactoring. This shows the diffs relative to the current double version, and I confirmed that all the refactoring was as expected. - Marked as reviewed by kcr (Lead). PR: https://git.openjdk.java.net/jfx/pull/379
Re: RFR: 8228363: ContextMenu.show with side=TOP does not work the first time in the presence of CSS
On Wed, 20 Jan 2021 16:37:04 GMT, Kevin Rushforth wrote: > This changes the specification in a way that will require prior discussion,. > It also will need a CSR. My hope is that it really doesn't change the specification in any way. All it should do is fix the bug. What part of the spec do you think this would change? - PR: https://git.openjdk.java.net/jfx/pull/381
Re: RFR: 8228363: ContextMenu.show with side=TOP does not work the first time in the presence of CSS
On Wed, 20 Jan 2021 11:57:04 GMT, Robert Lichtenberger wrote: > By using the anchor location facility of PopupWindows we can avoid > miscalculation of the > menu's height entirely. > This fix also cleans up some documentation issues. > This fix introduces tests that check the correct positioning (test_position_*) > test_position_withCSS reproduces the problem that is fixed with this patch. > The other test_position_* cases serve as "proof" that no regressions are > introduces. > They work before and after the fix is introduced. This changes the specification in a way that will require prior discussion,. It also will need a CSR. - PR: https://git.openjdk.java.net/jfx/pull/381
Re: RFR: 8228363: ContextMenu.show with side=TOP does not work the first time in the presence of CSS
On Wed, 20 Jan 2021 16:44:14 GMT, Robert Lichtenberger
wrote:
>> I recommend that you follow the instructions in the earlier comment about
>> pushing these changes to a new branch, resetting your master branch, and
>> creating a new PR from your new branch.
>
>> I recommend that you follow the instructions in the earlier comment about
>> pushing these changes to a new branch, resetting your master branch, and
>> creating a new PR from your new branch.
>
> Ah yes, will try to do so tomorrow.
Regarding the spec change, I was thinking of this section, which you removed:
* To clarify the purpose of the {@code hpos} and {@code vpos} parameters,
* consider that they are relative to the anchor node. As such, a {@code
hpos}
* and {@code vpos} of {@code CENTER} would mean that the ContextMenu
appears
* on top of the anchor, with the (0,0) position of the {@code ContextMenu}
* positioned at (0,0) of the anchor. A {@code hpos} of right would then
shift
* the {@code ContextMenu} such that its top-left (0,0) position would be
attached
* to the top-right position of the anchor.
As you pointed out, the reference to `hpos` and `vpos` is incorrect, but unless
I'm missing something, it still seems like the concept still needs to be
described.
-
PR: https://git.openjdk.java.net/jfx/pull/381
Integrated: JDK-8259718 Remove the Marlin rasterizer (single-precision)
On Sun, 17 Jan 2021 15:49:33 GMT, Laurent Bourgès wrote: > Changes: > - Removed single-precision Marlin renderer > - class name refactoring > - fix prism > - copyright year This pull request has now been integrated. Changeset: c1b14de9 Author:Laurent Bourgès URL: https://git.openjdk.java.net/jfx/commit/c1b14de9 Stats: 10877 lines in 33 files changed: 6 ins; 9689 del; 1182 mod 8259718: Remove the Marlin rasterizer (single-precision) Reviewed-by: kcr - PR: https://git.openjdk.java.net/jfx/pull/379
RFR: 8259635: Update to 610.2 version of WebKit
Update JavaFX WebKit to GTK WebKit 2.30 (610.2) - Commit messages: - 8259635: Update to 610.2 version of WebKit Changes: https://git.openjdk.java.net/jfx/pull/382/files Webrev: https://webrevs.openjdk.java.net/?repo=jfx=382=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8259635 Stats: 257339 lines in 5451 files changed: 125161 ins; 93742 del; 38436 mod Patch: https://git.openjdk.java.net/jfx/pull/382.diff Fetch: git fetch https://git.openjdk.java.net/jfx pull/382/head:pull/382 PR: https://git.openjdk.java.net/jfx/pull/382
Re: RFR: 8228363: ContextMenu.show with side=TOP does not work the first time in the presence of CSS
On Wed, 20 Jan 2021 16:49:35 GMT, Kevin Rushforth wrote:
> Regarding the spec change, I was thinking of this section, which you removed:
>
> ```
> * To clarify the purpose of the {@code hpos} and {@code vpos} parameters,
> * consider that they are relative to the anchor node. As such, a {@code
> hpos}
> * and {@code vpos} of {@code CENTER} would mean that the ContextMenu
> appears
> * on top of the anchor, with the (0,0) position of the {@code
> ContextMenu}
> * positioned at (0,0) of the anchor. A {@code hpos} of right would then
> shift
> * the {@code ContextMenu} such that its top-left (0,0) position would be
> attached
> * to the top-right position of the anchor.
> ```
>
> As you pointed out, the reference to `hpos` and `vpos` is incorrect, but
> unless I'm missing something, it still seems like the concept still needs to
> be described.
I assume that a very long time ago, there were hpos and vpos parameters for
this method. And that really required an explanation, because then you could
e.g. specify hpos=RIGHT and vpos=BOTTOM, which would give a very strange
position for the context menu (the top left corner of the menu would be in the
bottom right corner of the button).
It also was very counter-intuitive that giving CENTER/CENTER for hpos/vpos
would put the top left corner of the menu at the top left corner of the button.
However all this confusion was ended when Side was used instead of HPos/VPos.
At that very moment all the confusing cases went away. No more BOTTOM/RIGHT or
CENTER/CENTER placement.
So I think we can really just remove the (obsolete) explanation of hPos/vPos.
-
PR: https://git.openjdk.java.net/jfx/pull/381
Integrated: 8228363: ContextMenu.show with side=TOP does not work the first time in the presence of CSS
On Wed, 20 Jan 2021 11:57:04 GMT, Robert Lichtenberger wrote: > By using the anchor location facility of PopupWindows we can avoid > miscalculation of the > menu's height entirely. > This fix also cleans up some documentation issues. > This fix introduces tests that check the correct positioning (test_position_*) > test_position_withCSS reproduces the problem that is fixed with this patch. > The other test_position_* cases serve as "proof" that no regressions are > introduces. > They work before and after the fix is introduced. This pull request has now been integrated. Changeset: d8bb41d1 Author:Jose Pereda URL: https://git.openjdk.java.net/jfx/commit/d8bb41d1 Stats: 2 lines in 1 file changed: 1 ins; 0 del; 1 mod 8165749: java.lang.RuntimeException: dndGesture.dragboard is null in dragDrop Reviewed-by: kcr, arapte - PR: https://git.openjdk.java.net/jfx/pull/381
RFR: 8228363: ContextMenu.show with side=TOP does not work the first time in the presence of CSS
By using the anchor location facility of PopupWindows we can avoid miscalculation of the menu's height entirely. This fix also cleans up some documentation issues. This fix introduces tests that check the correct positioning (test_position_) test_position_withCSS reproduces the problem that is fixed with this patch. The other test_position_ cases serve as "proof" that no regressions are introduces. They work before and after the fix is introduced. - Commit messages: - 8228363: ContextMenu.show with side=TOP does not work the first time in the presence of CSS - Merge pull request #1 from openjdk/master - Merge remote-tracking branch 'upstream/master' - Merge remote-tracking branch 'upstream/master' - Merge remote-tracking branch 'upstream/master' - Merge remote-tracking branch 'upstream/master' - Merge remote-tracking branch 'upstream/master' - Merge remote-tracking branch 'upstream/master' - 8232524: Test cleanup: terminate background thread upon failure. - 8232524: SynchronizedObservableMap cannot be be protected for copying/iterating - ... and 1 more: https://git.openjdk.java.net/jfx/compare/d8bb41d1...48d6453a Changes: https://git.openjdk.java.net/jfx/pull/383/files Webrev: https://webrevs.openjdk.java.net/?repo=jfx=383=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8228363 Stats: 144 lines in 2 files changed: 115 ins; 14 del; 15 mod Patch: https://git.openjdk.java.net/jfx/pull/383.diff Fetch: git fetch https://git.openjdk.java.net/jfx pull/383/head:pull/383 PR: https://git.openjdk.java.net/jfx/pull/383
Re: RFR: 8228363: ContextMenu.show with side=TOP does not work the first time in the presence of CSS
On Thu, 21 Jan 2021 06:37:59 GMT, Robert Lichtenberger
wrote:
>> Regarding the spec change, I was thinking of this section, which you removed:
>>
>> * To clarify the purpose of the {@code hpos} and {@code vpos}
>> parameters,
>> * consider that they are relative to the anchor node. As such, a {@code
>> hpos}
>> * and {@code vpos} of {@code CENTER} would mean that the ContextMenu
>> appears
>> * on top of the anchor, with the (0,0) position of the {@code
>> ContextMenu}
>> * positioned at (0,0) of the anchor. A {@code hpos} of right would then
>> shift
>> * the {@code ContextMenu} such that its top-left (0,0) position would
>> be attached
>> * to the top-right position of the anchor.
>>
>> As you pointed out, the reference to `hpos` and `vpos` is incorrect, but
>> unless I'm missing something, it still seems like the concept still needs to
>> be described.
>
>> Regarding the spec change, I was thinking of this section, which you removed:
>>
>> ```
>> * To clarify the purpose of the {@code hpos} and {@code vpos}
>> parameters,
>> * consider that they are relative to the anchor node. As such, a {@code
>> hpos}
>> * and {@code vpos} of {@code CENTER} would mean that the ContextMenu
>> appears
>> * on top of the anchor, with the (0,0) position of the {@code
>> ContextMenu}
>> * positioned at (0,0) of the anchor. A {@code hpos} of right would then
>> shift
>> * the {@code ContextMenu} such that its top-left (0,0) position would
>> be attached
>> * to the top-right position of the anchor.
>> ```
>>
>> As you pointed out, the reference to `hpos` and `vpos` is incorrect, but
>> unless I'm missing something, it still seems like the concept still needs to
>> be described.
>
> I assume that a very long time ago, there were hpos and vpos parameters for
> this method. And that really required an explanation, because then you could
> e.g. specify hpos=RIGHT and vpos=BOTTOM, which would give a very strange
> position for the context menu (the top left corner of the menu would be in
> the bottom right corner of the button).
> It also was very counter-intuitive that giving CENTER/CENTER for hpos/vpos
> would put the top left corner of the menu at the top left corner of the
> button.
> However all this confusion was ended when Side was used instead of HPos/VPos.
> At that very moment all the confusing cases went away. No more BOTTOM/RIGHT
> or CENTER/CENTER placement.
> So I think we can really just remove the (obsolete) explanation of hPos/vPos.
I've just "recreated" this PR as https://github.com/openjdk/jfx/pull/383 which
is based on a separate branch as suggested.
-
PR: https://git.openjdk.java.net/jfx/pull/381
Re: RFR: 8228363: ContextMenu.show with side=TOP does not work the first time in the presence of CSS [v2]
> By using the anchor location facility of PopupWindows we can avoid > miscalculation of the > menu's height entirely. > This fix also cleans up some documentation issues. > This fix introduces tests that check the correct positioning (test_position_*) > test_position_withCSS reproduces the problem that is fixed with this patch. > The other test_position_* cases serve as "proof" that no regressions are > introduces. > They work before and after the fix is introduced. Robert Lichtenberger has refreshed the contents of this pull request, and previous commits have been removed. The incremental views will show differences compared to the previous content of the PR. - Changes: - all: https://git.openjdk.java.net/jfx/pull/381/files - new: https://git.openjdk.java.net/jfx/pull/381/files/48d6453a..d8bb41d1 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jfx=381=01 - incr: https://webrevs.openjdk.java.net/?repo=jfx=381=00-01 Stats: 144 lines in 2 files changed: 14 ins; 115 del; 15 mod Patch: https://git.openjdk.java.net/jfx/pull/381.diff Fetch: git fetch https://git.openjdk.java.net/jfx pull/381/head:pull/381 PR: https://git.openjdk.java.net/jfx/pull/381
Re: RFR: 8233678: [macos 10.15] System menu bar does not work initially on macOS Catalina [v3]
On Mon, 7 Dec 2020 16:21:57 GMT, Kevin Rushforth wrote: >> Marked as reviewed by aghaisas (Reviewer). > > I don't yet know whether there is an alternative that would allow the menubar > to work without deactivating the app, but I filed a follow-up bug to track > this: > > [JDK-8257835](https://bugs.openjdk.java.net/browse/JDK-8257835): Brief flash > in terminal window when launching FX app on macOS > > It seems like it might be possible to fix this, since AWT initialization > doesn't run into this. I note that AWT uses an Apple-provided "JavaRuntime > Support" (JRS) Framework. We would need to see whether there is something > else possible using public macOS APIs. @tomsontom Thanks for letting me know. He reached to me directly as well, so I'll plan to add the information to the follow-on bug, [JDK-8257835](https://bugs.openjdk.java.net/browse/JDK-8257835). - PR: https://git.openjdk.java.net/jfx/pull/361
Re: RFR: 8252935: Add treeShowing listener only when needed
On Tue, 8 Sep 2020 20:16:20 GMT, Kevin Rushforth wrote: >> @kevinrushforth >> >> I have another working alternative, but won't make a PR for it until it is >> more clear which direction the TableView / TreeTableView performance fixes >> are going to take. >> >> The alternative would convert the `treeShowing` property in `Node` into a >> "lazy" property (something which JavaFX does not support directly at the >> moment). A lazy property only binds to its dependencies when it is observed >> itself (so in this specific case, only when PopupWindow or >> ProgressIndicatorSkin are making use of it). >> >> This means the property stays a part of `NodeHelper` but will only register >> its listeners on Window and Scene when it is observed itself. Such lazy >> properties could be of great use in JavaFX in general, not just in this case. > > @hjohn Per [this > message](https://mail.openjdk.java.net/pipermail/openjfx-dev/2020-September/027534.html) > on the openjfx-dev mailing list, I have filed a new JBS issue for this PR to > use. Please change the title to: > > 8252935: Add treeShowing listener only when needed So, will this actually get reviewed? - PR: https://git.openjdk.java.net/jfx/pull/185
RFR: 8228363: ContextMenu.show with side=TOP does not work the first time in the presence of CSS
By using the anchor location facility of PopupWindows we can avoid miscalculation of the menu's height entirely. This fix also cleans up some documentation issues. This fix introduces tests that check the correct positioning (test_position_*) test_position_withCSS reproduces the problem that is fixed with this patch. The other test_position_* cases serve as "proof" that no regressions are introduces. They work before and after the fix is introduced. - Commit messages: - 8228363: ContextMenu.show with side=TOP does not work the first time in the presence of CSS - Merge pull request #1 from openjdk/master - Merge remote-tracking branch 'upstream/master' - Merge remote-tracking branch 'upstream/master' - Merge remote-tracking branch 'upstream/master' - Merge remote-tracking branch 'upstream/master' - Merge remote-tracking branch 'upstream/master' - Merge remote-tracking branch 'upstream/master' - 8232524: Test cleanup: terminate background thread upon failure. - 8232524: SynchronizedObservableMap cannot be be protected for copying/iterating - ... and 1 more: https://git.openjdk.java.net/jfx/compare/d8bb41d1...48d6453a Changes: https://git.openjdk.java.net/jfx/pull/381/files Webrev: https://webrevs.openjdk.java.net/?repo=jfx=381=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8228363 Stats: 144 lines in 2 files changed: 115 ins; 14 del; 15 mod Patch: https://git.openjdk.java.net/jfx/pull/381.diff Fetch: git fetch https://git.openjdk.java.net/jfx pull/381/head:pull/381 PR: https://git.openjdk.java.net/jfx/pull/381
