Re: JavaFX graphics performance and suitability for advanced animations
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?
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?
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
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
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
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
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
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
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)
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
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
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
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
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
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
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
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
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
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
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?)
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
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
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?
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?
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?
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?
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?
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?
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
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
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
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
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
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
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
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
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
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]
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
(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
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
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
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
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
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
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
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
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
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
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
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
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
> 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
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
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
> 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
> 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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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]
> 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]
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]
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]
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]
> 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]
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]
> 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]
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]
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
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
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]
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