Re: RFR: 8272870: Add convenience factory methods for border and background [v3]
On Thu, 9 Sep 2021 23:43:29 GMT, Nir Lisker wrote: >> Added convenience factory factory methods for Background and Border. > > Nir Lisker has updated the pull request incrementally with two additional > commits since the last revision: > > - Removed whitespaces > - Added tests and doc updates Added a couple of simple tests. My approach to the documentation here is that these are the simplest ways to create a border (background) where you provide only 1 color/gradient and not care about the rest, and what you are going to get is rather obvious. If you want to see what the implementation does and what all the defaults are, the `@implSpec` section has a link that you can follow. I prefer not to start giving these details since they end up taking more attention than those few important details of what the method does. When I tried to expand upon the behavior of the method it quickly became "noise", with details I didn't care for. If people insist I will add them, but think about: is what you get not what you expected to get with the current docs? What surprised you or what were you not sure about? - PR: https://git.openjdk.java.net/jfx/pull/610
Re: RFR: 8272870: Add convenience factory methods for border and background [v3]
> Added convenience factory factory methods for Background and Border. Nir Lisker has updated the pull request incrementally with two additional commits since the last revision: - Removed whitespaces - Added tests and doc updates - Changes: - all: https://git.openjdk.java.net/jfx/pull/610/files - new: https://git.openjdk.java.net/jfx/pull/610/files/bb54859e..345d759f Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jfx=610=02 - incr: https://webrevs.openjdk.java.net/?repo=jfx=610=01-02 Stats: 28 lines in 4 files changed: 20 ins; 2 del; 6 mod Patch: https://git.openjdk.java.net/jfx/pull/610.diff Fetch: git fetch https://git.openjdk.java.net/jfx pull/610/head:pull/610 PR: https://git.openjdk.java.net/jfx/pull/610
Re: RFR: 8272870: Add convenience factory methods for border and background [v2]
On Tue, 7 Sep 2021 06:31:18 GMT, Ambarish Rapte wrote:
>> Since the `Background` constructors take a list of `Paint` objects, I think
>> saying a "single" `{@code Paint}` is helpful. I can see how adding "for
>> `BackgroundFill`" (or maybe "as a `BackgroundFill`"?) might make it clearer.
>
> Sound good to me, It would read like `A convenience factory method for
> creating a Background by specifying a single {@code Paint} as a
> BackgroundFill`
Does mentioning `BackgroundFill` in the description contribute something? The
`@implSpec` tag gives that info. In my eyes, if the docs say the background
will use a single `Paint` then I know exactly what I'm going to get regardless
of what it uses in the implementation.
-
PR: https://git.openjdk.java.net/jfx/pull/610
Re: RFR: 8272870: Add convenience factory methods for border and background [v2]
On Fri, 3 Sep 2021 21:08:27 GMT, Kevin Rushforth wrote:
>> modules/javafx.graphics/src/main/java/javafx/scene/layout/Background.java
>> line 366:
>>
>>> 364: */
>>> 365: public static Background fill(Paint fill) {
>>> 366: return new Background(new BackgroundFill(fill, null, null));
>>
>> null **CornerRaddii** and null **Insets** will use **CornerRadii.EMPTY** and
>> **Insets.EMPTY**. Maybe we should use those here instead so it's more clear
>> for anyone having a look in the source code? I also always use those instead
>> of null.
>> Same for **BorderStroke**
>
> Are you talking about the implementation or the code? I guess both, since the
> `@implSepc` indicates what this call is equivalent to. I don't have a strong
> opinion on this one.
You don't need to look at the source code, I can link to the delegated
constructor and the docs will show those. I prefer using `null`s because the
point of these is that you want a border/background with 1 color and you don't
care about things like widths, insets and corners - they default to whatever
the default is. If you care what they are, this method is probably not what
you're looking for. It's also shorter and less to read through when the extra
info is defaults.
-
PR: https://git.openjdk.java.net/jfx/pull/610
Re: RFR: 8090547: Allow for transparent backgrounds in WebView [v8]
On Thu, 9 Sep 2021 08:52:31 GMT, Jose Pereda wrote: >> Currently, `WebPage` has already a public `setBackgroundColor()` method, but >> the class is not public. Therefore, public API is needed in `WebView` to >> allow developers access to it. >> >> In line with the `fontSmoothingType` property, this PR provides public >> support for setting the background color of a WebPage, by adding a >> `pageFill` property, and a CSR is required. >> >> The color for the background, that can be opaque, transparent or with any >> level of opacity, can be set via code or via CSS using `-fx-page-fill`. >> >> Unit tests and a system test are provided. > > Jose Pereda has updated the pull request incrementally with one additional > commit since the last revision: > > Changes required per reviewer Yes, I have been testing on MacOS (11.5.1), Linux (Ubuntu 20.04) and Windows (10) with a manual test. I do a quick visual test with transparent, translucent and solid color, including scrolling and selecting some text. However, the `PageFillTest` system test (which was actually based on this manual test) involves also taking snapshots and pixel comparison. On MacOS it works fine. On Linux I see this "ghosting" while scrolling, but snapshots are taken on a steady position, and it shouldn't be there. What assertion fails for you? - PR: https://git.openjdk.java.net/jfx/pull/563
Re: [External] : Re: apps compilation fails on ubuntu 20.04
I'm having the same issue on Windows. I'm not really sure what's different
about my setup. Originally I thought it was failing every time, but every
now and then it gets past that and successfully builds for me. After
changing from JDK 15.0.2 to 16.0.2 it worked, but that's when I realised it
was somewhat random. Based on this, it shouldn't be related to operating
system or JDK version.
I usually have more success after a clean, but not always...
Dean
On Fri, Aug 13, 2021 at 1:16 PM Abhinay Agarwal
wrote:
> The piece of code responsible for the exception is:
>
> for (File f : configurations.compileClasspath.files) {
> // Have to rename the swt jar because it is some platform specific
> name but
> // for the sake of the IDEs we need to have a single stable name that
> works
> // on every platform
> copy {
> into libsDir
> from f.getParentFile()
> include "**/*swt*.jar"
> includeEmptyDirs = false
> rename ".*swt.*jar", "swt-debug\\.jar"
> }
> }
>
> From the code snippet, it looks like Gradle is being very pro-active and
> complaining that this could end up with multiple jar files containing "swt"
> in their name and we need to provide a duplicate strategy for it.
>
>
> We can either provide a duplicatesStrategy or update the above to the
> following:
>
> copy {
> into libsDir
> from configurations.compileClasspath.files
> include "**/*swt*.jar"
> includeEmptyDirs = false
> rename ".*swt.*jar", "swt-debug\\.jar"
> // duplicatesStrategy = DuplicatesStrategy.WARN
> }
>
>
> From: Kevin Rushforth
> Sent: Friday, August 13, 2021 2:24 AM
> To: Abhinay Agarwal ;
> openjfx-dev@openjdk.java.net
> Subject: Re: [External] : Re: apps compilation fails on ubuntu 20.04
>
> Interesting. I wonder what it is about your environment that causes the
> duplicate, since we don't see the problem in our CI builds nor in the
> GitHub Actions builds. I presume you have done a clean build and still see
> it? It is possible that you have some SWT classes in your classpath or
> jlink'ed into your boot JDK?
>
> -- Kevin
>
>
> On 8/12/2021 1:39 PM, Abhinay Agarwal wrote:
> Hi Kevin,
>
> My local repository is up-to-date with the jfx repository.
>
> The failure happens while trying to copy swt-debug.jar:
>
> $ sh gradlew sdk
> ...
> > Task :swt:classes FAILED
>
> FAILURE: Build failed with an exception.
>
> * Where:
> Build file '/tmp/jfx/build.gradle' line: 2677
>
> * What went wrong:
> Execution failed for task ':swt:classes'.
> > Entry swt-debug.jar is a duplicate but no duplicate handling strategy
> has been set. Please refer to
> https://docs.gradle.org/7.0.1/dsl/org.gradle.api.tasks.Copy.html#org.gradle.api.tasks.Copy:duplicatesStrategy
> <
> https://urldefense.com/v3/__https://docs.gradle.org/7.0.1/dsl/org.gradle.api.tasks.Copy.html*org.gradle.api.tasks.Copy:duplicatesStrategy__;Iw!!ACWV5N9M2RV99hQ!fkRdlKBh300gss2krEHXu3UBrH7WOoht8YhvBygI4oIQd5dXyA2dXSrcTpkY5GOP5IQU$>
> for details.
>
> P.S. Adding a `duplicatesStrategy` to the copy task also helps with the
> issue.
>
> --Abhinay
>
>
> From: openjfx-dev openjfx-dev-r...@openjdk.java.net> on behalf of Kevin Rushforth <
> kevin.rushfo...@oracle.com>
> Sent: Friday, August 13, 2021 1:22 AM
> To: openjfx-dev@openjdk.java.net <
> openjfx-dev@openjdk.java.net>
> Subject: Re: apps compilation fails on ubuntu 20.04
>
> I still need to catch up on my email, since I still have the original
> question in my queue.
>
> The answer to the original query is that there is a known bug with ant
> 1.10.7 [1] that will cause this error. You can either use ant 1.10.5
> (which is the one we specify in build.properties) or use ant 1.10.8.
>
> As for the gradle sdk problem, I've not seen any problems relating to
> using gradle 7.0.1. Is your repo up to date? Specifically, you need the
> following commit:
>
>
> https://github.com/openjdk/jfx/commit/111bac4180a646662a81223bdbb56880789d5a90
> <
> https://urldefense.com/v3/__https://github.com/openjdk/jfx/commit/111bac4180a646662a81223bdbb56880789d5a90__;!!ACWV5N9M2RV99hQ!fkRdlKBh300gss2krEHXu3UBrH7WOoht8YhvBygI4oIQd5dXyA2dXSrcTpkY5KEBj0BS$
> >
>
> -- Kevin
>
> [1] https://bz.apache.org/bugzilla/show_bug.cgi?id=63874<
> https://urldefense.com/v3/__https://bz.apache.org/bugzilla/show_bug.cgi?id=63874__;!!ACWV5N9M2RV99hQ!fkRdlKBh300gss2krEHXu3UBrH7WOoht8YhvBygI4oIQd5dXyA2dXSrcTpkY5Bmaafi0$
> >
>
>
>
> On 8/12/2021 12:33 PM, Abhinay Agarwal wrote:
> > I am on Ubuntu 20.04. Using ANT 1.10.8, "gradlew apps" works for me.
> >
> > However, "gradlew sdk" fails for me since we upgraded to Gradle 7.0.1. I
> always switch back to Gradle 6.9 to build a local sdk.
> >
> > From: openjfx-dev openjfx-dev-r...@openjdk.java.net> on behalf of Thiago Milczarek Sayão <
>
Re: RFR: 8090547: Allow for transparent backgrounds in WebView [v8]
On Thu, 9 Sep 2021 08:52:31 GMT, Jose Pereda wrote: >> Currently, `WebPage` has already a public `setBackgroundColor()` method, but >> the class is not public. Therefore, public API is needed in `WebView` to >> allow developers access to it. >> >> In line with the `fontSmoothingType` property, this PR provides public >> support for setting the background color of a WebPage, by adding a >> `pageFill` property, and a CSR is required. >> >> The color for the background, that can be opaque, transparent or with any >> level of opacity, can be set via code or via CSS using `-fx-page-fill`. >> >> Unit tests and a system test are provided. > > Jose Pereda has updated the pull request incrementally with one additional > commit since the last revision: > > Changes required per reviewer Latest changes look good. The new test fails for me on Linux, though. I tried it both on my Ubuntu 20.04 VM and on an older Ubuntu 16.0.4 physical machine. I don't think it's just a test issue, either. When the OpenGL pipeline I get some odd looking ghosting around the text while the test is scrolling. Have you tested this on Linux? - PR: https://git.openjdk.java.net/jfx/pull/563
Re: [External] : Re: Pivot properties [was: Enhancements for JavaFX 18]
I have updated the PR to use DoubleProperties for the pivots and added 2 BooleanProperties for the normalized coordinates toggle on Node. To the Rotate/ScaleTransitions I added only the pivot properties that update those of the node. It's a prototype, so ignore docs and other miscellania. I also posted in a comment an interactive test app. It should be simple enough to modify it for local testing. I wasn't sure if the boolean properties should also be added to the transitions. The rotation axis and pivot properties are mostly a convenience since those can be updated on the node directly. My greatest concern is the confusion that happens with transforms: I need to be very careful when mixing those in since they use a different coordinate system by default. In a way, if Node and the transitions find it more natural to use normalized coordinates, shouldn't transforms find it natural too? Not sure if this is inconvenient only for me. On Sat, Aug 28, 2021 at 2:56 AM Nir Lisker wrote: > The problem I have with additional booleans is that they are not > reflected in the transitions, and to a lesser extent in the transforms. > > I'm going to have to write an implementation and test it out to know how > well this works and how much trouble it really saves. > > And while we could use NaN as an out of band value meaning "computed", >> that still runs into the problem of allowing for the odd case where X is >> computed and Y is specified. > > > I don't see this as a problem. Each direction is independent. The bigger > problem I see is the surprise factor of NaN as a computed value. > > On Tue, Aug 24, 2021 at 6:16 PM Kevin Rushforth < > kevin.rushfo...@oracle.com> wrote: > >> I definitely don't like having a "magic" initial value that can't be >> reset, so that won't work. And while we could use NaN as an out of band >> value meaning "computed", that still runs into the problem of allowing for >> the odd case where X is computed and Y is specified. >> >> If we do go with the idea of allowing the coordinates to be a percentage >> of the bounding box, with 0.5 as the default, then we could be informed by >> what ImagePattern does. It defines a "proportional" property as follows: >> >> "a boolean that indicates whether start and end locations are >> proportional or absolute. If this flag is true, the two end points are >> defined in a coordinate space where coordinates in the range [0..1] are >> scaled to map onto the bounds of the shape that the pattern fills. If this >> flag is false, then the coordinates are specified in the local coordinate >> system of the node." >> >> That's basically what we are doing if we go with Michael's idea. So if we >> do this, the two boolean properties could be named something like >> scalePivotProportional and rotatePivotProportional, with a default of true. >> >> What do you think? >> >> -- Kevin >> >> >> >> On 8/23/2021 7:25 PM, Michael Strauß wrote: >> >> I think normalized coordinates are a very natural fit for the >> definition of a pivot point, which is why the current default value is >> an implicitly-specified normalized coordinate. Adding general-purpose >> coordinate transformations here feels like bringing a sledgehammer to >> crack a nut. >> >> Having a property with an automagically updated "initial" value is >> quite non-standard behavior for properties. It would appear as if the >> property was bound, yet there's no binding. That's markedly different >> from a Transition's 'from' and 'to' properties, which are standard >> properties with normal behavior. A running transition doesn't bind to >> those properties, it uses a snapshot of their values when the >> transition starts. >> >> >> On Sun, Aug 22, 2021 at 9:55 PM Nir Lisker >> wrote: >> >> Now I understand what you meant. However, the concept of normalized >> coordinates does not appear anywhere in JavaFX (at least not in these >> contexts). I still think that coordinate transformations should be handled >> via dedicated means, like coordinate system transformations are. Maybe when >> we work on mapped bindings (I forgot that I need to review that) this pain >> point will be alleviated. >> >> Kevin, what if we only set the initial value of the pivot (doesn't matter >> what the implementation detail is) to the center of the node for backwards >> compatibility, but if the developer changes it to a custom value then it's >> on them to compute the value again if they want to go back? The default >> behavior remains. >> Another relevant point is that Transitions do something similar with their >> from_, to_ and by_ methods. They start with Double.NaN to signal that the >> value should be ignored. While the developer can reset the value to NaN, it >> is not something that is likely to happen during, say, an interpolation or >> as a result of a binding. >> >> >> >>
Re: RFR: 8268225: Support :focus-visible and :focus-within CSS pseudoclasses [v7]
On Thu, 9 Sep 2021 09:47:56 GMT, Michael Strauß wrote: > > > > Just curious: with this in place, would it be possible to use for > > supporting [JDK-8087926](https://bugs.openjdk.java.net/browse/JDK-8087926) > > (it's a bit vague, though, at least for me)? > > Yes, `:focus-within` can be used to select an ancestor of the > currently-focused node. cool - thanks :) - PR: https://git.openjdk.java.net/jfx/pull/475
Re: RFR: 8268225: Support :focus-visible and :focus-within CSS pseudoclasses [v7]
On Thu, 9 Sep 2021 09:15:06 GMT, Jeanette Winzenburg wrote: > Just curious: with this in place, would it be possible to use for supporting > [JDK-8087926](https://bugs.openjdk.java.net/browse/JDK-8087926) (it's a bit > vague, though, at least for me)? Yes, `:focus-within` can be used to select an ancestor of the currently-focused node. - PR: https://git.openjdk.java.net/jfx/pull/475
Re: RFR: 8268225: Support :focus-visible and :focus-within CSS pseudoclasses [v7]
On Fri, 20 Aug 2021 05:15:49 GMT, Michael Strauß wrote: >> This PR adds the `Node.focusVisible` and `Node.focusWithin` properties, as >> well as the corresponding `:focus-visible` and `:focus-within` CSS >> pseudo-classes. > > Michael Strauß has updated the pull request incrementally with one additional > commit since the last revision: > > fixed undeterministic test failures Just curious: with this in place, would it be possible to use for supporting [JDK-8087926](https://bugs.openjdk.java.net/browse/JDK-8087926) (it's a bit vague, though, at least for me)? - PR: https://git.openjdk.java.net/jfx/pull/475
Re: RFR: 8090547: Allow for transparent backgrounds in WebView [v8]
> Currently, `WebPage` has already a public `setBackgroundColor()` method, but > the class is not public. Therefore, public API is needed in `WebView` to > allow developers access to it. > > In line with the `fontSmoothingType` property, this PR provides public > support for setting the background color of a WebPage, by adding a `pageFill` > property, and a CSR is required. > > The color for the background, that can be opaque, transparent or with any > level of opacity, can be set via code or via CSS using `-fx-page-fill`. > > Unit tests and a system test are provided. Jose Pereda has updated the pull request incrementally with one additional commit since the last revision: Changes required per reviewer - Changes: - all: https://git.openjdk.java.net/jfx/pull/563/files - new: https://git.openjdk.java.net/jfx/pull/563/files/02061b9e..90ab86d4 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jfx=563=07 - incr: https://webrevs.openjdk.java.net/?repo=jfx=563=06-07 Stats: 5 lines in 2 files changed: 1 ins; 1 del; 3 mod Patch: https://git.openjdk.java.net/jfx/pull/563.diff Fetch: git fetch https://git.openjdk.java.net/jfx pull/563/head:pull/563 PR: https://git.openjdk.java.net/jfx/pull/563
Re: RFR: 8090547: Allow for transparent backgrounds in WebView [v7]
On Wed, 8 Sep 2021 21:08:40 GMT, Kevin Rushforth wrote:
>> Jose Pereda has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Address feedback from reviewers
>
> modules/javafx.web/src/main/java/com/sun/webkit/WebPage.java line 2589:
>
>> 2587: private static int getIntRgba(Color color) {
>> 2588: if (color == null) {
>> 2589: return -1;
>
> Maybe: `return 0x;` ? or else assign `color = Color.WHITE;` and fall
> through?
Probably better then to create a constant?
private static final int DEFAULT_BACKGROUND_INT_RGBA = 0x; //
Color.WHITE
-
PR: https://git.openjdk.java.net/jfx/pull/563
Re: RFR: 8090547: Allow for transparent backgrounds in WebView [v7]
On Wed, 8 Sep 2021 22:37:18 GMT, Kevin Rushforth wrote: >> modules/javafx.web/src/main/java/javafx/scene/web/WebView.java line 707: >> >>> 705: * level of transparency. >>> 706: * >>> 707: * However, if the HTML content being loaded sets its own >> >> I looked at the generated javadoc and this will read better without a >> paragraph break. > >> I looked at the generated javadoc and this will read better without a >> paragraph break. > > When you remove the paragraph break, please also remove the blank line to > make it clearer to future readers of the code that it is intended to be part > of the same paragraph as the previous sentence. Done - PR: https://git.openjdk.java.net/jfx/pull/563
Re: RFR: 8090547: Allow for transparent backgrounds in WebView [v7]
On Wed, 8 Sep 2021 16:15:36 GMT, Jose Pereda wrote: >> Currently, `WebPage` has already a public `setBackgroundColor()` method, but >> the class is not public. Therefore, public API is needed in `WebView` to >> allow developers access to it. >> >> In line with the `fontSmoothingType` property, this PR provides public >> support for setting the background color of a WebPage, by adding a >> `pageFill` property, and a CSR is required. >> >> The color for the background, that can be opaque, transparent or with any >> level of opacity, can be set via code or via CSS using `-fx-page-fill`. >> >> Unit tests and a system test are provided. > > Jose Pereda has updated the pull request incrementally with one additional > commit since the last revision: > > Address feedback from reviewers I've also updated the CSR - PR: https://git.openjdk.java.net/jfx/pull/563
Integrated: 8089398: [ChoiceBox, ComboBox] throws NPE on setting value on null selectionModel
On Wed, 30 Jun 2021 15:03:50 GMT, Marius Hanl wrote: > This PR fixes multiple NPEs in Choice-and ComboBox, when the selection model > is null. > > ChoiceBox: > - Null check in **valueProperty()** listener > > ComboBox: > - Null check in **editableProperty()** listener > - Null check in **valueProperty()** listener > - Null check in **ComboBoxListViewSkin#updateValue()** > - Null check in **ComboBoxListViewSkin#layoutChildren()** > - 2x Null check in **ComboBoxListViewSkin#createListView()** > > ~~The tests checks, that no NPE is printed to the console. > Some checks, that the set value is still displayed (either in the ComboBox > button cell or the ChoiceBox display label)~~ This pull request has now been integrated. Changeset: f987b181 Author:Marius Hanl Committer: Jeanette Winzenburg URL: https://git.openjdk.java.net/jfx/commit/f987b181aba1d3d56e745bd02fe350287ab57459 Stats: 141 lines in 5 files changed: 127 ins; 3 del; 11 mod 8089398: [ChoiceBox, ComboBox] throws NPE on setting value on null selectionModel Reviewed-by: fastegal, aghaisas - PR: https://git.openjdk.java.net/jfx/pull/557
Re: RFR: 8172095: Let Node.managed become CSS-styleable [v2]
On Sat, 4 Sep 2021 15:44:17 GMT, Abhinay Agarwal wrote: >> 8172095: Let Node.managed become CSS-styleable > > Abhinay Agarwal has updated the pull request incrementally with two > additional commits since the last revision: > > - Add default value tests for setting / getting the managed property > - Replace with HTML equivalent code Changes look good to me. The tests, managedSetFromCSS() and Node_cssMethods_Test fail before and pass after. Other tests do pass both before and after, which is expected behaviour. - Marked as reviewed by arapte (Reviewer). PR: https://git.openjdk.java.net/jfx/pull/602
Re: RFR: 8273485: Deadlock when also using Swing and exiting Fullscreen on Mac
On Wed, 8 Sep 2021 10:37:40 GMT, Florian Kirmaier wrote: > When using Swing it's possible to generate a Deadlock. > It's related to the nested eventloop started in enterFullScreenExitingLoop - > and the RenderLock aquired when using setView in Scene. > Sample Programm and Threaddump are added to the ticket. > > Removing the nested loop fixes the Problem. > I hope this doesn't have any side effect - so far i don't know of any. Thank you for the feedback - I will start writing an unit test for it because it will help on all further development regarding this issue. - PR: https://git.openjdk.java.net/jfx/pull/622
