HTMLEditor don't create new line when Enter pressed.
Hi, Did someone test HTMLEditor with OpenJFX 9 and 10 ? - HTMLEditor doesn't create a new line when one presses Enter. - It works with JavaFX 8u171 and 11-internal+0-2018-07-02-110935. - but not with JavaFX 9.0.4 and JavaFX 10.0.1. Best regards.
REMINDER: Milestone dates for OpenJFX 11 release
As a reminder, the RDP1 date for openjfx11 is July 9, at 23:59 Pacific time. This is the deadline for features, although we will have a process for getting in some smaller enhancements after RDP1 with approval (similar to what the JDK does). -- Kevin On 6/12/2018 3:18 PM, Kevin Rushforth wrote: We are getting close to the start of rampdown for JDK 11 [1]. Now that FX is no longer bundled we don't need to follow exactly the same schedule of milestones, although we do want to release this first unbundled FX 11 release at the same time JDK 11 ships (or slightly sooner). Here is the proposed rampdown schedule for FX 11. The main differences between the JDK schedule and the FX schedule is that we don't need as long a back end as a full JDK feature release 2018-07-09 : RDP1 (aka feature freeze) 2018-08-06 : RDP2 2018-08-27 : freeze for GAC build 2018-09-18 : GA I am proposing a shorter time after RDP2 than the JDK because I don't think we need the long back end of a full JDK feature release (not as many moving parts or stakeholders). I will send out the proposed ramp-down policies, but as a starting point I imagine we want something similar to what the JDK uses with a couple modifications: 1. Since this is our first unbundled release and there are likely to be plenty of bugs that need fixing during RDP1, so I propose to postpone forking the repo until RDP1. This means a 4 week downtime where there is no place to push new features / enhancements unless they are critical to openjfx 11 (in which case they will be an exception). I think this is a reasonable trade-off for this release 2. I don't plan to propose any restrictions on P4 bugs before RDP2. The focus should be on more serious bugs, and we likely won't fix many P4s, but if a safe P4 bug fix is proposed, reviewed, and tested, then I see no reason not to take it between RDP1 and RDP2. -- Kevin [1] http://openjdk.java.net/projects/jdk/11/
[11] Review request: 8090763: FX Robot API
Most of you should be aware of this already: The review for the public FX Robot API, contributed by Michael Ennen (and sponsored by me), has been proceeding on GitHub: https://bugs.openjdk.java.net/browse/JDK-8090763 https://github.com/javafxports/openjdk-jfx/pull/36 My review is now done, so as soon as we get one more review, I'll merge the PR on GitHub. This will also serve as the request to integrate this into the openjfx/jfx-dev master repo. -- Kevin
Re: [11] JDK-8204621: Upgrade MarlinFX to 0.9.2
Looks good.
+1 -- note that needs a second reviewer (doesn't need to be a capital-R
Reviewer).
-- Kevin
On 7/3/2018 8:56 AM, Kevin Rushforth wrote:
PS: I am not really satisfied by adding such noise in build.gradle,
but it can be improved later ...
Agreed. This can be a follow-on issue. I'll finish my review shortly.
-- Kevin
On 7/3/2018 8:45 AM, Laurent Bourgès wrote:
Kevin,
> I added the system property "ClipShapeTest.numTests" but it
requires a
build.gradle change to pass the parameter:
Yes, something like this is what I had in mind. As long as we
don't add too many of these, it is OK with me. Note that as coded,
the build will fail if you don't define ClipShapeTest.numTests, so
you will need to check for that. I note also that you used tabs in
build.gradle (so please change them to spaces). I recommend the
following logic:
if (rootProject.hasProperty("ClipShapeTest.numTests")) {
systemProperty "ClipShapeTest.numTests",
rootProject.getProperty("ClipShapeTest.numTests")
}
I adopted your proposal and updated the webrev:
http://cr.openjdk.java.net/~lbourges/marlinFX/marlinFX-092.2/
Re: [11] JDK-8204621: Upgrade MarlinFX to 0.9.2
PS: I am not really satisfied by adding such noise in build.gradle,
but it can be improved later ...
Agreed. This can be a follow-on issue. I'll finish my review shortly.
-- Kevin
On 7/3/2018 8:45 AM, Laurent Bourgès wrote:
Kevin,
> I added the system property "ClipShapeTest.numTests" but it requires a
build.gradle change to pass the parameter:
Yes, something like this is what I had in mind. As long as we
don't add too many of these, it is OK with me. Note that as coded,
the build will fail if you don't define ClipShapeTest.numTests, so
you will need to check for that. I note also that you used tabs in
build.gradle (so please change them to spaces). I recommend the
following logic:
if (rootProject.hasProperty("ClipShapeTest.numTests")) {
systemProperty "ClipShapeTest.numTests",
rootProject.getProperty("ClipShapeTest.numTests")
}
I adopted your proposal and updated the webrev:
http://cr.openjdk.java.net/~lbourges/marlinFX/marlinFX-092.2/
Re: [11] JDK-8204621: Upgrade MarlinFX to 0.9.2
>
On 7/3/2018 8:45 AM, Laurent Bourgès wrote:
Kevin,
> I added the system property "ClipShapeTest.numTests" but it requires a
build.gradle change to pass the parameter:
Yes, something like this is what I had in mind. As long as we
don't add too many of these, it is OK with me. Note that as coded,
the build will fail if you don't define ClipShapeTest.numTests, so
you will need to check for that. I note also that you used tabs in
build.gradle (so please change them to spaces). I recommend the
following logic:
if (rootProject.hasProperty("ClipShapeTest.numTests")) {
systemProperty "ClipShapeTest.numTests",
rootProject.getProperty("ClipShapeTest.numTests")
}
I adopted your proposal and updated the webrev:
http://cr.openjdk.java.net/~lbourges/marlinFX/marlinFX-092.2/
Re: [11] JDK-8204621: Upgrade MarlinFX to 0.9.2
Kevin,
> I added the system property "ClipShapeTest.numTests" but it requires a
> build.gradle change to pass the parameter:
>
> Yes, something like this is what I had in mind. As long as we don't add
> too many of these, it is OK with me. Note that as coded, the build will
> fail if you don't define ClipShapeTest.numTests, so you will need to check
> for that. I note also that you used tabs in build.gradle (so please change
> them to spaces). I recommend the following logic:
>
> if (rootProject.hasProperty("ClipShapeTest.numTests")) {
> systemProperty "ClipShapeTest.numTests",
> rootProject.getProperty("ClipShapeTest.numTests")
> }
>
I adopted your proposal and updated the webrev:
http://cr.openjdk.java.net/~lbourges/marlinFX/marlinFX-092.2/
PS: I am not really satisfied by adding such noise in build.gradle, but it
can be improved later ...
Laurent
Re: [11] JDK-8204621: Upgrade MarlinFX to 0.9.2
Hi Laurent,
> I added the system property "ClipShapeTest.numTests" but it requires
a build.gradle change to pass the parameter:
Yes, something like this is what I had in mind. As long as we don't add
too many of these, it is OK with me. Note that as coded, the build will
fail if you don't define ClipShapeTest.numTests, so you will need to
check for that. I note also that you used tabs in build.gradle (so
please change them to spaces). I recommend the following logic:
if (rootProject.hasProperty("ClipShapeTest.numTests")) {
systemProperty "ClipShapeTest.numTests",
rootProject.getProperty("ClipShapeTest.numTests")
}
-- Kevin
On 7/3/2018 2:13 AM, Laurent Bourgès wrote:
Kevin,
Thanks for the review !
2018-06-29 22:52 GMT+02:00 Kevin Rushforth >:
I'm giving a +1 on the implementation changes. I scanned the
webrev and didn't see anything out of place. I compared the diffs
of the FX Marlin 0.9.2 with the Java2D 0.9.1 changeset, and there
were a few more diffs than I might have expoected, but nothing
jumped out of me as a problem. Also, I've tested it pretty well on
all three platforms.
The overall +1 is pending the fixes needed for the test: at least
the copyright header and shortening or disabling the test.
Here is the updated webrev fixing the test:
http://cr.openjdk.java.net/~lbourges/marlinFX/marlinFX-092.1/
Re: [11] JDK-8204621: Upgrade MarlinFX to 0.9.2
Kevin,
Thanks for the review !
2018-06-29 22:52 GMT+02:00 Kevin Rushforth :
> I'm giving a +1 on the implementation changes. I scanned the webrev and
> didn't see anything out of place. I compared the diffs of the FX Marlin
> 0.9.2 with the Java2D 0.9.1 changeset, and there were a few more diffs than
> I might have expoected, but nothing jumped out of me as a problem. Also,
> I've tested it pretty well on all three platforms.
>
> The overall +1 is pending the fixes needed for the test: at least the
> copyright header and shortening or disabling the test.
>
Here is the updated webrev fixing the test:
http://cr.openjdk.java.net/~lbourges/marlinFX/marlinFX-092.1/
ClipShapeTest incremental diff:
--- /tmp/meld-tmpyWO0FS
+++
/home/bourgesl/libs/marlin/branches/marlin-fx-openjdk/src/test/java/test/manual/marlin/ClipShapeTest.java
@@ -4,7 +4,9 @@
*
* This code is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License version 2 only, as
- * published by the Free Software Foundation.
+ * published by the Free Software Foundation. Oracle designates this
+ * particular file as subject to the "Classpath" exception as provided
+ * by Oracle in the LICENSE file that accompanied this code.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
@@ -219,12 +221,10 @@
// cubic min/max error:
System.setProperty("prism.marlin.cubic_dec_d2", "1e-3");
-System.setProperty("prism.marlin.cubic_inc_d1", "1e-4"); // or
disabled ~ 1e-6
+System.setProperty("prism.marlin.cubic_inc_d1", "1e-4");
// quad max error:
System.setProperty("prism.marlin.quad_dec_d2", "5e-4");
-
-System.setProperty("javafx.animation.fullspeed", "true"); // full
speed
}
// Application class. An instance is created and initialized before
running
@@ -273,7 +273,7 @@
}
private static void resetOptions() {
-NUM_TESTS = 5000;
+NUM_TESTS = Integer.getInteger("ClipShapeTest.numTests", 100);
// shape settings:
SHAPE_MODE = ShapeMode.NINE_LINE_POLYS;
Changes:
- fixed license (Classpath exception)
- removed "javafx.animation.fullspeed" in the test setup
- use 100 tests by default to shorten the test duration (but I kept the
high timeout values if the following parameter is increased):
I added the system property "ClipShapeTest.numTests" but it requires a
build.gradle change to pass the parameter:
+// Marlin ClipShapeTest+
systemProperty "ClipShapeTest.numTests",
project.getProperty("ClipShapeTest.numTests")+
If it is not recommended to add such specific parameters into the
build.gradle file, what do you recommend ? (manual edit ?)
Best regards,
Laurent
>
> On 6/29/2018 11:25 AM, Kevin Rushforth wrote:
>
>> One more thing about the test. All of the OpenJFX unit tests should have
>> GPL v2 + Classpath Exception (this differs from the JDK).
>>
>> -- Kevin
>>
>>
>> On 6/29/2018 10:23 AM, Kevin Rushforth wrote:
>>
>>>
>>> I'll plan to review the code today if possible. This will need one more
>>> reviewer, so maybe Phil can also review it, since he reviewed the Java2D
>>> patch?
>>>
>>> As for my comments on the test:
>>>
>>> Finally I think this test should be manually run only if Marlin renderer
is modified.
How to do that ? use @Ignore or specific tags ?
>>>
>>> As a slight variation of this: How about running a small number (say,
>>> 200 or 250) by default, but adding a flag to run more? Alternatively, you
>>> could use a flag to enable it, but since you would need to do something
>>> extra (provide a flag or modify the test) to run it, we might as well get
>>> at least some testing all the time. Unless you really think there is no
>>> value in doing this.
>>>
>>> I deliberately set all these Marlin clip (runtime + always subdivider) /
curve quality settings (quads / cubics thresholds) to be sure of the
concrete Marlin setup as quality thresholds are sensitive to such values.
>>>
>>> As a best practice, tests should generally be run using the same
>>> settings as are used in production. Other than to verify how it behaves
>>> when you change these settings, I don't see the value in testing the system
>>> running in a mode that no application will ever see. I may be missing some
>>> point here.
>>>
>>> -- Kevin
>>>
>>>
>>> On 6/25/2018 9:01 AM, Laurent Bourgès wrote:
>>>
Kevin,
Here are my comments below:
2018-06-16 1:47 GMT+02:00 Kevin Rushforth >>> >:
I tested this on all three platforms and the updated rasterizer
looks good.
I spot checked the code changes, but didn't get time to do a
complete review yet. I was mostly looking for diffs between the
Java2D version which was already reviewed, and this one.
I do have
[11] Review request: 8201231 : java.lang.NullPointerException at WindowStage.setPlatformEnabled
Hi, Please review this fix : Webrev : http://cr.openjdk.java.net/~dkumar/primanga/8201231/webrev.00/ JBS : https://bugs.openjdk.java.net/browse/JDK-8201231 Thanks, Priyanka
[11] JDK-8191885 : [MacOS] JavaFX main window not resizable coming back from full screen mode in MacOS
Hi Kevin, Murali & Ajit, Please review this fix, Webrev: http://cr.openjdk.java.net/~pbansal/8191885/webrev.00/ JBS: https://bugs.openjdk.java.net/browse/JDK-8191885 Regards, Pankaj
