Re: JavaFX graphics performance and suitability for advanced animations

2013-06-03 Thread John Hendrikx

On 1/06/2013 02:24, Daniel Zwolenski wrote:

Can anyone recommend any good screen capture software for windows? Not sure
it would capture this problem but it would be useful for some of the other
problems I'm seeing in the TD game that I can't seem to reproduce in small
snippets of code.
I used http://camstudio.org/ before to record a JavaFX program (see here 
for the results: http://www.youtube.com/watch?v=8Mb15bOwIyE)


--John




Usecase for needsLayout property?

2013-06-05 Thread John Hendrikx
I'm having a bit of a chicken-egg problem with regards to some 
dynamically sized components I'm displaying in my application.


Description:
I have a group with a title that wraps some content and displays a title 
above it -- the entire group, including the title must be hidden and 
unmanaged when the content it is wrapping is empty, here it is in code:


protected Pane createTitledBlock(final String title, final Node content, 
final BooleanExpression visibleCondition) {

return new VBox() {{
setFillWidth(true);
getChildren().add(new Label(title) {{
getStyleClass().add(header);
}});
getChildren().add(content);
if(visibleCondition != null) {
managedProperty().bind(visibleCondition);
visibleProperty().bind(visibleCondition);
}
}};
}


Problem:
When the content becomes available and visibleCondition gets toggled to 
true, the content part of the titled group is not yet laid out. This 
causes the title to be visible without any content briefly until a 
layout pass occurs.


Timeline:
- Group with title is hidden/unmanaged
- The content gets updated and visibleCondition is set to true
- Group with title becomes visible/managed
- Briefly an empty group with title is displayed
- Layout occurs, group with title gets resized

Now, I'm thinking of playing with the needsLayout property and delaying 
the making visible/managed of the titled group until needsLayout is 
false... however, I have a feeling that won't work as an 
unmanaged/hidden group is probably not gonna participate in the layout, 
resulting in needsLayout never becoming false again as it is not 
visible/managed yet...


Any ideas how I could resolve this, and have the group with title only 
visible and managed when it is properly laid out?


Also, has any progress in general been made with regards to 
hidden/unmanaged controls and determining their expected size when 
they're made visible/managed?


--John


Re: Usecase for needsLayout property?

2013-06-05 Thread John Hendrikx

Thanks Martin,

I've done my best to quickly extract the code from my application... 
however, I cannot reproduce the issue there in a direct fashion -- 
however, I did discover an issue which may be the reason I see a partial 
group, I filed that as RT-30952 (TilePane doesn't appear until after 
Stage resized a bit).


Basically, my main application is much more active, and will do a lot 
of background stuff, which may trigger more layouts -- that's why there 
I may only see the partial group for a moment (flickering when I scroll 
through the big list) before it gets displayed completely.  In this test 
application, I see it much more directly where the group that I want to 
see is simply not complete until more action is taken.


I also filed another issue, RT-30951 because the resizing of a Stage is 
very jittery (the controls keep moving a bit from their correct 
positions when in the process of resizing a Stage)... seems like some 
font-metrics or other calculation is not producing repeatable results 
resulting in a slightly different layout every time...


--John

On 5/06/2013 13:04, Martin Sladecek wrote:

Hi John,
the problem you described is most likely caused by something else as 
the layout pass occurs BEFORE the actual rendering.
Also, you bind managed and visible of the entire VBox, not just the 
title. So it's strange that you see Label correctly laid-out, but not 
the content.


Can you make some sample and attach it to a JIRA issue, so I can have 
a look what's actually happening there?


Thanks,
-Martin

On 06/05/2013 11:29 AM, John Hendrikx wrote:
I'm having a bit of a chicken-egg problem with regards to some 
dynamically sized components I'm displaying in my application.


Description:
I have a group with a title that wraps some content and displays a 
title above it -- the entire group, including the title must be 
hidden and unmanaged when the content it is wrapping is empty, here 
it is in code:


protected Pane createTitledBlock(final String title, final Node 
content, final BooleanExpression visibleCondition) {

return new VBox() {{
setFillWidth(true);
getChildren().add(new Label(title) {{
getStyleClass().add(header);
}});
getChildren().add(content);
if(visibleCondition != null) {
managedProperty().bind(visibleCondition);
visibleProperty().bind(visibleCondition);
}
}};
}


Problem:
When the content becomes available and visibleCondition gets toggled 
to true, the content part of the titled group is not yet laid out. 
This causes the title to be visible without any content briefly until 
a layout pass occurs.


Timeline:
- Group with title is hidden/unmanaged
- The content gets updated and visibleCondition is set to true
- Group with title becomes visible/managed
- Briefly an empty group with title is displayed
- Layout occurs, group with title gets resized

Now, I'm thinking of playing with the needsLayout property and 
delaying the making visible/managed of the titled group until 
needsLayout is false... however, I have a feeling that won't work as 
an unmanaged/hidden group is probably not gonna participate in the 
layout, resulting in needsLayout never becoming false again as it is 
not visible/managed yet...


Any ideas how I could resolve this, and have the group with title 
only visible and managed when it is properly laid out?


Also, has any progress in general been made with regards to 
hidden/unmanaged controls and determining their expected size when 
they're made visible/managed?


--John






ObservableValue Stacktrace

2013-06-05 Thread John Hendrikx

Hi List,

I'm getting some log messages sometimes (see at the end) about 
properties being null (whereas I didn't get them before in JavaFX 2.2).


Is this intended as an informative message to the developer, something I 
should report, or just debug code for the JavaFX team?


In this case, the binding is null, and that's fine -- it will be 
populated later, but the binding is already in place -- I thought JavaFX 
was designed to allow nulls in a chain of bindings and
fall back to reasonable defaults for things like Strings, numbers etc?  
Am I doing something wrong?


Code:
{
  private final ObjectPropertyMedia data = new SimpleObjectProperty();
  public ObjectPropertyMedia dataProperty() { return data; }

  protected final ObjectBindingObservableListCasting castings = 
Bindings.select(dataProperty(), castings);

}

Log:

Jun 05, 2013 9:15:55 PM 
com.sun.javafx.binding.SelectBinding$SelectBindingHelper getObservableValue

WARNING: Exception while evaluating select-binding [castings]
Jun 05, 2013 9:15:55 PM 
com.sun.javafx.binding.SelectBinding$SelectBindingHelper getObservableValue

WARNING: Property 'castings' in ObjectProperty [bound, value: null] is null
java.lang.NullPointerException
at 
com.sun.javafx.binding.SelectBinding$SelectBindingHelper.getObservableValue(SelectBinding.java:481)
at 
com.sun.javafx.binding.SelectBinding$AsObject.computeValue(SelectBinding.java:92)

at javafx.beans.binding.ObjectBinding.get(ObjectBinding.java:152)
at 
javafx.beans.binding.ObjectExpression.getValue(ObjectExpression.java:49)
at 
com.sun.javafx.binding.ExpressionHelper.addListener(ExpressionHelper.java:53)
at 
javafx.beans.binding.ObjectBinding.addListener(ObjectBinding.java:71)
at 
javafx.beans.property.ObjectPropertyBase.bind(ObjectPropertyBase.java:170)
at 
hs.mediasystem.TitledBlockSample.createCastingsRow(TitledBlockSample.java:114)

at hs.mediasystem.TitledBlockSample.start(TitledBlockSample.java:78)
at com.sun.javafx.application.LauncherImpl$5.run(LauncherImpl.java:810)
at com.sun.javafx.application.PlatformImpl$6.run(PlatformImpl.java:260)
at 
com.sun.javafx.application.PlatformImpl$5$1.run(PlatformImpl.java:226)
at 
com.sun.javafx.application.PlatformImpl$5$1.run(PlatformImpl.java:223)

at java.security.AccessController.doPrivileged(Native Method)
at com.sun.javafx.application.PlatformImpl$5.run(PlatformImpl.java:223)
at 
com.sun.glass.ui.InvokeLaterDispatcher$Future.run(InvokeLaterDispatcher.java:95)

at com.sun.glass.ui.win.WinApplication._runLoop(Native Method)
at 
com.sun.glass.ui.win.WinApplication.access$300(WinApplication.java:39)

at com.sun.glass.ui.win.WinApplication$3$1.run(WinApplication.java:101)
at java.lang.Thread.run(Thread.java:724)

Regards,
--John



Re: ObservableValue Stacktrace

2013-06-06 Thread John Hendrikx
Hm, ok -- it is correct that it doesn't fail, the code runs without any 
problem and everything works as expected.


But, what would be the way to avoid these messages in my log then?  
Something like:


  Bindings.select( Bindings.when(dataProperty().isNull()).then( ??? 
).otherwise(dataProperty()), castings) );


??

I'd prefer to just turn these warnings off unless there is a really good 
reason to have them (ie, they indicate a logic error or other bug, 
something I can resolve...)


In my case the dataProperty() is often bound to the selection of a 
ListView -- if you have something valid selected, then a Detail Pane is 
filled in with information about the selected item.  When nothing is 
selected (ie, it is null), then the Detail Pane should remain empty... I 
donot want to have to remove/recreate these bindings every time some 
property becomes null.


Also, it seems rather wierd that JavaFX will complain about nulls in the 
first part of the Bindings.select(), but will happily traverse the graph 
(with or without nulls) for the other parts.


Do I have to add an extra level of indirection to get rid off these 
messages, ie something like:


  private final ObjectPropertyMedia data = new SimpleObjectProperty();
  public ObjectPropertyMedia dataProperty() { return data; }
  private ObjectPropertyObjectPropertyMedia metaDataProperty = new 
SimpleObjectProperty(dataProperty());   // extra indirection...
  public ObjectPropertyObjectPropertyMedia metaDataProperty() { 
return metaDataProperty ; }


So that I can then do a binding like:

   Bindings.select(metaDataProperty(), data, castings);

That seems.. cumbersome :)

--John



On 6/06/2013 09:34, Martin Sladecek wrote:
JavaFX should not fail in these cases, but doesn't consider that a 
valid state, so it always prints a warning on the stderr, for the 
developer to see something went wrong.


Regards,
-Martin

On 5.6.2013 21:38, John Hendrikx wrote:

Hi List,

I'm getting some log messages sometimes (see at the end) about 
properties being null (whereas I didn't get them before in JavaFX 2.2).


Is this intended as an informative message to the developer, 
something I should report, or just debug code for the JavaFX team?


In this case, the binding is null, and that's fine -- it will be 
populated later, but the binding is already in place -- I thought 
JavaFX was designed to allow nulls in a chain of bindings and
fall back to reasonable defaults for things like Strings, numbers 
etc?  Am I doing something wrong?


Code:
{
  private final ObjectPropertyMedia data = new 
SimpleObjectProperty();

  public ObjectPropertyMedia dataProperty() { return data; }

  protected final ObjectBindingObservableListCasting castings = 
Bindings.select(dataProperty(), castings);

}

Log:

Jun 05, 2013 9:15:55 PM 
com.sun.javafx.binding.SelectBinding$SelectBindingHelper 
getObservableValue

WARNING: Exception while evaluating select-binding [castings]
Jun 05, 2013 9:15:55 PM 
com.sun.javafx.binding.SelectBinding$SelectBindingHelper 
getObservableValue
WARNING: Property 'castings' in ObjectProperty [bound, value: null] 
is null

java.lang.NullPointerException
at 
com.sun.javafx.binding.SelectBinding$SelectBindingHelper.getObservableValue(SelectBinding.java:481)
at 
com.sun.javafx.binding.SelectBinding$AsObject.computeValue(SelectBinding.java:92)

at javafx.beans.binding.ObjectBinding.get(ObjectBinding.java:152)
at 
javafx.beans.binding.ObjectExpression.getValue(ObjectExpression.java:49)
at 
com.sun.javafx.binding.ExpressionHelper.addListener(ExpressionHelper.java:53)
at 
javafx.beans.binding.ObjectBinding.addListener(ObjectBinding.java:71)
at 
javafx.beans.property.ObjectPropertyBase.bind(ObjectPropertyBase.java:170)
at 
hs.mediasystem.TitledBlockSample.createCastingsRow(TitledBlockSample.java:114)

at hs.mediasystem.TitledBlockSample.start(TitledBlockSample.java:78)
at 
com.sun.javafx.application.LauncherImpl$5.run(LauncherImpl.java:810)
at 
com.sun.javafx.application.PlatformImpl$6.run(PlatformImpl.java:260)
at 
com.sun.javafx.application.PlatformImpl$5$1.run(PlatformImpl.java:226)
at 
com.sun.javafx.application.PlatformImpl$5$1.run(PlatformImpl.java:223)

at java.security.AccessController.doPrivileged(Native Method)
at 
com.sun.javafx.application.PlatformImpl$5.run(PlatformImpl.java:223)
at 
com.sun.glass.ui.InvokeLaterDispatcher$Future.run(InvokeLaterDispatcher.java:95)

at com.sun.glass.ui.win.WinApplication._runLoop(Native Method)
at 
com.sun.glass.ui.win.WinApplication.access$300(WinApplication.java:39)
at 
com.sun.glass.ui.win.WinApplication$3$1.run(WinApplication.java:101)

at java.lang.Thread.run(Thread.java:724)

Regards,
--John







Re: ConcurrentModificationException during controls test runs

2013-07-17 Thread John Hendrikx


I just got one too, but not related to any ComboBox in b98 as I don't 
use any ComboBoxes.  Probably related to a TreeView control:


java.util.ConcurrentModificationException
at java.util.HashMap$HashIterator.nextEntry(HashMap.java:2395)
at java.util.HashMap$EntryIterator.next(HashMap.java:2466)
at java.util.HashMap$EntryIterator.next(HashMap.java:2464)
at 
javafx.scene.CssStyleHelper.resetToInitialValues(CssStyleHelper.java:280)
at 
javafx.scene.CssStyleHelper.createStyleHelper(CssStyleHelper.java:95)

at javafx.scene.Node.impl_processCSS(Node.java:8619)
at javafx.scene.Parent.impl_processCSS(Parent.java:1192)
at javafx.scene.control.Control.impl_processCSS(Control.java:863)
at javafx.scene.Node.processCSS(Node.java:8548)
at javafx.scene.Node.processCSS(Node.java:8539)
at javafx.scene.Node.processCSS(Node.java:8539)
at javafx.scene.Node.processCSS(Node.java:8539)
at javafx.scene.Node.processCSS(Node.java:8539)
at javafx.scene.Node.processCSS(Node.java:8539)
at javafx.scene.Node.processCSS(Node.java:8539)
at javafx.scene.Node.processCSS(Node.java:8539)
at javafx.scene.Node.processCSS(Node.java:8539)
at javafx.scene.Node.processCSS(Node.java:8539)
at javafx.scene.Node.processCSS(Node.java:8539)
at javafx.scene.Scene.doCSSPass(Scene.java:545)
at javafx.scene.Scene.access$3600(Scene.java:190)
at javafx.scene.Scene$ScenePulseListener.pulse(Scene.java:2373)
at com.sun.javafx.tk.Toolkit.firePulse(Toolkit.java:350)
at 
com.sun.javafx.tk.quantum.QuantumToolkit.pulse(QuantumToolkit.java:589)
at 
com.sun.javafx.tk.quantum.QuantumToolkit.pulse(QuantumToolkit.java:569)
at 
com.sun.javafx.tk.quantum.QuantumToolkit$16.run(QuantumToolkit.java:436)
at 
com.sun.glass.ui.InvokeLaterDispatcher$Future.run(InvokeLaterDispatcher.java:95)

at com.sun.glass.ui.win.WinApplication._runLoop(Native Method)
at 
com.sun.glass.ui.win.WinApplication.access$300(WinApplication.java:39)

at com.sun.glass.ui.win.WinApplication$3$1.run(WinApplication.java:101)
at java.lang.Thread.run(Thread.java:724)

On 17/07/2013 00:58, Richard Bair wrote:

Is anybody else seeing this?

javafx.scene.control.ComboBoxTest  test_rt31479 FAILED
 java.util.ConcurrentModificationException
 at java.util.HashMap$HashIterator.nextEntry(HashMap.java:2290)
 at java.util.HashMap$EntryIterator.next(HashMap.java:2361)
 at java.util.HashMap$EntryIterator.next(HashMap.java:2359)
 at 
javafx.scene.CssStyleHelper.resetToInitialValues(CssStyleHelper.java:280)
 at 
javafx.scene.CssStyleHelper.createStyleHelper(CssStyleHelper.java:177)
 at javafx.scene.Node.impl_processCSS(Node.java:8639)
 at javafx.scene.Parent.impl_processCSS(Parent.java:1202)
 at javafx.scene.control.Control.impl_processCSS(Control.java:863)
 at javafx.scene.Parent.impl_processCSS(Parent.java:1217)
 at 
javafx.scene.control.PopupControl$CSSBridge.impl_processCSS(PopupControl.java:1221)
 at javafx.scene.Parent.impl_processCSS(Parent.java:1217)
 at javafx.scene.Node.processCSS(Node.java:8571)
 at javafx.scene.Scene.doCSSPass(Scene.java:538)
 at javafx.scene.Scene.preferredSize(Scene.java:1562)
 at javafx.scene.Scene.impl_preferredSize(Scene.java:1571)
 at javafx.stage.Window$9.invalidated(Window.java:726)
 at 
javafx.beans.property.BooleanPropertyBase.markInvalid(BooleanPropertyBase.java:110)
 at 
javafx.beans.property.BooleanPropertyBase.set(BooleanPropertyBase.java:141)
 at javafx.stage.Window.setShowing(Window.java:800)
 at javafx.stage.Window.show(Window.java:815)
 at javafx.stage.PopupWindow.showImpl(PopupWindow.java:394)
 at javafx.stage.PopupWindow.show(PopupWindow.java:368)
 at 
com.sun.javafx.scene.control.skin.ComboBoxPopupControl.positionAndShowPopup(ComboBoxPopupControl.java:99)
 at 
com.sun.javafx.scene.control.skin.ComboBoxPopupControl.show(ComboBoxPopupControl.java:79)
 at 
com.sun.javafx.scene.control.skin.ComboBoxBaseSkin.handleControlPropertyChanged(ComboBoxBaseSkin.java:123)
 at 
com.sun.javafx.scene.control.skin.ComboBoxListViewSkin.handleControlPropertyChanged(ComboBoxListViewSkin.java:219)
 at 
com.sun.javafx.scene.control.skin.BehaviorSkinBase$2.call(BehaviorSkinBase.java:180)
 at 
com.sun.javafx.scene.control.skin.BehaviorSkinBase$2.call(BehaviorSkinBase.java:177)
 at 
com.sun.javafx.scene.control.MultiplePropertyChangeListenerHandler$1.changed(MultiplePropertyChangeListenerHandler.java:56)
 at 
javafx.beans.value.WeakChangeListener.changed(WeakChangeListener.java:88)
 at 
com.sun.javafx.binding.ExpressionHelper$SingleChange.fireValueChangedEvent(ExpressionHelper.java:176)
 at 
com.sun.javafx.binding.ExpressionHelper.fireValueChangedEvent(ExpressionHelper.java:80)
 at 

Re: A different way to handle pulse timing

2013-08-05 Thread John Hendrikx

On 5/08/2013 20:46, Richard Bair wrote:

As I wrote in the previous email, it seems that we currently are not blocked waiting for 
vsync(), at least on Windows with D3D pipeline. Anyway, even if we fix that, 
what you propose is that sometimes both threads will be blocked (the render thread 
waiting for vsync, the event thread waiting for the render thread), which doesn't sound 
perfect.

Right. That stinks.

So today, the FX thread will block waiting for the render thread at the point 
where we synchronize state between the FX thread and render thread. The problem 
with this proposal is that we will wait here much longer waiting for vsync in 
the case that we have animations happening, which is just dead time when we 
ought to be preparing the next frame.

Richard
Reading all this I get the distinct feeling that the current way of 
doing things is 'double buffering', where you have to wait until vsync 
arrives before you can start with the next frame, while you are looking 
for 'triple buffering', which allows a new frame to be prepared in a 
seperate buffer/graph/layout while the first (finished) 
buffer/graph/layout waits to be passed off to the render thread when 
vsync arrives.


--John



Re: JavaFX Media issues

2013-08-08 Thread John Hendrikx

On 9/08/2013 02:10, Scott Palmer wrote:

The Media APIs are mostly useless in their current state.  Other than
demoing that you can play a video, they don't go far enough to be of
practical value.  I tried to get someone to pay attention to them back in
the JavaFX 1.0 days
https://javafx-jira.kenai.com/browse/RT-2684


Unfortunately, I have to agree here.  I'd love to use the Media APIs of 
JavaFX, but they are way to limited.  At a minimum I'd need support for 
MKV containers and some common audio formats used with these.  Decoding 
AVI containers would be rather important as well.  Without those, JavaFX 
video is basically limited to bring your own videos only and 
completely unsuitable for playing back videos that end users might have...


Instead, I've been using VLC + VLCJ from day one while working with 
JavaFX as a work-around.



at least someone listened to the request to get H.264 support in there, but
that is just a workaround.  We need to be able to get our data into the
media pipeline.  This would allow those of us that have attempted to do a
video window to have a fighting chance.  Canvas can't keep up and will
likely crash the app with out of memory errors.  Support for drawing into a
native surface (OpenGL or D3D context) has been talked about, but doesn't
appear to be on the horizon yet.  If we just had a hook to get the dang
pixel data into the media pipeline so we could supply the next frame with
whatever we want - either from any native codec via JNI, or dynamically
generated from Java code, whatever... that would be just so dang useful...
(to me at least)
This would atleast allow me to decode the MKV container myself and 
supply data... not looking forward to having to write all that code, but 
I would if it meant I'd be able to go native JavaFX with video.


Canvas however is working for me even with HD video (copying 25 
frames/sec of HD video from VLCJ to Canvas).  Playback is smooth even 
with 1920x1080 video for hours long video, and I can't tell the 
difference with a native player or copying frame by frame using 
pixelWriter anymore.  There is some CPU penalty but on a my system it is 
well below 5%.  This is basically how that looks with VLCJ:


new RenderCallback() {
  @Override
  public void display(DirectMediaPlayer mp, Memory[] memory, 
final BufferFormat bufferFormat) {
final ByteBuffer byteBuffer = memory[0].getByteBuffer(0, 
memory[0].size());


Platform.runLater(new Runnable() {
  @Override
  public void run() {
pixelWriter.setPixels(0, 0, bufferFormat.getWidth(), 
bufferFormat.getHeight(), byteBgraInstance, byteBuffer, 
bufferFormat.getPitches()[0]);

  }
});
  }
}

However, the bug where Canvas keeps buffering data when off-screen 
sometimes bites me, resulting in out of memory errors -- even slight 
display delays can fairly easily cause this when video is running at 25 
fps (takes about a second to go through 256 MB of memory).


--John



Regards,

Scott



On Thu, Aug 8, 2013 at 5:04 PM, Fabrizio Giudici
fabrizio.giud...@tidalwave.it  wrote:


On Thu, 08 Aug 2013 22:57:51 +0200, Joe McGlynnjoe.mcgl...@oracle.com
wrote:

  I don't know why FX Media isn't in the FX 8 API docs, but that's clearly

an error.  Please file a bug on that.

In the meantime, you should look at the FX 2 media docs, there isn't a
lot of change from FX2 media in FX8.  Buffering and streaming (HTTP Live
Streaming) are both supported, as is playback from a URL.


What is the strategy for codecs? I mean, now we have ImageIO (there is
also JAI but it seems basically dead). ImageIO provides many image codecs
and there's a SPI that can be used to support more formats. Will it be
replaced by FX2 media or co-exist with it?


--
Fabrizio Giudici - Java Architect @ Tidalwave s.a.s.
We make Java work. Everywhere.
http://tidalwave.it/fabrizio/**bloghttp://tidalwave.it/fabrizio/blog  -
fabrizio.giud...@tidalwave.it





Re: JavaFX Media issues

2013-08-09 Thread John Hendrikx

On 9/08/2013 03:48, Scott Palmer wrote:
I have heard rumors of people being able to play HD video via Canvas. 
 I have tried everything and can't come close. (Yes, I have been 
careful about the pixel format.)  I mean, it looks like it is working 
for a few seconds, but then as the memory fills with the Canvas 
backlog it can lead to the GC using a lot more CPU, thus reducing the 
ability for Canvas to process its command queue even further, well it 
just collapses in on itself  and dies.
Is your app able to do *anything* else while the video is playing? 
 The slightest delay to the rendering and that Canvas buffering bug 
kills the app.  Not that it would matter if it could keep up, because 
the off-screen thing is also a deal breaker.
My app is basically just a video watching system -- while video is 
playing (in the bottom most layer of a StackPane), it shows overlays 
with time / position, possibly a menu (where you can download a matching 
subtitle for the video and adjust settings like playback speed and 
brightness).  These overlays are smoothly faded in / faded out just with 
the opacity property of another Pane that is at a higher level in the 
main StackPane.


None of this is really CPU intensive, nor is there a huge SceneGraph to 
deal with (I'm guessing its smaller than 100 nodes while video playback 
is running).  See picture here for an idea of what's going on while 
playback is occuring: 
https://github.com/hjohn/MediaSystem/blob/master/screenshot-1.jpg


The Stage used is a non-transparent one -- I mention this because a 
transparent change performs a lot worse than a non-transparent one.  
Before I used the Canvas solution, I'd actually just playback video in a 
java.awt.Frame with a transparent JavaFX Stage on top.   I'd have 2 
windows, with the transparent JavaFX Stage on top that would show 
overlays for the video, while the other window would show the video 
using java.awt.Canvas, accessed directly by VLC.


I do run into this issue every now and then though: 
https://javafx-jira.kenai.com/browse/RT-23312


It really needs to be fixed or Canvas is simply not safely usable in its 
current form (and IMHO never has been since its release then).


Of course 25fps is well below the 60fps required for full-speed video. 
 I suspect it is the frame rate more than the frame size that matters 
here.  Plain old, standard definition, interlaced, 60 fields per 
second will probably kill most apps with the current Canvas 
implementation.
I don't think I have aany 60fps video, if you can point me to something 
I can download that VLC can handle that is of the size you're using I 
could test with the setup I have here.  With 1920x1080 HD Video, the CPU 
uses 8% according to Task Manager (low-end quad core xeon, about 1 year 
old).  I'm running a standard Java VM (b99), no other tweaking, with 
default memory settings, 256 MB heap, Task Manager claims a working set 
of  +/- 600 MB, but some native memory might be involved -- when 
playback stops, working set drops to 340 MB, so there's definitely a lot 
going on.


It's solid though once playback starts (usually it only locks up at the 
start of playback, if it locks up) and can run for hours.  No frame 
stuttering (I'd notice this immediately on a horizontal pan of one of 
the test videos I use).  Even with a lot of other things going on on the 
same machine (although not by my Java process) playback stays solid -- I 
often have this machine running at 50-75% CPU for extended periods while 
enjoying a Video on the 3rd screen.


I'd agree though that more than doubling the frame rate is gonna be a 
huge impact.  I'm not certain if interlaced is going to cause any 
additional problems, I'm assuming that's handled by VLC and that I'd 
still be copying a full buffer for each frame (or do you mean 60 FPS 
interlaced = 120 FPS?).


We need something better.  I proposed having at least a JNI method so 
we could get native window handles from Stages but didn't get any 
traction either.  Security was brought up as a concern, which I 
totally do NOT understand as the use of JNI means you are out of the 
Java sandbox already.  If we had native window handles we might be 
able get our own window for rendering to at least interact nicely with 
the Stage.  (We already did this successfully in Swing via JAWT and a 
heavyweight component that we paint to from native code)
Getting a WindowHandle for a Stage would be great -- however, I think I 
actually hacked this in at one point, and then tried playing back video 
on a JavaFX Stage -- the video however always ended up in the 
foreground, obliterating any JavaFX rendering.  That would need to be 
solved as well if it is still an issue.


