Re: [Rev 03] RFR: 8130738: Add tabSize property to Text and TextFlow

[Kevin Rushforth]

On Wed, 11 Dec 2019 01:04:15 GMT, Scott Palmer  wrote:

>> As a follow-on point to the missing public method in TextFlow, can you add 
>> unit tests for the API methods on both `Text` and `TextFlow`? A good way to 
>> do that is to have a test for all combinations of setting the value via the 
>> setter and the property method, and getting the value via the getter and the 
>> property method.
> 
> The unit tests that were already added to `TextTest.java` cover the new 
> methods on Text, but not in every combination.  I'll add a couple more to 
> ensure all combinations are covered. `TextFlow` has no existing unit tests 
> that I could find!  Shall I add a new `TextFlowText.java` file for just these 
> tests?
> 
> The apps/toys folder seems to use ant-based projects.  The existing ones 
> don't build for me. If I made a new project there could I use the JavaFX 
> Gradle plugin and configure it to get JavaFX from the build/sdk folder?

I think a new `TextFlowTest.java` would be a good place for those tests.

Our build is set up to use `ant` so if you want to wire it up to the build, 
you'll need that (it should be as simple as having `ANT_HOME` set to 
`apache-ant-1.10.5`). It would be fine to defer this if you can't get it 
working.

-

PR: https://git.openjdk.java.net/jfx/pull/32

Re: [Rev 01] RFR: 8227808: Make GTK3 libraries mandatory for building on Linux

[Kevin Rushforth]

On Wed, 11 Dec 2019 01:18:55 GMT, Ambarish Rapte  wrote:

>> Need for the change 
>> [[JDK-8227808](https://bugs.openjdk.java.net/browse/JDK-8227808)]:
>> Currently GTK3 is the default on Linux. However, there is build logic that 
>> will skip building glassgtk3 if the gtk3 development libraries are not 
>> installed. This creates a situation where the build will succeed, but the 
>> resulting JavaFX library will fail to run unless you explicitly force GTK2 
>> with "-Djdk.gtk.version=2". 
>> 
>> How to verify the change:
>> On a Linux system without gtk2 and gtk3 packages,
>> 1. Clone jfx and run gradle sdk.
>> -> Build should fail with an error for missing GTK3 packages.
>> 
>> 2. Install gtk3 packages, remove build directory and run gradle clean sdk.
>> -> Build should fail with an error for missing GTK2 packages.
>> 
>> 3. Install gtk2 packages, remove build directory and run gradle clean sdk.
>> -> Build should finish without any error.
>> 
>> Build verification:
>> 1. Run a JavaFX application with -Djdk.gtk.verbose=true
>> Verify the verbose message, the default glassgtk3 library should be used.
>> 
>> 2. Run a JavaFX application with -Djdk.gtk.verbose=true -Djdk.gtk.version=2
>> Verify the verbose message, glassgtk2 library should be used.
>> 
>> 3. Run a JavaFX application with -Djdk.gtk.verbose=true -Djdk.gtk.version=3
>> Verify the verbose message, glassgtk3 library should be used.
> 
> The pull request has been updated with 1 additional commit.



-

Marked as reviewed by kcr (Lead).

PR: https://git.openjdk.java.net/jfx/pull/61

Re: [Rev 01] RFR: 8227808: Make GTK3 libraries mandatory for building on Linux

[Kevin Rushforth]

On Wed, 11 Dec 2019 01:18:42 GMT, Kevin Rushforth  wrote:

>> The pull request has been updated with 1 additional commit.
> 
> 

This will also need a second review from @johanvos

-

PR: https://git.openjdk.java.net/jfx/pull/61

Re: [Rev 03] RFR: 8130738: Add tabSize property to Text and TextFlow

[Scott Palmer]

On Tue, 10 Dec 2019 23:51:48 GMT, Kevin Rushforth  wrote:

>>> Should this PR also add a tabSize property to controls such as TextArea? Or 
>>> should that be a different PR after this one is merged?
>> 
>> This would need to be a new enhancement and would first need to be discussed 
>> on the openjfx-dev mailing list. There are additional things to consider in 
>> order to support tab size for controls, and it isn't clear whether there is 
>> enough benefit in doing it.
> 
> As a follow-on point to the missing public method in TextFlow, can you add 
> unit tests for the API methods on both `Text` and `TextFlow`? A good way to 
> do that is to have a test for all combinations of setting the value via the 
> setter and the property method, and getting the value via the getter and the 
> property method.

The unit tests that were already added to `TextTest.java` cover the new methods 
on Text, but not in every combination.  I'll add a couple more to ensure all 
combinations are covered. `TextFlow` has no existing unit tests that I could 
find!  Shall I add a new `TextFlowText.java` file for just these tests?

The apps/toys folder seems to use ant-based projects.  The existing ones don't 
build for me. If I made a new project there could I use the JavaFX Gradle 
plugin and configure it to get JavaFX from the build/sdk folder?

-

PR: https://git.openjdk.java.net/jfx/pull/32

Re: [Rev 03] RFR: 8225571: Port DND source to use GTK instead of GDK

[Thiago Milczarek Sayao]

> https://bugs.openjdk.java.net/browse/JDK-8225571
> 
> To run tests (on the root of the source tree):
> ./gradlew apps
> java @build/run.args -cp apps/toys/DragDrop/dist/DragDrop.jar 
> dragdrop.DragDropWithControls
> java @build/run.args -cp apps/toys/DragDrop/dist/DragDrop.jar 
> dragdrop.DragDropColor
> 
> java -Djdk.gtk.version=2 @build/run.args -cp 
> apps/toys/DragDrop/dist/DragDrop.jar dragdrop.DragDropWithControls
> java -Djdk.gtk.version=2 @build/run.args -cp 
> apps/toys/DragDrop/dist/DragDrop.jar dragdrop.DragDropColor

Previous commits in this pull request have been removed, probably due to a 
force push. The incremental views will show differences compared to the 
previous content of the PR.

-

Added commits:
 - 47155b08: Remove blank line

Changes:
  - all: https://git.openjdk.java.net/jfx/pull/4/files
  - new: https://git.openjdk.java.net/jfx/pull/4/files/554734c0..47155b08

Webrevs:
 - full: https://webrevs.openjdk.java.net/jfx/4/webrev.03
 - incr: https://webrevs.openjdk.java.net/jfx/4/webrev.02-03

  Stats: 2 lines in 2 files changed: 0 ins; 0 del; 2 mod
  Patch: https://git.openjdk.java.net/jfx/pull/4.diff
  Fetch: git fetch https://git.openjdk.java.net/jfx pull/4/head:pull/4

PR: https://git.openjdk.java.net/jfx/pull/4

Re: [Rev 02] RFR: 8225571: Port DND source to use GTK instead of GDK

[Thiago Milczarek Sayao]

> https://bugs.openjdk.java.net/browse/JDK-8225571
> 
> To run tests (on the root of the source tree):
> ./gradlew apps
> java @build/run.args -cp apps/toys/DragDrop/dist/DragDrop.jar 
> dragdrop.DragDropWithControls
> java @build/run.args -cp apps/toys/DragDrop/dist/DragDrop.jar 
> dragdrop.DragDropColor
> 
> java -Djdk.gtk.version=2 @build/run.args -cp 
> apps/toys/DragDrop/dist/DragDrop.jar dragdrop.DragDropWithControls
> java -Djdk.gtk.version=2 @build/run.args -cp 
> apps/toys/DragDrop/dist/DragDrop.jar dragdrop.DragDropColor

The pull request has been updated with 1 additional commit.

-

Added commits:
 - 554734c0: Remove blank line

Changes:
  - all: https://git.openjdk.java.net/jfx/pull/4/files
  - new: https://git.openjdk.java.net/jfx/pull/4/files/d16fc2c6..554734c0

Webrevs:
 - full: https://webrevs.openjdk.java.net/jfx/4/webrev.02
 - incr: https://webrevs.openjdk.java.net/jfx/4/webrev.01-02

  Stats: 3 lines in 3 files changed: 0 ins; 1 del; 2 mod
  Patch: https://git.openjdk.java.net/jfx/pull/4.diff
  Fetch: git fetch https://git.openjdk.java.net/jfx pull/4/head:pull/4

PR: https://git.openjdk.java.net/jfx/pull/4

Re: [Rev 01] RFR: 8225571: Port DND source to use GTK instead of GDK

[Thiago Milczarek Sayao]

> https://bugs.openjdk.java.net/browse/JDK-8225571
> 
> To run tests (on the root of the source tree):
> ./gradlew apps
> java @build/run.args -cp apps/toys/DragDrop/dist/DragDrop.jar 
> dragdrop.DragDropWithControls
> java @build/run.args -cp apps/toys/DragDrop/dist/DragDrop.jar 
> dragdrop.DragDropColor
> 
> java -Djdk.gtk.version=2 @build/run.args -cp 
> apps/toys/DragDrop/dist/DragDrop.jar dragdrop.DragDropWithControls
> java -Djdk.gtk.version=2 @build/run.args -cp 
> apps/toys/DragDrop/dist/DragDrop.jar dragdrop.DragDropColor

The pull request has been updated with 1 additional commit.

-

Added commits:
 - d16fc2c6: Restore DRAG_DONE behaviour

Changes:
  - all: https://git.openjdk.java.net/jfx/pull/4/files
  - new: https://git.openjdk.java.net/jfx/pull/4/files/3d650b2b..d16fc2c6

Webrevs:
 - full: https://webrevs.openjdk.java.net/jfx/4/webrev.01
 - incr: https://webrevs.openjdk.java.net/jfx/4/webrev.00-01

  Stats: 4 lines in 1 file changed: 0 ins; 3 del; 1 mod
  Patch: https://git.openjdk.java.net/jfx/pull/4.diff
  Fetch: git fetch https://git.openjdk.java.net/jfx pull/4/head:pull/4

PR: https://git.openjdk.java.net/jfx/pull/4

Re: [Rev 05] RFR: 8207957: TableSkinUtils should not contain actual code implementation

[Kevin Rushforth]

On Mon, 9 Dec 2019 23:20:19 GMT, Hadzic Samir  wrote:

>> I would like both @aghaisas and myself to review / approve this. I will 
>> sponsor it.
>> 
>> In addition, the CSR needs to be approved before this can be integrated.
> 
> Thanks for the review. I do not have access to a computer right now, I'll
> update next week on Monday.
> 
> Le lun. 9 déc. 2019 à 17:01, Kevin Rushforth  a
> écrit :
> 
>> I would like both @aghaisas  and myself to
>> review / approve this. I will sponsor it.
>>
>> In addition, the CSR needs to be approved before this can be integrated.
>>
>> —
>> You are receiving this because you authored the thread.
>> Reply to this email directly, view it on GitHub
>> ,
>> or unsubscribe
>> 
>> .
>>

I took the liberty of updating the CSR to add the missing comma, so I could 
mark it as Reviewed. You can Finalize the CSR when you are ready.

-

PR: https://git.openjdk.java.net/jfx/pull/6

Re: [Rev 02] RFR: 8130738: Add tabSize property to Text and TextFlow

[Kevin Rushforth]

On Wed, 27 Nov 2019 10:51:10 GMT, Jeanette Winzenburg  
wrote:

>> modules/javafx.graphics/src/main/java/javafx/scene/text/Text.java line 1895:
>> 
>>> 1894: }
>>> 1895: @Override public void set(int v) { super.set((v < 
>>> 1) ? 1 : v); }
>>> 1896: @Override protected void invalidated() {
>> 
>> For mutable properties, we usually clamp on usage, so that we don't have 
>> problems binding to the value. This preserves the invariant that `set(val); 
>> get() == val` for all values. If that is what we end up doing, then this 
>> overridden method should be removed.
> 
> Hmm ... so you are saying the clamping is the responsibility of client code 
> (internal as well as external)?

In this case, yes.

-

PR: https://git.openjdk.java.net/jfx/pull/32

Re: RFR: 8225571: Port DND source to use GTK instead of GDK

[Kevin Rushforth]

On Tue, 10 Dec 2019 23:53:27 GMT, Kevin Rushforth  wrote:

>> @kevinrushforth I changed that behaviour. Since the drag does not complete i 
>> find it counter-intuitive to send the DRAG_DONE event.
>> 
>> Does it send the event on other platforms? I can test on Windows.
>> 
>> Will look into the assertion error - it does not happen on 18.04.
> 
>> Does it send the event on other platforms? I can test on Windows.
> 
> Yes, it always sends the `DRAG_DONE` event on all platforms today (I tested 
> it on Mac, Windows, and Linux).
> 
>> Will look into the assertion error - it does not happen on 18.04.
> 
> It doesn't happen on 19.10 either. Only 16.04 as far as I have tested. I'll 
> also test on Oracle Linux 7.x.

I don't get an assertion error on Oracle Linux 7.7 -- so far only on Ubuntu 
16.04. I will resume reviewing once you restore the `DRAG_DONE` behavior.

-

PR: https://git.openjdk.java.net/jfx/pull/4

Re: RFR: 8225571: Port DND source to use GTK instead of GDK

[Kevin Rushforth]

On Tue, 10 Dec 2019 19:41:05 GMT, Thiago Milczarek Sayao  
wrote:

>> I'm starting to look at this. I see at least one behavioral difference that 
>> will need to be addressed. With the existing implementation, if I drag / 
>> drop onto a target that set up to receive the drop, the transfer completes 
>> with a transfer mode of `null`. With your patch, the transfer never 
>> completes -- the `DRAG_DONE` event is never sent.
>> 
>> This can be most easily seen by running the `DragDropWithControls` app and 
>> doing one of the following two things:
>> 
>> 1.  Drag from the source rectangle and drop somewhere _other than_ the 
>> target rectangle, for example, in the status text box at the bottom.
>> 2. Change the source data format to `HTML` (uncheck `PLAINTEXT`), leaving 
>> the target at `PLAINTEXT`, then drag from the source rectangle and drop onto 
>> the target rectangle
>> 
>> In addition to the drop not happening, I get the following assertion error 
>> on Ubuntu 16.04:
>> 
>> (java:22328): Gdk-CRITICAL **: gdk_frame_clock_get_frame_time: assertion 
>> 'GDK_IS_FRAME_CLOCK (frame_clock)' failed
>> 
>> I'm using the default GTK 3 library.
> 
> @kevinrushforth I changed that behaviour. Since the drag does not complete i 
> find it counter-intuitive to send the DRAG_DONE event.
> 
> Does it send the event on other platforms? I can test on Windows.
> 
> Will look into the assertion error - it does not happen on 18.04.

> Does it send the event on other platforms? I can test on Windows.

Yes, it always sends the `DRAG_DONE` event on all platforms today (I tested it 
on Mac, Windows, and Linux).

> Will look into the assertion error - it does not happen on 18.04.

It doesn't happen on 19.10 either. Only 16.04 as far as I have tested. I'll 
also test on Oracle Linux 7.x.

-

PR: https://git.openjdk.java.net/jfx/pull/4

Re: [Rev 03] RFR: 8130738: Add tabSize property to Text and TextFlow

[Kevin Rushforth]

On Tue, 10 Dec 2019 18:54:08 GMT, Kevin Rushforth  wrote:

>> Overall this looks good to me with one "must fix" API issue and one 
>> additional minor comment.
>> 
>> In addition to the automated unit test, it might be nice to have a simple 
>> app (in `apps/toys`) with a slider to control the tab size of a Text and/or 
>> TextFlow. This could be done as a follow-on issue if you prefer.
>> 
>> Once you fix the public API issue, you can add ithe API to the CSR and I'll 
>> review it. The javadoc, public methods, and CSS additions for the two 
>> classes should go into the CSR in the specification section. See 
>> [JDK-8195139](https://bugs.openjdk.java.net/browse/JDK-8195139) for a good 
>> example.
>> 
>> @prrace still needs to review the implementation and API as well.
> 
>> Should this PR also add a tabSize property to controls such as TextArea? Or 
>> should that be a different PR after this one is merged?
> 
> This would need to be a new enhancement and would first need to be discussed 
> on the openjfx-dev mailing list. There are additional things to consider in 
> order to support tab size for controls, and it isn't clear whether there is 
> enough benefit in doing it.

As a follow-on point to the missing public method in TextFlow, can you add unit 
tests for the API methods on both `Text` and `TextFlow`? A good way to do that 
is to have a test for all combinations of setting the value via the setter and 
the property method, and getting the value via the getter and the property 
method.

-

PR: https://git.openjdk.java.net/jfx/pull/32

Re: Proposed schedule for JavaFX 14

[Kevin Rushforth]

As a reminder, RDP1 for JavaFX 14 starts on January 6, 2020 (at 23:59 
Pacific time).


With the holidays approaching, please allow sufficient time for any 
feature that needs a CSR. New features should be far enough along in the 
review process so you can finalize the CSR before next Friday, Dec 20, 
or it is likely to miss the window for this release, in which case it 
can be targeted for JavaFX 15.


During rampdown of JavaFX 14, the "master" branch of the jfx repo will 
be open for JavaFX 15 fixes.


We will follow the same process as previous releases for getting fixes 
into JavaFX 14 during rampdown. I'll send a message with detailed 
information later, but generally candidates for fixing after RDP1 are 
P1-P3 bugs (as long as they are not risky) and test or doc bugs of any 
priority.


-- Kevin


On 9/20/2019 8:00 AM, Kevin Rushforth wrote:

Here is the proposed schedule for JavaFX 14.

RDP1: Jan 6, 2020 (aka “feature freeze”)
RDP2: Feb 3, 2020
Freeze: Feb 24, 2020
GA: March 10, 2020

We plan to fork a jfx14 stabilization branch at RDP1. The GA date is 
expected to be one week ahead of JDK 14, matching what we did for 13.


Please let Johan or me know if you have any questions.

-- Kevin

Re: RFR: 8225571: Port DND source to use GTK instead of GDK

[Thiago Milczarek Sayao]

On Tue, 10 Dec 2019 16:13:06 GMT, Kevin Rushforth  wrote:

>> Just a reminder to take a look when time allows.
> 
> I'm starting to look at this. I see at least one behavioral difference that 
> will need to be addressed. With the existing implementation, if I drag / drop 
> onto a target that set up to receive the drop, the transfer completes with a 
> transfer mode of `null`. With your patch, the transfer never completes -- the 
> `DRAG_DONE` event is never sent.
> 
> This can be most easily seen by running the `DragDropWithControls` app and 
> doing one of the following two things:
> 
> 1.  Drag from the source rectangle and drop somewhere _other than_ the target 
> rectangle, for example, in the status text box at the bottom.
> 2. Change the source data format to `HTML` (uncheck `PLAINTEXT`), leaving the 
> target at `PLAINTEXT`, then drag from the source rectangle and drop onto the 
> target rectangle
> 
> In addition to the drop not happening, I get the following assertion error on 
> Ubuntu 16.04:
> 
> (java:22328): Gdk-CRITICAL **: gdk_frame_clock_get_frame_time: assertion 
> 'GDK_IS_FRAME_CLOCK (frame_clock)' failed
> 
> I'm using the default GTK 3 library.

@kevinrushforth I changed that behaviour. Since the drag does not complete i 
find it counter-intuitive to send the DRAG_DONE event.

Does it send the event on other platforms? I can test on Windows.

Will look into the assertion error - it does not happen on 18.04.

-

PR: https://git.openjdk.java.net/jfx/pull/4

Re: [Rev 03] RFR: 8130738: Add tabSize property to Text and TextFlow

[Kevin Rushforth]

On Tue, 10 Dec 2019 18:37:50 GMT, Kevin Rushforth  wrote:

>> The pull request has been updated with 1 additional commit.
> 
> Overall this looks good to me with one "must fix" API issue and one 
> additional minor comment.
> 
> In addition to the automated unit test, it might be nice to have a simple app 
> (in `apps/toys`) with a slider to control the tab size of a Text and/or 
> TextFlow. This could be done as a follow-on issue if you prefer.
> 
> Once you fix the public API issue, you can add ithe API to the CSR and I'll 
> review it. The javadoc, public methods, and CSS additions for the two classes 
> should go into the CSR in the specification section. See 
> [JDK-8195139](https://bugs.openjdk.java.net/browse/JDK-8195139) for a good 
> example.
> 
> @prrace still needs to review the implementation and API as well.

> Should this PR also add a tabSize property to controls such as TextArea? Or 
> should that be a different PR after this one is merged?

This would need to be a new enhancement and would first need to be discussed on 
the openjfx-dev mailing list. There are additional things to consider in order 
to support tab size for controls, and it isn't clear whether there is enough 
benefit in doing it.

-

PR: https://git.openjdk.java.net/jfx/pull/32

Re: [Rev 03] RFR: 8130738: Add tabSize property to Text and TextFlow

[Kevin Rushforth]

On Tue, 10 Dec 2019 18:38:04 GMT, Scott Palmer  wrote:

>> Added tabSize property to Text and TextFlow and -fx-tab-size CSS attribute 
>> to both.  TextFlow's tab size overrides that of contained Text nodes.
> 
> The pull request has been updated with 1 additional commit.

Overall this looks good to me with one "must fix" API issue and one additional 
minor comment.

In addition to the automated unit test, it might be nice to have a simple app 
(in `apps/toys`) with a slider to control the tab size of a Text and/or 
TextFlow. This could be done as a follow-on issue if you prefer.

Once you fix the public API issue, you can add ithe API to the CSR and I'll 
review it. The javadoc, public methods, and CSS additions for the three classes 
should go into the CSR in the specification section. See 
[JDK-8195139](https://bugs.openjdk.java.net/browse/JDK-8195139) for a good 
example.

@prrace still needs to review the implementation and API as well.

modules/javafx.graphics/src/main/java/com/sun/javafx/scene/text/TextLayout.java 
line 78:

> 77: 
> 78: static final int DEFAULT_TAB_SIZE = 8;
> 79: 

Although it doesn't matter, since fields of public interfaces are public by 
default, it might be clearer to add `public` here for consistency with the 
other members.

modules/javafx.graphics/src/main/java/javafx/scene/text/TextFlow.java line 509:

> 508: 
> 509: final IntegerProperty tabSizeProperty() {
> 510: if (tabSize == null) {

The `tabSizeProperty` method needs to be public in order for it to be part of 
the API and for `textSize` to be a property (which is what we want).

Also, please move `tabSizeProperty` above the set / get methods (leaving the 
javadoc comment block where it is). I realize that we aren't consistent in how 
we do this, but for new properties, we are trying to have the javadoc comments 
be on the property method (it's OK if the comment block is above the private 
instance of the `Property`), followed by the setter and getter.

-

Changes requested by kcr (Lead).

PR: https://git.openjdk.java.net/jfx/pull/32

Re: RFR: 8223296: NullPointerException in GlassScene.java at line 325

[Kevin Rushforth]

On Tue, 10 Dec 2019 07:00:50 GMT, Ambarish Rapte  wrote:

> Issue: NPE in GlassScene.frameRendered().
> 
> Cause: scenePaintListener is set in setTKScenePaintListener(), used in 
> frameRendered() and 
> set to null in dispose().
> setTKScenePaintListener() and dispose() are called on JavaFX Application 
> Thread and 
> frameRendered() is called by QuantumRenderer thread.
> setTKScenePaintListener() and frameRendered() are synchronized but dispose() 
> is not.
> 
> Fix:
> dispose() should use the synchronized setTKScenePaintListener() to set 
> scenePaintListener to null.
> 
> Verification:
> This is a very rare issue and cannot be reliably reproduced with a test case.

The change looks OK as far as it goes, meaning that it will fix the specific 
NPE reported by the bug and is looks like a safe fix.

Two questions:

1. In addition to calling the synchronized `setTKScenePaintListener` method, 
you moved the call to the beginning of `dispose`. Is there a reason you needed 
to do this?
2. Do any of the other listeners or variables that are set in `dispose` have 
the same problem (i.e., are any the rest accessed from another thread)?

-



PR: https://git.openjdk.java.net/jfx/pull/64

Re: RFR: 8225571: Port DND source to use GTK instead of GDK

[Kevin Rushforth]

On Mon, 2 Dec 2019 11:09:05 GMT, Thiago Milczarek Sayao  
wrote:

>> Preliminary review was here: 
>> [javafxports/openjdk-jfx/pull/490](https://github.com/javafxports/openjdk-jfx/pull/490)
> 
> Just a reminder to take a look when time allows.

I'm starting to look at this. I see at least one behavioral difference that 
will need to be addressed. With the existing implementation, if I drag / drop 
onto a target that set up to receive the drop, the transfer completes with a 
transfer mode of `null`. With your patch, the transfer never completes -- the 
`DRAG_DONE` event is never sent.

This can be most easily seen by running the `DragDropWithControls` app and 
doing one of the following two things:

1.  Drag from the source rectangle and drop somewhere _other than_ the target 
rectangle, for example, in the status text box at the bottom.
2. Change the source data format to `HTML` (uncheck `PLAINTEXT`), leaving the 
target at `PLAINTEXT`, then drag from the source rectangle and drop onto the 
target rectangle

In addition to the drop not happening, I get the following assertion error on 
Ubuntu 16.04:

(java:22328): Gdk-CRITICAL **: gdk_frame_clock_get_frame_time: assertion 
'GDK_IS_FRAME_CLOCK (frame_clock)' failed

I'm using the default GTK 3 library.

-

PR: https://git.openjdk.java.net/jfx/pull/4

Re: [Rev 01] RFR: 8207759: VK_ENTER not consumed by TextField when default Button exists

[Jeanette Winzenburg]

On Tue, 10 Dec 2019 13:01:15 GMT, Jeanette Winzenburg  
wrote:

>> modules/javafx.controls/src/main/java/com/sun/javafx/scene/control/behavior/TextInputControlBehavior.java
>>  line 626:
>> 
>>> 625: protected void fire(KeyEvent event) { } // TODO move to 
>>> TextFieldBehavior
>>> 626: protected void cancelEdit(KeyEvent event) { };
>>> 627: 
>> 
>> Will this code removal re-introduce JDK-8145515?
> 
> no don't think so: the fix for JDK-8145515 was to prevent the triggering of 
> parent.fireEvent, so removing the method altogether should have no effect :) 
> Could add a test to explicitly guard against this.
> 
> Please note that the old combo issue is back in the form of 
> https://bugs.openjdk.java.net/browse/JDK-8229914 - before and after fixing 
> this, combo does very bad things ;)

just to let you know: locally added test to explicitly guard against 
JDK-8145515 (and an ignored one for the regression JDK-8229914) - they are 
passing/failing the same before/after this fix. Will push later ...

-

PR: https://git.openjdk.java.net/jfx/pull/15

Re: [Rev 01] RFR: 8207759: VK_ENTER not consumed by TextField when default Button exists

[Jeanette Winzenburg]

On Tue, 10 Dec 2019 12:03:03 GMT, Ajit Ghaisas  wrote:

>> The pull request has been updated with 1 additional commit.
> 
> modules/javafx.controls/src/main/java/com/sun/javafx/scene/control/behavior/TextInputControlBehavior.java
>  line 626:
> 
>> 625: protected void fire(KeyEvent event) { } // TODO move to 
>> TextFieldBehavior
>> 626: protected void cancelEdit(KeyEvent event) { };
>> 627: 
> 
> Will this code removal re-introduce JDK-8145515?

no don't think so: the fix for JDK-8145515 was to prevent the triggering of 
parent.fireEvent, so removing the method altogether should have no effect :) 
Could add a test to explicitly guard against this.

Please note that the old combo issue is back in the form of 
https://bugs.openjdk.java.net/browse/JDK-8229914 - before and after fixing 
this, combo does very bad things ;)

-

PR: https://git.openjdk.java.net/jfx/pull/15

Re: Blank stages when running JavaFX app in a macOS virtual machine

[Kevin Rushforth]

Yes, I see that it was transferred to the JDK project yesterday evening 
(or early morning today depending on your time zone). Usually it happens 
within a day or so, but sometimes there is a delay. Btw, if there is one 
that you or anyone else has submitted that you are ready to contribute a 
fix for, let me know and I can process it.


-- Kevin


On 12/10/2019 3:17 AM, thevenet.f...@free.fr wrote:

Well yes, it is indeed my bug report, thanks!
I can't find a response in my mailbox, and searching for it it didn't yield any 
result (bug was created in JBS 9 hours ago; might not be indexed yet).
Anyway, it doesn't matter; I can now go forward with my PR.

Thanks again.
Fred.


- Mail original -
De: "Michael Paus" 
À: "openjfx-dev" 
Envoyé: Mardi 10 Décembre 2019 11:59:30
Objet: Re: Blank stages when running JavaFX app in a macOS virtual machine

Hi,
maybe you lost the response mail or it went into your trash.
I guess this is your bug report in the JBS.
https://bugs.openjdk.java.net/browse/JDK-8235627
--Michael

Am 10.12.19 um 11:33 schrieb thevenet.f...@free.fr:

Hi,

I've submitted bug report via https://bugreport.java.com/bugreport/ regarding 
the issue below about a week ago but I haven't heard from it yet.
I understand this might not be the best place to inquire about it, but since 
this prevents me from submitting a PR (as I don't have an issue number) I was 
hoping someone here could offer some guidance on how to get this moving forward.

Many thanks,
Fred.

- Mail original -
De: "thevenet fred" 
À: "Kevin Rushforth" 
Cc: "openjfx-dev" 
Envoyé: Mardi 3 Décembre 2019 18:46:57
Objet: Re: Blank stages when running JavaFX app in a macOS virtual machine

Ok thanks, will do that.

Regards,
Frederic Thevenet

- Mail original -
De: "Kevin Rushforth" 
À: "openjfx-dev" 
Envoyé: Mardi 3 Décembre 2019 18:35:49
Objet: Re: Blank stages when running JavaFX app in a macOS virtual machine

A new follow-on bug ID will be needed. We never reopen a JBS issue once
it has been resolved as fixed by a changeset that has been pushed.

-- Kevin


On 12/3/2019 7:11 AM, thevenet.f...@free.fr wrote:

Hello,

When running a JavaFX application in macOS guest VM, the main stage is 
completely blank, with the following errors: CGLChoosePixelFormat error: 10002, 
CGLCreateContext error: 10002
This behavior was observed with a macOS 10.15 guest OS on both VMWare player 
and VirtualBox, on a Windows 10 host.

This is an old issue that has already been reported several times (notably as 
JDK-8154852) but is claimed to have been fixed by JDK-8154148.
However the fix provided by JDK-8154148 is incomplete; while it does fix a JVM 
crash, it doesn't prevent an incorrect pixel format to be retrieved, which is 
the root cause for the stage being empty.

The problematic code is located at #96 in GlassView3D.m:

095: CGLError err = CGLChoosePixelFormat(attributes, &pix, &npix);
096: if ((err == kCGLNoError) && (npix == 0))
097: {
098:   const CGLPixelFormatAttribute attributes2[] =
099:   {
100: kCGLPFAAllowOfflineRenderers,
101: (CGLPixelFormatAttribute)0
102:   };
103:   err = CGLChoosePixelFormat(attributes2, &pix, &npix);
104: }


In a comment in JDK-8154148, the following claim is made: "[...] an error happens is 
when something bad or invalid has occurred. An unsatisfied pixel format request is indicated 
by npix is 0 and pix is NULL. Hence I feel it is a cleaner and clearer logic to request a 
different format when npix is 0 and err is kCGLNoError." 
(https://bugs.openjdk.java.net/browse/JDK-8154148?focusedCommentId=13980465&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-13980465)

From what I could observe, this is not necessarily true; in this case where 
a pixel format satisfying the initial conditions could not be found, the return 
code is kCGLBadPixelFormat (10002); which means we don't try to get a pixel 
format with less restrictive condition.
This suggests that a better condition for the second call to CGLChoosePixelFormat (line #103) should be 
"if (pix == NULL)" instead of "if ((err == kCGLNoError) && (npix == 0))"
This is further supported by a sample in Apple developer's documentation on how 
to choose a pixel format: 
https://developer.apple.com/library/archive/documentation/GraphicsImaging/Conceptual/OpenGL-MacProgGuide/opengl_pixelformats/opengl_pixelformats.html

I have verified that this change produces the expected behavior and I propose 
to submit a PR with this change (I have already signed the OCA)

A quick question before I do, however; should I file a new issue prior to 
submitting a PR or is it better that someone with privileges reopens 
JDK-8154148?

Regards,

Frederic Thevenet

Re: [Rev 01] RFR: 8207759: VK_ENTER not consumed by TextField when default Button exists

[Ajit Ghaisas]

On Tue, 10 Dec 2019 12:26:27 GMT, Jeanette Winzenburg  
wrote:

>> This is a fix for https://bugs.openjdk.java.net/browse/JDK-8207759
>> 
>> The issue is that default/cancel button on a scene are triggered even though 
>> a onKeyPressed handler for ENTER/CANCEL consumes the keyEvent. See the bug 
>> for details on both cause and fix.
>> 
>> There are additional/changed tests to verify the fix. All old tests are 
>> passing.
> 
> The pull request has been updated with 1 additional commit.

Changes look OK apart from one doubt I have listed.

modules/javafx.controls/src/main/java/com/sun/javafx/scene/control/behavior/TextInputControlBehavior.java
 line 626:

> 625: protected void fire(KeyEvent event) { } // TODO move to 
> TextFieldBehavior
> 626: protected void cancelEdit(KeyEvent event) { };
> 627: 

Will this code removal re-introduce JDK-8145515?

-



PR: https://git.openjdk.java.net/jfx/pull/15

Re: Blank stages when running JavaFX app in a macOS virtual machine

[thevenet . fred]

Well yes, it is indeed my bug report, thanks!
I can't find a response in my mailbox, and searching for it it didn't yield any 
result (bug was created in JBS 9 hours ago; might not be indexed yet).
Anyway, it doesn't matter; I can now go forward with my PR.

Thanks again.
Fred.  


- Mail original -
De: "Michael Paus" 
À: "openjfx-dev" 
Envoyé: Mardi 10 Décembre 2019 11:59:30
Objet: Re: Blank stages when running JavaFX app in a macOS virtual machine

Hi,
maybe you lost the response mail or it went into your trash.
I guess this is your bug report in the JBS.
https://bugs.openjdk.java.net/browse/JDK-8235627
--Michael

Am 10.12.19 um 11:33 schrieb thevenet.f...@free.fr:
> Hi,
>
> I've submitted bug report via https://bugreport.java.com/bugreport/ regarding 
> the issue below about a week ago but I haven't heard from it yet.
> I understand this might not be the best place to inquire about it, but since 
> this prevents me from submitting a PR (as I don't have an issue number) I was 
> hoping someone here could offer some guidance on how to get this moving 
> forward.
>
> Many thanks,
> Fred.
>
> - Mail original -
> De: "thevenet fred" 
> À: "Kevin Rushforth" 
> Cc: "openjfx-dev" 
> Envoyé: Mardi 3 Décembre 2019 18:46:57
> Objet: Re: Blank stages when running JavaFX app in a macOS virtual machine
>
> Ok thanks, will do that.
>
> Regards,
> Frederic Thevenet
>
> - Mail original -
> De: "Kevin Rushforth" 
> À: "openjfx-dev" 
> Envoyé: Mardi 3 Décembre 2019 18:35:49
> Objet: Re: Blank stages when running JavaFX app in a macOS virtual machine
>
> A new follow-on bug ID will be needed. We never reopen a JBS issue once
> it has been resolved as fixed by a changeset that has been pushed.
>
> -- Kevin
>
>
> On 12/3/2019 7:11 AM, thevenet.f...@free.fr wrote:
>> Hello,
>>
>> When running a JavaFX application in macOS guest VM, the main stage is 
>> completely blank, with the following errors: CGLChoosePixelFormat error: 
>> 10002, CGLCreateContext error: 10002
>> This behavior was observed with a macOS 10.15 guest OS on both VMWare player 
>> and VirtualBox, on a Windows 10 host.
>>
>> This is an old issue that has already been reported several times (notably 
>> as JDK-8154852) but is claimed to have been fixed by JDK-8154148.
>> However the fix provided by JDK-8154148 is incomplete; while it does fix a 
>> JVM crash, it doesn't prevent an incorrect pixel format to be retrieved, 
>> which is the root cause for the stage being empty.
>>
>> The problematic code is located at #96 in GlassView3D.m:
>>
>> 095: CGLError err = CGLChoosePixelFormat(attributes, &pix, &npix);
>> 096: if ((err == kCGLNoError) && (npix == 0))
>> 097: {
>> 098:   const CGLPixelFormatAttribute attributes2[] =
>> 099:   {
>> 100: kCGLPFAAllowOfflineRenderers,
>> 101: (CGLPixelFormatAttribute)0
>> 102:   };
>> 103:   err = CGLChoosePixelFormat(attributes2, &pix, &npix);
>> 104: }
>>
>>
>> In a comment in JDK-8154148, the following claim is made: "[...] an error 
>> happens is when something bad or invalid has occurred. An unsatisfied pixel 
>> format request is indicated by npix is 0 and pix is NULL. Hence I feel it is 
>> a cleaner and clearer logic to request a different format when npix is 0 and 
>> err is kCGLNoError." 
>> (https://bugs.openjdk.java.net/browse/JDK-8154148?focusedCommentId=13980465&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-13980465)
>>
>>From what I could observe, this is not necessarily true; in this case 
>> where a pixel format satisfying the initial conditions could not be found, 
>> the return code is kCGLBadPixelFormat (10002); which means we don't try to 
>> get a pixel format with less restrictive condition.
>> This suggests that a better condition for the second call to 
>> CGLChoosePixelFormat (line #103) should be "if (pix == NULL)" instead of "if 
>> ((err == kCGLNoError) && (npix == 0))"
>> This is further supported by a sample in Apple developer's documentation on 
>> how to choose a pixel format: 
>> https://developer.apple.com/library/archive/documentation/GraphicsImaging/Conceptual/OpenGL-MacProgGuide/opengl_pixelformats/opengl_pixelformats.html
>>
>> I have verified that this change produces the expected behavior and I 
>> propose to submit a PR with this change (I have already signed the OCA)
>>
>> A quick question before I do, however; should I file a new issue prior to 
>> submitting a PR or is it better that someone with privileges reopens 
>> JDK-8154148?
>>
>> Regards,
>>
>> Frederic Thevenet

Re: Blank stages when running JavaFX app in a macOS virtual machine

[Michael Paus]

Hi,
maybe you lost the response mail or it went into your trash.
I guess this is your bug report in the JBS.
https://bugs.openjdk.java.net/browse/JDK-8235627
--Michael

Am 10.12.19 um 11:33 schrieb thevenet.f...@free.fr:

Hi,

I've submitted bug report via https://bugreport.java.com/bugreport/ regarding 
the issue below about a week ago but I haven't heard from it yet.
I understand this might not be the best place to inquire about it, but since 
this prevents me from submitting a PR (as I don't have an issue number) I was 
hoping someone here could offer some guidance on how to get this moving forward.

Many thanks,
Fred.

- Mail original -
De: "thevenet fred" 
À: "Kevin Rushforth" 
Cc: "openjfx-dev" 
Envoyé: Mardi 3 Décembre 2019 18:46:57
Objet: Re: Blank stages when running JavaFX app in a macOS virtual machine

Ok thanks, will do that.

Regards,
Frederic Thevenet

- Mail original -
De: "Kevin Rushforth" 
À: "openjfx-dev" 
Envoyé: Mardi 3 Décembre 2019 18:35:49
Objet: Re: Blank stages when running JavaFX app in a macOS virtual machine

A new follow-on bug ID will be needed. We never reopen a JBS issue once
it has been resolved as fixed by a changeset that has been pushed.

-- Kevin


On 12/3/2019 7:11 AM, thevenet.f...@free.fr wrote:

Hello,

When running a JavaFX application in macOS guest VM, the main stage is 
completely blank, with the following errors: CGLChoosePixelFormat error: 10002, 
CGLCreateContext error: 10002
This behavior was observed with a macOS 10.15 guest OS on both VMWare player 
and VirtualBox, on a Windows 10 host.

This is an old issue that has already been reported several times (notably as 
JDK-8154852) but is claimed to have been fixed by JDK-8154148.
However the fix provided by JDK-8154148 is incomplete; while it does fix a JVM 
crash, it doesn't prevent an incorrect pixel format to be retrieved, which is 
the root cause for the stage being empty.

The problematic code is located at #96 in GlassView3D.m:

095: CGLError err = CGLChoosePixelFormat(attributes, &pix, &npix);
096: if ((err == kCGLNoError) && (npix == 0))
097: {
098:   const CGLPixelFormatAttribute attributes2[] =
099:   {
100: kCGLPFAAllowOfflineRenderers,
101: (CGLPixelFormatAttribute)0
102:   };
103:   err = CGLChoosePixelFormat(attributes2, &pix, &npix);
104: }


In a comment in JDK-8154148, the following claim is made: "[...] an error happens is 
when something bad or invalid has occurred. An unsatisfied pixel format request is indicated 
by npix is 0 and pix is NULL. Hence I feel it is a cleaner and clearer logic to request a 
different format when npix is 0 and err is kCGLNoError." 
(https://bugs.openjdk.java.net/browse/JDK-8154148?focusedCommentId=13980465&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-13980465)

   From what I could observe, this is not necessarily true; in this case where 
a pixel format satisfying the initial conditions could not be found, the return 
code is kCGLBadPixelFormat (10002); which means we don't try to get a pixel 
format with less restrictive condition.
This suggests that a better condition for the second call to CGLChoosePixelFormat (line #103) should be 
"if (pix == NULL)" instead of "if ((err == kCGLNoError) && (npix == 0))"
This is further supported by a sample in Apple developer's documentation on how 
to choose a pixel format: 
https://developer.apple.com/library/archive/documentation/GraphicsImaging/Conceptual/OpenGL-MacProgGuide/opengl_pixelformats/opengl_pixelformats.html

I have verified that this change produces the expected behavior and I propose 
to submit a PR with this change (I have already signed the OCA)

A quick question before I do, however; should I file a new issue prior to 
submitting a PR or is it better that someone with privileges reopens 
JDK-8154148?

Regards,

Frederic Thevenet

Re: Blank stages when running JavaFX app in a macOS virtual machine

[thevenet . fred]

Hi,

I've submitted bug report via https://bugreport.java.com/bugreport/ regarding 
the issue below about a week ago but I haven't heard from it yet.
I understand this might not be the best place to inquire about it, but since 
this prevents me from submitting a PR (as I don't have an issue number) I was 
hoping someone here could offer some guidance on how to get this moving forward.

Many thanks,
Fred.

- Mail original -
De: "thevenet fred" 
À: "Kevin Rushforth" 
Cc: "openjfx-dev" 
Envoyé: Mardi 3 Décembre 2019 18:46:57
Objet: Re: Blank stages when running JavaFX app in a macOS virtual machine

Ok thanks, will do that.

Regards,
Frederic Thevenet

- Mail original -
De: "Kevin Rushforth" 
À: "openjfx-dev" 
Envoyé: Mardi 3 Décembre 2019 18:35:49
Objet: Re: Blank stages when running JavaFX app in a macOS virtual machine

A new follow-on bug ID will be needed. We never reopen a JBS issue once 
it has been resolved as fixed by a changeset that has been pushed.

-- Kevin


On 12/3/2019 7:11 AM, thevenet.f...@free.fr wrote:
> Hello,
>
> When running a JavaFX application in macOS guest VM, the main stage is 
> completely blank, with the following errors: CGLChoosePixelFormat error: 
> 10002, CGLCreateContext error: 10002
> This behavior was observed with a macOS 10.15 guest OS on both VMWare player 
> and VirtualBox, on a Windows 10 host.
>
> This is an old issue that has already been reported several times (notably as 
> JDK-8154852) but is claimed to have been fixed by JDK-8154148.
> However the fix provided by JDK-8154148 is incomplete; while it does fix a 
> JVM crash, it doesn't prevent an incorrect pixel format to be retrieved, 
> which is the root cause for the stage being empty.
>
> The problematic code is located at #96 in GlassView3D.m:
>
> 095: CGLError err = CGLChoosePixelFormat(attributes, &pix, &npix);
> 096: if ((err == kCGLNoError) && (npix == 0))
> 097: {
> 098:   const CGLPixelFormatAttribute attributes2[] =
> 099:   {
> 100: kCGLPFAAllowOfflineRenderers,
> 101: (CGLPixelFormatAttribute)0
> 102:   };
> 103:   err = CGLChoosePixelFormat(attributes2, &pix, &npix);
> 104: }
>
>
> In a comment in JDK-8154148, the following claim is made: "[...] an error 
> happens is when something bad or invalid has occurred. An unsatisfied pixel 
> format request is indicated by npix is 0 and pix is NULL. Hence I feel it is 
> a cleaner and clearer logic to request a different format when npix is 0 and 
> err is kCGLNoError." 
> (https://bugs.openjdk.java.net/browse/JDK-8154148?focusedCommentId=13980465&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-13980465)
>
>  From what I could observe, this is not necessarily true; in this case where 
> a pixel format satisfying the initial conditions could not be found, the 
> return code is kCGLBadPixelFormat (10002); which means we don't try to get a 
> pixel format with less restrictive condition.
> This suggests that a better condition for the second call to 
> CGLChoosePixelFormat (line #103) should be "if (pix == NULL)" instead of "if 
> ((err == kCGLNoError) && (npix == 0))"
> This is further supported by a sample in Apple developer's documentation on 
> how to choose a pixel format: 
> https://developer.apple.com/library/archive/documentation/GraphicsImaging/Conceptual/OpenGL-MacProgGuide/opengl_pixelformats/opengl_pixelformats.html
>
> I have verified that this change produces the expected behavior and I propose 
> to submit a PR with this change (I have already signed the OCA)
>
> A quick question before I do, however; should I file a new issue prior to 
> submitting a PR or is it better that someone with privileges reopens 
> JDK-8154148?
>
> Regards,
>
> Frederic Thevenet