Re: RFR: 8242523: Update the animation and clip envelope classes

[Ambarish Rapte]

On Wed, 13 May 2020 12:29:03 GMT, Kevin Rushforth  wrote:

>> I agree that these methods are better suited there, but I'm not sure the 
>> same epsilon value will be suitable for other
>> places that will want to use these. That value is somewhat arbitrary anyway 
>> I think.
>
> Either is fine with me.

> but I'm not sure the same epsilon value will be suitable for other places 
> that will want to use these

True, EPSILON is defined in some other classes with different value. Only 
Animation classes define EPSILON as 1e-12. So
it may not be reasonable to include Animation specific helper method in common 
Util class. These methods would be more
suitable for Animation only util class. I think I am good with the current way.

-

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

Re: RFR: 8242523: Update the animation and clip envelope classes

[Ambarish Rapte]

On Wed, 13 May 2020 00:38:03 GMT, Nir Lisker  wrote:

>> modules/javafx.graphics/src/main/java/com/sun/scenario/animation/shared/MultiLoopClipEnvelope.java
>>  line 48:
>> 
>>> 47:
>>> 48: protected boolean autoReverse() {
>>> 49: return autoReverse;
>> 
>> I would suggest the name to be `isAutoReverese`
>
> That would be the usual naming convention, yes, but I find that  this can be 
> more fluent to read. Perhaps I'm
> influenced by the upcoming `record`s feature. Can change.

Yes, please change.  Animation.java also has  `isAutoReverese` method, It would 
be good to be uniform.

-

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

Re: [Rev 01] RFR: 8244112: Skin implementations: must not violate contract of dispose

[Ambarish Rapte]

On Wed, 13 May 2020 11:20:25 GMT, Jeanette Winzenburg  
wrote:

>> modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/SkinDisposeContractTest.java
>>  line 159:
>> 
>>> 158: {TreeTableView.class, },
>>> 159: {TreeView.class, },
>>> 160: };
>> 
>> Should these controls be included too: `Tooltip`, `HTMLEditor`, 
>> `ContextMenu` ?
>
> good catch :)
> 
> Hmm .. thinking aloud:
> 
> - all in web is a bit very different as a small layer on top of the 
> webEngine, usually off my radar
> - the skinnable windows are no controls
> 
> so the easy way out - which I would tend take :) - is to narrow the issue to 
> include only controls in controls. What do
> you think?

Agree that HTMLEditorSkin is a web module. let's keep it off radar :)