I've actually tried almost every video player solution that I could find 
(all on Windows), including DirectShow (using DSJ), GStreamer-java, 
MPlayer (in seperate window), VLCJ, MediaPlayer Classic Home Cinema and 
Xuggler.  All of them except VLCJ had severe issues, ranging 

Re: Canvas blowing up (was Re: JavaFX Media issues)

2013-08-09 Thread John Hendrikx

On 9/08/2013 20:15, Richard Bair wrote:

Also the proposed clear() or empty() option only applies to Canvas correct?
i.e. WritableImages don't suffer from these kind of issues and don't require 
such methods?

That is correct (WritableImage we don't provide a 2D API to use to fill the 
buffer, you just bash the pixels yourself however you like, so we don't have to 
buffer anything up).
Hm, I didn't realize WritableImage had the same kind PixelWriter 
interface.  For my usecase, where I just render full frames to a 
PixelWriter so JavaFX can display them in some fashion, why should I not 
use a WriteableImage instead?


Looking at the docs, I see that one limitation is that the WritableImage 
cannot be resized after construction (something that I do use with 
Canvas), but I think I could work around that by just recreating the 
Image when its size changes... I could just wrap a class around it that 
does this transparently.


Going to take a look if I can rip out the Canvas code and replace it 
with a resizable WritableImage and see what the results are for full 
screen video playback...


--John



Re: Poor quality font rendering

2013-08-21 Thread John Hendrikx
I think I also noticed a change in font rendering around b99 
somewhere... the fonts seem to be thinner than before, or perhaps more 
poorly aligned with pixel boundaries.  I'd prefer glyphs laid out in the 
same way each time, ie. letters are always on a new pixel boundary, so 
the same letter will look the same regardless of what preceeds it.  I 
have LCD rendering turned off as I donot appreciate colored fringes on 
my glyphs.


On 21/08/2013 14:53, John C. Turnbull wrote:

I have only really tested JavaFX extensively on Windows so my comments here
apply mainly to that platform.



It seems that even with a font smoothing type of LCD, font rendering in
JavaFX is not at the same level of quality of native applications.  My
current experiences are with JavaFX 8 b103 and I find that all rendered text
in JavaFX appears of a significantly poorer quality than that which I would
see in Word for example or even in IE10 (which I believe uses the same text
rendering engine).  Also, these observations are based on text in standard
controls and the quality of font rendering is dramatically worse within the
Canvas control.



I am not an expert in font technology but I have read many times that the
levels of antialiasing for text that can be achieved in a GPU-based renderer
are always going to be less than that achieved in a CPU-based renderer.
This is often explained on the basis of graphics card drivers being
optimised for performance and the rapid rendering of triangles commonly
required in games rather than for rendering quality when it comes to text.



Is this the reason why JavaFX font rendering appears less legible and of a
lower quality than native apps?

If so, how does IE10 for example achieve a higher quality of rendering when
it seems to also use DirectWrite?

Is the quality of JavaFX font rendering ever going to improve?



Thanks,



-jct





Re: Poor quality font rendering

2013-08-22 Thread John Hendrikx


I took another good look, and I see what is bothering me is mostly how 
the glyph 2 is rendered on my system (it has a thick appearing curve 
attached to the base).  I've included a screenshot of my application 
that uses several different sizes fonts, but it seems only the ones in 
the top bar are rendered somewhat wierd.


http://ukyo.xs4all.nl/Digit2RenderedPoorlyInTopBar.png

I'm on Windows 7, JavaFX 8b99, 32-bit, using D3D pipeline (I get this 
stuff in log in an infinite loop, so must be D3D I think):


D3D Vram Pool: 129,613,674 used (48.3%), 129,613,674 managed (48.3%), 
268,435,456 total   -- 
com.sun.prism.impl.ManagedResource.printSummary(ManagedResource.java:134)
75 total resources being 
managed  
-- com.sun.prism.impl.ManagedResource.printSummary(ManagedResource.java:153)
4 permanent resources 
(5.3%)  
-- com.sun.prism.impl.ManagedResource.printSummary(ManagedResource.java:154)
2 resources locked 
(2.7%) 
-- com.sun.prism.impl.ManagedResource.printSummary(ManagedResource.java:156)
43 resources contain interesting data 
(57.3%) -- 
com.sun.prism.impl.ManagedResource.printSummary(ManagedResource.java:158)
0 resources disappeared 
(0.0%)
-- com.sun.prism.impl.ManagedResource.printSummary(ManagedResource.java:160)


I also have this in main, before Application.launch is called:

System.setProperty(prism.lcdtext, false);

In .root in CSS I have:

  -fx-font-family: Arial;
  -fx-font-size: 16px;
  -fx-font-weight: normal;

So all the fonts you see should be Arial (but the sizes and weights are 
tweaked depending on location).


--John

On 21/08/2013 20:51, Felipe Heidrich wrote:

John H:

In JFX we decided to go with sub-pixel positioned text (as opposite to pixel 
grid aligned).

That said, on Windows for grayscale text, we are not doing that (yet). Are you 
running Windows, with D3D pipeline ?
I would need to see a picture to be sure I understand the problem you describe.

Felipe


On Aug 21, 2013, at 10:19 AM, John Hendrikx wrote:


I think I also noticed a change in font rendering around b99 somewhere... the 
fonts seem to be thinner than before, or perhaps more poorly aligned with pixel 
boundaries.  I'd prefer glyphs laid out in the same way each time, ie. letters 
are always on a new pixel boundary, so the same letter will look the same 
regardless of what preceeds it.  I have LCD rendering turned off as I donot 
appreciate colored fringes on my glyphs.

On 21/08/2013 14:53, John C. Turnbull wrote:

I have only really tested JavaFX extensively on Windows so my comments here
apply mainly to that platform.



It seems that even with a font smoothing type of LCD, font rendering in
JavaFX is not at the same level of quality of native applications.  My
current experiences are with JavaFX 8 b103 and I find that all rendered text
in JavaFX appears of a significantly poorer quality than that which I would
see in Word for example or even in IE10 (which I believe uses the same text
rendering engine).  Also, these observations are based on text in standard
controls and the quality of font rendering is dramatically worse within the
Canvas control.



I am not an expert in font technology but I have read many times that the
levels of antialiasing for text that can be achieved in a GPU-based renderer
are always going to be less than that achieved in a CPU-based renderer.
This is often explained on the basis of graphics card drivers being
optimised for performance and the rapid rendering of triangles commonly
required in games rather than for rendering quality when it comes to text.



Is this the reason why JavaFX font rendering appears less legible and of a
lower quality than native apps?

If so, how does IE10 for example achieve a higher quality of rendering when
it seems to also use DirectWrite?

Is the quality of JavaFX font rendering ever going to improve?



Thanks,



-jct





Re: Poor quality font rendering

2013-08-22 Thread John Hendrikx


Those are all normal controls, the plot section is just a Label for 
example.


On 22/08/2013 13:39, John C. Turnbull wrote:

John H, it may be just me but pretty much *all* the fonts in your screenshot
look quite poor and noticeably different from native font rendering.  If you
look for instance at the text in the Plot section, to me that text looks
awful.

Is that inside a WebView or some other control?

-jct

-Original Message-
From: openjfx-dev-boun...@openjdk.java.net
[mailto:openjfx-dev-boun...@openjdk.java.net] On Behalf Of John Hendrikx
Sent: Thursday, 22 August 2013 17:29
To: openjfx-dev@openjdk.java.net
Subject: Re: Poor quality font rendering


I took another good look, and I see what is bothering me is mostly how the
glyph 2 is rendered on my system (it has a thick appearing curve attached
to the base).  I've included a screenshot of my application that uses
several different sizes fonts, but it seems only the ones in the top bar are
rendered somewhat wierd.

http://ukyo.xs4all.nl/Digit2RenderedPoorlyInTopBar.png

I'm on Windows 7, JavaFX 8b99, 32-bit, using D3D pipeline (I get this stuff
in log in an infinite loop, so must be D3D I think):

D3D Vram Pool: 129,613,674 used (48.3%), 129,613,674 managed (48.3%),
268,435,456 total   --
com.sun.prism.impl.ManagedResource.printSummary(ManagedResource.java:134)
75 total resources being
managed

-- com.sun.prism.impl.ManagedResource.printSummary(ManagedResource.java:153)
4 permanent resources
(5.3%)

-- com.sun.prism.impl.ManagedResource.printSummary(ManagedResource.java:154)
2 resources locked
(2.7%)

-- com.sun.prism.impl.ManagedResource.printSummary(ManagedResource.java:156)
43 resources contain interesting data
(57.3%) --
com.sun.prism.impl.ManagedResource.printSummary(ManagedResource.java:158)
0 resources disappeared
(0.0%)

-- com.sun.prism.impl.ManagedResource.printSummary(ManagedResource.java:160)

I also have this in main, before Application.launch is called:

  System.setProperty(prism.lcdtext, false);

In .root in CSS I have:

-fx-font-family: Arial;
-fx-font-size: 16px;
-fx-font-weight: normal;

So all the fonts you see should be Arial (but the sizes and weights are
tweaked depending on location).

--John

On 21/08/2013 20:51, Felipe Heidrich wrote:

John H:

In JFX we decided to go with sub-pixel positioned text (as opposite to

pixel grid aligned).

That said, on Windows for grayscale text, we are not doing that (yet). Are

you running Windows, with D3D pipeline ?

I would need to see a picture to be sure I understand the problem you

describe.

Felipe


On Aug 21, 2013, at 10:19 AM, John Hendrikx wrote:


I think I also noticed a change in font rendering around b99 somewhere...

the fonts seem to be thinner than before, or perhaps more poorly aligned
with pixel boundaries.  I'd prefer glyphs laid out in the same way each
time, ie. letters are always on a new pixel boundary, so the same letter
will look the same regardless of what preceeds it.  I have LCD rendering
turned off as I donot appreciate colored fringes on my glyphs.

On 21/08/2013 14:53, John C. Turnbull wrote:

I have only really tested JavaFX extensively on Windows so my
comments here apply mainly to that platform.



It seems that even with a font smoothing type of LCD, font rendering
in JavaFX is not at the same level of quality of native
applications.  My current experiences are with JavaFX 8 b103 and I
find that all rendered text in JavaFX appears of a significantly
poorer quality than that which I would see in Word for example or
even in IE10 (which I believe uses the same text rendering engine).

Also, these observations are based on text in standard

controls and the quality of font rendering is dramatically worse
within the Canvas control.



I am not an expert in font technology but I have read many times
that the levels of antialiasing for text that can be achieved in a
GPU-based renderer are always going to be less than that achieved in a

CPU-based renderer.

This is often explained on the basis of graphics card drivers being
optimised for performance and the rapid rendering of triangles
commonly required in games rather than for rendering quality when it

comes to text.



Is this the reason why JavaFX font rendering appears less legible
and of a lower quality than native apps?

If so, how does IE10 for example achieve a higher quality of
rendering when it seems to also use DirectWrite?

Is the quality of JavaFX font rendering ever going to improve?



Thanks,



-jct





Re: Poor quality font rendering

2013-08-22 Thread John Hendrikx

On 22/08/2013 16:45, Phil Race wrote:

On 8/22/13 12:29 AM, John Hendrikx wrote:




System.setProperty(prism.lcdtext, false);




So you deliberately asked for it to be different than usual Windows 
apps ?
I'm not sure why you do this, but if its in effect we can't expect to 
make any
comparisons since IE10 etc will use LCD text I think. Its is the case 
on Windows

pretty much everywhere MS can apply it.
Sorry, don't confuse me with John T. -- I don't know what he did.   I've 
turned it off simply because of the numerous issues with LCD rendering 
(but those are not JavaFX specific) -- it is turned off system-wide 
though, so if I wanted to make a comparison, I should still be able to.


Anyway, I'm not making any direct comparisons (certainly not with IE10), 
but look at the screenshot, and see how poorly the 2 glyph is rendered 
in the top bar.  This wasn't like that before and I've had that top bar 
in for well over a year now.


--John



Re: Poor quality font rendering

2013-08-22 Thread John Hendrikx

On 22/08/2013 21:24, Scott Palmer wrote:

I just tried turning off ClearType on Windows and found that the result was
pretty ugly.  I assume that with ClearType turned off, the results should
be comparable to what JavaFX is producing with grey-scale smoothing.
It depends on how you did it, if you turned it fully off it may not even 
do grayscale smoothing... also the default font in Windows is not 
suitable for this.  See below.


I noticed that the same web page rendered text differently between Chrome
and Firefox - Firefox seemed to have darker text. So even among native apps
you can get different results for the same font.


If you want to turn off cleartype in Windows 7, then just turning it off 
is not sufficient.  You need to replace the default font with Tahoma as 
well as the Windows 7 default font (Segoe UI) sucks without LCD 
smoothing.  I can recommend the cleartype switch program, see here: 
http://karpolan.com/software/cleartype-switch/


You'll want to keep smooth edges on, but cleartype off.



Scott


On Thu, Aug 22, 2013 at 3:21 PM, Phil Racephilip.r...@oracle.com  wrote:


John T,

Per a couple of earlier emails in this thread John H is deliberately
disabling LCD text
so has grey scale, hence you can't compare directly with a native app
since they all use LCD.

Any comparison has to be
1. Using the same string
2. Rendered in the same fashion : LCD or greyscale, hinted/unhinted.
3. Using the same font (face, size etc)
4. Using the same colours (fgbg) and watch out for the UI controls - they
used
to tweak your text colour so that (eg) black became dark grey, reducing
contrast
which reduced legibility, thus also invalidating any  test. I am not sure
if this is still the case,
but contrast is very important for text and you should make sure its as
high as possible.

-phil.




On 8/22/2013 11:35 AM, John C. Turnbull wrote:


Am I the only one who looks at that screenshot and thinks that the fonts
look really bad and obviously different from a native app?  Its not just
the Plot section, its all text on the screen.

This is what I am talking about.  I wouldn't even describe the
differences as subtle as to me there is a dramatic difference in quality.

Can this be fixed or are JavaFX apps always going to stand out for all
the wrong reasons like this?

On 23/08/2013, at 0:05, John Hendrikxhj...@xs4all.nl  wrote:

  Those are all normal controls, the plot section is just a Label for

example.

On 22/08/2013 13:39, John C. Turnbull wrote:


John H, it may be just me but pretty much *all* the fonts in your
screenshot
look quite poor and noticeably different from native font rendering.
  If you
look for instance at the text in the Plot section, to me that text
looks
awful.

Is that inside a WebView or some other control?

-jct

-Original Message-
From: 
openjfx-dev-bounces@openjdk.**java.netopenjfx-dev-boun...@openjdk.java.net
[mailto:openjfx-dev-bounces@**openjdk.java.netopenjfx-dev-boun...@openjdk.java.net]
On Behalf Of John Hendrikx
Sent: Thursday, 22 August 2013 17:29
To: openjfx-dev@openjdk.java.net
Subject: Re: Poor quality font rendering


I took another good look, and I see what is bothering me is mostly how
the
glyph 2 is rendered on my system (it has a thick appearing curve
attached
to the base).  I've included a screenshot of my application that uses
several different sizes fonts, but it seems only the ones in the top
bar are
rendered somewhat wierd.

http://ukyo.xs4all.nl/**Digit2RenderedPoorlyInTopBar.**pnghttp://ukyo.xs4all.nl/Digit2RenderedPoorlyInTopBar.png

I'm on Windows 7, JavaFX 8b99, 32-bit, using D3D pipeline (I get this
stuff
in log in an infinite loop, so must be D3D I think):

D3D Vram Pool: 129,613,674 used (48.3%), 129,613,674 managed (48.3%),
268,435,456 total   --
com.sun.prism.impl.**ManagedResource.printSummary(**
ManagedResource.java:134)
75 total resources being
managed

-- com.sun.prism.impl.**ManagedResource.printSummary(**
ManagedResource.java:153)
4 permanent resources
(5.3%)

-- com.sun.prism.impl.**ManagedResource.printSummary(**
ManagedResource.java:154)
2 resources locked
(2.7%)

-- com.sun.prism.impl.**ManagedResource.printSummary(**
ManagedResource.java:156)
43 resources contain interesting data
(57.3%) --
com.sun.prism.impl.**ManagedResource.printSummary(**
ManagedResource.java:158)
0 resources disappeared
(0.0%)

-- com.sun.prism.impl.**ManagedResource.printSummary(**
ManagedResource.java:160)

I also have this in main, before Application.launch is called:

   System.setProperty(prism.**lcdtext, false);

In .root in CSS I have:

 -fx-font-family: Arial;
 -fx-font-size: 16px;
 -fx-font-weight: normal;

So all the fonts you see should be Arial (but the sizes and weights are
tweaked depending on location).

--John

On 21/08/2013 20:51, Felipe Heidrich wrote:


John H:

In JFX we decided to go with sub-pixel positioned text (as opposite to


pixel grid aligned

Re: Poor quality font rendering

2013-08-22 Thread John Hendrikx

On 22/08/2013 17:38, Felipe Heidrich wrote:


John, for the sake of testing.

Could you enable LCD text for the plot Label in your app ?
Please let me know if that improves the position of each glyph (relative to one 
another).

You should also try your app using our previous text rasterizer, run with 
-Dprism.text=t2k
Is that better for you ?


Sure, I've created approximately the same shots with different settings 
for comparison:


prism.lcdtext=false
http://ukyo.xs4all.nl/Digit2RenderedPoorlyInTopBar.png

prism.lcdtext=true
http://ukyo.xs4all.nl/LCDSmoothed.png

prism.lcdtext=true, prism.text=t2k
http://ukyo.xs4all.nl/LCDSmoothedT2K.png

prism.lcdtext=false, prism.text=t2k
http://ukyo.xs4all.nl/GrayscaleT2K.png

Now that I compared them all, the last one is how it was for the longest 
time.  Now I also noticed that in the last shot the font height is a 
pixel smaller, which is one the changes I couldn't put my finger on.


The LCD smoothed versions look quite good, but (as you can see in 
screenshot 2) it for same reason garbles my memory indicator when the 
background image is fading in.  After the fade-in completes, the text is 
correct (I screenshotted it just before the fade-in completes to show 
the garbled text).  This occurs with both the normal and t2k renderer 
when LCD text is on.


So, to make it clear:

1) Grayscale only: some glyphs look akward when bolded
2) Grayscale only + t2k: this is how I expected it to be (I tuned the 
look for that).  Apparently this default has changed(?).  The font in 
the top bar seems to be 1 pixel less high then in all the other version.
3) LCD only: Looks quite good on my normal LCD screens (better than 
Windows in my opinion, but perhaps the light-on-dark vs dark-on-light is 
the cause of that)

4) LCD only + t2k: Also looks good, I think (3) looks better though

3+4 fail when I drag the Window to my projector.  Colored fringes are 
visible.  This is probably a driver problem as Windows may not realize 
that my Projector is not using LCD (DLP projector).  I'm not sure how 
JavaFX figures this out, but I assume it asks Windows for this information.


--John




Regards,
Felipe



On Aug 22, 2013, at 7:05 AM, John Hendrikx wrote:


Those are all normal controls, the plot section is just a Label for example.

On 22/08/2013 13:39, John C. Turnbull wrote:

John H, it may be just me but pretty much *all* the fonts in your screenshot
look quite poor and noticeably different from native font rendering.  If you
look for instance at the text in the Plot section, to me that text looks
awful.

Is that inside a WebView or some other control?

-jct

-Original Message-
From: openjfx-dev-boun...@openjdk.java.net
[mailto:openjfx-dev-boun...@openjdk.java.net] On Behalf Of John Hendrikx
Sent: Thursday, 22 August 2013 17:29
To: openjfx-dev@openjdk.java.net
Subject: Re: Poor quality font rendering


I took another good look, and I see what is bothering me is mostly how the
glyph 2 is rendered on my system (it has a thick appearing curve attached
to the base).  I've included a screenshot of my application that uses
several different sizes fonts, but it seems only the ones in the top bar are
rendered somewhat wierd.

http://ukyo.xs4all.nl/Digit2RenderedPoorlyInTopBar.png

I'm on Windows 7, JavaFX 8b99, 32-bit, using D3D pipeline (I get this stuff
in log in an infinite loop, so must be D3D I think):

D3D Vram Pool: 129,613,674 used (48.3%), 129,613,674 managed (48.3%),
268,435,456 total   --
com.sun.prism.impl.ManagedResource.printSummary(ManagedResource.java:134)
75 total resources being
managed

-- com.sun.prism.impl.ManagedResource.printSummary(ManagedResource.java:153)
4 permanent resources
(5.3%)

-- com.sun.prism.impl.ManagedResource.printSummary(ManagedResource.java:154)
2 resources locked
(2.7%)

-- com.sun.prism.impl.ManagedResource.printSummary(ManagedResource.java:156)
43 resources contain interesting data
(57.3%) --
com.sun.prism.impl.ManagedResource.printSummary(ManagedResource.java:158)
0 resources disappeared
(0.0%)

-- com.sun.prism.impl.ManagedResource.printSummary(ManagedResource.java:160)

I also have this in main, before Application.launch is called:

  System.setProperty(prism.lcdtext, false);

In .root in CSS I have:

-fx-font-family: Arial;
-fx-font-size: 16px;
-fx-font-weight: normal;

So all the fonts you see should be Arial (but the sizes and weights are
tweaked depending on location).

--John

On 21/08/2013 20:51, Felipe Heidrich wrote:

John H:

In JFX we decided to go with sub-pixel positioned text (as opposite to

pixel grid aligned).

That said, on Windows for grayscale text, we are not doing that (yet). Are

you running Windows, with D3D pipeline ?

I would need to see a picture to be sure I understand the problem you

describe.

Felipe


On Aug 21, 2013, at 10:19 AM, John Hendrikx wrote:


I think I also noticed a change in font rendering around b99

Re: Poor quality font rendering

2013-08-22 Thread John Hendrikx

On 22/08/2013 21:41, John C. Turnbull wrote:

Hi Phil,

I was actually aware that LCD text was disabled in this case but what I am 
saying is that I am seeing similar stark disparities between JavaFX fonts and 
native fonts even with LCD turned on.

I think the screenshot shows what I am talking about though perhaps the quality 
is poorer than my case because of the LCD issue.

The point is that there are still differences no matter what I do and to me at least 
these differences are very, very obvious.  I am trying to establish if what we have now 
is as good as it gets or if things are going to improve.

Also, it's clear from this thread that not everyone agrees on what looks good. 
It would seem that John H is concerned only with the weird '2' and feels that 
grayscale text is better than LCD text whereas I am more concerned with 
resembling native rendering.
I made new screenshots with different combinations of settings if you 
wish to compare those.  The strange 2 is what jumped out at me as 
something that changed recently, but on closer inspection I see many 
bolded glyphs that look poor in the original screenshot:


The 1 has a rather big top edge.  The H is thicker in one leg than 
the other.  The s has a thick right curve.  The / in top bar looks 
uneven.  The o, 0, 6, 9 and 5 glyphs hardly look bold at all.


I think the LCD smoothed versions looks quite good on my regular LCD 
monitor -- the problem is that it doesn't look good on the projector I'm 
using so I just turned it off for that.


Oh.. I forgot to say this, but...

Screenshots taken with LCD smoothing on are always gonna end up looking 
different on different monitors... if for example your monitor has 
slightly different spacing or a different order of the subpixels (or you 
rotated it), then the screenshot will look wierd.  I prefer to keep LCD 
smoothing off as I make screenshots / videos regularly and I donot know 
on what system they'll be viewed on.  So if one of shots has 
particularly bad colored fringes, it is likely you have a monitor that 
has a different configuration than mine.


--John



On 23/08/2013, at 5:21, Phil Racephilip.r...@oracle.com  wrote:


John T,

Per a couple of earlier emails in this thread John H is deliberately disabling 
LCD text
so has grey scale, hence you can't compare directly with a native app since 
they all use LCD.

Any comparison has to be
1. Using the same string
2. Rendered in the same fashion : LCD or greyscale, hinted/unhinted.
3. Using the same font (face, size etc)
4. Using the same colours (fgbg) and watch out for the UI controls - they used
to tweak your text colour so that (eg) black became dark grey, reducing contrast
which reduced legibility, thus also invalidating any  test. I am not sure if 
this is still the case,
but contrast is very important for text and you should make sure its as high as 
possible.

-phil.



On 8/22/2013 11:35 AM, John C. Turnbull wrote:

Am I the only one who looks at that screenshot and thinks that the fonts look 
really bad and obviously different from a native app?  Its not just the Plot 
section, its all text on the screen.

This is what I am talking about.  I wouldn't even describe the differences as 
subtle as to me there is a dramatic difference in quality.

Can this be fixed or are JavaFX apps always going to stand out for all the 
wrong reasons like this?

On 23/08/2013, at 0:05, John Hendrikxhj...@xs4all.nl  wrote:


Those are all normal controls, the plot section is just a Label for example.

On 22/08/2013 13:39, John C. Turnbull wrote:

John H, it may be just me but pretty much *all* the fonts in your screenshot
look quite poor and noticeably different from native font rendering.  If you
look for instance at the text in the Plot section, to me that text looks
awful.

Is that inside a WebView or some other control?

-jct

-Original Message-
From: openjfx-dev-boun...@openjdk.java.net
[mailto:openjfx-dev-boun...@openjdk.java.net] On Behalf Of John Hendrikx
Sent: Thursday, 22 August 2013 17:29
To: openjfx-dev@openjdk.java.net
Subject: Re: Poor quality font rendering


I took another good look, and I see what is bothering me is mostly how the
glyph 2 is rendered on my system (it has a thick appearing curve attached
to the base).  I've included a screenshot of my application that uses
several different sizes fonts, but it seems only the ones in the top bar are
rendered somewhat wierd.

http://ukyo.xs4all.nl/Digit2RenderedPoorlyInTopBar.png

I'm on Windows 7, JavaFX 8b99, 32-bit, using D3D pipeline (I get this stuff
in log in an infinite loop, so must be D3D I think):

D3D Vram Pool: 129,613,674 used (48.3%), 129,613,674 managed (48.3%),
268,435,456 total   --
com.sun.prism.impl.ManagedResource.printSummary(ManagedResource.java:134)
75 total resources being
managed

-- com.sun.prism.impl.ManagedResource.printSummary(ManagedResource.java:153)
4 permanent resources
(5.3%)

-- com.sun.prism.impl.ManagedResource.printSummary

Re: Font size

2013-08-22 Thread John Hendrikx

On 22/08/2013 22:58, Peter Penzov wrote:

Hi,
I'm working on JavaFX application based on Java 8. It seems that from
version b97 there is a issue with -fx-font-size. It's not working properly.
If you compare the two images from application tested on JavaFX 2.2 and
JavaFX 8 b103 you will see that there is a difference. Can you give me some
information when this issue will be fixed?


Could you try do:

 System.setProperty(prism.text, t2k);

or:

 -Dprism.text=t2k

I noticed somewhere between the builds you mentioned that the apparent 
font size in some areas changed slightly.  Perhaps the above setting 
will help locate the problem.




Regards




Re: Poor quality font rendering

2013-08-28 Thread John Hendrikx

On 27/08/2013 17:59, Felipe Heidrich wrote:

Hi John

Thank you for the images!

Comparing DirectWrite versus T2K for grayscale text the main difference that 
jumps at me is that in DirectWrite (which is what you get when prism.text=t2k 
is not set) the glyphs look a touch taller, looking somewhat stretched. 
Inspecting it closely you can see the actual pixel size is the same, it is the 
anti-aliasing at the top that is different.
I'll see if anything can be done differently in DirectWrite to avoid this 
'stretched' look, would that be better ?
The stretched look doesn't really bother me, it was just something that 
changed (the apparent height) that I couldn't put my finger on.  What 
botheres me more is that the bolded glyphs (especially the 2)  look 
wierd, the thickness of parts of the same glyph seems to vary (not just 
the 2 is affected, more glyphs look a touch worse, especially in the 
top bar in my first screenshot: 
http://ukyo.xs4all.nl/Digit2RenderedPoorlyInTopBar.png -- the legs of 
the M and H for example have different thickness.. the 2 jumps out though)


