Re: [Rev 07] RFR: WIP: 8236651: Simplify and update glass gtk backend
> This proposed change does the following: > > - Ports DND target to use GTK reducing code size and adding extra text / > image formats (such as .gif); > - Use gtk signals instead of gdk events (also to reduce code size); > - Simplifies geometry (sizing/positioning) with a more straightforward code > (less special cases) ; > - Replaces (pointer and focus) grabbing with a gtk approach; > - Reworked frame extents (the wm extension to get decoration sizes) to reduce > size and complexity; > - Simplified cursor changing; > - Reduced the use of gtk/gdk deprecated functions; > > > In general it reduces code size and complexity and hands more work to gtk. > > Important notice: As I could not test the code for handling web start and web > applets because browsers do not support it anymore and java has removed > "javaws", I took the liberty to remove the code. Will restore if necessary. > >  The pull request has been updated with 1 additional commit. - Added commits: - 209b7ac7: Better alternative calculation for no _NET_FRAME_EXTENTS WM extension Changes: - all: https://git.openjdk.java.net/jfx/pull/77/files - new: https://git.openjdk.java.net/jfx/pull/77/files/0dc70ab3..209b7ac7 Webrevs: - full: https://webrevs.openjdk.java.net/jfx/77/webrev.07 - incr: https://webrevs.openjdk.java.net/jfx/77/webrev.06-07 Stats: 70 lines in 2 files changed: 54 ins; 3 del; 13 mod Patch: https://git.openjdk.java.net/jfx/pull/77.diff Fetch: git fetch https://git.openjdk.java.net/jfx pull/77/head:pull/77 PR: https://git.openjdk.java.net/jfx/pull/77
Re: RFR: 8236753: Animations do not play backwards after being stopped
On Fri, 10 Jan 2020 00:37:58 GMT, Kevin Rushforth wrote: >> The private field `lastPlayFinished` is responsible for 2 cases where an >> animation in `STOPPED` status does not play after `play()` is called if the >> rate is negative: >> >> 1. When the animation is created, it is `STOPPED` and `lastPlayFinished` is >> `false`. Setting a negative rate and calling `play()` will not jump to the >> end of the animation (in order to play it backwards) because the `if >> (lastPlayedFinished)` check is `false`. Creating the animation with >> `lastPlayFinished = true` fixes this. However, >> `SequentialTransitionPlayTest#testCycleReverse`'s initial state test implies >> that the original behavior is correct. *That test currently fails with this >> change.* Either the fix is reverted or the test is corrected. >> 2. When the animation is stopped (if it was not `STOPPED` already), >> `jumpTo(Duration.ZERO)` sets `lastPlayFinished` to `false`, which causes the >> same issue above with `play()`. Setting `lastPlayFinished = true` at the end >> `stop()` fixes this issue. >> >> A test was added for case 2 to check that the playing head indeed jumps to >> the end of the animation. Without this fix, it stays at the start. >> >> I'm still somewhat confused as to what constitutes a "last play finished". >> Any `jumpTo` resets `lastPlayFinished` to `false`, even if the jump is to >> the start/end of the animation. In this case, stopping an animation, jumping >> to its start/end, setting the rate to negative/positive, and playing, will >> do nothing as the end condition is reached immediately. This is what the >> behavior that was fixed for cases 1 and 2, but maybe this is also incorrect >> behavior for jumping to start/end. >> >> A test app is included in the "parent" >> [bug](https://bugs.openjdk.java.net/browse/JDK-8210238), which also mentions >> a bug relating to **pausing** and playing backwards, so be mindful of it >> when testing. > > I'll review this next week. This seems a fine candidate for openjfx14, so it > (along with a couple other pending reviews that can be for 14) will be a good > test of targeting a PR to the stabilization branch. > > I also request @arapte to review. I should add that it will depend on whether there are any regressions. One thing we do need to be careful of is introducing regressions during rampdown. - PR: https://git.openjdk.java.net/jfx/pull/82
Re: RFR: 8236753: Animations do not play backwards after being stopped
On Wed, 8 Jan 2020 23:52:07 GMT, Nir Lisker wrote: > The private field `lastPlayFinished` is responsible for 2 cases where an > animation in `STOPPED` status does not play after `play()` is called if the > rate is negative: > > 1. When the animation is created, it is `STOPPED` and `lastPlayFinished` is > `false`. Setting a negative rate and calling `play()` will not jump to the > end of the animation (in order to play it backwards) because the `if > (lastPlayedFinished)` check is `false`. Creating the animation with > `lastPlayFinished = true` fixes this. However, > `SequentialTransitionPlayTest#testCycleReverse`'s initial state test implies > that the original behavior is correct. *That test currently fails with this > change.* Either the fix is reverted or the test is corrected. > 2. When the animation is stopped (if it was not `STOPPED` already), > `jumpTo(Duration.ZERO)` sets `lastPlayFinished` to `false`, which causes the > same issue above with `play()`. Setting `lastPlayFinished = true` at the end > `stop()` fixes this issue. > > A test was added for case 2 to check that the playing head indeed jumps to > the end of the animation. Without this fix, it stays at the start. > > I'm still somewhat confused as to what constitutes a "last play finished". > Any `jumpTo` resets `lastPlayFinished` to `false`, even if the jump is to the > start/end of the animation. In this case, stopping an animation, jumping to > its start/end, setting the rate to negative/positive, and playing, will do > nothing as the end condition is reached immediately. This is what the > behavior that was fixed for cases 1 and 2, but maybe this is also incorrect > behavior for jumping to start/end. > > A test app is included in the "parent" > [bug](https://bugs.openjdk.java.net/browse/JDK-8210238), which also mentions > a bug relating to **pausing** and playing backwards, so be mindful of it when > testing. I'll review this next week. This seems a fine candidate for openjfx14, so it (along with a couple other pending reviews that can be for 14) will be a good test of targeting a PR to the stabilization branch. I also request @arapte to review. - PR: https://git.openjdk.java.net/jfx/pull/82
Re: [Rev 03] RFR: 8217472: Add attenuation for PointLight
On Thu, 9 Jan 2020 18:00:19 GMT, Nir Lisker wrote: >> The error was for the cases of 2 and 3 lights (I was testing 1) and should >> be fixed now. My fault with copy-paste... that's why we use loops, but I >> guess this is some optimization for the es2 pipeline. I wonder if it's >> really worth it over a single shader looping over the number of lights like >> d3d does. > >> This will need a fair bit of testing to ensure that there are no regressions >> either in functionality or (especially) performance, in addition to tests >> for the new functionality. > > Which tests for the new functionality should I write? Up to the shader itself > it's mostly just passing on variables, and the API is standard > `DoubleProperty`s. I can test the dirty bits / redraw logic. > >> On the performance aspect, the inner loop of the lighting calculation now >> has an additional if test for the max range and additional arithmetic >> calculations for the attenuation. What we will need is a test program that >> we can run on Mac and Windows to measure the performance of rendering in a >> fill-rate-limited case. Ideally, we would not pay much of a performance hit >> in the default case where `ca == 1, la == 0, qa == 0`, but we first need to >> be able to measure the drop in performance before we can say whether it is >> acceptable. > > Can you point me to a similar performance test? I didn't see a way to easily > measure FPS. > I don't know how to avoid the `if` test for the `maxRange`, I'm open to > suggestions. The only thing I can think of is if `maxRange` is infinite (the > default), we will swap the shader for one that doesn't make that check. > However, swapping shaders also costs performance. We have a few performance tests in apps/performance, but I don't know how up to date they are. They might give you a starting point on how to measure FPS, but really the harder part is going to be coming up with a workload -- a scene with a number of Shape3Ds with large triangles (so that they are fill-rate limited) and 1-3 lights, etc -- that you can use to measure rendering performance without measuring overhead. Typically you want a scene that is rendering continuously in the < 30 fps range, and more like 10 fps to minimize the overhead even more. Before we figure out whether we need to double the number of shaders (which was one of the ideas that I had as well), we need to know how much of a problem it is. Is it < 2% performance drop on typical cases on a variety of machines or it is a 25% drop (or more likely somewhere in between). If the perf drop is negligible, then it isn't worth doubling the shaders. If it is significant, then we probably need to. If we do need to double the shaders, I wouldn't do it based on the maxRange being infinite, rather I would do it based on whether attenuation is being used. That way the existing shaders would be unchanged, while the new shader would deal with both attenuation and the maxRange test. Hopefully, there won't be enough of a perf hit to require doubling the shaders, but we need to be able to measure it. For functional testing, in addition to the simple API tests, we want to make sure that the basic rendering is working with and without attenuation. Some sort of visual test where you verify that attenuation is / isn't happening as well as testing the cutoff. I wouldn't get too fancy with these...just a sanity test. - PR: https://git.openjdk.java.net/jfx/pull/43
Re: [Rev 03] RFR: 8217472: Add attenuation for PointLight
On Sat, 4 Jan 2020 18:22:29 GMT, Nir Lisker wrote:
>> Right. This needs to talk about pixels. Perhaps there is a way to make it
>> more clear that we are talking about pixels that are part of a rendered
>> Shape3D, but I don't have a good suggestion right now.
>
> Maybe
>
> A light source that radiates light equally in all directions away from
> itself. The location of the light
> source is a single point in space. The light affects {@code Shape3D}s in its
> {@code scope}. Any pixels in
> the light's {@code range} that belong to a {@code Shape3D} will be
> illuminated by it according to the
> computation specified in {@link PhongMaterial}.
> The docs of `PhongMaterial` will need need to be updated too.
Yes, I think that change to the docs looks good.
-
PR: https://git.openjdk.java.net/jfx/pull/43
Re: RFR: 8235772: Remove use of deprecated finalize method from PiscesRenderer and AbstractSurface
On Thu, 9 Jan 2020 16:20:19 GMT, Ambarish Rapte wrote: >> I also looked at the code and don't see any mismatches (including those >> called from the `ACQUIRE_SURFACE` macro). I suppose you could restore the >> call, but since we are in a disposer thread throwing an exception doesn't >> seem like the right thing to do. You could log the error, but if we are sure >> there can be no pending errors, it might not be worth the effort. > > Yes Kevin, After looking at the code it can be concluded that there are no > mismatches. So It seems good to remove those checks. But just in case of > safety can keep those as logs too. I don't have a strong preference. Either removing the checks or leaving them in (with a log message rather than an exception) is fine with me. - PR: https://git.openjdk.java.net/jfx/pull/66
Re: [Rev 03] RFR: 8217472: Add attenuation for PointLight
On Fri, 3 Jan 2020 23:02:56 GMT, Nir Lisker wrote: >>> I still need to test your sample app on Mac. >> >> I get the error with your sample app. It fails on Mac or Linux (Ubuntu >> 16.04) with the same error as I reported above. > > The error was for the cases of 2 and 3 lights (I was testing 1) and should be > fixed now. My fault with copy-paste... that's why we use loops, but I guess > this is some optimization for the es2 pipeline. I wonder if it's really worth > it over a single shader looping over the number of lights like d3d does. > This will need a fair bit of testing to ensure that there are no regressions > either in functionality or (especially) performance, in addition to tests for > the new functionality. Which tests for the new functionality should I write? Up to the shader itself it's mostly just passing on variables, and the API is standard `DoubleProperty`s. I can test the dirty bits / redraw logic. > On the performance aspect, the inner loop of the lighting calculation now has > an additional if test for the max range and additional arithmetic > calculations for the attenuation. What we will need is a test program that we > can run on Mac and Windows to measure the performance of rendering in a > fill-rate-limited case. Ideally, we would not pay much of a performance hit > in the default case where `ca == 1, la == 0, qa == 0`, but we first need to > be able to measure the drop in performance before we can say whether it is > acceptable. Can you point me to a similar performance test? I didn't see a way to easily measure FPS. I don't know how to avoid the `if` test for the `maxRange`, I'm open to suggestions. The only thing I can think of is if `maxRange` is infinite (the default), we will swap the shader for one that doesn't make that check. However, swapping shaders also costs performance. - PR: https://git.openjdk.java.net/jfx/pull/43
Re: RFR: 8235772: Remove use of deprecated finalize method from PiscesRenderer and AbstractSurface
On Fri, 3 Jan 2020 23:50:29 GMT, Kevin Rushforth wrote: >> Hi Johan, I did miss to verify this angle. >> But have checked the code now and can confirm that, in a given function for >> all possible calls to `setMemErrorFlag()` there exists a call to >> `readAndClearMemErrorFlag()` in the same function. So it looks safe to >> remove the memory checks from `dispose()` > > I also looked at the code and don't see any mismatches (including those > called from the `ACQUIRE_SURFACE` macro). I suppose you could restore the > call, but since we are in a disposer thread throwing an exception doesn't > seem like the right thing to do. You could log the error, but if we are sure > there can be no pending errors, it might not be worth the effort. Yes Kevin, After looking at the code it can be concluded that there are no mismatches. So It seems good to remove those checks. But just in case of safety can keep those as logs too. - PR: https://git.openjdk.java.net/jfx/pull/66
Re: RFR: 8236733: Change JavaFX release version to 15
On Thu, 9 Jan 2020 02:39:09 GMT, Kevin Rushforth wrote: > JDK-8236733](https://bugs.openjdk.java.net/browse/JDK-8236733). > > Bump the release version to 15. As noted in the JBS issue, I will wait until > right after the `jfx14` fork to integrate this. Looks good to me. - Marked as reviewed by arapte (Reviewer). PR: https://git.openjdk.java.net/jfx/pull/83
DnD Move does not work on OS-X
Hi,
I must be doing something completely wrong or there is a major bug in
JavaFX-DnD from Java-8 upwards to JavaFX-14-ea.
If you run the attached application on OS-X the and press the CMD-key
while dragging you get null for the getTransferMode() inside DragOver
and you don't get a DragDropped and null on the DragDone.
I tracked that back to View#notifyDragOver() where one gets 0 from the
native side. So am I missing something in my code?
I also tried with
https://docs.oracle.com/javafx/2/drag_drop/jfxpub-drag_drop.htm and it
does not work either.
So I guess it's a major bug in openjfx but before filing a jira-ticket I
wanted to make extra sure it's not a stupid mistake, my german operating
system, ...
Tom
--
Tom Schindl, CTO
BestSolution.at EDV Systemhaus GmbH
Salurnerstrasse 15. A-6020 Innsbruck
Reg. Nr. FN 222302s am Firmenbuchgericht Innsbruck
package bla;
import java.util.UUID;
import javafx.application.Application;
import javafx.scene.Scene;
import javafx.scene.input.ClipboardContent;
import javafx.scene.input.Dragboard;
import javafx.scene.input.TransferMode;
import javafx.scene.layout.HBox;
import javafx.scene.layout.VBox;
import javafx.scene.paint.Color;
import javafx.scene.shape.Rectangle;
import javafx.stage.Stage;
public class SampleApp extends Application {
@Override
public void start(Stage primaryStage) throws Exception {
HBox b = new HBox(50);
Rectangle sourceR = new Rectangle(300, 300);
sourceR.setOnDragDetected( evt -> {
Dragboard dragboard =
sourceR.startDragAndDrop(TransferMode.ANY);
ClipboardContent content = new ClipboardContent();
content.putString(UUID.randomUUID().toString());
dragboard.setContent(content);
evt.consume();
});
sourceR.setOnDragDone( evt -> System.err.println("Done: " +
evt.getTransferMode()));
sourceR.setFill(Color.RED);
Rectangle targetR = new Rectangle(300, 300);
targetR.setOnDragOver( evt -> {
evt.acceptTransferModes(TransferMode.ANY);
System.err.println(evt.getTransferMode());
evt.consume();
});
targetR.setOnDragDropped( evt -> {
System.err.println("Completed: " +
evt.getTransferMode());
evt.setDropCompleted(true);
evt.consume();
});
targetR.setFill(Color.BLUE);
b.getChildren().setAll(sourceR, targetR);
Scene s = new Scene(new VBox(b), 800, 600);
primaryStage.setScene(s);
primaryStage.show();
}
public static void main(String[] args) {
launch(args);
}
}
RE: [EXTERNAL] Memory leak in JavaFX 8 when changing skins
Possible workarounds would be to use Control.setSkin instead of -fx-skin, or to bind the skin property after it has been set to something other than the default. CSS should not mess with a bound property. > -Original Message- > From: openjfx-dev On Behalf Of > Adam Granger > Sent: Thursday, January 9, 2020 3:04 AM > To: openjfx-dev@openjdk.java.net > Subject: [EXTERNAL] Memory leak in JavaFX 8 when changing skins > > > Greetings, > > I realise this is now legacy for most people but we still widely use JavaFX 8. > > I appear to have discovered a memory leak when skin is changed > > The constructor > com.sun.javafx.scene.control.skin.BehaviorSkinBase.BehaviorSkinBase > adds an event listener > > control.addEventHandler(ContextMenuEvent.CONTEXT_MENU_REQUESTE > D, > contextMenuHandler); > > However this is never removed in the dispose() which prevents garbage > collection of the previous skin. > > This problem is amplified by fact JavaFX appears to reload skins unnecessarily > in a complex use case involving JFXPanel - I've as-yet been unable to > produce a SSCCE for this > > What I've discovered so far > - CSS is reprocessed > > - javafx.scene.CssStyleHelper.canReuseStyleHelper(Node, StyleMap) > returns false > - node.styleHelper.resetToInitialValues(node); is called which then causes > the "stock" skin load > - custom skin (-fx-skin in our application CSS) is then loaded > > - stock skin cannot be GC'd and neither can our custom skin > - appears to affect any subclass of BehaviorSkinBase > > Please could you confirm my analysis of this problem and suggest any > workarounds? > > Regards, > > Adam
Memory leak in JavaFX 8 when changing skins
Greetings, I realise this is now legacy for most people but we still widely use JavaFX 8. I appear to have discovered a memory leak when skin is changed The constructor com.sun.javafx.scene.control.skin.BehaviorSkinBase.BehaviorSkinBase adds an event listener control.addEventHandler(ContextMenuEvent.CONTEXT_MENU_REQUESTED, contextMenuHandler); However this is never removed in the dispose() which prevents garbage collection of the previous skin. This problem is amplified by fact JavaFX appears to reload skins unnecessarily in a complex use case involving JFXPanel - I've as-yet been unable to produce a SSCCE for this What I've discovered so far - CSS is reprocessed - javafx.scene.CssStyleHelper.canReuseStyleHelper(Node, StyleMap) returns false - node.styleHelper.resetToInitialValues(node); is called which then causes the "stock" skin load - custom skin (-fx-skin in our application CSS) is then loaded - stock skin cannot be GC'd and neither can our custom skin - appears to affect any subclass of BehaviorSkinBase Please could you confirm my analysis of this problem and suggest any workarounds? Regards, Adam