TooltipSkin and ContextMenuSkin implement Skin and override the dispose() 
method. I think these Skins should also
behave similar to other and can be included here in test. and also should 
consider them under cleanup issue
[JDK-8241364](https://bugs.openjdk.java.net/browse/JDK-8241364). Both the 
dispose() seem safe from NPE.

-

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

Re: [Rev 04] RFR: 8217472: Add attenuation for PointLight

[Kevin Rushforth]

On Wed, 13 May 2020 23:42:40 GMT, Nir Lisker  wrote:

>> If this were an even remotely representative use case, then no, the 
>> performance hit would not be OK. The test was
>> designed as an artificial "worst-case" stress test: a single mesh with a 
>> large number of very large (window-sized)
>> quads stacked on top of each other. Any real-world use case won't do this.  
>> We should make sure that we aren't seeing
>> any significant performance drop when rendering spheres (at a couple 
>> different tessellation levels) or boxes.
>
>> We should make sure that we aren't seeing any significant performance drop 
>> when rendering spheres (at a couple
>> different tessellation levels) or boxes.
> 
> I missed this. Do you mean that the test should create a mesh of a sphere 
> instead of a flat surface?

I would say in addition to rather than instead of, since both are useful.

What might help is to add the sphere test plus the pathological test I put 
together into your test program so we can
select between them. And then get a few of us to run that updated program and 
post results.

-

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

Re: [Rev 04] RFR: 8217472: Add attenuation for PointLight

[Nir Lisker]

On Sat, 25 Apr 2020 17:07:21 GMT, Kevin Rushforth  wrote:

> We should make sure that we aren't seeing any significant performance drop 
> when rendering spheres (at a couple
> different tessellation levels) or boxes.

I missed this. Do you mean that the test should create a mesh of a sphere 
instead of a flat surface?

-

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

Re: RFR: 8242523: Update the animation and clip envelope classes

[Nir Lisker]

On Thu, 7 May 2020 08:50:23 GMT, Ambarish Rapte  wrote:

>> Mostly refactoring in preparation of the upcoming fixes. The changes might 
>> look like a lot, but it's mostly rearranging
>> of methods. Summery of changes:
>> ### Animation
>> * Added `isNearZero` and `areNearEqual` methods that deal with `EPSILON` 
>> checks.
>> * Added `isStopped`, `isRunning` and `isPaused` convenience methods.
>> * Added `runHandler` method to deal with running the handler.
>> * Moved methods to be grouped closer to where they are used rather than by 
>> visibility.
>> * Removed the static import for `TickCalculation`.
>> * Various small subjective readability changes.
>> * Behavioral changes: switching `autoReverse` and `onFinished` properties 
>> from "Simple" to "Base" properties; and lazily
>>   initializing the `cuePoints` map.
>> 
>> ### Clip Envelopes
>> * Added `MultiLoopClipEnvelope` as an intermediate parent for infinite and 
>> finite clip envelopes.
>> * Rearranged methods order to be consistent.
>> * Replaced the `checkBounds` method with a new overload of `Utils.clamp`.
>> * Renamed `pos` to `cyclePos`.
>> * Extracted common methods: `changedDirection` and `ticksRateChange`
>> * Added internal documentation.
>> 
>> Also corrected a typo in `TicksCalculation` and added an explanation for an 
>> animation test.
>
> modules/javafx.graphics/src/main/java/com/sun/scenario/animation/shared/SingleLoopClipEnvelope.java
>  line 102:
> 
>> 101: long ticksChange = Math.round(currentTick * currentRate);
>> 102: ticks = Utils.clamp(0, deltaTicks + ticksChange, 
>> cycleTicks);
>> 103: AnimationAccessor.getDefault().playTo(animation, ticks, 
>> cycleTicks);
> 
> This could remain unchanged. The `ticksChange` value is not really used 
> elsewhere here.

This change was made with extracting similar code from the other clip envelopes 
in mind, though there would need to be
some future discussion about it. I think it also makes the code more readable 
anyway.

-

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

Re: RFR: 8242523: Update the animation and clip envelope classes

[Kevin Rushforth]

On Wed, 13 May 2020 00:17:17 GMT, Nir Lisker  wrote:

>> modules/javafx.graphics/src/main/java/com/sun/scenario/animation/shared/ClipEnvelope.java
>>  line 46:
>> 
>>> 45:  */
>>> 46: public abstract class ClipEnvelope {
>>> 47:
>> 
>> I think the removal of line 46 was unintended change.
>
> I think that there is no empty line between the docs and the declaration, but 
> can revert.

Correct, so no need to revert.

>> modules/javafx.graphics/src/main/java/javafx/animation/Animation.java line 
>> 142:
>> 
>>> 141: }
>>> 142:
>>> 143: /*
>> 
>> Both these methods seem like they belong to Utils.java. If moved to 
>> Utils.java then the all the calls
>> `(Math.abs(currentRate - rate) < EPSILON)` can be changed to use these 
>> methods from Utils.java. If we move these
>> methods then, Utils.java also needs to declare  `static final double EPSILON 
>> = 1e-12;`  and the EPSILON here can be
>> initialized as `private static final double EPSILON = Utils.EPSILON;`
>
> I agree that these methods are better suited there, but I'm not sure the same 
> epsilon value will be suitable for other
> places that will want to use these. That value is somewhat arbitrary anyway I 
> think.

Either is fine with me.

-

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

Re: RFR: 8238080: FXMLLoader: if script engines implement javax.script.Compilable compile scripts

[Rony G . Flatscher]

On Wed, 13 May 2020 09:22:20 GMT, Rony G. Flatscher 
 wrote:

>> Doc addition for the CSR:
>> =
>> The following text was added to the [Introduction to
>> FXML](https://github.com/openjdk/jfx/blob/master/modules/javafx.fxml/src/main/docs/javafx/fxml/doc-files/introduction_to_fxml.html)
>> at two locations, the "Script Event Handlers" ("#script_event_handlers") and 
>> the "Scripting" (#scripting") sections:
>>> Hint: to turn off automatic compilation of script code place the 
>>> processing instruction >> class="code">?compile false? before the script. To turn on 
>>> compilation of script code again use the
>>> processing instruction ?compile true? (or 
>>> short: >> class="code">?compile?). The compile processing instruction 
>>> can be used repeatedly to turn compilation
>>> of script code off and on.
>> 
>> This documents the compile processing instruction, that it compilation is on 
>> by default and that it can be turned off
>> and on repeatedly. It is hoped that this formulation plays well with the 
>> context where this paragraph got introduced.
>
> Doc addition for the CSR (update)
> -
> In the event handler section:
>> Note: to turn off automatic compilation of script code place the 
>> processing instruction > class="code">?compile false? before the element that contains 
>> the script. To turn on compilation of
>> script code again use the processing instruction > class="code">?compile true? (or short: > class="code">?compile?). The compile processing instruction 
>> can be used repeatedly to turn compilation
>> of script code off and on.
> 
> In the Scripting section:
>> Note: to turn off automatic compilation of script code place the 
>> processing instruction > class="code">?compile false? before the script element. To 
>> turn on compilation of script code again use
>> the processing instruction ?compile true? 
>> (or short: > class="code">?compile?). The compile processing instruction 
>> can be used repeatedly to turn compilation
>> of script code off and on.

/solves

-

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

Re: [Rev 02] RFR: 8244112: Skin implementations: must not violate contract of dispose

[Jeanette Winzenburg]

> some skins have not been guarding themselves against multiple calls to 
> dispose (see issue for details)
> 
> Fixed by backing out off dispose if skinnable is null. Added test 
> (parameterized in control class) for all controls in
> the controls package. Those that failed for the misbehaving skins before are 
> passing after the fix.

Jeanette Winzenburg has updated the pull request incrementally with one 
additional commit since the last revision:

  cleanup as per review

-

Changes:
  - all: https://git.openjdk.java.net/jfx/pull/209/files
  - new: https://git.openjdk.java.net/jfx/pull/209/files/fe76204d..2a362614

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

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

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

Re: [Rev 01] RFR: 8244112: Skin implementations: must not violate contract of dispose

[Jeanette Winzenburg]

On Wed, 13 May 2020 10:09:40 GMT, Ambarish Rapte  wrote:

>> Jeanette Winzenburg has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   cleanup
>>   
>>   - corrected incorrect bug id in commented
>>   - removed unrelated test change from TextAreaTest
>>   - updated copyright year
>
> modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/SkinDisposeContractTest.java
>  line 159:
> 
>> 158: {TreeTableView.class, },
>> 159: {TreeView.class, },
>> 160: };
> 
> Should these controls be included too: `Tooltip`, `HTMLEditor`, `ContextMenu` 
> ?

good catch :)

Hmm .. thinking aloud:

- all in web is a bit very different as a small layer on top of the webEngine, 
usually off my radar
- the skinnable windows are no controls

so the easy way out - which I would tend take :) - is to narrow the issue to 
include only controls in controls. What do
you think?

-

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

Re: [Rev 01] RFR: 8244418: MenuBar: IOOB exception on requestFocus on empty bar

[Jeanette Winzenburg]

On Tue, 12 May 2020 12:21:14 GMT, Ajit Ghaisas  wrote:

>> modules/javafx.controls/src/main/java/javafx/scene/control/skin/MenuBarSkin.java
>>  line 485:
>> 
>>> 484: }
>>> 485:
>>> 486: if (focusedMenu != null && focusedMenuIndex != -1) {
>> 
>> don't quite understand why this change is necessary? The guard in the 
>> listener seems to be enough, at least the test
>> passes without this. If it's needed to fix another potentially breaking path 
>> to this, would it be possible to add a
>> test method that fails/passes before/after?
>
> This was the place where exception was occurring. Hence, I added this check.
> When I ran against the test, I still got the exception at caller lambda. I 
> added that check later.
> Wanted to remove this check - but simple code scan revealed this method gets 
> called from engine.addTraverseListener()
> listener with index = 0. Hence, I have kept this check. Covering this 
> scenario in test seems tough.

Hmm .. re-reading my comment: was assuming and then formulating round a corner, 
sry. Will try again

My assumption (from the method implementation before the fix) was that the 
method expects only valid indices (either -1
or an index in range) such that

 assertEquals(focusedMenuIndex, getMenus().indexOf(focusedMenu);

Which would leave the validity check to the caller.

With that assumption, the old implementation were just fine. The one location 
that calls it with an invalid index is
the one you fixed (not called if empty), the other is the traversalListener 
(which I silently and incorrectly dropped
from my mind, because it's never notified anyway - which is nothing but a bold 
guess ;)

With the change, the constraint doesn't hold (as it didn't before if the caller 
passed in an invalid index) it just
doesn't blow. Not sure what - if any - bad might happen if we have a 
focusedMenuIndex >= getMenu().size() (in
particular empty menus).

The other way round: the method might take the responsibility to protect itself 
(no precondition and the postcondition
to guarantee the constraint above).

My preference would be to keep the method as is, and change the callers to 
check for a valid index. Hard to test, one
way or other ;)

-

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

Re: [Integrated] RFR: 8244647: Wrong first layout pass of Scrollbar controls on touch supported devices

[Jose Pereda]

On Fri, 8 May 2020 11:25:03 GMT, Jose Pereda  wrote:

> There is a visual glitch when the scrollbar controls are laid out on touch 
> enabled devices.
> 
> The first time they are laid out in the wrong location (20 px from right or 
> bottom), while the next passes are correct
> (8 px from right or bottom).
> The reason for this glitch is the use of `getWidth()` or `getHeight()` in the 
> `resizeRelocate` calls to relocate the
> bars before the controls have been properly resized yet: The initial w/h 
> values are set to the default (20 px / 100
> px), while the pref values are correctly set to 8 px.  This PR fixes that, by 
> using the same prefWidth/prefHeight for
> both resizing and relocating.
> It has been tested on Mac OS and Linux (with `-Dcom.sun.javafx.touch=true`), 
> and on Android and iOS.

This pull request has now been integrated.

Changeset: dbb64373
Author:Jose Pereda 
Committer: Ajit Ghaisas 
URL:   https://git.openjdk.java.net/jfx/commit/dbb64373
Stats: 10 lines in 1 file changed: 0 ins; 4 del; 6 mod

8244647: Wrong first layout pass of Scrollbar controls on touch supported 
devices

Reviewed-by: aghaisas

-

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

Re: [Rev 01] RFR: 8244112: Skin implementations: must not violate contract of dispose

[Ambarish Rapte]

On Mon, 11 May 2020 04:51:51 GMT, Ambarish Rapte  wrote:

>> Jeanette Winzenburg has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   cleanup
>>   
>>   - corrected incorrect bug id in commented
>>   - removed unrelated test change from TextAreaTest
>>   - updated copyright year
>
> modules/javafx.controls/src/main/java/javafx/scene/control/skin/MenuButtonSkinBase.java
>  line 222:
> 
>> 221: // FIXME : JDK-8244112 - backout if we are already disposed
>> 222: // should check for getSkinnable to be null and return
>> 223: if (getSkinnable() == null) return;
> 
> This comment can be removed. FIXME is getting fixed. :)

Ajit has already commented about it, please resolve this comment.

-

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

Re: [Rev 01] RFR: 8244112: Skin implementations: must not violate contract of dispose

[Ambarish Rapte]

On Wed, 6 May 2020 11:46:27 GMT, Jeanette Winzenburg  
wrote:

>> some skins have not been guarding themselves against multiple calls to 
>> dispose (see issue for details)
>> 
>> Fixed by backing out off dispose if skinnable is null. Added test 
>> (parameterized in control class) for all controls in
>> the controls package. Those that failed for the misbehaving skins before are 
>> passing after the fix.
>
> Jeanette Winzenburg has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   cleanup
>   
>   - corrected incorrect bug id in commented
>   - removed unrelated test change from TextAreaTest
>   - updated copyright year

Eventually we might need to add this check to all the skin's dispose() which 
will be fixed for cleanup.
Looks good to me, suggested some minor changes.

modules/javafx.controls/src/main/java/javafx/scene/control/skin/MenuButtonSkinBase.java
 line 222:

> 221: // FIXME : JDK-8244112 - backout if we are already disposed
> 222: // should check for getSkinnable to be null and return
> 223: if (getSkinnable() == null) return;

This comment can be removed. FIXME is getting fixed. :)

modules/javafx.controls/src/shims/java/javafx/scene/control/ControlShim.java 
line 38:

> 37:  *
> 38:  * @param control the control the set the default skin on
> 39:  */

typo:- the control the set => the control **to** set

modules/javafx.controls/src/shims/java/javafx/scene/control/ControlShim.java 
line 34:

> 33:  *
> 34:  * Note that this has no noticeable effect if the the control's
> 35:  * skin already is set to the default skin (see skinProperty for

typo:-  if the the control's => if the control's

modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/SkinDisposeContractTest.java
 line 159:

> 158: {TreeTableView.class, },
> 159: {TreeView.class, },
> 160: };

Should these controls be included too: `Tooltip`, `HTMLEditor`, `ContextMenu` ?

-

Changes requested by arapte (Reviewer).

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

Re: [Rev 01] RFR: 8244647: Wrong first layout pass of Scrollbar controls on touch supported devices

[Ajit Ghaisas]

On Wed, 13 May 2020 09:38:23 GMT, Jose Pereda  wrote:

>> There is a visual glitch when the scrollbar controls are laid out on touch 
>> enabled devices.
>> 
>> The first time they are laid out in the wrong location (20 px from right or 
>> bottom), while the next passes are correct
>> (8 px from right or bottom).
>> The reason for this glitch is the use of `getWidth()` or `getHeight()` in 
>> the `resizeRelocate` calls to relocate the
>> bars before the controls have been properly resized yet: The initial w/h 
>> values are set to the default (20 px / 100
>> px), while the pref values are correctly set to 8 px.  This PR fixes that, 
>> by using the same prefWidth/prefHeight for
>> both resizing and relocating.
>> It has been tested on Mac OS and Linux (with `-Dcom.sun.javafx.touch=true`), 
>> and on Android and iOS.
>
> Jose Pereda has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Apply code formatting

Marked as reviewed by aghaisas (Reviewer).

-

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

Re: [Rev 01] RFR: 8244112: Skin implementations: must not violate contract of dispose

[Ajit Ghaisas]

On Wed, 6 May 2020 11:46:27 GMT, Jeanette Winzenburg  
wrote:

>> some skins have not been guarding themselves against multiple calls to 
>> dispose (see issue for details)
>> 
>> Fixed by backing out off dispose if skinnable is null. Added test 
>> (parameterized in control class) for all controls in
>> the controls package. Those that failed for the misbehaving skins before are 
>> passing after the fix.
>
> Jeanette Winzenburg has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   cleanup
>   
>   - corrected incorrect bug id in commented
>   - removed unrelated test change from TextAreaTest
>   - updated copyright year

Fix looks good. Nice test as well.
I have minor comments.

modules/javafx.controls/src/main/java/javafx/scene/control/skin/MenuButtonSkinBase.java
 line 221:

> 220: @Override public void dispose() {
> 221: // FIXME : JDK-8244112 - backout if we are already disposed
> 222: // should check for getSkinnable to be null and return

Remove this FIXME comment.

modules/javafx.controls/src/shims/java/javafx/scene/control/ControlShim.java 
line 38:

> 37:  *
> 38:  * @param control the control the set the default skin on
> 39:  */

"the control the set the default skin on" --> "the control to set the default 
skin on"

-

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

Re: [Rev 01] RFR: 8244647: Wrong first layout pass of Scrollbar controls on touch supported devices

[Jose Pereda]

On Wed, 13 May 2020 09:14:02 GMT, Ajit Ghaisas  wrote:

>> Jose Pereda has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Apply code formatting
>
> modules/javafx.controls/src/main/java/javafx/scene/control/skin/VirtualFlow.java
>  line 2449:
> 
>> 2448: double prefHeight = 
>> hbar.prefHeight(viewportBreadth);
>> 2449: hbar.resizeRelocate(0, viewportLength-prefHeight,
>> 2450: viewportBreadth, prefHeight);
> 
> Minor : Code formatting convention: Add spaces around '-' at 4 places in your 
> changes.
> It was not followed in earlier version, but let's correct it while touching 
> these lines.

Yes, makes sense, I thought about doing it while removing the unnecessary 
parenthesis but didn't want to be too
intrusive. Done.

-

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

Re: [Rev 01] RFR: 8244647: Wrong first layout pass of Scrollbar controls on touch supported devices

[Jose Pereda]

> There is a visual glitch when the scrollbar controls are laid out on touch 
> enabled devices.
> 
> The first time they are laid out in the wrong location (20 px from right or 
> bottom), while the next passes are correct
> (8 px from right or bottom).
> The reason for this glitch is the use of `getWidth()` or `getHeight()` in the 
> `resizeRelocate` calls to relocate the
> bars before the controls have been properly resized yet: The initial w/h 
> values are set to the default (20 px / 100
> px), while the pref values are correctly set to 8 px.  This PR fixes that, by 
> using the same prefWidth/prefHeight for
> both resizing and relocating.
> It has been tested on Mac OS and Linux (with `-Dcom.sun.javafx.touch=true`), 
> and on Android and iOS.

Jose Pereda has updated the pull request incrementally with one additional 
commit since the last revision:

  Apply code formatting

-

Changes:
  - all: https://git.openjdk.java.net/jfx/pull/215/files
  - new: https://git.openjdk.java.net/jfx/pull/215/files/fcce06aa..fea9d186

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

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

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

Re: RFR: 8238080: FXMLLoader: if script engines implement javax.script.Compilable compile scripts

[Rony G . Flatscher]

On Tue, 12 May 2020 17:33:06 GMT, Rony G. Flatscher 
 wrote:

>> Suggested CSR:
>> 
>> Summary
>> ===
>> Have javafx.fxml.FXMLLoader compile FXML scripts before evaluating them, if 
>> the script engine
>> implements the javax.script.Compilable interface to speed up execution. In 
>> case compilation
>> throws a javax.script.ScriptException fall back to evaluating the uncompiled 
>> script. Allow
>> control of script compilation with a "compile" PI for FXML files.
>> 
>> Problem
>> ===
>> javafx.fxml.FXMLLoader is able to execute scripts in Java script languages
>> (javax.script.ScriptEngine implementations) referred to or embedded in a 
>> FXML file.
>> 
>> If a script engine implements the javax.script.Compilable interface, then 
>> such scripts could be
>> compiled and the resulting javax.script.CompiledScript could be executed 
>> instead using its
>> eval() methods.
>> 
>> Evaluating the javax.script.CompiledScript objects may help speed up the 
>> execution of script
>> invocations, especially for scripts defined for event attributes in FXML 
>> elements (e.g. like
>> onMouseMove) which may be repetitively invoked and evaluated.
>> 
>> Solution
>> 
>> Before evaluating the script code test whether the javax.script.ScriptEngine 
>> implements
>> javax.script.Compilable. If so, compile the script to a 
>> javax.script.CompiledScript object first
>> and then use its eval() method to evaluate the script, otherwise continue to 
>> use the
>> javax.script.ScriptEngine's eval() method instead. Should compilation of a 
>> script yield
>> (unexpectedly) a javax.script.ScriptException then fall back to using the
>> javax.script.ScriptEngine's eval() method. A new process instruction 
>> "compile" allows control of
>> the compilation of scripts ("true" sets compilation on, "false" to set 
>> compilation off) in FXML
>> files.
>> 
>> Specification
>> =
>> If a javax.script.ScriptEngine implements the javax.script.Compilable 
>> interface, then use its
>> compile() method to compile the script to a javax.script.CompiledScript 
>> object and use its
>> eval() method to run the script. In the case that the compilation throws 
>> (unexpectedly) a
>> javax.script.ScriptException log a warning and fall back to using the
>> javax.script.ScriptEngine's eval() method instead.
>> To allow setting this feature off and on while processing the FXML file a 
>> "compile" process
>> instruction ("" or "") gets defined that 
>> allows to turn
>> compilation off and on throughout a FXML file.
>
> Doc addition for the CSR:
> =
> The following text was added to the [Introduction to
> FXML](https://github.com/openjdk/jfx/blob/master/modules/javafx.fxml/src/main/docs/javafx/fxml/doc-files/introduction_to_fxml.html)
> at two locations, the "Script Event Handlers" ("#script_event_handlers") and 
> the "Scripting" (#scripting") sections:
>> Hint: to turn off automatic compilation of script code place the 
>> processing instruction > class="code">?compile false? before the script. To turn on 
>> compilation of script code again use the
>> processing instruction ?compile true? (or 
>> short: > class="code">?compile?). The compile processing instruction 
>> can be used repeatedly to turn compilation
>> of script code off and on.
> 
> This documents the compile processing instruction, that it compilation is on 
> by default and that it can be turned off
> and on repeatedly. It is hoped that this formulation plays well with the 
> context where this paragraph got introduced.

Doc addition for the CSR (update)
-
In the event handler section:
> Note: to turn off automatic compilation of script code place the 
> processing instruction  class="code">?compile false? before the element that contains 
> the script. To turn on compilation of
> script code again use the processing instruction  class="code">?compile true? (or short:  class="code">?compile?). The compile processing instruction 
> can be used repeatedly to turn compilation
> of script code off and on.

In the Scripting section:
> Note: to turn off automatic compilation of script code place the 
> processing instruction  class="code">?compile false? before the script element. To 
> turn on compilation of script code again use
> the processing instruction ?compile true? 
> (or short:  class="code">?compile?). The compile processing instruction 
> can be used repeatedly to turn compilation
> of script code off and on.

-

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

Re: RFR: 8244647: Wrong first layout pass of Scrollbar controls on touch supported devices

[Ajit Ghaisas]

On Fri, 8 May 2020 11:25:03 GMT, Jose Pereda  wrote:

> There is a visual glitch when the scrollbar controls are laid out on touch 
> enabled devices.
> 
> The first time they are laid out in the wrong location (20 px from right or 
> bottom), while the next passes are correct
> (8 px from right or bottom).
> The reason for this glitch is the use of `getWidth()` or `getHeight()` in the 
> `resizeRelocate` calls to relocate the
> bars before the controls have been properly resized yet: The initial w/h 
> values are set to the default (20 px / 100
> px), while the pref values are correctly set to 8 px.  This PR fixes that, by 
> using the same prefWidth/prefHeight for
> both resizing and relocating.
> It has been tested on Mac OS and Linux (with `-Dcom.sun.javafx.touch=true`), 
> and on Android and iOS.

The changes look fine.
I have a minor comment on code formatting.

modules/javafx.controls/src/main/java/javafx/scene/control/skin/VirtualFlow.java
 line 2449:

> 2448: double prefHeight = 
> hbar.prefHeight(viewportBreadth);
> 2449: hbar.resizeRelocate(0, viewportLength-prefHeight,
> 2450: viewportBreadth, prefHeight);

Minor : Code formatting convention: Add spaces around '-' at 4 places in your 
changes.
It was not followed in earlier version, but let's correct it while touching 
these lines.

-

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

Re: HID multitouch display support in JavaFX

[Michael Paus]

Having proper and consistent Multitouch support on all platforms is 
certainly
an interesting goal, especially if your development is targeting a 
mobile platform. If one

could test the input behavior already on the desktop, this would be an
enormous time saver because the turn-around times 
(edit-compile-build-deploy-debug-curse)
for mobile are currently still extremely high and thus not very 
development-friendly.

I learned from your mail that the situation on Windows seems to be better
than on macOS, which is my current development environment. That gives
me an idea :-)

Am 13.05.20 um 10:15 schrieb jfx user2:

Nobody is biting... so is anyone else out there interested in using HID
touchscreens with JavaFX on OSX (or any other plaform) without using a
third party driver?

On Fri, May 8, 2020 at 4:18 AM jfx user2  wrote:


Multitouch display support JavaFX on Windows, iOS, and Android seems
straightforward.  If the OS recognizes the display as multitouch, then the
chances are that JavaFX will also work with it since it relies on the
native multitouch support of the OS.

When it comes to Linux and OSX, it's not always as easy.  Linux sometimes
requires device mapping.  I'm not sure if it works at all in OSX unless you
have specific drivers for the display.

All of the platforms seem to support HID so maybe JavaFX could have direct
support for HID devices (optionally) if they are present instead of just
the current method.  It may also improve performance in cases where the OS
is just piping HID events through their own API's to support multitouch.
Why not just go straight to the HID source and bypass that conduit.

This would vastly improve multitouch app development on OSX.  It would be
possible to connect a HID display and use it for testing apps that are
destined for platforms that formally support multitouch.

Is there any work being done in this area?  Is there a bigger picture
(e.g. better multi-display and connect/disconnect support in JavaFX)?

... coming from a frustrated OSX user with lots of HID displays and no
easy JavaFX support.

https://www.usb.org/hid

Re: [Rev 01] RFR: 8238080: FXMLLoader: if script engines implement javax.script.Compilable compile scripts

[Rony G . Flatscher]

> This WIP adds the ability for a fallback in case compilation of scripts 
> fails, in which case a warning gets issued
> about this fact and evaluation of the script will be done without 
> compilation. Because of the fallback scripts get
> compiled with this version by default. It extends PR 187 
> .  To further ease
> spotting scripts that cause a ScriptException a message in the form of 
> "filename: caused ScriptException" gets added to
> the exception handling in either of the three locations: an error message, a 
> stack trace or a wrap-up into a
> RuntimeException (having three different kinds of reporting ScriptExceptions 
> may be questioned, however none of these
> tear down the FXML GUI).

Rony G. Flatscher has updated the pull request incrementally with one 
additional commit since the last revision:

  Reword the compile processing instructions (replace 'Hint' with 'Note:', 
adjust text to context), remove spurious empty
  line in script example code.

-

Changes:
  - all: https://git.openjdk.java.net/jfx/pull/192/files
  - new: https://git.openjdk.java.net/jfx/pull/192/files/44b0f9f8..59a16c9f

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

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

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

Re: TableView help

[Dirk Lemmermann]

Hi John,

the model class used by your TableView is called “SimpleImmutableEntry”. The 
middle part of this name should be enough to tell you what the problem is :-)

Solution: create your own custom model class and provide setters and getters 
for “key” and “value”. Then the TableView will write back into your POJO.

In other news: this mailing list is meant for "Technical discussion related to 
the OpenJFX Project”, so you might wanna find a different place for support 
questions like this one. In this mailing list we discuss things like bugs in 
OpenJFX and their potential fixes.

Dirk

> On 13 May 2020, at 02:05, John Scancella  wrote:
> 
> Hello All,
> 
> I am hoping to get some help with setting up a TableView. What I want is a
> simple two column table with each column containing Strings. I want to be
> able to double click to create a new row if I double clicked on empty
> space. And I want to be able to edit any cell already in the table.
> 
> Currently I am struggling with editing the data already in the table. I can
> click and edit, but then if I double click again on the cell it reverts
> back to the original value. If I try and get the value programmatically, it
> is still getting the original value, even though the display is showing the
> new value.
> 
> You can see my code here: https://github.com/jscancella/heirloom
> I am using java version "1.8.0_251"
> 
> Thank you in advance for any help!
> 
> 
> -- 
> ~John Scancella

Re: HID multitouch display support in JavaFX

[jfx user2]

Nobody is biting... so is anyone else out there interested in using HID
touchscreens with JavaFX on OSX (or any other plaform) without using a
third party driver?

On Fri, May 8, 2020 at 4:18 AM jfx user2  wrote:

> Multitouch display support JavaFX on Windows, iOS, and Android seems
> straightforward.  If the OS recognizes the display as multitouch, then the
> chances are that JavaFX will also work with it since it relies on the
> native multitouch support of the OS.
>
> When it comes to Linux and OSX, it's not always as easy.  Linux sometimes
> requires device mapping.  I'm not sure if it works at all in OSX unless you
> have specific drivers for the display.
>
> All of the platforms seem to support HID so maybe JavaFX could have direct
> support for HID devices (optionally) if they are present instead of just
> the current method.  It may also improve performance in cases where the OS
> is just piping HID events through their own API's to support multitouch.
> Why not just go straight to the HID source and bypass that conduit.
>
> This would vastly improve multitouch app development on OSX.  It would be
> possible to connect a HID display and use it for testing apps that are
> destined for platforms that formally support multitouch.
>
> Is there any work being done in this area?  Is there a bigger picture
> (e.g. better multi-display and connect/disconnect support in JavaFX)?
>
> ... coming from a frustrated OSX user with lots of HID displays and no
> easy JavaFX support.
>
> https://www.usb.org/hid
>

Re: com.sun.javafx vs javafx (with example/request)

[jfx user2]

Turns out that I partially misunderstood the use of Path2D in
GraphicsContext.  I had been using appendSVGPath for nearly everything and
within a single beginPath/closePath.  Under those circumstances
 converting GraphicsContext.path from Path2D to Path (using code from
Shape.createFromGeomShape(Path2D p)) gives you a relatively simple path
that can be used as a simplified version of what is rendered in the
Canvas.  It can be used as a very "degraded" version of what was placed in
the canvas and is way faster than other solutions.

However, it would only apply to the last beginPath/closePath and it does
not include any of the other path methods in GraphicsContext.  Path IS used
by those methods but is empty after a call to e.g. fillRect.  That's
unfortunate... If path did retain everything, I'd continue to push for a
"getPath" method on GraphicsContext.  I'll have to stick with my current
implementations.

As for the performance of Canvas vs. the Scene, I'm also speaking from
experience.  But I'm referring to highly dynamic applications.  If you have
an app with lots of objects (and lots dynamic behavior such as motion of
all types, animation, and color) Canvas will be many times more performant
than the Scene graph.  I think Gerrit Grunwald's TileFX is a good example
(more complex usage of Canvas than most but still not even close to the
level of dynamic behavior that I'm referring to).

On Fri, May 8, 2020 at 7:13 PM Michael Paus  wrote:

> I think I know quite a bit about JavaFX graphics and I do not generally
> agree with your statements.
> Especially the statement that the Canvas is so much superior is a myth
> from the old days of Java 8
> where there were a few performance bugs in the scene graph handling. But
> that's a long time ago.
> (I even gave a workshop on this at the JavaLand conference some time ago.)
> Maybe you could present
> a few more details about your use-case and not so much about the technique
> that you think is the
> best fit for it. Maybe even some demo code somewhere. I am always
> interested in a discussion about
> graphics as long as it is supported by facts. Neither the scene graph nor
> the canvas is a silver bullet.
> It always depends on your use-case.
> Michael
>
> Am 09.05.20 um 00:18 schrieb jfx user2:
>
> From the JavaDoc "Canvas is an image that can be drawn on using a set of
> graphics commands provided by a GraphicsContext."  This a bit of a
> misnomer.  While canvas can be used to draw in image (actually
> GraphicsContext not canvas itself), the image based methods of
> GraphicsContext are far outweighed by the "vector" or path based methods.
> The GraphicsContext gives you the ability to freely create dynamic graphics
> without the constraints of the Scene Graph.  It can be highly performant
> and scalable if done properly.  The Scene Graph will not perform like the
> GraphicsContext.  If you haven't worked with highly dynamic graphics, you
> might not have encountered any issues.  The Scene Graph works ok up to a
> certain number of objects and it is not good at adding and removing objects
> often.  GraphicsContext does not have the same restrictions.  It can
> involve more work but the end result will scale far beyond what you can do
> in a Scene Graph... I think this is already widely accepted.
>
> The proposed method on the GraphicsContext simply returns what is already
> there.  It would convert the already stored Path2D to a Path.  Why reinvent
> what is already present and only private?  This in turn CAN be used in the
> Scene Graph but it can also be used as a container to draw back onto the
> GraphicsContext.  You see,  a Path that is calculated once and then used
> repeatedly to draw in the GraphicsContext (possibly even transformed) is
> better than sub-optimally calculating that Path on every pass through the
> GraphicsContext or storing it as an image or Shape (those were some of my
> workarounds).
>
> Anyway, I'm requesting that a private API be used to create a new public
> method.  This is really no different than existing public methods that use
> private APIs.  I'm not asking to expose private APIs (not in this request
> :o).  There isn't even much code.  It's reusing what's already there.
>
> Ask Gerrit Grunwald about his experience with the Scene Graph vs
> Canvas/GraphicsContext.
>
>
> On Fri, May 8, 2020 at 4:27 PM Michael Paus  wrote:
>
>> Hi,
>> I have to say that your requirements sound a little bit strange to me,
>> but maybe you can make it clearer what your real use-case behind them is.
>> What I do not understand is why you are using the canvas at all.
>> Conceptually the canvas is for direct mode rendering into an image. The
>> fact that
>> this is handled a little bit different internally is an implementation
>> detail, you should not rely on. Why don't you use the scene graph which
>> seems
>> to provide many of the aspects that you need? I admit that there are a
>> few hidden gems internally that I would also like to be made