Whether those are result of the stretched look or not I cannot say.  I 
can also imagine that if you're using DirectWrite that there's little 
that can be done about it.  Are there any advantages of using DW over t2k?



DirectWrite LCD for me is the one that gives the best result. That said, that 
is something wrong going on in the text over the progress bar.
Yes, DirectWrite LCD certainly looks crisp, unfortunately I cannot use 
it as my target is not an LCD screen.


I think however I know now what causes the text bug in the progress 
bar.  It's not really a progress bar, but just memory bar with text on 
it that's supposed to be readable no matter what using an inversion 
effect... It's this effect that I suspect causes problems with the 
tricks used for LCD rendering, as a simple inversion of LCD rendered 
glyphs is probably not gonna give the desired smooth result.


It's basically this, a Rectangle (with some CSS styling) and a Label in 
a StackPane.  The BlendMode.DIFFERENCE is what I suspect is causing the 
issue.  If so, LCD rendering may have to be disabled in these cases:


  private final Rectangle memoryBar = new Rectangle();
  private final Label memText = new Label();

  private final StackPane gc = new StackPane() {{
getStyleClass().addAll(element, memory);

getChildren().add(new StackPane() {{
  getStyleClass().add(bar);
  getChildren().addAll(memoryBar, memText);
}});

memText.setBlendMode(BlendMode.DIFFERENCE);

memoryBar.setWidth(150);
memoryBar.heightProperty().bind(memText.heightProperty());
  }};


Is it possible to run the same test case on Mac ? (Grayscale text, 
prism.text=t2k *not* set).

Unfortunately, I donot own a mac, and can't easily borrow one at this time.

--John



Regards
Felipe


On Aug 22, 2013, at 12:54 PM, John Hendrikx wrote:


On 22/08/2013 17:38, Felipe Heidrich wrote:

John, for the sake of testing.

Could you enable LCD text for the plot Label in your app ?
Please let me know if that improves the position of each glyph (relative to one 
another).

You should also try your app using our previous text rasterizer, run with 
-Dprism.text=t2k
Is that better for you ?

Sure, I've created approximately the same shots with different settings for 
comparison:

prism.lcdtext=false
http://ukyo.xs4all.nl/Digit2RenderedPoorlyInTopBar.png

prism.lcdtext=true
http://ukyo.xs4all.nl/LCDSmoothed.png

prism.lcdtext=true, prism.text=t2k
http://ukyo.xs4all.nl/LCDSmoothedT2K.png

prism.lcdtext=false, prism.text=t2k
http://ukyo.xs4all.nl/GrayscaleT2K.png

Now that I compared them all, the last one is how it was for the longest time.  
Now I also noticed that in the last shot the font height is a pixel smaller, 
which is one the changes I couldn't put my finger on.

The LCD smoothed versions look quite good, but (as you can see in screenshot 2) 
it for same reason garbles my memory indicator when the background image is 
fading in.  After the fade-in completes, the text is correct (I screenshotted 
it just before the fade-in completes to show the garbled text).  This occurs 
with both the normal and t2k renderer when LCD text is on.

So, to make it clear:

1) Grayscale only: some glyphs look akward when bolded
2) Grayscale only + t2k: this is how I expected it to be (I tuned the look for 
that).  Apparently this default has changed(?).  The font in the top bar seems 
to be 1 pixel less high then in all the other version.
3) LCD only: Looks quite good on my normal LCD screens (better than Windows in 
my opinion, but perhaps the light-on-dark vs dark-on-light is the cause of that)
4) LCD only + t2k: Also looks good, I think (3) looks better though

3+4 fail when I drag the Window to my projector.  Colored fringes are visible.  
This is probably a driver problem as Windows may not realize that my Projector 
is not using LCD (DLP projector).  I'm not sure how JavaFX figures this out

Re: Why is almost everything in the API final

2013-09-03 Thread John Hendrikx

On 3/09/2013 20:34, Richard Bair wrote:

I would strongly recommend leaving the shared JRE install world behind.

As a suggestion, try JWrapper - we have flawless installs now, even using an 
OSGI deployment procedure! Bundled JVMs are really the only dependable way to 
go now it seems?

If my business were betting on it, I'd not use a shared install for a couple 
reasons:
   - I want to control the *exact* version of the JRE such that my app testing 
was done against a specific version of the JRE
   - I have the freedom to modify the JRE as needed for my app
   - I can deploy as a normal desktop app using normal mechanisms
   - Related, I don't have to field support requests around what version of 
Java is installed or not, or Java install problems

I can still have auto-update with an app cobundle, so I don't miss out there 
either.

None of these points are suggesting the problem is with WebStart's 
implementation, they all hold even if WebStart were completely bug free. 
They're just the natural side-effect of a shared install system.
Actually, one of the best things of Java is that in like 99.9% of the 
cases, my app will run flawlessly on whatever version of the JDK or JRE 
is installed... on multiple platforms.  If Java goes down the road of 
requiring testing against a specific JRE (and thus specific platform by 
extension) then that is its biggest advantage lost.


In fact, any framework that does not aspire to do the same (being 
backwards compatible to a flaw) is not a framework I'll consider for 
production use.  I want to be able to upgrade, and at most run my unit 
tests to have the confidence that it will perform as expected.  There 
can always be mistakes, both in the frameworks or in my code doing 
something that was undocumented or incorrectly interpreted, but in the 
end backwards compatibility is what makes or breaks a framework -- a big 
reason for that is that such frameworks are actually designed with some 
care, exactly because they aspire to be backward compatible... and a 
good API is everything.


--John



Re: Moving on to a round house kick (forked from Re: JavaOne roundup?)

2013-09-30 Thread John Hendrikx

On 30/09/2013 17:38, Anton Epple wrote:

Hi guys,

  I understand your frustration about the cancelled sessions, and I share it. 
But when I talk to the engineers and see their posts here, they're clearly 
interested in the same stuff we'd like to see in JavaFX. I guess nobody was 
more frustrated that these sessions were cancelled than the engineers who 
submitted them. If you want to talk about something new and exiting you will 
have to let some company lawyers approve it. This takes some time. My guess is, 
that the approval for the talks might not have arrived in time.
To be honest, it is likely JavaFX already missed its window to become 
relevant on Android and iOS.  These platforms are not waiting for anyone 
and are being aggressively pushed into all kinds of new areas (console 
gaming, entertainment hubs, general productivity, etc).  Oracle should 
count itself lucky that Google even used a Java-like language for its 
platform or they'd stand no chance at all anymore in this space. There 
are already dozens of frameworks that work with Dalvik that compete in 
atleast part of the same space as JavaFX -- many of them cross 
platform.  Just one of these needs to be actively pushed by a big name 
and gone is your opportunity.


These markets donot move at the snails pace that lawyers and courts 
move.  Limiting yourself to the speed of your legal department is a 
guaranteed way to become irrelevant.


My 2 cents
--John



If I was right, and the reason for the talks being removed are just of temporary nature, 
then I guess the best strategy now is to keep calm and carry on for a bit.

Regards

--Toni

P.s.: @Matthias:
Regarding your thoughts about JavaFX in a browser:
- WORA matters - I think it's the whole point that started this discussion.
- Using Cordova you can package your app as a native app. So you've got a 
working solution, which is admittedly not feature complete and not usable for 
every application, but much better than nothing.
- JavaScript is a huge problem as it leads to ugly unmaintainable code. Right 
now there are tons of projects desperately trying to solve that issue (GWT, 
typescript, ...). bck2brwsr is one of the solutions. It enables you to write 
clean Java(FX) code and still run in the browser without the need to install 
any plugin. So bck2brwsr solves a real world problem. That's why it matters.




Am 30.09.2013 um 14:03 schrieb Matthias Hänelhae...@ultramixer.com:


Hi,


@Felix: you are kidding are you? We cannot take another breath without choking 
on it. Sure there
are many positive things about JavaFX but in the real world I can't be happy 
over and over again about
the same things. A university can just devlop until a certain point, but we 
have a running bussiness
where we need to decide the future of the underlaying technology.

This is my very first post to this mailing list. My collegue tobi is an active 
member of this community.
He is head of the java devlopement department in our company and I am the 
counterpart by managing the
backend native codes and the interfacing to JNI/Java for the upper layers.
Since Javafx could be a game changer for our company we have had internal 
workshops for the developers
to get a common sense about the furture of development directions. This summer 
we focused our development
on JavaFX for further products. This meant reworking all UI-stuff, cleaning 
APIs and fixing JNI for java8.

Tobi was soo excited to see the new technologies and his presentation to our 
fellow developers has been
more than ethusiastic. It sounded almost like the old dream 
code-once-run-anywhere comes true. The closer
JavaOne got and the more session of interest for us has been canceled, the more 
we got fed up over here.
As a result non of the session that had been a sort of interest for us had been 
held. Just to summarize
our feeling about that, we are taking this really personally. There is 
investment of money and time on
one side and on the other side it is personal investment into a future 
technology.


I would like to give you an overview of the things that happend and how they 
appear over here.

What did we heard over here from JavaOne?

1. JavaFX is still in development
2. Dukepad is released
3. Oracle wong a sailing cup
(4. Javafx runs in a browser)


I'll start at the bottom:

(4. When Javafx runs in a browser, why do I need it? I could use JavaScript and 
web technologies as well.
This is quite a failure of time investment. Sure write-once-run-anywhere 
applies but all tough real world
applications are not buildable since there is no native interfacing and won't 
be cross platform in the near future.)

3. Larry Ellison spent 200 million dollar to win a sailing cup.
I don't want to image what Oracle could have been done to revolutionize the 
world. I don't speak only about JavaFX,
there is a lot to be done with the right power. But doesn't lead to much here.

2. Wow, there is a JavaFX enabled Dukepad. Beeing a soldering nerd myself, 

Problem with Timeline keeping references around

2013-10-15 Thread John Hendrikx

Hi List,

I just submitted https://javafx-jira.kenai.com/browse/RT-33600 which is 
asking for the Timeline JavaDocs to be more clear on when and where hard 
references are being created and how to properly clean up after 
oneself.  Most of the docs hardly mention anything when it comes to 
references causing memory leaks, and Timeline is no exception.


However, apart from the docs not really discussing this topic, I 
actually cannot get proper cleanup to work at all when working with a 
Timeline.


If my Timeline is referencing properties from a StackPane and the 
Timeline was run atleast once, I cannot figure out how to get this Pane 
to be gc'd.  Calling stop() and even doing getKeyFrames().clear() 
doesn't help.  I'm sure it must be the Timeline because if I remove the 
KeyFrames that access the StackPane, it will get gc'd properly.  If I 
never run the animation, it also gets gc'd properly.


See code below.  The System.outs will occur exactly as expected when the 
StackPane becomes part of the Scene and when it is removed from the 
Scene.  I call stop() on my Timeline and even clear its KeyFrames, but 
the StackPane refuses to be gc'd (even after hitting the GC button I 
created for this occasion dozens of times).


Not calling playFromStart() or not using the KeyFrames that refer to 
detailsOverlay.opacityProperty() will solve the problem, and the 
finalize() System.out will occur usually after the first or second time 
I press my GC button.


Anything else I can try or is this a bug?
--John

public class PlaybackOverlayPane extends StackPane {
  private final Timeline fadeInSustainAndFadeOut = new Timeline(
new KeyFrame(Duration.seconds(0)),
new KeyFrame(Duration.seconds(1), new 
KeyValue(detailsOverlay.opacityProperty(), 1.0)),
new KeyFrame(Duration.seconds(6), new 
KeyValue(detailsOverlay.opacityProperty(), 1.0)),
new KeyFrame(Duration.seconds(9), new 
KeyValue(detailsOverlay.opacityProperty(), 0.0))

  );

  private final ChangeListenerScene sceneChangeListener = new 
ChangeListenerScene() {

@Override
public void changed(ObservableValue? extends Scene observable, 
Scene oldValue, Scene newValue) {

  if(newValue != null) {
System.out.println( Starting fadeIn anim);
fadeInSustainAndFadeOut.playFromStart();
  }
  else {
System.out.println( Stopping fadeIn anim);
fadeInSustainAndFadeOut.stop();
fadeInSustainAndFadeOut.getKeyFrames().clear();
fadeInSustainAndFadeOut.stop();
  }
}
  };

  private final GridPane detailsOverlay = GridPaneUtil.create(new 
double[] {5, 20, 5, 65, 5}, new double[] {45, 50, 5});


 ...

  @Override
  protected void finalize() throws Throwable {
super.finalize();
System.out.println( Finalized  + this);
  }
}


Re: Problem with Timeline keeping references around

2013-10-15 Thread John Hendrikx
 F Northover wrote:
This looks like a bug.  Timeline should not be holding on to 
references.  Please enter a JIRA.


Steve

On 2013-10-15 6:35 PM, John Hendrikx wrote:

Hi List,

I just submitted https://javafx-jira.kenai.com/browse/RT-33600 which 
is asking for the Timeline JavaDocs to be more clear on when and 
where hard references are being created and how to properly clean up 
after oneself.  Most of the docs hardly mention anything when it 
comes to references causing memory leaks, and Timeline is no exception.


However, apart from the docs not really discussing this topic, I 
actually cannot get proper cleanup to work at all when working with a 
Timeline.


If my Timeline is referencing properties from a StackPane and the 
Timeline was run atleast once, I cannot figure out how to get this 
Pane to be gc'd.  Calling stop() and even doing 
getKeyFrames().clear() doesn't help.  I'm sure it must be the 
Timeline because if I remove the KeyFrames that access the StackPane, 
it will get gc'd properly.  If I never run the animation, it also 
gets gc'd properly.


See code below.  The System.outs will occur exactly as expected when 
the StackPane becomes part of the Scene and when it is removed from 
the Scene.  I call stop() on my Timeline and even clear its 
KeyFrames, but the StackPane refuses to be gc'd (even after hitting 
the GC button I created for this occasion dozens of times).


Not calling playFromStart() or not using the KeyFrames that refer to 
detailsOverlay.opacityProperty() will solve the problem, and the 
finalize() System.out will occur usually after the first or second 
time I press my GC button.


Anything else I can try or is this a bug?
--John

public class PlaybackOverlayPane extends StackPane {
  private final Timeline fadeInSustainAndFadeOut = new Timeline(
new KeyFrame(Duration.seconds(0)),
new KeyFrame(Duration.seconds(1), new 
KeyValue(detailsOverlay.opacityProperty(), 1.0)),
new KeyFrame(Duration.seconds(6), new 
KeyValue(detailsOverlay.opacityProperty(), 1.0)),
new KeyFrame(Duration.seconds(9), new 
KeyValue(detailsOverlay.opacityProperty(), 0.0))

  );

  private final ChangeListenerScene sceneChangeListener = new 
ChangeListenerScene() {

@Override
public void changed(ObservableValue? extends Scene observable, 
Scene oldValue, Scene newValue) {

  if(newValue != null) {
System.out.println( Starting fadeIn anim);
fadeInSustainAndFadeOut.playFromStart();
  }
  else {
System.out.println( Stopping fadeIn anim);
fadeInSustainAndFadeOut.stop();
fadeInSustainAndFadeOut.getKeyFrames().clear();
fadeInSustainAndFadeOut.stop();
  }
}
  };

  private final GridPane detailsOverlay = GridPaneUtil.create(new 
double[] {5, 20, 5, 65, 5}, new double[] {45, 50, 5});


 ...

  @Override
  protected void finalize() throws Throwable {
super.finalize();
System.out.println( Finalized  + this);
  }
}






Re: Use ScenePulseListener to avoid expensive recalculations?

2013-11-06 Thread John Hendrikx

On 5/11/2013 20:10, Jonathan Giles wrote:

You're right in that controls don't tend to use ScenePulseListener. This
may be due to an oversight on our part, or just that our requirements
differ (I don't honestly know).

You're also right that it is important to batch up property changes and
do them once per pulse, rather than as they come in. If this is not done
you do indeed get a severe performance hit (I remember back in the day
before I optimised many of the controls in this way, the performance
impact of not doing this was staggering).

The way I do it is simple: in places where I receive events from
properties / bindings / listeners, where I know that there is
potentially a lot of changes coming through, I create a boolean (e.g.
'refreshView' or somesuch). I then set this boolean to true, and in the
appropriate place in the code (most commonly layoutChildren(), but
sometimes in the computePref*() methods), I start the method with code
like this:

if (refreshView) {
 doExpensiveOperationThatShouldHappenOnlyOncePerPulse();
 refreshView = false;
}

The reason why I sometimes use layoutChildren() and sometimes the
computePref*() methods comes down to the (new) rule in JavaFX 8.0 that
you can not change the children list of a node within the
layoutChildren() method. Because of this, if I need to modify the
children list, I put the above code in the computePref*() method.


I'm not sure in which circumstances layoutChildren() will be called or 
if I can mark my control as needing layout, since the properties I want 
to trigger on are not directly attached to anything that JavaFX will be 
monitoring.  The ScenePulseListener therefore seems like a good choice 
-- the reason I'm asking about is that I cannot find any examples and 
the documentation for it is also pretty lean.


I'll experiment with a ScenePulseListener and see how it pans out, it 
looks like it would be an elegant solution.


Thanks Jonathan!




I hope that helps.

-- Jonathan

On 6/11/2013 3:58 a.m., John Hendrikx wrote:

Hi List,

I'm considering using a ScenePulseListener to avoid expensive
recalculations when multiple bindings trigger an action.

My problem is this:

I like to build Views (Controls) that are dumb and expose properties
that can be manipulated by whatever is using the View.  The View
listens to its own Properties and adjusts its state accordingly.  Some
of these adjustments are related (like width + height) and can be
expensive when calculated immediately.  So I would like to mark the
View as invalid and recalculate its state (if invalid) on the next
Pulse.

My current use case I'm looking at is a View that wraps (amongst
others) a TreeView.  The View exposes an ObservableList and a
BooleanProperty that decides whether the first level of the Tree
should be displayed as Tabs or as Nodes (which has an impact on what
Nodes actually are added to the TreeView, and which are added as
Tabs).  User code will thus often set a new list of nodes + change the
boolean to show tabs or nodes.  The View currently naively has
InvalidationListeners on both of these properties which cause
TreeNodes to be created after the first change... then discarded and
recreated after the second change by the user code, ie:

   view.nodesProperty().setAll(nodes);   // Recreates all
Tabs/TreeNodes with the current value of expand top level, as we donot
know another change might be incoming...
   view.expandTopLevelProperty().set(false);   // Recreates all
Tabs/TreeNodes again if expand top level was changed...

This specific problem might be done in a better way, but the point
remains, how do I avoid expensive calculations when multiple
properties get changed one after the other by the user code?  I'm
assuming that JavaFX controls already avoid these kinds of things, and
I'd like to know whether using a ScenePulseListener is the way to go,
or that it can/should be done in a different way -- examining the code
for TreeView (and its superclasses), I couldn't find uses of
ScenePulseListener...

--John








Re: Use ScenePulseListener to avoid expensive recalculations?

2013-11-07 Thread John Hendrikx

On 7/11/2013 09:20, Martin Sladecek wrote:

On 11/06/2013 07:31 PM, John Hendrikx wrote:

On 5/11/2013 20:10, Jonathan Giles wrote:
You're right in that controls don't tend to use ScenePulseListener. 
This

may be due to an oversight on our part, or just that our requirements
differ (I don't honestly know).

You're also right that it is important to batch up property changes and
do them once per pulse, rather than as they come in. If this is not 
done

you do indeed get a severe performance hit (I remember back in the day
before I optimised many of the controls in this way, the performance
impact of not doing this was staggering).

The way I do it is simple: in places where I receive events from
properties / bindings / listeners, where I know that there is
potentially a lot of changes coming through, I create a boolean (e.g.
'refreshView' or somesuch). I then set this boolean to true, and in the
appropriate place in the code (most commonly layoutChildren(), but
sometimes in the computePref*() methods), I start the method with code
like this:

if (refreshView) {
 doExpensiveOperationThatShouldHappenOnlyOncePerPulse();
 refreshView = false;
}

The reason why I sometimes use layoutChildren() and sometimes the
computePref*() methods comes down to the (new) rule in JavaFX 8.0 that
you can not change the children list of a node within the
layoutChildren() method. Because of this, if I need to modify the
children list, I put the above code in the computePref*() method.


I'm not sure in which circumstances layoutChildren() will be called 
or if I can mark my control as needing layout, since the properties I 
want to trigger on are not directly attached to anything that JavaFX 
will be monitoring.  The ScenePulseListener therefore seems like a 
good choice -- the reason I'm asking about is that I cannot find any 
examples and the documentation for it is also pretty lean.
If you call requestLayout(), the layoutChildren() will be called on 
the next pulse. The ScenePulseListener is not a public API and the 
layout is the only way how Controls can react on pulse. Can you be 
more specific on what you need to do on pulse? If it's not related to 
the layout, maybe it would be reasonable to introduce some API that 
would allow you to execute code on each pulse.



Hm, I found it googling, and since it showed up here:

http://docs.oracle.com/javafx/2/api/javafx/scene/Scene.ScenePulseListener.html

I figured it was public, but I just noticed the class is defined package 
private.


Anyway, what I need to do:

1) I have two listeners that keep an eye on two properties, 
expandTopLevel and nodes:


expandTopLevel.addListener(new InvalidationListener() {
  @Override
  public void invalidated(Observable observable) {
buildTree();
  }
});

mediaNodes.addListener(new ListChangeListenerMediaNode() {
  @Override
  public void onChanged(ListChangeListener.Change? extends 
MediaNode change) {

buildTree();
  }
});

2) buildTree() will call TreeView#setRoot() directly/indirectly in both 
cases, either with the full list of nodes or a subtree, depending on 
expandTopLevel state.  The other possible subtree choices are moved into 
a tab-like control, which will have either 1 tab (if not expandTopLevel) 
or multiple tabs (one for each top level node).


Now, TreeView itself might be smart enough now to actually draw the sets 
of Nodes twice when both properties change, however buildTree will still 
get called twice, which itself is doing quite some work (although not 
enough to move it off the event thread), but also can cause things like 
selection to change.


None of this will affect the size of these controls, which I assumed 
would mean layoutChildren() does not get called (definitely not on the 
Pane containing both the tab-like control and the TreeView control).  
Also, if I donot call buildTree, but wait, I'd at the very least would 
need to change something to trigger a layout -- requestLayout() would 
work me there.


--John


-Martin



I'll experiment with a ScenePulseListener and see how it pans out, it 
looks like it would be an elegant solution.


Thanks Jonathan!




I hope that helps.

-- Jonathan

On 6/11/2013 3:58 a.m., John Hendrikx wrote:

Hi List,

I'm considering using a ScenePulseListener to avoid expensive
recalculations when multiple bindings trigger an action.

My problem is this:

I like to build Views (Controls) that are dumb and expose properties
that can be manipulated by whatever is using the View.  The View
listens to its own Properties and adjusts its state accordingly.  Some
of these adjustments are related (like width + height) and can be
expensive when calculated immediately.  So I would like to mark the
View as invalid and recalculate its state (if invalid) on the next
Pulse.

My current use case I'm looking at is a View that wraps (amongst
others) a TreeView.  The View exposes an ObservableList and a
BooleanProperty that decides

Re: Use ScenePulseListener to avoid expensive recalculations?

2013-11-07 Thread John Hendrikx

On 7/11/2013 14:08, Tomas Mikula wrote:

On Thu, Nov 7, 2013 at 11:58 AM, John Hendrikxhj...@xs4all.nl  wrote:

Hm, I found it googling, and since it showed up here:

http://docs.oracle.com/javafx/2/api/javafx/scene/Scene.ScenePulseListener.html

I figured it was public, but I just noticed the class is defined package
private.

Although not part of the public API, you can use

 import com.sun.javafx.tk.TKPulseListener;
 import com.sun.javafx.tk.Toolkit;

 Toolkit.getToolkit().addSceneTkPulseListener(new TKPulseListener(){...})


Anyway, I don't think deferring property invalidation until the next
pulse is very useful in general, for the following reasons:

1) It can lead to inconsistent observable state of your objects.
Consider an object with properties p, q, where the value of q depends
on the value of p, and consider changing the value of p. Now, right
after
 p.set(x);
returns, the state of the object is inconsistent until the next pulse,
and this inconsistency is observable to the outside world.
Well, in my case the properties being modified can immediately be 
queried back (so appear consistent) -- there are no dependent 
properties publically visible.  What you donot see is that the 
underlying controls (that you cannot access directly) will only get 
updated on the next pulse.  So my TreePane has two properties you can 
play with, and will either immediately or on the next pulse update a 
child control (a TreeView).  Since you cannot access this TreeView 
directly you will not be able to see that the update was deferred.

2) It doesn't (in general) avoid recalculations. Consider properties
p, q, r, s, whose invalidation listeners are deferred until the next
pulse, with bindings
 p- q- r- s
 p- s
and consider changing the value of p in pulse 0. The invalidation
listeners will fire as follows:
pulse 1: p
pulse 2: q, s
pulse 3: r
pulse 4: s
As you see, the listeners of s are called twice, causing potentially
expensive recalculation.
Not in this case -- the calculation I'm trying to avoid is the updating 
of the internal TreeView -- modifying the externally visible properties 
multiple times would trigger a recalculation each time in a simplistic 
implementation as you have to update the underlying controls after each 
modification.  What I want to do is to avoid this and allow these 
external properties to be modified as often as a user likes, but only 
update the hidden underlying controls on each Pulse (when needed).  This 
reduces the number of calculations to one per pulse, instead of one per 
property modification.


--John




enterNestedEventLoop as public API?

2013-11-12 Thread John Hendrikx

Hi List,

Any chance that Toolkit.getToolkit().enterNestedEventLoop() will in the 
future become public API?


I'm currently using this to create Dialogs based on a Pane to avoid 
creating Stages (which have the nice show and showAndWait 
functionality).  I duplicated this functionality in a Pane, allowing me 
to create Dialogs on top of existing Scenes without creating a Stage, 
and it makes use of the enterNestedEventLoop and exitNestedEventLoop 
functions in com.sun.javafx.tk.Toolkit.


The reason I'm avoiding the Stages is because they donot play well with 
an application that never has the mouse or keyboard focus (my 
application is fully remote controlled) -- creating a Stage, even one to 
just show a Dialog, will cause Windows to try and attract the user's 
attention by flashing its taskbar button (for which I filed a 
bug/feature request) and this is undesired.


Regards,
John

(Here's a part of the DialogPane to show and close it:)

  public R showDialog(Scene scene, boolean synchronous) {
this.synchronous = synchronous;
this.scene = scene;
this.oldFocusOwner = scene.getFocusOwner();

Parent root = scene.getRoot();

stackPane.getChildren().add(root);
stackPane.getChildren().add(this);

scene.setRoot(stackPane);

requestFocus();

if(synchronous) {
  return (R)Toolkit.getToolkit().enterNestedEventLoop(this);
}

return null;
  }

  public void close() {
Parent originalRoot = (Parent)stackPane.getChildren().remove(0);

scene.setRoot(originalRoot);
scene = null;

if(oldFocusOwner != null) {
  oldFocusOwner.requestFocus();
}

if(synchronous) {
  Toolkit.getToolkit().exitNestedEventLoop(this, getResult());
}
  }




Re: enterNestedEventLoop as public API?

2013-11-13 Thread John Hendrikx

On 13/11/2013 17:27, Tom Schindl wrote:

Currently the API requires to pass the same object to enter and exit how would 
your code work with multiple nested event loops?

Tom


Dialog d = new Dialog() {
   public void onClose() {
  Platform.exitNestedEventLoop(d, null);
   }
}
Platform.enterNestedEventLoop(d);

An example with multiple gets complicated quick, but not due to the 
nested event loop, just hard to show in a single example, normally 
nesting dialogs would just use another instance that handles these 
details -- in my code that uses the nested event loops, there is no 
difference between calling the first dialog or a deeply nested dialog -- 
caller code is unaware of how deep the nesting goes (I can post that if 
you like, it is fully functional apart from using non-public API):


Dialog d = new Dialog(Do you want exit the program?) {
   public void onExitClick() {
 Dialog e = new Dialog(Are you  absolutely sure?) {
public void onYesClick() {
   Platform.exitNestedEventLoop(e, true);
}
public void onNoClick() {
   Platform.exitNestedEventLoop(e, false);
}
 }
  e.show();
  if(Platform.enterNestedEventLoop(e)) {
close();  // closes first dialog, if yes was selected in 
confirmation dialog

  }
   }

   public void onCancelClick() {
  close();  // closes first dialog on cancel
   }

   public void close() {
  Platform.exitNestedEventLoop(d);
   }
}
d.show();
Platform.enterNestedEventLoop(d);

--John



Von meinem iPhone gesendet


Am 13.11.2013 um 16:35 schrieb Stephen F 
Northoversteve.x.northo...@oracle.com:

What is the difference?

Dialog d = new Dialog() {
   public void onClose() {
  Platform.exitNestedEventLoop();
   }
}
Platform.enterNestedEventLoop();


Steve


On 2013-11-13 5:28 AM, Tom Schindl wrote:
What bothers me with the API as it is today is that I have call
enter/exit, I would find it more easy to work with an API like:

---8---
WaitCondition c = new WaitCondition();
Dialog d = new Dialog() {
public void onClose() {
   c.release();
}
}
Platform.spinNestedEventLoop(c);
---8---

Tom


On 13.11.13 11:18, Artem Ananiev wrote:
I also think it's a good request for public API. In AWT/Swing, people
had been using ugly workarounds with modal dialogs just to enter a
nested event loop, until public java.awt API was finally provided:

http://docs.oracle.com/javase/7/docs/api/java/awt/SecondaryLoop.html

http://docs.oracle.com/javase/7/docs/api/java/awt/EventQueue.html#createSecondaryLoop()


The same is here in JavaFX: unless Toolkit.enter/exitNestedEventLoop()
is exposed at javafx.* level, people will have to workaround it by using
Stage, or calling into com.sun.javafx.*, which is not good.

Thanks,

Artem


On 11/13/2013 10:15 AM, John Hendrikx wrote:
Hi List,

Any chance that Toolkit.getToolkit().enterNestedEventLoop() will in the
future become public API?

I'm currently using this to create Dialogs based on a Pane to avoid
creating Stages (which have the nice show and showAndWait
functionality).  I duplicated this functionality in a Pane, allowing me
to create Dialogs on top of existing Scenes without creating a Stage,
and it makes use of the enterNestedEventLoop and exitNestedEventLoop
functions in com.sun.javafx.tk.Toolkit.

The reason I'm avoiding the Stages is because they donot play well with
an application that never has the mouse or keyboard focus (my
application is fully remote controlled) -- creating a Stage, even one to
just show a Dialog, will cause Windows to try and attract the user's
attention by flashing its taskbar button (for which I filed a
bug/feature request) and this is undesired.

Regards,
John

(Here's a part of the DialogPane to show and close it:)

public R showDialog(Scene scene, boolean synchronous) {
  this.synchronous = synchronous;
  this.scene = scene;
  this.oldFocusOwner = scene.getFocusOwner();

  Parent root = scene.getRoot();

  stackPane.getChildren().add(root);
  stackPane.getChildren().add(this);

  scene.setRoot(stackPane);

  requestFocus();

  if(synchronous) {
return (R)Toolkit.getToolkit().enterNestedEventLoop(this);
  }

  return null;
}

public void close() {
  Parent originalRoot = (Parent)stackPane.getChildren().remove(0);

  scene.setRoot(originalRoot);
  scene = null;

  if(oldFocusOwner != null) {
oldFocusOwner.requestFocus();
  }

  if(synchronous) {
Toolkit.getToolkit().exitNestedEventLoop(this, getResult());
  }
}




Re: enterNestedEventLoop as public API?

2013-11-13 Thread John Hendrikx

On 13/11/2013 16:35, Stephen F Northover wrote:

What is the difference?

Dialog d = new Dialog() {
   public void onClose() {
  Platform.exitNestedEventLoop();
   }
}
Platform.enterNestedEventLoop();

I find the current API to work well, it is just in the wrong package tree :)

Considering I didn't know about nested event loops before, I find the 
solution quite elegant -- leave the old event loop on the call stack, 
and start a fresh one that can be exited to continue again where the 
main event loop was halted -- it's almost like a 2nd thread gets 
started, but there isn't :)


--John



Steve

On 2013-11-13 5:28 AM, Tom Schindl wrote:

What bothers me with the API as it is today is that I have call
enter/exit, I would find it more easy to work with an API like:

---8---
WaitCondition c = new WaitCondition();
Dialog d = new Dialog() {
public void onClose() {
   c.release();
}
}
Platform.spinNestedEventLoop(c);
---8---

Tom

On 13.11.13 11:18, Artem Ananiev wrote:

I also think it's a good request for public API. In AWT/Swing, people
had been using ugly workarounds with modal dialogs just to enter a
nested event loop, until public java.awt API was finally provided:

http://docs.oracle.com/javase/7/docs/api/java/awt/SecondaryLoop.html

http://docs.oracle.com/javase/7/docs/api/java/awt/EventQueue.html#createSecondaryLoop() 




The same is here in JavaFX: unless Toolkit.enter/exitNestedEventLoop()
is exposed at javafx.* level, people will have to workaround it by 
using

Stage, or calling into com.sun.javafx.*, which is not good.

Thanks,

Artem

On 11/13/2013 10:15 AM, John Hendrikx wrote:

Hi List,

Any chance that Toolkit.getToolkit().enterNestedEventLoop() will in 
the

future become public API?

I'm currently using this to create Dialogs based on a Pane to avoid
creating Stages (which have the nice show and showAndWait
functionality).  I duplicated this functionality in a Pane, 
allowing me

to create Dialogs on top of existing Scenes without creating a Stage,
and it makes use of the enterNestedEventLoop and exitNestedEventLoop
functions in com.sun.javafx.tk.Toolkit.

The reason I'm avoiding the Stages is because they donot play well 
with

an application that never has the mouse or keyboard focus (my
application is fully remote controlled) -- creating a Stage, even 
one to

just show a Dialog, will cause Windows to try and attract the user's
attention by flashing its taskbar button (for which I filed a
bug/feature request) and this is undesired.

Regards,
John

(Here's a part of the DialogPane to show and close it:)

public R showDialog(Scene scene, boolean synchronous) {
  this.synchronous = synchronous;
  this.scene = scene;
  this.oldFocusOwner = scene.getFocusOwner();

  Parent root = scene.getRoot();

  stackPane.getChildren().add(root);
  stackPane.getChildren().add(this);

  scene.setRoot(stackPane);

  requestFocus();

  if(synchronous) {
return (R)Toolkit.getToolkit().enterNestedEventLoop(this);
  }

  return null;
}

public void close() {
  Parent originalRoot = (Parent)stackPane.getChildren().remove(0);

  scene.setRoot(originalRoot);
  scene = null;

  if(oldFocusOwner != null) {
oldFocusOwner.requestFocus();
  }

  if(synchronous) {
Toolkit.getToolkit().exitNestedEventLoop(this, getResult());
  }
}








Two Level Focus

2013-11-16 Thread John Hendrikx

Hi list,

I'm wondering how well the Conditional Feature TWO_LEVEL_FOCUS works, 
and if it is allowed to be used on non-embedded platforms.


My main JavaFX project is basically a Windows/Linux application that 
runs without focus and is controlled with a remote control (no mouse or 
keyboard, although keyboard events are synthesized).  This means I often 
run into problems related to the Window not having focus (controls donot 
show it, combobox drop downs close automatically, etc.)


While fixing yet another of these problems in custom controls / skins, I 
ran into the Conditional Feature above, which quite accurately describes 
what I'm doing (remote control only has left/right/up/down and an OK 
button... and a back button) which qualifies as two level navigation I 
think.  So I tried turning this on on Windows using:


System.setProperty(com.sun.javafx.twoLevelFocus, true);

...which I dug up out of Jira.  This changes ComoBox control atleast to 
not respond to up and down keys when it has the focus, allowing me 
to move away from it.  However, if you do click on it with the mouse (by 
selecting one of its items), focus is stuck there and cannot be moved 
anymore with just the 5 buttons.


Anyway, this sounds like it is exactly what I need and may save me a lot 
of work, any more information about it is appreciated!


--John





Re: [announce] InhiBeans: mitigate redundant recalculations

2013-12-15 Thread John Hendrikx
Since you are only allowed to modify properties on the JavaFX thread (in 
most cases), I've been using Platform.runLater() to make sure I observe 
only complete changes.


Basically I register an InvalidationListener on the properties that are 
relevant, and when one gets triggered I set a boolean and trigger a 
Runnable that
will be run on the JavaFX thread later (and after the change 
completes).  The Runnable checks the relevant values and acts on them, 
then resets the boolean.


I use this mechanism for example when I'm observing Objects with many 
properties that need to be stored in a database.  To make sure I only 
store consistent objects, I only observe the values of the properties 
when my own little piece of observer code runs on the JavaFX thread.  
Since nothing can modify the properties except on the JavaFX thread, 
this is almost like a form of transactions, ensuring that every change 
has completed before observing the results.


--John

On 15/12/2013 18:39, Scott Palmer wrote:

Interesting idea.

There is a case I have been curious about and wonder what the best
practices are for it.  Suppose you have a case when you are changing
multiple different properties that will be used in a single calculation.
  You want to deal with a single change to all of them in one go.  E.g.
imagine you have an area property that is bound to both width and
height.  You want to write code like:

obj.setWidth(w);
obj.setHeight(h);

and have only ONE recalculation of the area property happen.  Currently the
way bindings work the area will be calculated twice.  The intermediate
calculation is really not a value that you ever want to observe.

Are there helpers for this sort of situation?  Are there guidelines in the
JavaFX docs somewhere?

Regards,

Scott


On Sat, Dec 14, 2013 at 11:54 PM, Tomas Mikulatomas.mik...@gmail.comwrote:


Hello,

I just published a small extension of javafx bindings and properties
that can help you reduce redundant recalculations.

They provide two additional methods:

public void block();
public void release();

Call p.block() when you suspect your actions will lead to multiple
invalidations of p, and call p.release() when you are done and want to
deliver a single invalidation notification to p's observers.

https://github.com/TomasMikula/InhiBeans

Regards,
Tomas





Future of Skins

2014-01-07 Thread John Hendrikx

Hi List,

I'm wondering if Skins in their current form are finished or that more 
changes are still likely to come.  The Skin interface is public, and 
SkinBase is as well.  Unfortunately, it seems that even though SkinBase 
is treated differently by JavaFX from regular Skins, the opportunity was 
not used to give SkinBase an initialize method and make Skins truly 
reusable.


The reason I'm asking is because I'm developing a control that would 
greatly benefit from customizable and switchable skins, which differs 
quite a bit from the use patterns I've seen so far for Controls in 
JavaFX (I don't think any of them has more than one Skin out of the 
box).  My control would need:


- Multiple different layouts, each with potentially different (CSS) 
properties to customize the chosen layout further (the properties would 
only make sense for the given layout (or skin)).

- Easy switching between different layouts
- Highly customizable (possibility to override methods in the layouts or 
provide custom code for certain parts of the rendering)


I've been looking into providing this with Skins, but I keep bumping 
into odd choices made, especially related to their resuability.  As far 
as I know, a Skin is created with a reference to the control it is 
supposed to Skin -- at this time the Skin (if it is a SkinBase) 
apparently has done all kinds of stuff to the control, whether setSkin 
is called or not ehr??  Then later you are supposed to call setSkin 
on the control with the newly created Skin... which is apparently an 
almost optional step, unless it is not a SkinBase...?


- What happens when I create the Skin but donot call setSkin?  -- 
Undefined, depends on whether or not it is extended from SkinBase (?)... 
SkinBase skins seem to make this step almost optional.
- What happens when I call setSkin on a different control with this 
Skin? -- Depends on whether or not SkinBase is extended.  Would break 
controls in most situations -- no check is done if getSkinnable matches 
AFAICS.
- What happens when a disposed skin is setSkin'ned again? -- Seems to 
break most controls, no check is done if getSkinnable is null AFAICS.


This arrangement where the Skin class must be constructed with some 
parameter that must later match with the setSkin feels incredibly 
fragile to me.  It would be far nicer if a Skin was simply given an 
initialize method (which would also nicely match the dispose method) 
that takes the Skinnable as a parameter.  This initialization can be 
enforced in the setSkin method.  There is even a remark in the code 
about this:


// Note I do not remove any children here, because the
// skin will have already configured all the children
// by the time setSkin has been called. This is because
// our Skin interface was lacking an initialize method 
(doh!)

// and so the Skin constructor is where it adds listeners
// and so forth. For SkinBase implementations, the
// constructor is also where it will take ownership of
// the children.

Would it not have been a better idea to add an #initialize method to 
SkinBase if the code has two paths for legacy Skins and SkinBase skins 
anyway?  Or a Skin2 interface with the extra method + instanceof check?  
Or Java8 default #initialize method which does nothing by default?


Any of those would open up Skins to be far easier to use by making them 
reusable.  I really want to be able to do something like this:


ComboBoxSkin comboBox = new 
ComboBox(FXCollections.observableArrayList(

new Skin1(),
new Skin2(),
new Skin3(),
new SubClassOfSkin1(Color.RED)),
new Skin2() {
   // with some methods overriden
}
));

Then bind those simply with:


myControl.skinProperty().bind(comboBox.getSelectionModel().selectedItemProperty());


This would allow a ComboBox to control the Skin of another Control, 
because skins are reusable AND donot require knowledge of the control 
they are Skinning at construction time.  This is currently not possible 
without creating somekind of SkinFactory and a ChangeListener on the 
ComboBox that does something like:


myControl.setSkin(skinFactory.create(myControl));

This problem gets worse when you also want to customize the Skins a 
bit.  All the customization must be done in the SkinFactory part, every 
time the Skin is switched.  With reusable skins, they can just be 
configured once.


So I'm left with a bit of a choice.  Do I make multiple Skins and a 
bunch of Factories?  Or do I simply create just one fixed Skin that just 
delegates everything it does to a second light-weight Skinning mechanism 
that is easier to switch, customize and extend?


--John


Re: Future of Skins

2014-01-07 Thread John Hendrikx

On 7/01/2014 14:50, Tomas Mikula wrote:
Interesting ideas. I'm wondering, do you switch skins often enough 
that you are worried about performance (and thus care about 
reusability of skins)? Because I don't see how reusability of skins 
saves you lines of code - whether the code is in the constructor or in 
the initialize() method, it is there somewhere. In my opinion, reusing 
objects is more complicated than creating fresh instances and the only 
justification for it is performance.


To address your last point first, if you already are required to write a 
proper dispose method, then fresh instances should be just as easy/hard 
to write as reusable ones.  Everything is already in place, just rename 
the constructor.  Of course, if you did not write a proper dispose 
method, then your object will stick around or cause other trouble.  With 
fresh instances you won't notice this so readily -- in JavaFX for 
example, the problem of having objects that are no longer actively 
reachable through the SceneGraph, but are still participating in Event 
handling (because they registered non-weak listeners) can be a nice 
source of surprises.  With reusable objects, you'll notice the bugs in 
your cleanup code likely the first time you reuse it.


Anyway, for me, making Skins reusable makes them easier to use with 
bindings, and it ofcourse saves creating a factory.  I see the skin of 
an object as the same as say its background color.  There is no reason 
(anymore I think) that one should be treated so differently from the other.


   private final Skin someExistingSkin = new SkinA();
   private final Skin someExistingSkin2 = new SkinB();

   void changeToSquareLook() {
  myControl.setSkin(someExistingSkin);
   }

   void changeToRoundLook() {
  myControl.setSkin(someExistingSkin2);
   }

vs.

  private final SkinFactory skinFactory = new SkinFactory() {
 public Skin createSkin(Control control) {
 return new SkinA(control);
 }
  };

  private final SkinFactory skinFactory2 = new SkinFactory() {
 public Skin createSkin(Control control) {
 return new SkinB(control);
 }
  };

  void changeToSquareLook() {
 myControl.setSkin(skinFactory.createSkin(myControl));
  }

   void changeToRoundLook() {
 myControl.setSkin(skinFactory2.createSkin(myControl));
   }

It's not really about performance, but ease of use.  The binding case 
requires a ChangeListener instead of just bind().


I agree with you on the problem of separation of skin initialization 
from setSkin(). Another way to address this could be deprecating the 
original setSkin() method and introducing


setSkin(SkinProviderC skinProvider);

where SkinProvider is a SAM interface and thus the above method could 
be called like this:


setSkin(control - new MySkin(control));

I know this defeats reusability of skins altogether, so you (and 
others) may disagree.
Maybe if there was a skinProviderProperty... then I could bind to that 
atleast.  Still introduces lots of factories (or functions).


--John



CSS metadata boilerplate

2014-01-07 Thread John Hendrikx

Hi List,

I'm in the process of adding CSS metadata to a new control, and I 
noticed there is a lot of boilerplate.


In order to reduce this, I've created some custom classes 
StyleableProperty classes (SimpleStyleableXXXProperty), which reduces 
the boilerplate significantly without sacrificing much (if any) 
performance.  The only thing that I cannot easily provide in this 
fashion is the static getClassCssMetaData method.  From the 
documentation I understand it is there just for convience for subclass 
creators and is not used by the CSS engine at all -- atleast, everything 
seems to work.


The shortened version for CSS aware properties basically looks like:

  private final SimpleStyleableDoubleProperty cellAlignment = new 
SimpleStyleableDoubleProperty(this, cellAlignment, 
-fx-cell-alignment, 0.8);
  private final SimpleStyleableDoubleProperty density= new 
SimpleStyleableDoubleProperty(this, density, -fx-density, 0.02);
  private final SimpleStyleableBooleanProperty reflectionEnabled= new 
SimpleStyleableBooleanProperty(this, reflectionEnabled, 
-fx-reflection-enabled, true);
  private final SimpleStyleableBooleanProperty clipReflections= new 
SimpleStyleableBooleanProperty(this, clipReflections, 
-fx-clip-reflections, true);


With one small bit of supporting code in the relevant class (Skin or 
Control), which is basically a non-static implementation of the standard 
CSS List example code:


  private static ListCssMetaData? extends Styleable, ? cssMetaData;

  @Override
  public ListCssMetaData? extends Styleable, ? getCssMetaData() {  
// Unsynchronized. WC: list gets initialized multiple times.

if(cssMetaData == null) {
  ListCssMetaData? extends Styleable, ? metaData = new 
ArrayList(super.getCssMetaData());

  Collections.addAll(metaData,
  cellAlignment.getCssMetaData(),
  density.getCssMetaData(),
  reflectionEnabled.getCssMetaData(),
  clipReflections.getCssMetaData()
  );
  cssMetaData = Collections.unmodifiableList(metaData);
}

return cssMetaData;
  }

Note that the List is static and lazy-final.  The same goes for the 
getCssMetaData method in the SimpleStyleableXXXProperty classes.  There 
is a slight performance decrease in those classes as getCssMetaData is 
looked up from a static Map (indexed by Class + css property name) and 
lazily created as needed -- a Map lookup however should be quite fast 
enough.


I'm sure the design had good reason to do things as they are, and I'm 
wondering if reducing the boilerplate has left me missing something 
important.


I welcome any insights!

--John


Re: CSS metadata boilerplate

2014-01-08 Thread John Hendrikx

On 8/01/2014 14:28, David Grieve wrote:

The reason things are as they are is because most properties in the core 
classes are lazily created and I didn't want cause the property to be created 
just to see if it was bound. But that is a particular concern for the core 
classes and not so much for controls.
I see, this mechanism certainly doesn't try to do that.  It only works 
when the properties are created in advance.

I'm not sure if you are aware of the javafx.css.SimpleStyleableXXXProperty 
classes. I'd be interested to see how your custom classes dovetail into these 
as I'd be more than willing to consider taking  your additions and calling them 
my own… uh, um, I mean, rolling them into the source code.
I wasn't aware of those, but I just took a look at them.  They do make 
the process a little bit easier, although still require creating a 
CssMetaData object yourself.  If you want to use the code or just the 
idea, please feel free -- it would be very nice for creating custom 
controls if it was part of JavaFX.

Have you looked at the initialization-on-demand holder pattern for creating the 
List?
That's the Super-lazy instantiation pattern from Bill Pugh ?  I've 
seen it several times in the JavaFX sources, and I think it is a neat 
way to avoid certain statics from being created immediately after the 
class loads.


However, I'm not sure if it applies here, the first getCssMetaData call 
basically is the trigger that causes all CssMetaData objects and the 
List to get created...  I'm not sure how I could make it any more lazy 
than that.  The Map I use in the properties themselves is static, so 
shouldn't cause too much footprint.


The code I use for the one of the SimpleStyleableXXXProperty is below.  
It relies on a Map that will cache CssMetaData objects (for different 
instances of the same property in a Control) and are only created when 
they're needed.  CssMetaData objects only get created one time on 
access, after that further calls just do a quick map lookup.


It makes assumptions about the kind of converter needed, which could be 
parameterized further with another constructor option.


public class SimpleStyleableDoubleProperty extends StyleableDoubleProperty {
  private static final MapClass?, MapString, CssMetaData? extends 
Styleable, Number CSS_META_DATA_BY_NAME_BY_CLASS = new HashMap();


  private final Object bean;
  private final String name;
  private final String cssName;
  private final double initialValue;

  public SimpleStyleableDoubleProperty(Object bean, String name, String 
cssName, double initialValue) {

super(initialValue);

this.bean = bean;
this.name = name;
this.cssName = cssName;
this.initialValue = initialValue;
  }

  @Override
  public CssMetaData? extends Styleable, Number getCssMetaData() {
// TODO consider synchronizing CSS_META_DATA_BY_NAME_BY_CLASS -- 
depends on whether or not this ever gets called by more than one thread.
MapString, CssMetaData?, Number cssMetaDataByName = 
CSS_META_DATA_BY_NAME_BY_CLASS.get(bean.getClass());


if(cssMetaDataByName == null) {
  cssMetaDataByName = new HashMap();
  CSS_META_DATA_BY_NAME_BY_CLASS.put(bean.getClass(), 
cssMetaDataByName);

}

CssMetaData? extends Styleable, Number cssMetaData = 
cssMetaDataByName.get(cssName);


if(cssMetaData == null) {
  cssMetaData = new CssMetaDataStyleable, Number(cssName, 
StyleConverter.getSizeConverter(), initialValue) {

@Override
public boolean isSettable(Styleable styleable) {
  return !SimpleStyleableDoubleProperty.this.isBound();
}

@Override
public StyleableDoubleProperty getStyleableProperty(Styleable 
styleable) {

  return SimpleStyleableDoubleProperty.this;
}
  };
}

return cssMetaData;
  }

  @Override
  public Object getBean() {
return bean;
  }

  @Override
  public String getName() {
return name;
  }
}

--John




On Jan 7, 2014, at 9:34 PM, John Hendrikxhj...@xs4all.nl  wrote:


Hi List,

I'm in the process of adding CSS metadata to a new control, and I noticed there 
is a lot of boilerplate.

In order to reduce this, I've created some custom classes StyleableProperty 
classes (SimpleStyleableXXXProperty), which reduces the boilerplate 
significantly without sacrificing much (if any) performance.  The only thing 
that I cannot easily provide in this fashion is the static getClassCssMetaData 
method.  From the documentation I understand it is there just for convience for 
subclass creators and is not used by the CSS engine at all -- atleast, 
everything seems to work.

The shortened version for CSS aware properties basically looks like:

  private final SimpleStyleableDoubleProperty cellAlignment = new 
SimpleStyleableDoubleProperty(this, cellAlignment, -fx-cell-alignment, 0.8);
  private final SimpleStyleableDoubleProperty density= new SimpleStyleableDoubleProperty(this, 
density, -fx-density, 0.02);
  private final 

Re: Future of Skins

2014-01-08 Thread John Hendrikx


That's basically how I've solved it so far (although I call the reusable 
skins Layouts for lack of a more imaginitive name).


However, I'd like to support CSS properties as well for the delegates, 
and I'm not sure how the CSS engine deals with Skins that can change 
their properties when some other property (the delegated Skin) changes.  
I suspect that changing properties will work fine for changing normal 
Skins, but that it won't be able to detect the properties having changed 
when the a Skin I delegate to changes.


Perhaps though I'm going a bit too far in order to provide that little 
bit of extra convience and customization possibilities to the end-user 
of the Control.


--John

On 7/01/2014 17:04, Richard Bair wrote:

Could you write a single skin that delegates to one or more reusable skins?





Re: Future of Skins

2014-01-08 Thread John Hendrikx

On 7/01/2014 18:11, Tomas Mikula wrote:
With a non-reusable skin, dispose is pretty much just removing the 
listeners. With a reusable instance, I suspect there is more work to 
reset the state of the instance (e.g. removing children, or, if you 
were concerned about performance, returning them to a pool of 
objects). So I would argue fresh instances are never more complicated 
than reusable ones, while the opposite is true only in simple cases.
Well fair enough.  Making them fully reusable is more involved anyway 
(as in settable on multiple controls).


Anyway, for me, making Skins reusable makes them easier to use
with bindings,


 My understanding of skins is that they are the view of the MVC 
pattern and that the rest of the application should not depend on the 
particular implementation.

I'm not sure what you mean by this.

Skins that radically change the appearance of a Control may have new CSS 
properties to control that appearance (and standard JavaFX Skins do 
introduce CSS properties that are not available on the control itself).  
Big changes, like a Tree that is displayed in standard filesystem style 
and change it into a graph with linked nodes (perhaps 3D) that can be 
navigated in a similar fashion -- without having to switch to a 
completely different control (after all, the code does not care, it's 
still a tree).  It's possible -- and that's what I'm doing.


I just found it akward to provide users with controls to change the Skin 
(of one control) on demand.  Something like a SkinFactory would be 
required as Skins themselves cannot be precreated and re-used as 
needed.  If it were only a matter of changing the skins, I could also 
just change the CSS skin property's class name, but some of these Skins 
can be configured extensively which would need to be handled in either a 
Factory or by creating yet another Skin with these configurations preset.


Consider the graph case for a tree strucuture... there are many possible 
visualizations, 2D, 3D, visible node levels, fade in/out rules for 
nodes, scaling, etcetera.  Just one Skin offers all of these as 
Properties, and I'd like to provide say 5 or 6 defaults from which the 
user can choose.  Factories would be my only option at the moment.


As the underlying control needs no changes, I think Skins are ideal here.


and it ofcourse saves creating a factory.  I see the skin of an
object as the same as say its background color.  There is no
reason (anymore I think) that one should be treated so differently
from the other.


One outstanding difference is that a skin is stateful, while a 
background color is stateless (immutable). Thus you can use the same 
instance of background color for many controls, but you cannot do that 
with skin instances. In this respect, a skin provider is a better 
analogy to background color (you can use the same skin provider with 
many controls).
This is true, a SkinProvider would atleast not carry State, and it would 
blend in a lot better with almost all other properties.
Furthermore, neither the current state nor your proposal prevents you 
from doing this:


SkinMyControl skin = new MySkin();
myControl1.setSkin(skin);
myControl2.setSkin(skin);

My proposal does not allow you to write such code.
Well, your proposal would make the above code work, if all of those are 
SkinProviders, and even do exactly what you'd expect :)


--John


Re: CSS metadata boilerplate

2014-01-09 Thread John Hendrikx

Noticed a slight bug in the code.

The line:

  cssMetaDataByName.put(cssName, cssMetaData);

...needs to be added in the if block where the CssMetaData is created.

--John


Re: Move to JIRA [was: Re: [8u] API Request: RT-25613, ObservableValue should have a hasListener(listener) method]

2014-01-22 Thread John Hendrikx
Unfortunately, discussing things in JIRA works very poorly and is a 
good way to end a productive discussion IMHO.  Mailinglists are much 
better suited to the task, as thousands of interesting mailinglists 
accross many developer communities will atest to.


Keeping a record is good, aren't these mailinglists archived?

--John

On 22/01/2014 18:47, Daniel Blaukopf wrote:

Hi Martin, Randahl, Tom, Richard, Tomas and Ali,

This is a productive discussion, but once we get to this level of detail JIRA 
is the place to have it, so that we don’t lose our record of it. Would you 
continue the discussion on https://javafx-jira.kenai.com/browse/RT-25613 ?

See 
https://wiki.openjdk.java.net/display/OpenJFX/Code+Reviews#CodeReviews-TechnicalDiscussionsandCodeReviews

Thanks,
Daniel

On Jan 22, 2014, at 7:23 PM, Stephen F Northoversteve.x.northo...@oracle.com  
wrote:


If we add this API, I like addListener(InvalidationListener, boolean) better 
than ensureListener().

Steve

On 2014-01-22 8:20 AM, Ali Ebrahimi wrote:

I suggest adding another overload for addListener method taking boolean
parameter  duplicateAllowed or duplicateNotAllowed.


On Wed, Jan 22, 2014 at 3:00 PM, Richard Bairrichard.b...@oracle.comwrote:


The default implementation (for Observable) would look like this:

public default void ensureListener(InvalidationListener listener) {
removeListener(listener);
addListener(listener);
}

subclasses might do something more effective. The same would apply to
ObservableValue and ChangeListener and Observable[List|Set|Map] and
[List|Set|Map]ChangeListener.

Well this would destroy the order! I expect listeners to be called in
the correct order not?

That’s a good point :-(


Why doing a remove and not simply check if the
listener has already been added?

Because there is no way to check, except in the implementation. From the
Observable interface level, there is no way to a) force all implementations
of the interface to implement the method correctly (without breaking source
compatibility anyway), or b) to provide a reasonable default implementation.

Maybe this is one of those things we can’t fix on the Observable interface
and just have to provide implementations of on our concrete properties.

Richard




Re: Future of Skins

2014-01-25 Thread John Hendrikx

On 24/01/2014 22:28, David Grieve wrote:

On Jan 24, 2014, at 4:02 PM, John Hendrikxhj...@xs4all.nl  wrote:


I've got an update on this.

I've rewritten the code now to make use of multiple skins, and doing some 
trickery with Factories to make them easily switchable.

The main reason I've rewritten them is because I think I will not be able to provide CSS 
properties that are accessible if they are not specifically defined on a Skin class (or 
Control class) -- but that may be lack of understanding how CSS properties are discovered 
(I don't see how they could get discovered if they're defined on a delegated 
skin).

A control returns the CssMetaData of its skin (provided the skin extends from SkinBase).  
Also, if a node is in the scene-graph, then CSS will discover it.
Yes, and a Skin could return the CssMetaData for a Skin it delegates 
to... my problem is, if I change the delegate (by changing a property) 
while the main Skin itself remains the same, can I inform the CSS engine 
of potential changes in the CssMetaData?  Each delegate Skin may have 
additional CSS properties.  When changing a Skin on a Control that's 
probably automatic, but I'm not so sure how that would work with a 
delegate skin.


--John




Anyway, the solution works, but I found some odd issues, for which I filed two 
JIRA's:

https://javafx-jira.kenai.com/browse/RT-35528
https://javafx-jira.kenai.com/browse/RT-35529

One deals with anonymously created Skins (my Skins have protected methods that 
can be easily overriden to customize the appearance).  Unfortunately, JavaFX 
doesn't like it when a Skin is an anonymous inner class with one or more 
methods overriden with small customizations.  The setting of the Skin itself 
works, it just complains about it later when a CSS pass occurs (I think).
The second issue deals with something odd that I noticed when a Skin is 
specified in CSS (with -fx-skin) and when I later override it with setSkin().  
JavaFX seems to reset it back to the skin defined by CSS when a CSS pass 
occurs.  I'm not aware of any other properties that behave like that.

All in all, it works, but it feels somewhat fragile.

--John

On 7/01/2014 17:04, Richard Bair wrote:

Could you write a single skin that delegates to one or more reusable skins?

On Jan 7, 2014, at 7:26 AM, John Hendrikxhj...@xs4all.nl   wrote:


On 7/01/2014 14:50, Tomas Mikula wrote:

Interesting ideas. I'm wondering, do you switch skins often enough that you are 
worried about performance (and thus care about reusability of skins)? Because I 
don't see how reusability of skins saves you lines of code - whether the code 
is in the constructor or in the initialize() method, it is there somewhere. In 
my opinion, reusing objects is more complicated than creating fresh instances 
and the only justification for it is performance.

To address your last point first, if you already are required to write a proper 
dispose method, then fresh instances should be just as easy/hard to write as 
reusable ones.  Everything is already in place, just rename the constructor.  
Of course, if you did not write a proper dispose method, then your object will 
stick around or cause other trouble.  With fresh instances you won't notice 
this so readily -- in JavaFX for example, the problem of having objects that 
are no longer actively reachable through the SceneGraph, but are still 
participating in Event handling (because they registered non-weak listeners) 
can be a nice source of surprises.  With reusable objects, you'll notice the 
bugs in your cleanup code likely the first time you reuse it.

Anyway, for me, making Skins reusable makes them easier to use with bindings, and it 
ofcourse saves creating a factory.  I see the skin of an object as the same 
as say its background color.  There is no reason (anymore I think) that one should be 
treated so differently from the other.

   private final Skin someExistingSkin = new SkinA();
   private final Skin someExistingSkin2 = new SkinB();

   void changeToSquareLook() {
  myControl.setSkin(someExistingSkin);
   }

   void changeToRoundLook() {
  myControl.setSkin(someExistingSkin2);
   }

vs.

  private final SkinFactory skinFactory = new SkinFactory() {
 public Skin createSkin(Control control) {
 return new SkinA(control);
 }
  };

  private final SkinFactory skinFactory2 = new SkinFactory() {
 public Skin createSkin(Control control) {
 return new SkinB(control);
 }
  };

  void changeToSquareLook() {
 myControl.setSkin(skinFactory.createSkin(myControl));
  }

   void changeToRoundLook() {
 myControl.setSkin(skinFactory2.createSkin(myControl));
   }

It's not really about performance, but ease of use.  The binding case requires 
a ChangeListener instead of just bind().


I agree with you on the problem of separation of skin initialization from 
setSkin(). Another way to address this could be deprecating the original 
setSkin() method and introducing


Layout issue

2014-02-11 Thread John Hendrikx
From an earlier posting on this list, I came to understand that in 
JavaFX 8 it is no longer allowed to modify the children list in 
layoutChildren, and that such modifications may need to be moved to the 
computerPref* methods.


However, I get a different odd issue, and I'm wondering exactly what is 
allowed and what isn't when it comes to layout (any documentation on this?)


What's happening is the following:

I got a (subclass of) BorderPane, at the top I have a tab-like control 
(let's call it a FilterControl).  At the center I got a TreeView.


When the BorderPane gets laid out, I set up the content for both the 
TreeView and the FilterControl.  I do this in layoutChildren or in 
computerPref* of the BorderPane -- it makes no difference.  Setting up 
the content for the FilterControl involves changing its children list.  
The TreeView probably will do the same (adding/removing Cells as needed).


Now, I'm seeing layout issues.  The BorderPane for example is often 
putting the TreeView right on top of the FilterControl (stuff is 
transparent, so I can see it).  With other attempts, the FilterControl 
is not visible at all (zero height).  Forcing an update (by changing 
focus) usually clears up the issue and the FilterControl will take its 
rightful spot and force the TreeView to be a little less high.


I'm thinking this is somekind of issue with layout and that I'm 
approaching this wrong causing the layout problems.  However, I don't 
quite understand what I'm possibly doing wrong -- modifying the children 
of the Top node of the BorderPane during BorderPane's own 
computePref/layoutChildren call should be perfectly fine right?


Any help appreciated!

--John



Re: Layout issue

2014-02-12 Thread John Hendrikx
I've tried to create a simple test application in 
https://javafx-jira.kenai.com/browse/RT-35830.


It is still fairly long, but it shows the problem right from the start 
when the Stage pops up.


Thanks :)
--John

On 11/02/2014 20:41, Jonathan Giles wrote:

If you can create a simple test application it would be great if you
can log this as a Jira issue so that we can take a proper look into
this for you.

Thanks,
-- Jonathan

On Wednesday, 12 February 2014 5:50:15 a.m., John Hendrikx wrote:

 From an earlier posting on this list, I came to understand that in
JavaFX 8 it is no longer allowed to modify the children list in
layoutChildren, and that such modifications may need to be moved to
the computerPref* methods.

However, I get a different odd issue, and I'm wondering exactly what
is allowed and what isn't when it comes to layout (any documentation
on this?)

What's happening is the following:

I got a (subclass of) BorderPane, at the top I have a tab-like control
(let's call it a FilterControl).  At the center I got a TreeView.

When the BorderPane gets laid out, I set up the content for both the
TreeView and the FilterControl.  I do this in layoutChildren or in
computerPref* of the BorderPane -- it makes no difference.  Setting up
the content for the FilterControl involves changing its children
list.  The TreeView probably will do the same (adding/removing Cells
as needed).

Now, I'm seeing layout issues.  The BorderPane for example is often
putting the TreeView right on top of the FilterControl (stuff is
transparent, so I can see it).  With other attempts, the FilterControl
is not visible at all (zero height).  Forcing an update (by changing
focus) usually clears up the issue and the FilterControl will take its
rightful spot and force the TreeView to be a little less high.

I'm thinking this is somekind of issue with layout and that I'm
approaching this wrong causing the layout problems.  However, I don't
quite understand what I'm possibly doing wrong -- modifying the
children of the Top node of the BorderPane during BorderPane's own
computePref/layoutChildren call should be perfectly fine right?

Any help appreciated!

--John





Re: Layout issue

2014-02-12 Thread John Hendrikx

On 11/02/2014 21:09, Martin Sladecek wrote:
The rule of thumb in case you modify content during the layout is that 
content should depend on layout pane size, not the other way around. 
It means that changing the content won't modify the min/pref/max size 
of the pane as that would trigger another layout pass (possibly 
falling into a loop).


BorderPane
  +- (TOP) Filter (subclass of FlowPane)
  +- (CENTER) TreeView

I modify the content during the compute* method of the BorderPane.  For 
TreeView I change the root; for the Filter (FlowPane) I add/remove children.


For some reason that seems to be the wrong time to do it, despite the 
fact that this happens almost as the first thing during the layout 
process.  LayoutChildren will get called later on BorderPane, and no 
more changes were made, and the layout is incorrect.  Then when the 
focus changes, the compute* methods get called again (but I change 
nothing since everything is valid still) and layoutChildren gets called 
again on BorderPane... and then everything looks correct.


This is why it's not recommended to change children during the layout, 
as default computation of min/pref/max size depends on the children. 
But if you modify compute* methods and return something that is not 
dependent on the children, you can safely modify them while doing the 
layout.


So, e.g. your filter control might return pref size that is approx. 
for 2-3 tabs (or whatnot). Since BorderPane resizes top to the full 
width, you would always get the maximum width you can get. Based on 
the assigned width, you can then compute the layout and add as much 
tabs as you can. Since that would not change the pref width, the 
layout is done.
It is actually a FlowPane -- I'd like it to accomodate the tabs as well 
as possible, and even add an extra row if they don't all fit.  Since I 
donot myself override its compute* methods, I didn't think the issue 
might be in that area (see the Filter class included in the JIRA I 
created -- it's really simple).
Anyway, as Jonathan already noted, a JIRA issue with some sample would 
be great.

Yes, I just did, RT-35830 has a nice sample.

Thanks for giving some more insight in the layout process.

--John


Thanks,
-Martin

On 02/11/2014 05:50 PM, John Hendrikx wrote:
From an earlier posting on this list, I came to understand that in 
JavaFX 8 it is no longer allowed to modify the children list in 
layoutChildren, and that such modifications may need to be moved to 
the computerPref* methods.


However, I get a different odd issue, and I'm wondering exactly what 
is allowed and what isn't when it comes to layout (any documentation 
on this?)


What's happening is the following:

I got a (subclass of) BorderPane, at the top I have a tab-like 
control (let's call it a FilterControl).  At the center I got a 
TreeView.


When the BorderPane gets laid out, I set up the content for both the 
TreeView and the FilterControl.  I do this in layoutChildren or in 
computerPref* of the BorderPane -- it makes no difference. Setting up 
the content for the FilterControl involves changing its children 
list.  The TreeView probably will do the same (adding/removing Cells 
as needed).


Now, I'm seeing layout issues.  The BorderPane for example is often 
putting the TreeView right on top of the FilterControl (stuff is 
transparent, so I can see it).  With other attempts, the 
FilterControl is not visible at all (zero height).  Forcing an update 
(by changing focus) usually clears up the issue and the FilterControl 
will take its rightful spot and force the TreeView to be a little 
less high.


I'm thinking this is somekind of issue with layout and that I'm 
approaching this wrong causing the layout problems.  However, I don't 
quite understand what I'm possibly doing wrong -- modifying the 
children of the Top node of the BorderPane during BorderPane's own 
computePref/layoutChildren call should be perfectly fine right?


Any help appreciated!

--John







Re: Proposal on getting warning free (controls) packages

2014-03-21 Thread John Hendrikx

On 20/03/2014 20:57, Tom Schindl wrote:

Hi,

I've just started looking into getting the controls package warning free
and/or suppress them in case not fixable.

Most of the generic warnings I've come accross in a first pass involve
StyleableProperty cast like this:

((StyleableProperty)graphicProperty()).applyStyle(origin, null);

In fact above code has 2 warnings:
a) unchecked type cast
b) usage of raw-type

the raw type in this case can be fixed with:

((StyleablePropertyNode)graphicProperty()).applyStyle(origin, null);

leaving us with the unchecked cast so we could add:

@SuppressWarnings({ unchecked })

e.g. on the method but I'm uncertain this is a good idea because it
might hide unchecked warnings we have a possibility to fix.


If you are willing to expend another line of code, you could first 
assign graphicProperty to a variable:


  StylablePropertyNode  property = (StyleablePropertyNode)graphicProperty();

property.applyStyle(origin, null);

You can then apply the unchecked annotation to the assignment only, so 
future unchecked warnings in the same method will still be noticed.


--John

So what are other options:
a) Create a static helper in Util to make the cast and for us (we could
even add it as a static method in the StyleableProperty-interface)

b) provide a private/package-scoped method the public one delegates

Both of them work but have different problems where I think b) has the
bigger ones like blowing up class-file, modification of the field to be
of type StyleableObjectProperty.

I'd really like to get the source warning free.

Thoughts?

Tom





Re: Exposing native surface or opengl handle

2014-06-26 Thread John Hendrikx


On 13/06/2014 08:57, Robert Krüger wrote:

Hi,

it has been discussed a number of time in the passed but let me
quickly summarize:

A number of people have requested a feature that provides the ability
to have native code draw into a surface provided by a JavaFX
application as fast as technically possible, i.e. with no indirection
or copying because use cases for this were mostly cases where
performance was critical, e.g. HD/UHD video players, real-time
visualization etc. where losing even e few percent would make a
software written in JavaFX unable to compete with native products
(e.g. in the video area nobody will use a video player that is not
able to play the content smoothly that VLC player or Quicktime can on
the same machine).
Although copying is used, I've combined JavaFX and VLC in this fashion 
for over a year already, and video is smooth and stable -- stable enough 
to watch full length HD movies, at 20% increased speed (the speed I 
normally watch them).


Of course, if the target machine is barely able to play these, then the 
extra copying overhead (which is smaller than people think) may be too much.


--John



Re: The trouble with Skins

2015-03-22 Thread John Hendrikx

On 22/03/2015 09:59, Tom Eugelink wrote:

On 22-3-2015 00:12, John Hendrikx wrote:


What I do need however is a way to restore the control to the exact 
same state it was in before (the same amount of pixels scrolled, the 
same item at the top, the same item at the bottom).


That is an interesting use case. Can you describe it a bit more?

Tom

My app works more like a browser, so when I go back, I expect the same 
screen layout again (even though I have to reconstruct the screen 
again).  With a ListView, this cannot be done as the #scrollTo method 
only shows an item.  It doesn't remember however if that item was 
somewhere in the middle, top or bottom.  It's just convenient if it was 
in the same spot, as the user might expect it there.  Just like I expect 
my browser to go back to the same spot helps me a bit (eg: I clicked the 
link at the bottom of the screen somewhere, and there was something else 
interesting to the left of it -- that fails if it is now somewhere else).


--John


Re: The trouble with Skins

2015-03-22 Thread John Hendrikx

On 22/03/2015 16:18, Tom Eugelink wrote:


On 22-3-2015 13:53, John Hendrikx wrote:

On 22/03/2015 09:59, Tom Eugelink wrote:

On 22-3-2015 00:12, John Hendrikx wrote:


What I do need however is a way to restore the control to the exact 
same state it was in before (the same amount of pixels scrolled, 
the same item at the top, the same item at the bottom).


That is an interesting use case. Can you describe it a bit more?

Tom

My app works more like a browser, so when I go back, I expect the 
same screen layout again (even though I have to reconstruct the 
screen again).  With a ListView, this cannot be done as the #scrollTo 
method only shows an item.  It doesn't remember however if that item 
was somewhere in the middle, top or bottom.  It's just convenient if 
it was in the same spot, as the user might expect it there.  Just 
like I expect my browser to go back to the same spot helps me a bit 
(eg: I clicked the link at the bottom of the screen somewhere, and 
there was something else interesting to the left of it -- that fails 
if it is now somewhere else).




Ah, ok. So I am curious; even though the browser main scrollbar might 
return to the position you were before (or even when refreshing a site 
might do), will it also return divs-with-scrollbar to the same 
position? I doubt it.


But HTML is much more low level than JavaFX's controls; if you were to 
build a JavaFX screen just using primitive controls, so create your 
own list with a pane and a scrollbar, then that scrollbar's API is 
available and you can do what you want. As soon as you start 
encapsulating things, then it becomes more interesting. Does for 
example JSF's list control allow you to specify the scroll position, 
or JQuery tables?


Okay, in what way does that change the fact that I'd like to show the 
user a screen that was how he left it?  I could keep the entire page in 
memory I suppose, but I'd even like it to be the same between restarts 
(when navigating to the same part of the system again).  Anyway, it's 
super easy to do, the List just has to expose the scroll position in 
pixels, and allow me to put it back there -- exactly how I solved it 
with a custom control.


--John



Re: The trouble with Skins

2015-03-21 Thread John Hendrikx

On 14/03/2015 08:31, Tom Eugelink wrote:

Hi Tomas,

I have looked into it, but not yet attempted, but I did do a lot of 
custom controls. And I agree that it is dubious that a control is a 
node, and has the properties that come with it. I try to maintain a 
strict separation in my controls in JFXtras; controls only have 
functional methods, the skin and CSS do all the layout. For example 
the gauge I've ported from Enzo; my version does not have a 
setFillColor() like the one in Enzo, that is something that the user 
needs to do through CSS.


That said, if a control were only a model, than it would not be a 
control, right? We would create model-skins, so there is something 
that differentiates a control from a model. To me that is an 
abstraction of the skin. For example: not knowning HOW it is rendered, 
a textbox does know that it can receive the focus, it is implicit to 
what it is. So there is a certain logic for allowing controls to have 
rudimentary rendering info, as long as it does not expose the actual 
layout details.


Looking at ListView, there is a logic in that a list can scroll, so 
onScroll on the control makes sense. Whether that scrolling is done 
via a scrollbar or buttons is a skin detail that should not leak out. 
So the fact that ListView does not have a scroll position makes sense 
to me.


As someone that has been tempted to write a new Control that replaces 
ListView atleast half a dozen times now because of restrictions or 
idioms that don't match my needs, I'd disagree.  A ListView doesn't need 
to scroll at all.  An application that isn't mouse or touchscreen 
controlled (keyboard or remote controlled for example) has zero need for 
scrollbars except maybe as information to show the relative size of the 
view.


A List of items could be paged only, or they could flip.  I'd like to be 
able to take a List, and wrap it in a ScrollBarView... or in a 
PagerView, FlipOverView or CoverFlowView (with 30 new properties to set 
things like reflections, 3d parameters, distance between items, etc).   
It is possible to do this with Skins, but it feels like a hack rather 
than simply a different LookFeel in the end.


 After all, a ListView is a container for an unbounded list of items.  
I can think of half a dozen ways of how that can be shown to the user, 
and the current ListView is just one way to do it.  The promise of Skins 
here is that I could just change the look  feel, but unfortunately way 
too many details of the default look  feel leak through in the 
Control itself.


--John


Tom


On 14-3-2015 04:33, Tomas Mikula wrote:

A quick poll:

Has anyone ever implemented a custom skin for some of the more complex
controls like ListView, TableView, TreeView, TextArea?

The problem I have with the Control/Skin architecture is that a
Control, being a Node in the scene graph, cannot be a pure model (in
the MVC sense) - it is inherently a view (in the MVC sense). What is
the model of a check box? For me, a model of a check box is a boolean
property; certainly not something that has boundsInParent or
onZoomFinished properties. CheckBox is hardly a model. It is a view.
Everything in the scene graph is inherently a view.

There is this idea that the view aspect of a control is implemented by
the skin. The control itself does not know what the skin looks like or
even what skin class is used. So a ListView knows about its items, but
it does not know about its scroll position. But users sometimes want
to know the scroll position. Why should there be onScrollProperty, but
not way to get the current scroll position? Why should there be
TextArea.getBoundsInParent(), but not TextArea.getCaretBounds()? There
is no good way to implement the latter methods using custom skins. The
ListView or TextArea don't know anything about the skin, thus they
don't know anything about the current scroll position or caret bounds.
They cannot ask the skin, because there might be no skin yet, and even
if there is, all they know about it is that it is an instance of
Skin? - not much one can do with it (certainly not get caret
bounds).

I'm leaning more and more towards not supporting custom skins at all.
The whole idea of overriding skins via CSS looks to me like dependency
injection via CSS, except without any possibility to constrain the
type of what can be injected.

I would like to know the community opinion on this. Even hear your
success story how skins are awesome, if there is such.

Regards,
Tomas






Re: The trouble with Skins

2015-03-21 Thread John Hendrikx


For me, I'd like to simply change the Look and Feel and have the same 
data presented differently (perhaps related to space restrictions, 
orientation, user preferences).  Currently, this can be achieved by 
changing the Control, or maybe only change the Skin, depending on how 
radical the transformation is.  I'd like that to be the same thing for 
all these cases and have the same general API (show item X, get 
position, set position, go to first item, go to last item).


If Skins aren't intended for that (despite being able to change the 
complete appearance and even change all the navigation handling), then 
fine -- we'll need to do it with Controls, and have similar controls 
all implement the same interface.


ListView looked to me like a very close match, that just needs a small 
push to get what I want (re-implementing all the virtual cell logic is 
not something I want to do  for every skin).  It could be shown as 
pages easily (with a nice pager in the top right corner something).  
This does not affect the API.  I can still call show item X, or ask 
for it to give me an object that represents the current position (making 
it possible to restore it there later).  I don't need to know it is page 
based, or even know the page number -- just like I don't need to know 
there are scrollbars or what the current scroll position is.  What I do 
need however is a way to restore the control to the exact same state it 
was in before (the same amount of pixels scrolled, the same item at the 
top, the same item at the bottom).


--John

On 21/03/2015 20:32, Scott Palmer wrote:

My point is that there are many ways to display the same data set, but they shouldn't all be 
crammed into a single control type. Particularly one that changes radically based on the skin just 
because the data model can be represented by the same data structure.  For a ListView it could 
still fit if it was a list that scrolled a page at a time,  perhaps that was a bad 
example. I would still expect the control to be a Node and to have direct access via the control to 
the current scroll position. I would not expect a ListView to have a page-based API, 
nor would I expect to get access to such an API by accessing the Skin. The skin is an 
implementation detail that should remain distinct from how my program interacts with the control.

Scott


On Mar 21, 2015, at 2:53 PM, Tom Eugelinkt...@tbee.org  wrote:

A page based view can perfectly be a list view; I would have no problem having 
a paging skin or a scrollbar skin for ListView.




On 21-3-2015 19:46, Scott Palmer wrote:
But that's not right.  A *List* is 'a container for an unbounded list of 
items.  A ListView is a specific type of control that gives you a view to that 
list *in a specific way*.  It has properties in addition to the list itself.  It 
*should* have a scroll position because that is a property of that kind of control - 
even though the skin will implement it. These controls are visible elements and must 
expose properties that will ultimately be implemented in the skin. They must be 
Nodes.  Otherwise they will be far too cumbersome to use.
  You may not need to expose where the scroll bars are, or get access to arrow 
buttons, etc., but you should have access to what items are visible, what the 
scroll position is, etc.  you should be able to change the scroll position 
programmatically without knowing the skin.

A page based view is a different kind of control. It can still use a List 
model, but flipping to a page system from a scrollable list is not something I 
would expect when changing the look and feel.  I might expect to have the view 
change from pixel-based scrolling to line based scrolling, or have up/down 
arrows at one end of the scroll bar instead of each end.

Scott


On Sat, Mar 21, 2015 at 12:01 PM, John Hendrikxhj...@xs4all.nl  wrote:

As someone that has been tempted to write a new Control that replaces ListView 
atleast half a dozen times now because of restrictions or idioms that don't 
match my needs, I'd disagree.  A ListView doesn't need to scroll at all.  An 
application that isn't mouse or touchscreen controlled (keyboard or remote 
controlled for example) has zero need for scrollbars except maybe as 
information to show the relative size of the view.

A List of items could be paged only, or they could flip.  I'd like to be able to 
take a List, and wrap it in a ScrollBarView... or in a PagerView, FlipOverView or 
CoverFlowView (with 30 new properties to set things like reflections, 3d 
parameters, distance between items, etc).   It is possible to do this with Skins, 
but it feels like a hack rather than simply a different LookFeel in the end.

  After all, a ListView is a container for an unbounded list of items.  I can think of half a 
dozen ways of how that can be shown to the user, and the current ListView is just one way to do 
it.  The promise of Skins here is that I could just change the look  feel, but unfortunately 
way too 

Build fails at graphics:linkWinFont

2018-08-24 Thread John Hendrikx

Hi,

I'm trying to get openjfx build on Windows, but I've run into an issue
I haven't been able to resolve.

Any tips I can try?

--John


> Task :graphics:linkWinFont FAILED
   Creating library 
P:\Dev\git\openjdk-jfx\modules\javafx.graphics\build\libs\font\win\javafx_font.lib 
and object 
P:\Dev\git\openjdk-jfx\modules\javafx.graphics\build\libs\font\win\javafx_font.exp
fontpath.obj : error LNK2019: unresolved external symbol 
__imp_EnumFontFamiliesExW referenced in function DifferentFamily
fontpath.obj : error LNK2019: unresolved external symbol 
__imp_GetDeviceCaps referenced in function 
Java_com_sun_javafx_font_PrismFontFactory_getSystemFontSizeNative
C:/Program Files (x86)/Windows Kits/10/lib/10.0.16299.0/um/x86\gdi32.lib 
: warning LNK4272: library machine type 'X86' conflicts with target 
machine type 'x64'
P:\Dev\git\openjdk-jfx\modules\javafx.graphics\build\libs\font\win\javafx_font.dll 
: fatal error LNK1120: 2 unresolved externals


FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':graphics:linkWinFont'.
> Process 'command 'C:/Program Files (x86)/Microsoft Visual 
Studio/2017/Enterprise/VC/Tools/MSVC/14.10.25017/bin/HostX64/x64/link.exe'' 
finished with non-zero exit value 1120




Re: Build fails at graphics:linkWinFont

2018-08-25 Thread John Hendrikx
There should have been a gdi32.lib in the x64 folder as well, but there 
wasn't.  Downloading a Windows SDK (10.0.16299) and installing it solved 
this.


--John

On 25/08/2018 00:24, Kevin Rushforth wrote:



C:/Program Files (x86)/Windows
Kits/10/lib/10.0.16299.0/um/x86\gdi32.lib : warning LNK4272: library
machine type 'X86' conflicts with target machine type 'x64'


This looks like a problem where the tools are confused about 32-bit
versus 64-bit. It might be something missing from your Visual Studio
installation, but it's hard to say without more information. Take a look
at build/windows_tools.properties and see if anything looks odd. One
thing you could try is to 'rm -rf build', then run 'gradle clean' then
try the build again.

-- Kevin


On 8/24/2018 3:07 PM, John Hendrikx wrote:

Hi,

I'm trying to get openjfx build on Windows, but I've run into an issue
I haven't been able to resolve.

Any tips I can try?

--John


> Task :graphics:linkWinFont FAILED
   Creating library
P:\Dev\git\openjdk-jfx\modules\javafx.graphics\build\libs\font\win\javafx_font.lib
and object
P:\Dev\git\openjdk-jfx\modules\javafx.graphics\build\libs\font\win\javafx_font.exp

fontpath.obj : error LNK2019: unresolved external symbol
__imp_EnumFontFamiliesExW referenced in function DifferentFamily
fontpath.obj : error LNK2019: unresolved external symbol
__imp_GetDeviceCaps referenced in function
Java_com_sun_javafx_font_PrismFontFactory_getSystemFontSizeNative
C:/Program Files (x86)/Windows
Kits/10/lib/10.0.16299.0/um/x86\gdi32.lib : warning LNK4272: library
machine type 'X86' conflicts with target machine type 'x64'
P:\Dev\git\openjdk-jfx\modules\javafx.graphics\build\libs\font\win\javafx_font.dll
: fatal error LNK1120: 2 unresolved externals

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':graphics:linkWinFont'.
> Process 'command 'C:/Program Files (x86)/Microsoft Visual
Studio/2017/Enterprise/VC/Tools/MSVC/14.10.25017/bin/HostX64/x64/link.exe''
finished with non-zero exit value 1120





Review Request for JDK-8209968: Fix rounding error in image scale calculation

2018-09-01 Thread John Hendrikx

Hi,

This is a review request for:

https://bugs.openjdk.java.net/browse/JDK-8209968

The PR is on Github:

https://github.com/javafxports/openjdk-jfx/pull/170

--John


Re: JavaFX Application Thread is recursively re-entrant into Eventhandler handle() method under some circumstances

2018-09-09 Thread John Hendrikx

I see nothing special in the stack trace.

When you remove the component, a new MouseEvent *must* trigger 
(MouseEvent.EXITED) as it always needs to match with MouseEvent.ENTERED.


So, the call to 'remove' triggers a new event, which gets handled by the 
same handler.  It is indeed entered recursively, but in a normal 
fashion.  This has nothing to do with another thread or compiler bugs.


Don't be confused by the double "handle" lines in the stacktrace.  This 
happens when methods are overriden (the line number is 1).


There are two relevant lines in this trace:

   LabelEventHandler.handle(LabelEventHandler.java:95)

...where Remove is called, which triggers the recursive call later on:

   LabelEventHandler.handle(LabelEventHandler.java:88)

... for the MouseEvent.EXITED event.

The full stack trace is this:

	at 
bareBonesJavaFXBugExample.LabelEventHandler.handle(LabelEventHandler.java:88)
	at 
bareBonesJavaFXBugExample.LabelEventHandler.handle(LabelEventHandler.java:1)
	at 
javafx.base/com.sun.javafx.event.CompositeEventHandler$NormalEventHandlerRecord.handleBubblingEvent(CompositeEventHandler.java:218)
	at 
javafx.base/com.sun.javafx.event.CompositeEventHandler.dispatchBubblingEvent(CompositeEventHandler.java:80)
	at 
javafx.base/com.sun.javafx.event.EventHandlerManager.dispatchBubblingEvent(EventHandlerManager.java:238)
	at 
javafx.base/com.sun.javafx.event.EventHandlerManager.dispatchBubblingEvent(EventHandlerManager.java:191)
	at 
javafx.base/com.sun.javafx.event.CompositeEventDispatcher.dispatchBubblingEvent(CompositeEventDispatcher.java:59)
	at 
javafx.base/com.sun.javafx.event.BasicEventDispatcher.dispatchEvent(BasicEventDispatcher.java:58)
	at 
javafx.base/com.sun.javafx.event.EventDispatchChainImpl.dispatchEvent(EventDispatchChainImpl.java:114)
	at 
javafx.base/com.sun.javafx.event.BasicEventDispatcher.dispatchEvent(BasicEventDispatcher.java:56)
	at 
javafx.base/com.sun.javafx.event.EventDispatchChainImpl.dispatchEvent(EventDispatchChainImpl.java:114)
	at 
javafx.base/com.sun.javafx.event.EventUtil.fireEventImpl(EventUtil.java:74)

at 
javafx.base/com.sun.javafx.event.EventUtil.fireEvent(EventUtil.java:49)
at javafx.base/javafx.event.Event.fireEvent(Event.java:198)
at javafx.base/com.sun.javafx.event.EventQueue.fire(EventQueue.java:48)
	at 
javafx.graphics/javafx.scene.Scene$MouseHandler.handleNodeRemoval(Scene.java:3717)
	at 
javafx.graphics/javafx.scene.Scene$MouseHandler.access$7800(Scene.java:3604)

at 
javafx.graphics/javafx.scene.Scene.generateMouseExited(Scene.java:3601)
at 
javafx.graphics/javafx.scene.Parent$3.onProposedChange(Parent.java:613)
	at 
javafx.base/com.sun.javafx.collections.VetoableListDecorator.remove(VetoableListDecorator.java:329)
	at 
javafx.base/com.sun.javafx.collections.VetoableListDecorator.remove(VetoableListDecorator.java:221)
	at 
bareBonesJavaFXBugExample.LabelEventHandler.handle(LabelEventHandler.java:95)
	at 
bareBonesJavaFXBugExample.LabelEventHandler.handle(LabelEventHandler.java:1)
	at 
javafx.base/com.sun.javafx.event.CompositeEventHandler$NormalEventHandlerRecord.handleBubblingEvent(CompositeEventHandler.java:218)


(... rest cut off as it is not needed ... )

--John

On 09/09/2018 19:05, jav...@use.startmail.com wrote:

Hi All,
I spent some time refactoring the program which displays this bug. It's
now small enough to share the source in an email, but it is very very
dense and the proof of bug, one  specific line to standard I/O, requires
the source code to be read and understood in order to see the bug. As I
said previously, more comprehensible and user-friendly versions of this
program are  available at . The javadoc is more copious, the bug is
manifested as an exception and the side effect of the bug are more
consequential.

This brief version cnosists of just two classes. Here is the JavaFX
Application class:

***

package bareBonesJavaFXBugExample;


import javafx.application.Application;
import javafx.scene.Scene;
import javafx.scene.control.Label;
import javafx.scene.input.MouseEvent;
import javafx.scene.layout.Pane;
import javafx.stage.Stage;



/**
 * An {@link Application} with one {@link Pane} containing one {@link
Label}. The {@link Label} has a single {@link
javafx.event.EventHandler}, {@link LabelEventHandler} which processes
all {@link MouseEvent}s the {@link Label} receives.
 * 
 * To trigger the bug, run the application, then spend a second mouse
over the little label in the upper left hand corner of the scrren. You
will see output to standard I/O. Then, click the label, which will then
disppear. Check the I/O for Strings ending in debugCounter is 1. 
 * What that String means and how it proves that the JavaFX Application
Thread has become reentrant is explained in the javadoc of {@link
LabelEventHandler}.
 */
public class JavaFXAnomalyBareBonesApplication extends Application
{
public void start(Stage primaryStage)
 

Q: Rotated labels, layout and reflow

2018-12-14 Thread John Hendrikx

Hi list,

I get the impression that rotation of Labels needs to be something that 
is directly supported by Label instead of handling this with a Rotate 
transform (setRotate).


I want to achieve something quite trivial if no rotation was involved, a 
layout like this, an HBox with 3 labels in it:


 +-HBox+
 || Long text that can reflow to multiple ||
 | Short Text | lines if needed...| Short Text |
 ||   ||
 +-+

The center label would be given grow Priority.ALWAYS.

Now... the rotated version just goes wrong in so many ways.

First, I need to use Groups in order to get the layout bounds 
reasonable... however, these are unaware of how much space is available 
and will kill the reflow in the center Label.


If I put a Group around the whole HBox, the same issue occurs as the 
Group blocks any awareness of how big the area is where the three labels 
are going to appear, effectively rendering the center label as one long 
line.


What I'm actually trying to achieve is a layout that looks like this:

  ++---+
  ||   |
  ||   |
  ||   |
  ||   |
  ||   |
  ||   |
  ||   |
  ||   |


Q: Rotated labels, layout and reflow

2018-12-14 Thread John Hendrikx



(Sent this twice, first message got sent prematurely)

Hi list,

I get the impression that rotation of Labels needs to be something that 
is directly supported by Label instead of handling this with a Rotate 
transform (setRotate).


I want to achieve something quite trivial if no rotation was involved, a 
layout like this, an HBox with 3 labels in it:


  +-HBox+
  || Long text that can reflow to multiple ||
  | Short Text | lines if needed...| Short Text |
  ||   ||
  +-+

The center label would be given grow Priority.ALWAYS.

Now... the rotated version just goes wrong in so many ways.

First, I need to use Groups in order to get the layout bounds 
reasonable... however, these are unaware of how much space is available 
and will kill the reflow in the center Label.


If I put a Group around the whole HBox, the same issue occurs as the 
Group blocks any awareness of how big the area is where the three labels 
are going to appear, effectively rendering the center label as one long 
line.


What I'm actually trying to achieve is a layout that looks like this:

   ++---+
   |  T |   |
   |  e |   |
   |  x |   |
   |  t |   |
   ++   |
   ||   |
   |  T |   |
   |  e |   Image   |
   |  x |   |
   |  t |   |
   ||   |
   ++   |
   |  T |   |
   |  e |   |
   |  x |   |
   |  t |   |
   ++---+

Except of course the left area should be the rotated HBox.

Is this really not possible at the moment, without using a Canvas or 
something and a lot of layout calculations (to get reflow working)?


Any feedback appreciated :)

--John


Re: Q: Rotated labels, layout and reflow

2018-12-15 Thread John Hendrikx



I asked here because, although not a bug, it may be a good feature to 
support -- and I was looking for confirmation that this really isn't 
currently possible.  It's not a bug because a rotation transform is 
expected to not change the layout bounds.  Making use of Group fixes the 
layout bounds but makes it impossible to do proper dynamic layouts with 
labels that have been rotated 90 degrees.


Questions similar like this one, without good resolutions, show up on 
forums and stackoverflow, and, looking at the bug database, I think even 
some of the graph components part of JavaFX that support rotated labels 
have similar layout problems when texts needs to be cut-off or reflowed 
in such labels.


I even looked at MigLayout already, and noticed a similar issue reported 
there where rotated labels are not handled properly, probably because 
the layout bounds don't take the rotation into account, which no doubt 
MigLayout relies on to do its thing.


Now, I would love to contribute a fix for this (I contributed some small 
things before), however I think this might be a tough issue to resolve. 
The way I see it, this cannot be solved without Label taking the 
rotation into account itself and providing proper layout bounds -- this 
is needed because Label decides if text reflow needs to occur depending 
on where it is placed, and this information is lost when it is put in a 
Group.


So I think Label(ed) would need a new Rotate property, which only 
supports 0, 90, 180 and 270 degrees, or perhaps an extension to the text 
direction property (left-right, right-left, top-down, down-top), but I 
think that it serves a different purpose that is independent from 
rotation.


With this extra information, Label can then do the proper layout 
calculations with potential reflow of text and provide correct layout 
bounds.  The actual text rendering would also need to be rotated 
somehow, and I'm not quite sure how that would be accomplished yet for 
Labels.


All in all, it sounds like quite some effort that would need a good 
design, especially since Label already has a short-cut property to add a 
Rotate transform that cannot be re-used for this, so a new property 
would have to make the difference very clear.


--John

On 15/12/2018 09:18, Tom Eugelink wrote:

It's a bit grey. If this goes towards a bug in the layout, it could be
considered OpenJFX development. It could also go towards a patch,
because instead of using Canvas I would suggest (John) to look at the
HBox and see if you can figure out why it is not doing what you want.
And if that is too complex; write a layout that does this, and
contribute it to OpenJFX, ControlsFX or JFXtras. (I believe OpenJFX also
is the sum of all the extending libraries, not making the suck-it-all-in
mistake Java made.) The layout logic should be similar to when doing it
in Canvas, only reusable.

Also I have found that when rotating is involved, a lot of layouts do
not what you expect them to do. Have you given MigLayout a try? It
sometimes has surprising results (both positive and negative) ;-)




On 15-12-2018 03:14, John-Val Rose wrote:

My feedback would to ask this kind of question on a more appropriate
list or forum.

I believe this list is exclusively to discuss issues related to the
development of OpenJFX itself.

Graciously,

John-Val


On 15 Dec 2018, at 12:50, John Hendrikx  wrote:


(Sent this twice, first message got sent prematurely)

Hi list,

I get the impression that rotation of Labels needs to be something
that is directly supported by Label instead of handling this with a
Rotate transform (setRotate).

I want to achieve something quite trivial if no rotation was
involved, a layout like this, an HBox with 3 labels in it:

  +-HBox+
  || Long text that can reflow to multiple ||
  | Short Text | lines if needed...| Short Text |
  ||   ||
  +-+

The center label would be given grow Priority.ALWAYS.

Now... the rotated version just goes wrong in so many ways.

First, I need to use Groups in order to get the layout bounds
reasonable... however, these are unaware of how much space is
available and will kill the reflow in the center Label.

If I put a Group around the whole HBox, the same issue occurs as the
Group blocks any awareness of how big the area is where the three
labels are going to appear, effectively rendering the center label as
one long line.

What I'm actually trying to achieve is a layout that looks like this:

   ++---+
   |  T |   |
   |  e |   |
   |  x |   |
   |  t |   |
   ++   |
   ||   |
   |  T |   |
   |  e |   Image   |
   |  x |   |
   |  t

Re: RFR: 8185886: Improve scrolling performance of TableView and TreeTableView

2020-04-16 Thread John Hendrikx
On Thu, 16 Apr 2020 09:45:20 GMT, dannygonzalez 
 wrote:

>> @dannygonzalez  Could you perhaps debug your application and take a look at 
>> how large the following array is: a random
>> node -> `scene` -> `value` -> `window` -> `readOnlyProperty` -> `helper` -> 
>> `changeListeners`.  I just tested this with
>> a custom control displaying 200 cells on screen at once (each cell 
>> consisting of about 30 nodes itself), and I saw
>> about 2 change listeners registered on this single Scene Window 
>> property.  However, this custom control is not
>> creating/destroying cells beyond the initial allocation, so there wouldn't 
>> be any registering and unregistering going
>> on, scrolling was still smooth >30 fps.
>
> @hjohn I have 12136 change listeners when debugging our application as you 
> suggested.
> 
> Please note that I see the issue when the TableView is having items added to 
> it. If you just have a static TableView I
> do not see the issue.
> It is only when you add items to the TableView which causes a myriad of 
> listeners to be deregistered and registered.
> The Visual VM snapshot I attached above was taken as our application was 
> adding items to the TableView.

I've tested this pull request locally a few times, and the performance 
improvement is quite significant.   A test with
20.000 nested stack panes resulted in these average times:

- Add all 51 ms
- Remove all 25 ms

Versus the unmodified code:

- Add all 34 ms
- Remove all 135 ms

However, there are some significant functional changes as well that might 
impact current users:

1. The new code ensures that all listeners are notified even if the list is 
modified during iteration by always making
a **copy** when an event is fired.  The old code only did so when it was 
**actually** modified during iteration.  This
can be mitigated by making the copy in the code that modifies the list (as the 
original did) using the `locked` flag to
check whether an iteration was in progress.

2. There is a significant increase in memory use.  Where before each listener 
occupied an entry in an array, now each
listener is wrapped by `Map.Entry` (the Integer instance used per entry can be 
disregarded).  I estimate around 4-8x
more heap will be consumed (the numbers are small however, still well below 1 
MB for 20.000 entries).  If this is an
issue, a further level could be introduced in the listener implementation 
hierarchy (singleton -> array -> map).

3. Even though the current version of this pull request takes care to notify 
duplicate listeners the correct amount of
times, it does not notify them in the correct order with respect to other 
listeners.  If one registers listeners (a, b,
a) the notification order will be (a, a, b).

The last point is not easily solved and could potentially cause problems.

Finally I think this solution, although it performs well is not the full 
solution.  A doubling or quadrupling of nodes
would again run into serious limitations.  I think this commit
https://github.com/openjdk/jfx/commit/e21606d3a1b73cd4b44383babc750a4b4721edfd 
should not have introduced another
listener for each Node on the Window class.  A better solution would be to only 
have the Scene listen to Window and
have Scene provide a new combined status property that Node could use for its 
purposes.

Even better however would be to change the properties involved to make use of 
the hierarchy naturally present in Nodes,
having child nodes listen to their parent, and the top level node listen to the 
scene.  This would reduce the amount of
listeners on a single property in Scene and Window immensely, instead spreading 
those listeners over the Node
hierarchy, keeping listener lists much  shorter, which should scale a lot 
better.

-

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


RFR: 8242548: Honor line spacing in Labeled reflow calculation

2020-04-13 Thread John Hendrikx
This is a solution for 8242548.  There was zero test coverage, so I added a few 
tests for this as well.

-

Commit messages:
 - 8242548: Honor line spacing in Labeled reflow calculation

Changes: https://git.openjdk.java.net/jfx/pull/173/files
 Webrev: https://webrevs.openjdk.java.net/jfx/173/webrev.00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8242548
  Stats: 44 lines in 4 files changed: 40 ins; 0 del; 4 mod
  Patch: https://git.openjdk.java.net/jfx/pull/173.diff
  Fetch: git fetch https://git.openjdk.java.net/jfx pull/173/head:pull/173

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


Re: RFR: 8185886: Improve scrolling performance of TableView and TreeTableView

2020-04-14 Thread John Hendrikx
On Tue, 10 Mar 2020 06:08:25 GMT, yosbits 
 wrote:

>>> technically true - but the implementation was linear with a fixed sequence 
>>> since-the-beginning-of-java-desktop-time
>>> (and sometimes, for good ol' beans properties, even exposed as api to 
>>> access the array of listeners). So technically,
>>> we could go the path of explicitely spec'ing that the order is unspecified. 
>>> Pretty sure that doing so and implementing
>>> it will break tons of application code that's subtly relying on fifo 
>>> notification (f.i. register before or after skin
>>> has its own is a simple wide-spread trick) .. which all did it wrong and 
>>> might deserve it ;-) But then if even internal
>>> code does it ..
>> 
>> So we can choose to specify it as ordered.
>> 
>> These are the 2 options I mentioned:
>> 1. Not specify the behavior and change the implementation in favor of 
>> performance. This could break applications as
>> you've mentioned. 2. Specify that the order is preserved and keep the 
>> ordered implementation behavior. This will allow
>> applications to rely on the behavior safely.
>> Right now we're losing on both sides. We keep a promise and we don't tell 
>> anyone about it. The only reason for this is
>> if we intend to change the behavior in the future, in which case we should 
>> add a doc comment saying that the order is
>> unspecified and is planned to change in the future, so there will be at 
>> least some warning.  Once we choose what to do
>> here it will make sense to continue to review the code with that decision in 
>> mind.
>
> In a minimal test I wrote (not a microbenchmark that removes listeners), I 
> tried this PR code, but did not reproduce
> the performance improvement. I have attached a test program in my PR(#125)

@dannygonzalez You mentioned "There are multiple change listeners on a Node for 
example, so you can imagine with the
number of nodes in a TableView this is going to be a problem if the 
unregistering is slow.".

Your changes in this PR seem to be focused on improving performance of 
unregistering listeners when there are thousands
of listeners listening on changes or invalidations of the **same** property.  
Is this actually the case?  Is there a
single property in TableView or TreeTableView where such a high amount of 
listeners are present?  If so, I think the
problem should be solved there.

As it is, I donot think this PR will help unregistration performance of 
listeners when the listeners are  spread out
over dozens of properties, and although high in total number, the total 
listeners per property would be low and their
listener lists would be short.  Performance of short lists (<50 entries) is 
almost unbeatable, so it is relevant for
this PR to know if there are any properties with long listener lists.

-

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


Re: Remove JavaFX JPMS enforcement

2020-04-20 Thread John Hendrikx




With a not very intuitive hack you can circumvent this problem already now.
Just add a line like this to the file which contains your main class
extending
Application (MyApp).

class MyAppLauncher {public static void main(String[] args)
{MyApp.main(args);}}


I do exactly the same, I construct a fat jar with all of JavaFX included 
and run it as above.


I was looking forward to the module system allowing a clean and easy way 
to have a visibility scope inbetween public and package (by making 
packages hierarchical for example), but it turned into a monster that 
just seems to make projects fragile and hard to build/run.


I suppose it may be possible to just generate and publish JavaFX jars 
without all the module-info files included, but I'm not sure it is as 
simple as that.


--John


Re: RFR: 8185886: Improve scrolling performance of TableView and TreeTableView

2020-04-17 Thread John Hendrikx
On Fri, 17 Apr 2020 07:22:20 GMT, dannygonzalez 
 wrote:

>> @hjon
>> 
>>> 3. Even though the current version of this pull request takes care to 
>>> notify duplicate listeners the correct amount of
>>> times, it does not notify them in the correct order with respect to other 
>>> listeners. If one registers listeners (a, b,
>>> a) the notification order will be (a, a, b).
>> 
>> Unless I'm missing something I don't think this is the case. I used a 
>> LinkedHashMap which preserved the order of
>> notifications. Actually some unit tests failed if the notifications weren't 
>> carried out in the same order as
>> registration which was the case when I used a HashMap. See here:
>> https://github.com/openjdk/jfx/pull/108#issuecomment-590883183
>
> @hjohn
> 
>> 2. There is a significant increase in memory use. Where before each listener 
>> occupied an entry in an array, now each
>> listener is wrapped by Map.Entry (the Integer instance used per entry can be 
>> disregarded). I estimate around 4-8x more
>> heap will be consumed (the numbers are small however, still well below 1 MB 
>> for 20.000 entries). If this is an issue, a
>> further level could be introduced in the listener implementation hierarchy 
>> (singleton -> array -> map).
> 
> There was discussion about lazy initialisation of the LinkedHashMap when 
> needed and/or have some sort of strategy where
> we could use arrays or lists if the number of listeners are less than some 
> threshold (i.e. introducing another level to
> the hierarchy as you mentioned). This was mentioned here also:
> https://github.com/openjdk/jfx/pull/108#issuecomment-590838942

I've implemented an alternative solution: Removing the listeners on 
`Window.showingProperty` and `Scene.windowProperty`
completely.  They are in fact only used in two places: `PopupWindow` in order 
to remove itself if the `Window` is no
longer showing, and `ProgressIndicatorSkin`.  These two can be easily replaced 
with their own listeners for these
properties instead of burdening **all** nodes with these listeners only to 
support these two classes.

I left the `isTreeShowing` method in, and implemented it simply as 
`isTreeVisible() && isWindowShowing()` as that's
really the only difference between "visible" and "showing" apparently.

Here is the test result with 20.000 nested StackPanes with only this change in:

- Add all 45 ms
- Remove all 25 ms

I think this might be a good solution as it completely avoids these listeners.

-

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


Re: RFR: 8185886: Improve scrolling performance of TableView and TreeTableView

2020-04-17 Thread John Hendrikx
On Fri, 17 Apr 2020 08:48:35 GMT, dannygonzalez 
 wrote:

>> @dannygonzalez I added a proof of concept here if you want to play with it: 
>> https://github.com/openjdk/jfx/pull/185
>
> @hjohn Thanks for looking into this. It looks like your changes do improve 
> the issue with the JavaFX thread being
> swamped with listener de-registrations. Looking at the JavaFX thread in 
> VisualVM, the removeListener call has dropped
> off the hotspots in the same way it did with my pull request.  I wasn't fully 
> confident of making changes to the Node
> hierarchy to remove listeners hence why I approached the issue from the other 
> direction i.e. the obvious bottleneck
> which was the listener de-registration slowness.  I do worry however that any 
> changes down the road which add listeners
> to the Node hierarchy again without fully understanding the implications 
> would lead us to the same point we are now
> where the slowness of listener de-registrations becomes an issue again. There 
> are no tests that catch this scenario.  I
> feel that ideally both solutions are needed but am happy to bow to the more 
> experienced OpenJFX devs opinions here as I
> know my changes may be more fundamental and hence risky.

The problem is that there are usually many nodes, but only very few scenes and 
windows, so registering a listener for
each node on a scene or window is pretty bad design (also consider the amount 
of notifications that a scene change
would trigger in such scenarios).  As far as I can see, this is the only such 
listener and only needed for two very
limited cases, and its addition in the past may have slipped through the cracks.

Adding a performance unit test that specifically checks add/remove performance 
of nodes may prevent such future
regressions.

-

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


Re: RFR: 8185886: Improve scrolling performance of TableView and TreeTableView

2020-04-16 Thread John Hendrikx
On Thu, 16 Apr 2020 06:34:39 GMT, yosbits 
 wrote:

>> @hjohn
>> I haven't quantified exactly where all the listeners are coming from but the 
>> Node class for example has various
>> listeners on it.
>> The following changeset  (which added an extra listener on the Node class) 
>> impacted TableView performance significantly
>> (although it was still bad before this change in my previous tests):
>>> commit e21606d3a1b73cd4b44383babc750a4b4721edfd
>>> Author: Florian Kirmaier mailto:fk at sandec.de>>
>>> Date:   Tue Jan 22 09:08:36 2019 -0800
>>> 
>>> 8216377: Fix memoryleak for initial nodes of Window
>>> 8207837: Indeterminate ProgressBar does not animate if content is added 
>>> after scene is set on window
>>> 
>>> Add or remove the windowShowingChangedListener when the scene changes
>> 
>> As you can imagine, a TableView with many columns and rows can be made up of 
>> many Node classes. The impact of having
>> multiple listeners deregistering on the Node when new rows are being added 
>> to a TableView can be a significant
>> performance hit on the JavaFX thread.   I don't have the time or knowledge 
>> to investigate why these listeners are all
>> needed especially around the Node class. I went directly to the source of 
>> the problem which was the linear search to
>> deregister each listener.  I have been running my proposed fix in our JavaFX 
>> application for the past few months and it
>> has saved it from being unusable due to the JavaFx thread being swamped.
>
> The patch proposed here does not share the case where the listener deletion 
> performance becomes a bottleneck.
> 
> I think that it is necessary to reproduce it by testing first, but
> 
> If you just focus on improving listener removal performance,
> 
> If the lifespan of a large number of registered listeners is biased,
> It seems like the next simple change can improve delete performance without 
> changing the data structure.
> 
> * Change the search from the front of the list to the search from the back.
> 
> This will reduce the number of long-life listeners matching.

Looking at the commit  
https://github.com/openjdk/jfx/commit/e21606d3a1b73cd4b44383babc750a4b4721edfd
 it seems that the long listener lists are actually part of the `Scene`'s 
`Window` property and the `Window`'s `Showing`
 property.  Each `Node` registers itself on those and so the listener lists for 
those properties would scale with the
 number of nodes.

A test case showing this problem would really be great as then the patch can 
also be verified to solve the problem, but
I suppose it could be reproduced simply by having a large number of Nodes in a 
scene.  @dannygonzalez could you give us
an idea how many Nodes we're talking about?  1000? 10.000?

It also means there might be other options, do Nodes really need to add these 
listeners and for which functionality and
are there alternatives?  It would also be possible to target only these 
specific properties with an optimized listener
list to reduce the impact of this change.

-

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


Re: RFR: 8185886: Improve scrolling performance of TableView and TreeTableView

2020-04-16 Thread John Hendrikx
On Thu, 16 Apr 2020 08:24:16 GMT, dannygonzalez 
 wrote:

>> The listeners added by `Node` are apparently internally required for 
>> internal properties `TreeShowing` and
>> `TreeVisible`, and are used to take various decisions like whether to 
>> play/pause animations.  There is also a couple of
>> listeners registering on these properties in turn (in `PopupWindow`, 
>> `SwingNode`, `WebView` and `MediaView`).  A lot of
>> the checks for visibility / showing could easily be done by using the 
>> `Scene` property and checking visibility /
>> showing status from there.  No need for so many listeners.  The other 
>> classes mentioned might register their own
>> listener, instead of having `Node` do it for them (and thus impacting 
>> *every* node).  Alternatively, `Node` may lazily
>> register the listeners for Scene.Window and Window.Showing only when needed 
>> (which from what I can see is for pretty
>> specific classes only, not classes that you'd see a lot in a TableView...)
>
> If it is of any help, I have attached a VisualVM snapshot (v1.4.4) where the 
> ExpressionHelper.removeListener is using
> 61% of the JavaFX thread whilst running our application.
> [snapshot-1587024308245.nps.zip](https://github.com/openjdk/jfx/files/4485728/snapshot-1587024308245.nps.zip)
> 
> If you show only the JavaFX Application thread, press the "HotSpot" and 
> "Reverse Calls" button you can take a look to
> see which classes are calling the removeListener function.
> ![Screenshot 2020-04-16 at 09 16
> 11](https://user-images.githubusercontent.com/6702882/79432046-2f788800-7fc3-11ea-930a-98fed0190628.png)

@dannygonzalez  Could you perhaps debug your application and take a look at how 
large the following array is: a random
node -> `scene` -> `value` -> `window` -> `readOnlyProperty` -> `helper` -> 
`changeListeners`.  I just tested this with
a custom control displaying 200 cells on screen at once (each cell consisting 
of about 30 nodes itself), and I saw
about 2 change listeners registered on this single Scene Window property.

However, this custom control is not creating/destroying cells beyond the 
initial allocation, so there wouldn't be any
registering and unregistering going on, scrolling was still smooth >30 fps.

-

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


Re: RFR: 8185886: Improve scrolling performance of TableView and TreeTableView

2020-04-16 Thread John Hendrikx
On Thu, 16 Apr 2020 07:11:58 GMT, John Hendrikx  wrote:

>> The patch proposed here does not share the case where the listener deletion 
>> performance becomes a bottleneck.
>> 
>> I think that it is necessary to reproduce it by testing first, but
>> 
>> If you just focus on improving listener removal performance,
>> 
>> If the lifespan of a large number of registered listeners is biased,
>> It seems like the next simple change can improve delete performance without 
>> changing the data structure.
>> 
>> * Change the search from the front of the list to the search from the back.
>> 
>> This will reduce the number of long-life listeners matching.
>
> Looking at the commit  
> https://github.com/openjdk/jfx/commit/e21606d3a1b73cd4b44383babc750a4b4721edfd
>  it seems that the long listener lists are actually part of the `Scene`'s 
> `Window` property and the `Window`'s `Showing`
>  property.  Each `Node` registers itself on those and so the listener lists 
> for those properties would scale with the
>  number of nodes.
> 
> A test case showing this problem would really be great as then the patch can 
> also be verified to solve the problem, but
> I suppose it could be reproduced simply by having a large number of Nodes in 
> a scene.  @dannygonzalez could you give us
> an idea how many Nodes we're talking about?  1000? 10.000?  It also means 
> there might be other options, do Nodes really
> need to add these listeners and for which functionality and are there 
> alternatives?  It would also be possible to
> target only these specific properties with an optimized listener list to 
> reduce the impact of this change.

The listeners added by `Node` are apparently internally required for internal 
properties `TreeShowing` and
`TreeVisible`, and are used to take various decisions like whether to 
play/pause animations.  There is also a couple of
listeners registering on these properties in turn (in `PopupWindow`, 
`SwingNode`, `WebView` and `MediaView`).

A lot of the checks for visibility / showing could easily be done by using the 
`Scene` property and checking visibility
/ showing status from there.  No need for so many listeners.  The other classes 
mentioned might register their own
listener, instead of having `Node` do it for them (and thus impacting *every* 
node).

Alternatively, `Node` may lazily register the listeners for Scene.Window and 
Window.Showing only when needed (which
from what I can see is for pretty specific classes only, not classes that you'd 
see a lot in a TableView...)

-

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


Re: RFR: 8185886: Improve scrolling performance of TableView and TreeTableView

2020-04-17 Thread John Hendrikx
On Fri, 17 Apr 2020 07:22:20 GMT, dannygonzalez 
 wrote:

>> @hjon
>> 
>>> 3. Even though the current version of this pull request takes care to 
>>> notify duplicate listeners the correct amount of
>>> times, it does not notify them in the correct order with respect to other 
>>> listeners. If one registers listeners (a, b,
>>> a) the notification order will be (a, a, b).
>> 
>> Unless I'm missing something I don't think this is the case. I used a 
>> LinkedHashMap which preserved the order of
>> notifications. Actually some unit tests failed if the notifications weren't 
>> carried out in the same order as
>> registration which was the case when I used a HashMap. See here:
>> https://github.com/openjdk/jfx/pull/108#issuecomment-590883183
>
> @hjohn
> 
>> 2. There is a significant increase in memory use. Where before each listener 
>> occupied an entry in an array, now each
>> listener is wrapped by Map.Entry (the Integer instance used per entry can be 
>> disregarded). I estimate around 4-8x more
>> heap will be consumed (the numbers are small however, still well below 1 MB 
>> for 20.000 entries). If this is an issue, a
>> further level could be introduced in the listener implementation hierarchy 
>> (singleton -> array -> map).
> 
> There was discussion about lazy initialisation of the LinkedHashMap when 
> needed and/or have some sort of strategy where
> we could use arrays or lists if the number of listeners are less than some 
> threshold (i.e. introducing another level to
> the hierarchy as you mentioned). This was mentioned here also:
> https://github.com/openjdk/jfx/pull/108#issuecomment-590838942

@dannygonzalez I added a proof of concept here if you want to play with it: 
https://github.com/openjdk/jfx/pull/185

-

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


RFR: 8243115: Unregister bindings when unbind called multiple times

2020-04-27 Thread John Hendrikx
This fixes a bug where the first call to unbind would clear the internal 
invalidation listener used, resulting in
subsequent unbind calls to be no-ops, unless bind was called again first.

I had to rewrite the parameterized test slightly as Parameterized will only 
call the parameters method once, and my new
test modifies the internal state of the bindings used as parameters (by doing 
some unbind calls) which was making other
tests fail.

-

Commit messages:
 - 8243115: Unregister bindings when unbind called multiple times

Changes: https://git.openjdk.java.net/jfx/pull/198/files
 Webrev: https://webrevs.openjdk.java.net/jfx/198/webrev.00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8243115
  Stats: 206 lines in 9 files changed: 147 ins; 43 del; 16 mod
  Patch: https://git.openjdk.java.net/jfx/pull/198.diff
  Fetch: git fetch https://git.openjdk.java.net/jfx pull/198/head:pull/198

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


Re: [Rev 03] RFR: 8242548: Honor line spacing in Labeled reflow calculation

2020-04-27 Thread John Hendrikx
> This is a solution for 8242548.  There was zero test coverage, so I added a 
> few tests for this as well.

John Hendrikx has updated the pull request incrementally with one additional 
commit since the last revision:

  Fix typo in comment

-

Changes:
  - all: https://git.openjdk.java.net/jfx/pull/173/files
  - new: https://git.openjdk.java.net/jfx/pull/173/files/20ccd127..8fa8f3f0

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

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

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


Re: RFR: 8242548: Honor line spacing in Labeled reflow calculation

2020-04-22 Thread John Hendrikx
On Wed, 22 Apr 2020 12:46:03 GMT, Ajit Ghaisas  wrote:

>> This is a solution for 8242548.  There was zero test coverage, so I added a 
>> few tests for this as well.
>
> modules/javafx.controls/src/main/java/com/sun/javafx/scene/control/skin/Utils.java
>  line 426:
> 
>> 425:
>> 426: height += lineSpacing;
>> 427:
> 
> Modifying 'height' parameter seems incorrect as this parameter is used in 
> other calculations below. If needed, use a
> separate local variable to use 'height+lineSpacing'

The height used needs to be changed everywhere.  However, I can move this to 
the caller instead, with a comment
explaining why:

> The computeClippedWrappedText call works with full lines (height of line + 
> line spacing), including the last line.
> However the wrap height does not include the line spacing for the last line.  
> In order to avoid wrapping the last line
> of text even though there is sufficient space the wrap height passed to 
> computeClippedWrappedText is increased with the
> line spacing so it computes the correct text.

-

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


Re: RFR: 8242548: Honor line spacing in Labeled reflow calculation

2020-04-22 Thread John Hendrikx
On Wed, 22 Apr 2020 12:51:54 GMT, Ajit Ghaisas  wrote:

>> This is a solution for 8242548.  There was zero test coverage, so I added a 
>> few tests for this as well.
>
> modules/javafx.controls/src/main/java/com/sun/javafx/scene/control/skin/Utils.java
>  line 427:
> 
>> 426: height += lineSpacing;
>> 427:
>> 428: String ellipsis = (truncationStyle == CLIP) ? "" : 
>> ellipsisString;
> 
> This method makes a call to -
> double eHeight = computeTextHeight(font, ellipsis, 0, boundsType);
> 
> What is the behavior if we make a call to the other method that takes in 
> lineSpacing?
> double eHeight = computeTextHeight(font, ellipsis, 0, lineSpacing, 
> boundsType);

The call only determines the ellipsis height (eHeight) to see if the space is 
so small that even an ellipsis wouldn't
fit.  For a multi-line ellipsis (if there is such a thing) we could pass in the 
line spacing here.

-

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


Re: [Rev 01] RFR: 8242548: Honor line spacing in Labeled reflow calculation

2020-04-23 Thread John Hendrikx
> This is a solution for 8242548.  There was zero test coverage, so I added a 
> few tests for this as well.

John Hendrikx 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. The 
pull request contains one new commit since
the last revision:

  8242548: Honor line spacing in Labeled reflow calculation

-

Changes:
  - all: https://git.openjdk.java.net/jfx/pull/173/files
  - new: https://git.openjdk.java.net/jfx/pull/173/files/86b2f011..8d6084eb

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

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

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


Re: [Rev 02] RFR: 8242548: Honor line spacing in Labeled reflow calculation

2020-04-23 Thread John Hendrikx
> This is a solution for 8242548.  There was zero test coverage, so I added a 
> few tests for this as well.

John Hendrikx 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. The 
pull request contains one new commit since
the last revision:

  8242548: Honor line spacing in Labeled reflow calculation

-

Changes:
  - all: https://git.openjdk.java.net/jfx/pull/173/files
  - new: https://git.openjdk.java.net/jfx/pull/173/files/8d6084eb..20ccd127

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

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

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


Re: [Rev 02] RFR: 8242548: Honor line spacing in Labeled reflow calculation

2020-04-23 Thread John Hendrikx
On Thu, 23 Apr 2020 10:51:05 GMT, Ajit Ghaisas  wrote:

>> John Hendrikx 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. The 
>> pull request contains one new commit since
>> the last revision:
>>   8242548: Honor line spacing in Labeled reflow calculation
>
> Marked as reviewed by aghaisas (Reviewer).

I added a comment clarification and passed the line spacing to the ellipsis 
height calculation as well (just in case
there is a multi line ellipsis).

The test indeed doesn't quite cover the change, the StubTextLayout is not 
advanced enough to actually do line wrapping
calculations (it ignores wrap height), and would need to be updated.

-

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


Re: [Integrated] RFR: 8242548: Wrapped labeled controls using -fx-line-spacing cut text off

2020-05-12 Thread John Hendrikx
On Sun, 12 Apr 2020 21:21:07 GMT, John Hendrikx  wrote:

> This is a solution for 8242548.  There was zero test coverage, so I added a 
> few tests for this as well.

This pull request has now been integrated.

Changeset: 7b061900
Author:    John Hendrikx 
Committer: Ajit Ghaisas 
URL:   https://git.openjdk.java.net/jfx/commit/7b061900
Stats: 53 lines in 4 files changed: 0 ins; 48 del; 5 mod

8242548: Wrapped labeled controls using -fx-line-spacing cut text off

Reviewed-by: aghaisas, kcr

-

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


Re: RFR: 8243115: Spurious invalidations due to bug in IntegerBinding and other classes

2020-05-12 Thread John Hendrikx
On Tue, 12 May 2020 15:56:32 GMT, Nir Lisker  wrote:

> The fix looks correct and the tests pass. I just wonder why the change to 
> reflection-based construction with
> `bindingMockClassConstructor`?

The Parameterized test constructs some standard Binding objects to run multiple 
tests with.  This works fine if those
objects are immutable (or aren't modified during the tests).  My new test 
however does modify them, and other tests
would fail with such modified objects (as `unbind` was called on some).  So I 
rewrote this a little bit to construct
fresh objects for each test, and for that I used some reflection magic to avoid 
having to write a specific test for
each of the 8 binding types.

-

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


Re: [Rev 03] RFR: 8242548: Wrapped labeled controls using -fx-line-spacing cut text off

2020-05-12 Thread John Hendrikx
On Tue, 12 May 2020 13:44:53 GMT, Ajit Ghaisas  wrote:

>> Marked as reviewed by kcr (Lead).
>
> @hjohn, this PR is ready to be merged.
> You need to comment /integrate on this PR as instructed by the bot above. I 
> will sponsor it once you do it.

Sorry I figured only a committer was allowed to use that command.

-

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


Re: RFR: 8243115: Spurious invalidations due to bug in IntegerBinding and other classes

2020-05-12 Thread John Hendrikx
On Mon, 11 May 2020 22:27:27 GMT, Nir Lisker  wrote:

> As I started my review I noticed that `unbind` does not null-check its 
> argument `dependencies` like `bind` does and it
> can lead to NPEs. If it is out of scope for this PR to fix this, a new issue 
> should be filed.

I'm fine with doing a fix, but I need to know which one.  Avoiding NPE's and 
silently doing nothing is IMHO not very
desirable as this will give the user of the API no feedback that something went 
wrong.

So I would prefer to fix this by documenting that these cases will result in a 
NPE.

The `bind` method has a similar issue -- it doesn't check its array elements 
for `null`, and will throw a NPE when
attempting to add a listener to `null`.  Again, I would just document the NPE 
so what is clearly a mistake doesn't go
unnoticed.

-

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


Re: Unable to allocate direct buffer memory

2020-05-10 Thread John Hendrikx

They're related.  32767x1137x4 = 149024316.

It would help to know what your app might be doing.  Although it could 
be a bug in JavaFX, it seems more likely that a canvas/image or cached 
group or something is a bit bigger than reasonable.


I think I've seen such errors in my app before I restricted maximum 
sizes of images which were supplied from uncontrolled sources.


--John

On 07/05/2020 20:51, Ty Young wrote:


On 5/6/20 12:39 PM, Ty Young wrote:

After hours of running my JavaFX application I start getting this
error spammed in console:


java.lang.OutOfMemoryError: Cannot reserve 149024316 bytes of direct
buffer memory (allocated: 149066725, limit: 209715200)
at java.base/java.nio.Bits.reserveMemory(Bits.java:178)
at
java.base/java.nio.DirectByteBuffer.(DirectByteBuffer.java:120)
at java.base/java.nio.ByteBuffer.allocateDirect(ByteBuffer.java:329)
at
javafx.graphics/com.sun.prism.impl.BufferUtil.newByteBuffer(BufferUtil.java:90)

at
javafx.graphics/com.sun.prism.impl.BufferUtil.newIntBuffer(BufferUtil.java:121)

at
javafx.graphics/com.sun.prism.impl.QueuedPixelSource.getUnusedPixels(QueuedPixelSource.java:155)

at
javafx.graphics/com.sun.javafx.tk.quantum.UploadingPainter.run(UploadingPainter.java:156)

at
java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:515)

at
java.base/java.util.concurrent.FutureTask.runAndReset(FutureTask.java:305)

at javafx.graphics/com.sun.javafx.tk.RenderJob.run(RenderJob.java:58)
at
java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1130)

at
java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:630)

at
javafx.graphics/com.sun.javafx.tk.quantum.QuantumRenderer$PipelineRunnable.run(QuantumRenderer.java:125)

at java.base/java.lang.Thread.run(Thread.java:832)


Ironically every single error is about allocating the exact same
amount of bytes.



Anything on this? I'm also getting:


java.lang.RuntimeException: Requested texture dimensions (32767x1137)
require dimensions (0x1137) that exceed maximum texture size (16384)
at
javafx.graphics/com.sun.prism.es2.ES2RTTexture.create(ES2RTTexture.java:220)

at
javafx.graphics/com.sun.prism.es2.ES2ResourceFactory.createRTTexture(ES2ResourceFactory.java:165)

at
javafx.graphics/com.sun.javafx.tk.quantum.UploadingPainter.run(UploadingPainter.java:124)

at
java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:515)

at
java.base/java.util.concurrent.FutureTask.runAndReset(FutureTask.java:305)
at javafx.graphics/com.sun.javafx.tk.RenderJob.run(RenderJob.java:58)
at
java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1130)

at
java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:630)

at
javafx.graphics/com.sun.javafx.tk.quantum.QuantumRenderer$PipelineRunnable.run(QuantumRenderer.java:125)

at java.base/java.lang.Thread.run(Thread.java:832)


spammed alternatively.



Re: Possible approaches to JDK-8185886: Improve scrolling performance of TableView and TreeTableView

2020-09-01 Thread John Hendrikx
The "dynamic update performance" performance issue we are seeing is a 
regression from JDK-8090322.  In this change the Node treeShowing 
property was introduced.  The JDK-8090322 warns in its description about:


"""Considerations:
* This is too expensive to calculate for all nodes by default. So the 
simplest way to provide it would be a special binding implementation or 
a util class. Where you create a instance and pass in the node you are 
interested in. It can then register listeners all the way up the tree 
and listen to what it needs.  """


The above comment is warning against the fact that registering listeners 
for EACH Node on Window and Scene is going to be a performance issue. 
As nodes can number in the 1000's or 10.000's, that's a lot of listeners 
to store in a List data structure.


PR 185 targets this issue and implements the feature in JDK-8090322 in
the way that was suggested in the above comment, instead of how it 
currently is implemented (registering listeners for every Node, just in 
case someone needs the treeShowing property).


It's scope is not as broad as it would seem (because of a change in 
Node).  It effectively only makes a small change in two controls 
(PopupWindow and ProgressIndicatorSkin).


--John



On 31/08/2020 16:54, Jeanette Winzenburg wrote:


Looking at the examples provided in 108/125: apart from both having many
columns (> 300 makes them really nasty) they differ in

Table content:
125 - static data
108 - items are frequently modified (added)

Perceived performance:
125 - vertical scrolling: thumb/content lags behind mouse
108 - with enough columns, all interaction is sluggish (mouse, keys,
..), scrolling being just one of them

Both have examples, running those against the suggested fixes (with 108a
for Jose's approach)
125 - fixes its own example, does nothing for the other
108, 108a, 185 - improves its own example, does nothing for other

So we seem to have multiple issues that are (mostly) orthogonal: one
being the broken/missing horizontal virtualization (125), the other
related to dynamic update of table content (108, 108a, 185). We need to
solve both, the solution/s for one looks (mostly?) unrelated to the
solution to the other.

125 is the only one PR for its use-case, and it seems to do its job.
From those targeting the dynamic data update all except Jose's (not yet
formalized into a PR) approach feel too broad: table's reaction to items
modifications is .. suboptimal in more than one aspect. Changing overall
notification implementation to improve that, sounds like covering it up.

-- Jeanette

Zitat von Kevin Rushforth :


Sorry for the badly formatted html. Here it is again.

I see some progress being made on the {Tree}TableView performance
issue. To summarize where I think we are:

There are currently 2 different approaches under review:

1. https://github.com/openjdk/jfx/pull/108 : optimization in
javafx.base to make removing listeners faster
2. https://github.com/openjdk/jfx/pull/125 : optimization in TableView
to reduce the number of add / removes

In addition, the following is a WIP PR that the author thinks could be
a 3rd approach:

3. https://github.com/openjdk/jfx/pull/185 : optimization in scene
graph to avoid install treeShowing listeners on Window and Scene for
all nodes

Jose has proposed a 4th approach as a comment to PR #108, and as I
understand it, he will propose a PR shortly.

4. Don't clear the list of children in a VirtualFlow when the number
of items changes.

So the first thing that is needed is to evaluate the approaches and
decide which one to pursue.

Options 1 and 3 are more broad in their scope, option #2 is more
targeted (to TableView), but within that area is a larger change.
Option #3 would remove the (internal) treeShowing property as a
generally available concept and only use it for controls like
ProgressIndicator that really need it. Option #4 affects only controls
that use VirtualFlow (ListView, TableVIew, TreeTableView), and seems
not to be a large change (presuming we can verify that no leak is
introduced).

I note that these fixes are not mutually exclusive, but I do think we
need to settle on a primary approach and use that to fix this issue.
If there are still performance problems after that fix, we can
consider one (or more) of the others.

Comments?

-- Kevin






Re: Possible approaches to JDK-8185886: Improve scrolling performance of TableView and TreeTableView

2020-09-02 Thread John Hendrikx

Hi Jeanette,

Thanks for taking a look.

The PR was sort of inspired by JDK-8185886 (PR 108) because changes were 
made there to make performance with long listener lists better (by using 
a Map structure).  I wanted to know the root cause of the long lists, 
and found out that the long lists are on properties carried by Window 
and Scene.  This then was traced to the treeShowing property in Node 
which registers listeners on Window and Scene.


I think Kevin then mentioned the JDK-8090322 issue which gave me further 
background on why the treeShowing solution was introduced.


The change I proposed to fix this regression was also tested by the 
author of PR 108 (Danny Gonzalez) and addressed the issue as well, see 
this comment: https://github.com/openjdk/jfx/pull/108#issuecomment-615125904


I'm not aware of any other side effects of the change introduces with 
JDK-8090322, but anything with lots of nodes in a single scene (in the 
thousands) may see some performance degradation when adding/removing 
nodes -- complex Tables or TreeTables are the primary culprit.  It might 
also affect the performance of hiding/showing a Window (when there are 
many nodes), but that's not an action that is usually done a lot.


I did not include a JDK number -- I can include JDK-8185886 as it was 
supposed to address that issue, or file a new issue referring both that 
issue and JDK-8090322.


I donot think it will fix the performance issue with having many columns 
in a table, although it may reduce its impact a bit.  The issue there is 
more that columns are not virtualized, so a table rotated 90 degrees 
would exhibit the same problems as an unvirtualized table would because 
of the sheer number of nodes involved.


--John

On 02/09/2020 14:24, Jeanette Winzenburg wrote:


Hi John,

thanks for the clarification :)

Hmm .. but then it's not really a PR against JDK-8185886 (scrolling
performance was always bad with many columns) but against - yet to be
reported - side-effect of JDK-8090322 which happens to detoriate
tableView performance even further (there might be other side-effects)?

-- Jeanette

Zitat von John Hendrikx :


The "dynamic update performance" performance issue we are seeing is a
regression from JDK-8090322.  In this change the Node treeShowing
property was introduced.  The JDK-8090322 warns in its description about:

"""Considerations:
* This is too expensive to calculate for all nodes by default. So the
simplest way to provide it would be a special binding implementation
or a util class. Where you create a instance and pass in the node you
are interested in. It can then register listeners all the way up the
tree and listen to what it needs.  """

The above comment is warning against the fact that registering
listeners for EACH Node on Window and Scene is going to be a
performance issue. As nodes can number in the 1000's or 10.000's,
that's a lot of listeners to store in a List data structure.

PR 185 targets this issue and implements the feature in JDK-8090322 in
the way that was suggested in the above comment, instead of how it
currently is implemented (registering listeners for every Node, just
in case someone needs the treeShowing property).

It's scope is not as broad as it would seem (because of a change in
Node).  It effectively only makes a small change in two controls
(PopupWindow and ProgressIndicatorSkin).

--John



On 31/08/2020 16:54, Jeanette Winzenburg wrote:


Looking at the examples provided in 108/125: apart from both having many
columns (> 300 makes them really nasty) they differ in

Table content:
125 - static data
108 - items are frequently modified (added)

Perceived performance:
125 - vertical scrolling: thumb/content lags behind mouse
108 - with enough columns, all interaction is sluggish (mouse, keys,
..), scrolling being just one of them

Both have examples, running those against the suggested fixes (with 108a
for Jose's approach)
125 - fixes its own example, does nothing for the other
108, 108a, 185 - improves its own example, does nothing for other

So we seem to have multiple issues that are (mostly) orthogonal: one
being the broken/missing horizontal virtualization (125), the other
related to dynamic update of table content (108, 108a, 185). We need to
solve both, the solution/s for one looks (mostly?) unrelated to the
solution to the other.

125 is the only one PR for its use-case, and it seems to do its job.
From those targeting the dynamic data update all except Jose's (not yet
formalized into a PR) approach feel too broad: table's reaction to items
modifications is .. suboptimal in more than one aspect. Changing overall
notification implementation to improve that, sounds like covering it up.

-- Jeanette

Zitat von Kevin Rushforth :


Sorry for the badly formatted html. Here it is again.

I see some progress being made on the {Tree}TableView performance
issue. To summarize where I think we are:

There are currently 2 d

RFR: 8252935: Add treeShowing listener only when needed

2020-09-09 Thread John Hendrikx
This is a PoC for performance testing.

It contains commented out code in PopupWindow and ProgressIndicatorSkin and two 
tests are failing because of that.

This code avoids registering two listeners (on Scene and on Window) for each 
and every Node to support the
aforementioned classes.  The complete change should update these two classes to 
add their own listeners instead.

-

Commit messages:
 - WIP: Moved treeShowingProperty into its own class

Changes: https://git.openjdk.java.net/jfx/pull/185/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jfx=185=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8252935
  Stats: 275 lines in 6 files changed: 161 ins; 109 del; 5 mod
  Patch: https://git.openjdk.java.net/jfx/pull/185.diff
  Fetch: git fetch https://git.openjdk.java.net/jfx pull/185/head:pull/185

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


Re: RFR: 8252935: Add treeShowing listener only when needed

2020-09-09 Thread John Hendrikx
On Fri, 17 Apr 2020 23:47:42 GMT, John Hendrikx  wrote:

>> @kevinrushforth I don't propose to remove them, only to move them to where 
>> they are needed.
>> 
>> Currently they are part of Node, which means all nodes, whether they need to 
>> know the state of the Window or not are
>> registering a listener.  However, only PopupWindow and the 
>> ProgressIndicatorSkin are registering a listener on this
>> property (other uses are limited to a simple "isTreeShowing" checks which 
>> can be determined directly).It is very
>> wasteful to have all nodes register this listener, as there are potentially 
>> tens of thousands of nodes.  All of these
>> nodes are listening on the exact same two properties (when there is only one 
>> Scene and Window), causing huge listener
>> lists there.  The listener is not registered in a lazy fashion  (ie, only 
>> registered if something else is listening to
>> the property downstream, like reactfx would do).This also means that 
>> when a Scene change or Window showing change
>> occurs, thousands of nodes will receive a change event, but only 2 types of 
>> nodes are actually interested.  The other
>> nodes are just doing a lot of house keeping to keep watching the correct 
>> property (as there is an indirection from
>> Scene to Window).  Therefore my proposal is to have the two cases involved 
>> register their own listener on Scene and
>> Window.  There will not be any regressions.  The two tests that currently 
>> fail in this PR are expected to fail as I
>> commented out the listener code in the classes involved, but that can easily 
>> be fixed by adding the correct listeners
>> there.  I'll update it so you can see all tests will pass.
>
> @kevinrushforth I've updated this PR now with proper listeners added in again 
> for PopupWindow and
> ProgressIndicatorSkin.  In short, the functionality to support the 
> `treeShowingProperty` has been extracted to a
> separate class `TreeShowingExpression` which is now used by the two classes 
> mentioned.  All tests pass, including the
> memory leak tests that failed before.
> The issue JDK-8090322 you mentioned actually cautioned for not adding such 
> listeners for all nodes and seemed to
> propose the kind of solution I constructed here with a separate class for the 
> `treeShowingProperty`:
>> This is too expensive to calculate for all nodes by default. So the simplest 
>> way to provide it would be a special
>> binding implementation or a util class. Where you create a instance and pass 
>> in the node you are interested in. It can
>> then register listeners all the way up the tree and listen to what it needs.

@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.

-

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


Re: RFR: 8252935: Add treeShowing listener only when needed

2020-09-09 Thread John Hendrikx
On Fri, 17 Apr 2020 16:59:29 GMT, Kevin Rushforth  wrote:

>> This is a PoC for performance testing.
>> 
>> It contains commented out code in PopupWindow and ProgressIndicatorSkin and 
>> two tests are failing because of that.
>> 
>> This code avoids registering two listeners (on Scene and on Window) for each 
>> and every Node to support the
>> aforementioned classes.  The complete change should update these two classes 
>> to add their own listeners instead.
>
> These listeners exist for a good reason. Removing them will almost certainly 
> cause regressions in behavior. See
> [JDK-8090322](https://bugs.openjdk.java.net/browse/JDK-8090322) as well as 
> the follow-on fixes (since that was a rather
> involved fix in the first place).

@kevinrushforth I don't propose to remove them, only to move them to where they 
are needed.

Currently they are part of Node, which means all nodes, whether they need to 
know the state of the Window or not are
registering a listener.  However, only PopupWindow and the 
ProgressIndicatorSkin are registering a listener on this
property (other uses are limited to a simple "isTreeShowing" checks which can 
be determined directly).

It is very wasteful to have all nodes register this listener, as there are 
potentially tens of thousands of nodes.  All
of these nodes are listening on the exact same two properties (when there is 
only one Scene and Window), causing huge
listener lists there.  The listener is not registered in a lazy fashion  (ie, 
only registered if something else is
listening to the property downstream, like reactfx would do).

This also means that when a Scene change or Window showing change occurs, 
thousands of nodes will receive a change
event, but only 2 types of nodes are actually interested.  The other nodes are 
just doing a lot of house keeping to
keep watching the correct property (as there is an indirection from Scene to 
Window).

Therefore my proposal is to have the two cases involved register their own 
listener on Scene and Window.  There will
not be any regressions.  The two tests that currently fail in this PR are 
expected to fail as I commented out the
listener code in the classes involved, but that can easily be fixed by adding 
the correct listeners there.

I'll update it so you can see all tests will pass.

-

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


Re: RFR: 8252935: Add treeShowing listener only when needed

2020-09-09 Thread John Hendrikx
On Fri, 17 Apr 2020 17:18:00 GMT, John Hendrikx  wrote:

>> These listeners exist for a good reason. Removing them will almost certainly 
>> cause regressions in behavior. See
>> [JDK-8090322](https://bugs.openjdk.java.net/browse/JDK-8090322) as well as 
>> the follow-on fixes (since that was a rather
>> involved fix in the first place).
>
> @kevinrushforth I don't propose to remove them, only to move them to where 
> they are needed.
> 
> Currently they are part of Node, which means all nodes, whether they need to 
> know the state of the Window or not are
> registering a listener.  However, only PopupWindow and the 
> ProgressIndicatorSkin are registering a listener on this
> property (other uses are limited to a simple "isTreeShowing" checks which can 
> be determined directly).It is very
> wasteful to have all nodes register this listener, as there are potentially 
> tens of thousands of nodes.  All of these
> nodes are listening on the exact same two properties (when there is only one 
> Scene and Window), causing huge listener
> lists there.  The listener is not registered in a lazy fashion  (ie, only 
> registered if something else is listening to
> the property downstream, like reactfx would do).This also means that when 
> a Scene change or Window showing change
> occurs, thousands of nodes will receive a change event, but only 2 types of 
> nodes are actually interested.  The other
> nodes are just doing a lot of house keeping to keep watching the correct 
> property (as there is an indirection from
> Scene to Window).  Therefore my proposal is to have the two cases involved 
> register their own listener on Scene and
> Window.  There will not be any regressions.  The two tests that currently 
> fail in this PR are expected to fail as I
> commented out the listener code in the classes involved, but that can easily 
> be fixed by adding the correct listeners
> there.  I'll update it so you can see all tests will pass.

@kevinrushforth I've updated this PR now with proper listeners added in again 
for PopupWindow and
ProgressIndicatorSkin.  In short, the functionality to support the 
`treeShowingProperty` has been extracted to a
separate class `TreeShowingExpression` which is now used by the two classes 
mentioned.

All tests pass, including the memory leak tests that failed before.

The issue JDK-8090322 you mentioned actually cautioned for not adding such 
listeners for all nodes and seemed to
propose the kind of solution I constructed here with a separate class for the 
`treeShowingProperty`:

> This is too expensive to calculate for all nodes by default. So the simplest 
> way to provide it would be a special
> binding implementation or a util class. Where you create a instance and pass 
> in the node you are interested in. It can
> then register listeners all the way up the tree and listen to what it needs.

-

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


Re: RFR: 8185886: Improve scrolling performance of TableView and TreeTableView

2020-08-26 Thread John Hendrikx
On Wed, 26 Aug 2020 17:44:20 GMT, yosbits 
 wrote:

>> So far, there are two alternative fixes for the bad performance issue while 
>> scrolling TableView/TreeTableViews:
>> 
>> - this one #108, that tries to improve the performance of excessive number 
>> of `removeListener` calls
>> - the WIP #185 that avoids registering two listeners (on Scene and on 
>> Window) for each and every Node.
>> 
>> For the case presented, where new items are constantly added to the 
>> TableView, the trace of calls that reach
>> `com.sun.javafx.binding.ExpressionHelper.removeListener()` is something like 
>> this:
>> ![TraceOpenJFX16ea1](https://user-images.githubusercontent.com/2043230/91322208-c2ba9900-e7bf-11ea-8918-cdbecd457cf2.png)
>>  
>> As can be seen, all those calls are triggered by the change of the number of 
>> cells in
>> [VirtualFlow::setCellCount](https://github.com/openjdk/jfx/blob/master/modules/javafx.controls/src/main/java/javafx/scene/control/skin/VirtualFlow.java#L804).
>> Whenever the cell count changes there is this
>> [call](https://github.com/openjdk/jfx/blob/master/modules/javafx.controls/src/main/java/javafx/scene/control/skin/VirtualFlow.java#L843):
>> sheetChildren.clear();
>> 
>> This happens every time the number of items in the `TableView` changes, as 
>> the `VirtualFlow` keeps track of the number
>> of virtual cells (`cellCount` is the total number of items) while the number 
>> of actual cells or number of visible nodes
>> used is given by `sheetChildren.size()`.  This means that all the actual 
>> cells (nodes) that are used by the
>> `VirtualFlow` are disposed and recreated all over again every time the 
>> number of items changes, triggering all those
>> calls to unregister those nodes from the scene that ultimately lead to 
>> remove the listeners with
>> `ExpressionHelper.removeListener`.  However, this seems unnecessary, as the 
>> number of actual cells/nodes doesn't change
>> that much, and causes this very bad performance.  On a quick test over the 
>> sample posted, just removing that line gives
>> a noticeable improvement in performance..
>> There is a concern though due to the
>> [comment](https://github.com/openjdk/jfx/blob/master/modules/javafx.controls/src/main/java/javafx/scene/control/skin/VirtualFlow.java#L839):
>> // Fix for RT-13965: Without this line of code, the number of items in
>> // the sheet would constantly grow, leaking memory for the life of the
>> // application. This was especially apparent when the total number of
>> // cells changes - regardless of whether it became bigger or smaller.
>> sheetChildren.clear();
>> 
>> There are some methods in VirtualFlow that already manage the lifecycle of 
>> this list of nodes (clear, remove cells, add
>> cells, ...), so I don't think that is the case anymore for that comment: the 
>> number of items in the sheet doesn't
>> constantly grow and there is no memory leak.  Running the attached sample 
>> for a long time, and profiling with VisualVM,
>> shows improved performance (considerable drop in CPU usage), and no issues 
>> regarding memory usage.
>> So I'd like to propose this third alternative, which would affect only 
>> VirtualFlow and the controls that use it, but
>> wouldn't have any impact in the rest of controls as the other two options 
>> (as `ExpressionHelper` or `Node` listeners
>> wouldn't be modified).  Thoughts and feedback are welcome.
>
> I confirmed the sample code (JavaFX Sluggish),
> This is not scroll performance
> It seems to reproduce the additional performance issue.
> Therefore, it is not considered appropriate as a fix for JDK-8185886.
> I know you are reproducing another performance issue, but
> I'm proposing to fix scrolling performance issues in #125.

The https://github.com/openjdk/jfx/pull/185 is a full fix, not a WIP.  It 
avoids registering the listeners on Scene and
Window and moves the only uses of those listeners to their respective controls, 
PopupWindow  and ProgressIndicatorSkin
(the property involved is internal, so there is no risk of affecting 3rd 
parties).

Please have a closer look.

I updated the title to make it more clear it is no longer a WIP, unless someone 
has review comments.

-

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


Re: Possible approaches to JDK-8185886: Improve scrolling performance of TableView and TreeTableView

2020-08-27 Thread John Hendrikx



I just want to quickly mention that option 3 
(https://github.com/openjdk/jfx/pull/185) is no longer a WIP -- all 
functionality is working all tests pass.


The PR creates a TreeShowingExpression class which encapsulates the tree 
showing logic that was in Node class.  This class is then only used by 
ProgressIndicatorSkin and PopupWindow.  This effectively means that 
instead of registering a listener on Window and Scene for all nodes 
they're only registered for those classes that need to know the showing 
status of a Scene.


--John

On 27/08/2020 02:17, Kevin Rushforth wrote:

Sorry for the badly formatted html. Here it is again.

I see some progress being made on the {Tree}TableView performance issue.
To summarize where I think we are:

There are currently 2 different approaches under review:

1. https://github.com/openjdk/jfx/pull/108 : optimization in javafx.base
to make removing listeners faster
2. https://github.com/openjdk/jfx/pull/125 : optimization in TableView
to reduce the number of add / removes

In addition, the following is a WIP PR that the author thinks could be a
3rd approach:

3. https://github.com/openjdk/jfx/pull/185 : optimization in scene graph
to avoid install treeShowing listeners on Window and Scene for all nodes

Jose has proposed a 4th approach as a comment to PR #108, and as I
understand it, he will propose a PR shortly.

4. Don't clear the list of children in a VirtualFlow when the number of
items changes.

So the first thing that is needed is to evaluate the approaches and
decide which one to pursue.

Options 1 and 3 are more broad in their scope, option #2 is more
targeted (to TableView), but within that area is a larger change. Option
#3 would remove the (internal) treeShowing property as a generally
available concept and only use it for controls like ProgressIndicator
that really need it. Option #4 affects only controls that use
VirtualFlow (ListView, TableVIew, TreeTableView), and seems not to be a
large change (presuming we can verify that no leak is introduced).

I note that these fixes are not mutually exclusive, but I do think we
need to settle on a primary approach and use that to fix this issue. If
there are still performance problems after that fix, we can consider one
(or more) of the others.

Comments?

-- Kevin





Re: Preparing migrations to inline classes

2020-09-28 Thread John Hendrikx

Perhaps some of the classes in javafx.util.  Duration springs to mind.

Others I think could be Bounds/BoundingBox, Rectangle2D.

On 27/09/2020 20:29, Nir Lisker wrote:

Hi,

Project Valhalla is planning to create an annotation to apply to classes
that are planned to be migrated to inline classes [1]. As part of the
migration requirements, classes can't have a public or protected
constructor, and this is the main breaking change. They must also be final,
though I doubt many of these were extended.

We should start compiling a list of classes that are candidates
for migration so we will have enough time to deprecate constructors and
otherwise prepare developers for these changes when inline classes will be
ready.

As an example, Point2D/3D are good migration candidates. They will need to
have their constrictors removed and the classes made final. Other
candidates are:
javafx.css.Size
javafx.geometry.Dimension2D
javafx.geometry.Insets

Internal classes are less of a problem and can be migrated without
preparation.

- Nir

[1] https://openjdk.java.net/jeps/390



Re: RFR: 8252935: Add treeShowing listener only when needed

2021-01-20 Thread John Hendrikx
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


Re: RFR: 8252935: Add treeShowing listener only when needed [v2]

2021-01-24 Thread John Hendrikx
> This is a PoC for performance testing.  
> 
> It contains commented out code in PopupWindow and ProgressIndicatorSkin and 
> two tests are failing because of that.
> 
> This code avoids registering two listeners (on Scene and on Window) for each 
> and every Node to support the aforementioned classes.  The complete change 
> should update these two classes to add their own listeners instead.

John Hendrikx has updated the pull request with a new target base due to a 
merge or a rebase. The pull request now contains one commit:

  WIP: Moved treeShowingProperty into its own class

-

Changes: https://git.openjdk.java.net/jfx/pull/185/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jfx=185=01
  Stats: 275 lines in 6 files changed: 161 ins; 109 del; 5 mod
  Patch: https://git.openjdk.java.net/jfx/pull/185.diff
  Fetch: git fetch https://git.openjdk.java.net/jfx pull/185/head:pull/185

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


Re: RFR: 8252935: Add treeShowing listener only when needed [v2]

2021-01-24 Thread John Hendrikx
On Fri, 22 Jan 2021 21:32:28 GMT, Kevin Rushforth  wrote:

>> John Hendrikx has updated the pull request with a new target base due to a 
>> merge or a rebase. The pull request now contains one commit:
>> 
>>   WIP: Moved treeShowingProperty into its own class
>
> modules/javafx.controls/src/main/java/javafx/scene/control/skin/ProgressIndicatorSkin.java
>  line 239:
> 
>> 237: super.dispose();
>> 238: 
>> 239: treeShowingExpression.dispose();
> 
> This could be removed if you are able to use `registerChangeListener` to add 
> the listener.

I'm not so sure, the listener will be removed from the `TreeShowingExpression` 
that was created, but the `TreeShowingExpression` itself is also registering 
listeners on the `ProgressIndicator`.  I don't see any `dispose` calls being 
done by the helper code.  If I'm correct, switching skins multiple times on a 
`ProgressIndicator` would create multiple `TreeShowingExpression`s which would 
create listeners on the `ProgressIndicator` which in turn reference the 
expression -- this would prevent the discarded skins from being GC'd.

-

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


Re: RFR: 8252935: Add treeShowing listener only when needed [v2]

2021-01-24 Thread John Hendrikx
On Fri, 22 Jan 2021 21:36:53 GMT, Kevin Rushforth  wrote:

>> John Hendrikx has updated the pull request with a new target base due to a 
>> merge or a rebase. The pull request now contains one commit:
>> 
>>   WIP: Moved treeShowingProperty into its own class
>
> I left a few inline comments below.

Sorry about the force push, merging over a year of changes from master did not 
seem right to me.  It was only for getting up to date, I will do the other 
commits normally.

-

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


Re: RFR: 8252935: Add treeShowing listener only when needed [v2]

2021-01-24 Thread John Hendrikx
On Fri, 22 Jan 2021 21:31:42 GMT, Kevin Rushforth  wrote:

>> John Hendrikx has updated the pull request with a new target base due to a 
>> merge or a rebase. The pull request now contains one commit:
>> 
>>   WIP: Moved treeShowingProperty into its own class
>
> modules/javafx.controls/src/main/java/javafx/scene/control/skin/ProgressIndicatorSkin.java
>  line 137:
> 
>> 135: 
>> 136: treeShowingExpression = new TreeShowingExpression(control);
>> 137: treeShowingExpression.addListener((obs, old, current) -> 
>> updateAnimation());
> 
> Is there a reason not to still use `registerChangeListener` for this?

`registerChangeListener` can be used here, I just didn't see it.  However, 
there is no need to unregister the listener from the expression (it is local to 
the skin, it will never have more than one listener and will be GC'd with the 
Skin).  I will change it though, as it looks a bit cleaner, and there is no 
harm unregistering the listener.

-

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


Re: RFR: 8252935: Add treeShowing listener only when needed [v3]

2021-01-24 Thread John Hendrikx
> This is a PoC for performance testing.  
> 
> It contains commented out code in PopupWindow and ProgressIndicatorSkin and 
> two tests are failing because of that.
> 
> This code avoids registering two listeners (on Scene and on Window) for each 
> and every Node to support the aforementioned classes.  The complete change 
> should update these two classes to add their own listeners instead.

John Hendrikx has updated the pull request incrementally with two additional 
commits since the last revision:

 - Add performance test
 - Use registerChangeListener in ProgressIndicatorSkin

-

Changes:
  - all: https://git.openjdk.java.net/jfx/pull/185/files
  - new: https://git.openjdk.java.net/jfx/pull/185/files/5ac5d9e4..e68a886d

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jfx=185=02
 - incr: https://webrevs.openjdk.java.net/?repo=jfx=185=01-02

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

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


Re: RFR: 8252935: Add treeShowing listener only when needed [v3]

2021-01-24 Thread John Hendrikx
On Sun, 24 Jan 2021 09:02:17 GMT, John Hendrikx  wrote:

>> I left a few inline comments below.
>
> Sorry about the force push, merging over a year of changes from master did 
> not seem right to me.  It was only for getting up to date, I will do the 
> other commits normally.

> 1. Merge in the latest changes from the upstream master branch into your 
> local feature branch? Among other things, this will enable running tests via 
> GitHub Actions.

I hadn't seen that in practice yet, it looks really nice.

> 2. Add a test program under `tests/performance` that can be used to verify 
> the performance gain. Even better would be if you can create an automated 
> test under `tests/system`. I was thinking something like what we did for PR 
> #34 where there was a pathological performance bug fixed and the test checked 
> that the operation in question didn't take more than some small number of 
> seconds. That might be overkill for this fix, though.

I put a test under `tests/system` using the example you gave.  It creates a 
Scene with about 16k nodes, and then does 10.000 operations on them.  This runs 
in around 2-2.5 seconds before the fix and in about 100-200 ms after.  I put 
the cut-off point somewhere in the middle (800 ms).

I am planning to address the rest of the comments somewhere in the coming week.

-

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


Re: RFR: 8252935: Add treeShowing listener only when needed [v4]

2021-01-31 Thread John Hendrikx
> This is a PoC for performance testing.  
> 
> It contains commented out code in PopupWindow and ProgressIndicatorSkin and 
> two tests are failing because of that.
> 
> This code avoids registering two listeners (on Scene and on Window) for each 
> and every Node to support the aforementioned classes.  The complete change 
> should update these two classes to add their own listeners instead.

John Hendrikx has updated the pull request incrementally with four additional 
commits since the last revision:

 - Add missing TreeShowingExpressionTest which also tests SubScene
 - Also dispose listeners on Window/Showing in dispose
 - Fix review comments
 - Update copyrights

-

Changes:
  - all: https://git.openjdk.java.net/jfx/pull/185/files
  - new: https://git.openjdk.java.net/jfx/pull/185/files/e68a886d..2b6df779

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jfx=185=03
 - incr: https://webrevs.openjdk.java.net/?repo=jfx=185=02-03

  Stats: 342 lines in 7 files changed: 325 ins; 0 del; 17 mod
  Patch: https://git.openjdk.java.net/jfx/pull/185.diff
  Fetch: git fetch https://git.openjdk.java.net/jfx pull/185/head:pull/185

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


Re: RFR: 8252935: Add treeShowing listener only when needed [v3]

2021-01-31 Thread John Hendrikx
On Thu, 28 Jan 2021 13:37:59 GMT, Kevin Rushforth  wrote:

>> John Hendrikx has updated the pull request incrementally with two additional 
>> commits since the last revision:
>> 
>>  - Add performance test
>>  - Use registerChangeListener in ProgressIndicatorSkin
>
> The updated fix using `registerChangeListener` and the test look good. I left 
> a few mostly-minor comments.

I think this change is complete now, including an integration test and tests on 
all new code.  Please let me know if anything else needs to be done.

> modules/javafx.graphics/src/main/java/com/sun/javafx/scene/TreeShowingExpression.java
>  line 2:
> 
>> 1: /*
>> 2:  * Copyright (c) 2013, 2020, Oracle and/or its affiliates. All rights 
>> reserved.
> 
> change 2020 to 2021

Solved, and also did this in all other files included in this change.

-

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


Re: RFR: 8252935: Add treeShowing listener only when needed [v4]

2021-01-31 Thread John Hendrikx
On Fri, 22 Jan 2021 21:36:06 GMT, Kevin Rushforth  wrote:

>> John Hendrikx has updated the pull request incrementally with four 
>> additional commits since the last revision:
>> 
>>  - Add missing TreeShowingExpressionTest which also tests SubScene
>>  - Also dispose listeners on Window/Showing in dispose
>>  - Fix review comments
>>  - Update copyrights
>
> modules/javafx.graphics/src/main/java/javafx/scene/SubScene.java line 312:
> 
>> 310: _value.setTreeVisible(isTreeVisible());
>> 311: _value.setDisabled(isDisabled());
>> 312: _value.setTreeShowing(isTreeShowing());
> 
> This looks fine. It might be worth adding a unit test to ensure that 
> `isTreeShowing` works correctly for nodes in a `SubScene`.

I've added a test for this in `TreeShowingExpressionTest`.

-

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


Re: [External] : Re: Convenience factories for Border and Background

2021-06-09 Thread John Hendrikx
I'm not entirely convinced taking width is all that useful. The (final) 
pxiel width of a border is normally a calculated value based on the 
chosen unit and scaling needs of the platform.


A lot of methods in JavaFX work with pixel values only, while for proper 
scaling I often find myself needing a different unit, in which case CSS 
is the only option.


Perhaps however this is a different issue, as it would apply to much 
more than just borders.


--John

On 08/06/2021 17:29, Scott Palmer wrote:

+1 for having a variant that takes a width. Colour, width, and radius are the 
main parameters I need, so a variant that takes all three would help:

 Border.stroke(Paint p, double width, double radii)

(Though to be honest for my larger projects most of the time this stuff is set 
in a style sheet.  It’s smaller tools and utilities that I find this 
particularly tedious as the verbosity of the code becomes annoying.)

Scott


On Jun 8, 2021, at 9:59 AM, Kevin Rushforth  wrote:

I think that the convenience methods should just cover the most common cases, 
so I'd rather skip the dotted and dashed variants. It is a good question as to 
whether there ought to be a variant that takes width. I wouldn't do that as the 
only method, though. I'd lean towards not taking the width. Once you start 
getting into more parameters you can just use the constructors without much 
more trouble.

As for the names, I have a slight preference for Border.stroke and 
Background.fill.

-- Kevin


On 6/8/2021 4:25 AM, Nir Lisker wrote:

Are dashed and dotted used frequently? I find that I only use solid unless I'm 
doing something fancy.

On Tue, Jun 8, 2021 at 5:21 AM Michael Strauß mailto:michaelstr...@gmail.com>> wrote:

   What do you think about this variation?

   Border.solid(Paint color, double width) ->
   new Border(new BorderStroke(color, BorderStrokeStyle.SOLID,
   null, new BorderWidths(width)))

   Border.dashed(Paint color, double width)  ->
   new Border(new BorderStroke(color, BorderStrokeStyle.DASHED,
   null, new BorderWidths(width)))

   Border.dotted(Paint color, double width) ->
   new Border(new BorderStroke(color, BorderStrokeStyle.DOTTED,
   null, new BorderWidths(width)))

   Background.fill(Paint color) ->
   new BackgroundFill(color, null, null)

   This gives developers a good deal of customizability before needing to
   fall back to using constructors.


   Am Di., 8. Juni 2021 um 03:21 Uhr schrieb Nir Lisker
   mailto:nlis...@gmail.com>>:
   >
   > The new API:
   >
   > 1. `Border.of(Paint stroke)` or `Border.stroke(Paint stroke)`
   that does
   > `new Border(new BorderStroke(Paint stroke ,
   BorderStrokeStyle.SOLID, null,
   > null));`
   > 2. `Background.of((Paint fill)` or `Background.fill(Paint fill)`
   that does
   > `new Background(new BackgroundFill(Paint fill, null, null));`
   >
   > I don't mind either name choice.
   >







Re: RFR: 8267551: Support loading images from inline data-URIs

2021-06-05 Thread John Hendrikx

I have a question about this.

Why would you make this part of JavaFX directly instead of offering it 
as a dependency that people can include in their project?


As far as I know, it is possible to register custom URL handlers in 
Java, which should be used automatically by JavaFX assuming it 
constructs its URL's in the standard way when extracting them from the CSS.


This can be done since Java 9+ via URLStreamHandlerProvider. You need to 
store the name of your provider in `META-INF/services`.


Any reason why this route isn't being taken?

--John

On 21/05/2021 19:58, Michael Strauß wrote:

This PR adds support for loading images from [inline data 
URIs](https://en.wikipedia.org/wiki/Data_URI_scheme), which is also widely 
supported by web browsers. This enables developers to package small images in 
CSS files, rather than separately deploying the images alongside the CSS file.

-

Commit messages:
 - Added data-URI loading support for images

Changes: https://git.openjdk.java.net/jfx/pull/508/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jfx=508=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8267551
  Stats: 437 lines in 8 files changed: 414 ins; 9 del; 14 mod
  Patch: https://git.openjdk.java.net/jfx/pull/508.diff
  Fetch: git fetch https://git.openjdk.java.net/jfx pull/508/head:pull/508

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



Re: RFR: 8267551: Support loading images from inline data-URIs [v13]

2021-06-05 Thread John Hendrikx
On Sat, 5 Jun 2021 16:10:59 GMT, Kevin Rushforth  wrote:

>> In this specific case, image loading has failed for some reason. The call to 
>> `DataURI.tryParse` is only there to potentially call `DataURI.toString()` 
>> for a truncated log output, instead of logging the entire `url` String. If 
>> data-URI truncation is not wanted, this code can be reverted.
>
> Maybe add a brief comment to that effect?

Ok clear thanks

-

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


  1   2   3   >