Re: RFR: 8284654: Modal behavior returns to wrong stage [v3]
On Tue, 10 May 2022 12:56:53 GMT, Thiago Milczarek Sayao wrote: >> When there's an APPLICATION_MODAL window, all other windows are disabled and >> re-enabled when the APPLICATION_MODAL window closes. This causes >> `requestToFront()` to be called on every window, and it does not guarantee >> order. >> >> The fix also works for: >> https://bugs.openjdk.java.net/browse/JDK-8269429 >> >> But this one might need another fix: >> https://bugs.openjdk.java.net/browse/JDK-8240640 > > Thiago Milczarek Sayao has updated the pull request incrementally with one > additional commit since the last revision: > > Revert "Set the focus on the latest window when re-enabling them" > > This reverts commit b024de586c187f9000f791dab99507a4979cc027. Looks good now. - Marked as reviewed by kcr (Lead). PR: https://git.openjdk.java.net/jfx/pull/784
Re: RFR: 8274066: Polygon filled outside its area when very large coordinates are used [v2]
On Wed, 25 May 2022 07:37:38 GMT, Laurent Bourgès wrote: > keep dpqs for corner cases and keep my coding life simple I think this approach is fine. Diverging the code would likely make it less stable, and you answered the question about the provenance of the code, so there's no issue there. We should try to get this in before RDP1 of JavaFX 19 if possible. One more thing, as I wrote in my above comment: > We should file a new JBS Enhancement issue -- similar to what was done for > Marlin 0.9.2 via > [JDK-8204621](https://bugs.openjdk.java.net/browse/JDK-8204621) rather than > using a narrow bug, since that bug is only part of what's being done. The > current bug can either be added to the PR, or else that JBS bug (JDK-8274066) > can be closed as a duplicate of the new Enhancement. I took the liberty of filing [JDK-8287604](https://bugs.openjdk.java.net/browse/JDK-8287604) for this enhancement. Can you change the title to: 8287604: Update MarlinFX to 0.9.4.5 - PR: https://git.openjdk.java.net/jfx/pull/674
Re: RFR: 8274771: Map, FlatMap and OrElse fluent bindings for ObservableValue [v14]
On Tue, 22 Mar 2022 07:46:40 GMT, John Hendrikx wrote:
>> This is an implementation of the proposal in
>> https://bugs.openjdk.java.net/browse/JDK-8274771 that me and Nir Lisker
>> (@nlisker) have been working on. It's a complete implementation including
>> good test coverage.
>>
>> This was based on https://github.com/openjdk/jfx/pull/434 but with a smaller
>> API footprint. Compared to the PoC this is lacking public API for
>> subscriptions, and does not include `orElseGet` or the `conditionOn`
>> conditional mapping.
>>
>> **Flexible Mappings**
>> Map the contents of a property any way you like with `map`, or map nested
>> properties with `flatMap`.
>>
>> **Lazy**
>> The bindings created are lazy, which means they are always _invalid_ when
>> not themselves observed. This allows for easier garbage collection (once the
>> last observer is removed, a chain of bindings will stop observing their
>> parents) and less listener management when dealing with nested properties.
>> Furthermore, this allows inclusion of such bindings in classes such as
>> `Node` without listeners being created when the binding itself is not used
>> (this would allow for the inclusion of a `treeShowingProperty` in `Node`
>> without creating excessive listeners, see this fix I did in an earlier PR:
>> https://github.com/openjdk/jfx/pull/185)
>>
>> **Null Safe**
>> The `map` and `flatMap` methods are skipped, similar to `java.util.Optional`
>> when the value they would be mapping is `null`. This makes mapping nested
>> properties with `flatMap` trivial as the `null` case does not need to be
>> taken into account in a chain like this:
>> `node.sceneProperty().flatMap(Scene::windowProperty).flatMap(Window::showingProperty)`.
>> Instead a default can be provided with `orElse`.
>>
>> Some examples:
>>
>> void mapProperty() {
>> // Standard JavaFX:
>> label.textProperty().bind(Bindings.createStringBinding(() ->
>> text.getValueSafe().toUpperCase(), text));
>>
>> // Fluent: much more compact, no need to handle null
>> label.textProperty().bind(text.map(String::toUpperCase));
>> }
>>
>> void calculateCharactersLeft() {
>> // Standard JavaFX:
>>
>> label.textProperty().bind(text.length().negate().add(100).asString().concat("
>> characters left"));
>>
>> // Fluent: slightly more compact and more clear (no negate needed)
>> label.textProperty().bind(text.orElse("").map(v -> 100 - v.length() +
>> " characters left"));
>> }
>>
>> void mapNestedValue() {
>> // Standard JavaFX:
>> label.textProperty().bind(Bindings.createStringBinding(
>> () -> employee.get() == null ? ""
>> : employee.get().getCompany() == null ? ""
>> : employee.get().getCompany().getName(),
>> employee
>> ));
>>
>> // Standard JavaFX + Optional:
>> label.textProperty().bind(Bindings.createStringBinding(
>> () -> Optinal.ofNullable(employee.get())
>> .map(Employee::getCompany)
>> .map(Company::getName)
>> .orElse(""),
>> employee
>> ));
>>
>> // Fluent: no need to handle nulls everywhere
>> label.textProperty().bind(
>> employee.map(Employee::getCompany)
>> .map(Company::getName)
>> .orElse("")
>> );
>> }
>>
>> void mapNestedProperty() {
>> // Standard JavaFX:
>> label.textProperty().bind(
>> Bindings.when(Bindings.selectBoolean(label.sceneProperty(),
>> "window", "showing"))
>> .then("Visible")
>> .otherwise("Not Visible")
>> );
>>
>> // Fluent: type safe
>> label.textProperty().bind(label.sceneProperty()
>> .flatMap(Scene::windowProperty)
>> .flatMap(Window::showingProperty)
>> .orElse(false)
>> .map(showing -> showing ? "Visible" : "Not Visible")
>> );
>> }
>>
>> Note that this is based on ideas in ReactFX and my own experiments in
>> https://github.com/hjohn/hs.jfx.eventstream. I've come to the conclusion
>> that this is much better directly integrated into JavaFX, and I'm hoping
>> this proof of concept will be able to move such an effort forward.
>
> John Hendrikx has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Fix wording
I reviewed the public API changes, and this looks like a great addition to
JavaFX bindings. I think there might be time to get this into JavaFX 19,
presuming that there are no issues with the testing or implementation, so let's
proceed down that path.
I left one comment on the API docs as well as pointed out the public methods
that will need an `@since 19` javadoc tag.
Once that is updated you can propagate the javadoc changes to the CSR
(including the `@since` tags) and move it to "Proposed". I'll formally review
it later, once the code review is closer to being done.
Re: RFR: 8088420: JavaFX WebView memory leak via EventListener [v9]
On Fri, 27 May 2022 16:47:04 GMT, Jay Bhaskar wrote: >> This PR is new implementation of JavaEvent listener memory management. >> Issue >> [JDK-8088420](https://bugs.openjdk.java.net/browse/JDK-8088420?filter=-1) >> >> 1. Calling remove event listener does not free jni global references. >> 2. When WebView goes out of scope (disposed from app) , its Event Listeners >> are not being garbage collected. >> >> Solution: >> 1. Detached the jni global reference from JavaEventListener. >> 2. Create scoped ref counted wrapper class JavaObjectWrapperHandler for jni >> global reference. >> 3. Create unique JavaObjectWrapperHandler object for each JavaEventListener. >> 4. EventListenerManager is a singleton class , which stores the >> JavaObjectWrapperHandler mapped with JavaEventListener. >> 5. EventListenerManager also stores the JavaEventListener mapped with >> DOMWindow. >> 6. When Event listener explicitly removed , JavaEventListener is being >> forwarded to EventListenerManager to clear the listener. >> 7. When WebView goes out of scope, EventListenerManager will de-registered >> all the event listeners based on the ref counts attached with WebView >> DOMWindow. > > Jay Bhaskar has updated the pull request incrementally with one additional > commit since the last revision: > > Adingef else block to unregisterListener Looks good. - Marked as reviewed by kcr (Lead). PR: https://git.openjdk.java.net/jfx/pull/799
Re: RFR: 8088420: JavaFX WebView memory leak via EventListener [v8]
On Fri, 27 May 2022 12:13:43 GMT, Jay Bhaskar wrote: >> This PR is new implementation of JavaEvent listener memory management. >> Issue >> [JDK-8088420](https://bugs.openjdk.java.net/browse/JDK-8088420?filter=-1) >> >> 1. Calling remove event listener does not free jni global references. >> 2. When WebView goes out of scope (disposed from app) , its Event Listeners >> are not being garbage collected. >> >> Solution: >> 1. Detached the jni global reference from JavaEventListener. >> 2. Create scoped ref counted wrapper class JavaObjectWrapperHandler for jni >> global reference. >> 3. Create unique JavaObjectWrapperHandler object for each JavaEventListener. >> 4. EventListenerManager is a singleton class , which stores the >> JavaObjectWrapperHandler mapped with JavaEventListener. >> 5. EventListenerManager also stores the JavaEventListener mapped with >> DOMWindow. >> 6. When Event listener explicitly removed , JavaEventListener is being >> forwarded to EventListenerManager to clear the listener. >> 7. When WebView goes out of scope, EventListenerManager will de-registered >> all the event listeners based on the ref counts attached with WebView >> DOMWindow. > > Jay Bhaskar has updated the pull request incrementally with one additional > commit since the last revision: > > Change for unregisterDomWindow function and code cleanup The changes look good. While doing my final review I noticed one more thing that I think should be changed. modules/javafx.web/src/main/native/Source/WebCore/bindings/java/EventListenerManager.cpp line 57: > 55: } > 56: > 57: if (it->second && it->second->use_count() > 1) Shouldn't this be `else if`? The previous block calls `erase(it)`, so it isn't valid to dereference `it` after that call. - PR: https://git.openjdk.java.net/jfx/pull/799
Re: RFR: 8088420: JavaFX WebView memory leak via EventListener [v7]
On Thu, 26 May 2022 07:26:38 GMT, Jay Bhaskar wrote:
>> This PR is new implementation of JavaEvent listener memory management.
>> Issue
>> [JDK-8088420](https://bugs.openjdk.java.net/browse/JDK-8088420?filter=-1)
>>
>> 1. Calling remove event listener does not free jni global references.
>> 2. When WebView goes out of scope (disposed from app) , its Event Listeners
>> are not being garbage collected.
>>
>> Solution:
>> 1. Detached the jni global reference from JavaEventListener.
>> 2. Create scoped ref counted wrapper class JavaObjectWrapperHandler for jni
>> global reference.
>> 3. Create unique JavaObjectWrapperHandler object for each JavaEventListener.
>> 4. EventListenerManager is a singleton class , which stores the
>> JavaObjectWrapperHandler mapped with JavaEventListener.
>> 5. EventListenerManager also stores the JavaEventListener mapped with
>> DOMWindow.
>> 6. When Event listener explicitly removed , JavaEventListener is being
>> forwarded to EventListenerManager to clear the listener.
>> 7. When WebView goes out of scope, EventListenerManager will de-registered
>> all the event listeners based on the ref counts attached with WebView
>> DOMWindow.
>
> Jay Bhaskar has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Adding space for map include
Comments inline.
modules/javafx.web/src/main/native/Source/WebCore/bindings/java/EventListenerManager.cpp
line 106:
> 104: else
> 105: isReferringToOtherListener = true;
> 106: }
I think I now understand what this is trying to do, but it doesn't looks like
it's implemented correctly nor is it optimal.
It seems that the `isReferringToOtherListener` flag is intended to be true iff
there is any `JavaEventListener` with a ref count > 1 associated with the
Window being unregistered. Ignoring any possible bugs in how it is implemented,
there are two problems with this approach. First, you want to remove entries
associated with a particular listener if _that_ listener isn't used by another
window. So a global "is any listener referring to another window" isn't what
you want. Second, since this is multimap, it would seem better to remove
individual `(key,value)` pairs associated with the specific window being
removed rather than calling erase only for those listeners that aren't being
shared at all. If this is feasible, then you wouldn't have to even care if that
listener is used by a node in another window. I'm not familiar enough with C++
multimap calls to know how easy this would be, but presumably, it shouldn't be
too hard.
Not directly related to this, it might be cleaner to move this logic to
`unregisterDOMWindow` instead of having a separate `resetDOMWindow` method,
since this is really part of the same conceptual operation.
modules/javafx.web/src/test/java/test/javafx/scene/web/EventListenerLeakTest.java
line 607:
> 605: */
> 606: @Test
> 607: public void TestStrongRefNewContentLoad() throws Exception {
Minor: method names should start with a lower-case letter.
modules/javafx.web/src/test/java/test/javafx/scene/web/EventListenerLeakTest.java
line 730:
> 728: });
> 729:
> 730: // Verify that the events are delivered to the listeners (0 and
> 2 are same)
Minor: all three listeners are the same, not just 0 and 2.
modules/javafx.web/src/test/java/test/javafx/scene/web/EventListenerLeakTest.java
line 959:
> 957: // Verify that the event is delivered to the listener
> 958: assertEquals("Click count", 1, listeners.get(0).getClickCount());
> 959: assertEquals("Click count", 1, listeners.get(0).getClickCount());
The second check should be `listeners.get(1)`
-
PR: https://git.openjdk.java.net/jfx/pull/799
Re: RFR: 8088420: JavaFX WebView memory leak via EventListener [v4]
On Tue, 24 May 2022 15:20:01 GMT, Kevin Rushforth wrote:
>> Jay Bhaskar has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Adding JGObject in plave of raw jni object
>
> modules/javafx.web/src/test/java/test/javafx/scene/web/EventListenerLeakTest.java
> line 765:
>
>> 763: assertEquals("Click count", 6,
>> listeners.get(1).get().getClickCount() +
>> listeners.get(0).get().getClickCount());
>> 764:
>> 765: // add events listeners again
>
> that should be "remove"
I think you may have misunderstood my comment. I didn't mean to suggest that
you should remove the blocks of code that you removed in the most recent update
-- it was performing a quite useful check. Rather I meant that since you were
removing the listeners, the comment `add listeners again` was wrong and should
say `remove listeners again`.
-
PR: https://git.openjdk.java.net/jfx/pull/799
Re: RFR: 8088420: JavaFX WebView memory leak via EventListener [v4]
On Sun, 22 May 2022 14:06:41 GMT, Jay Bhaskar wrote:
>> This PR is new implementation of JavaEvent listener memory management.
>> Issue
>> [JDK-8088420](https://bugs.openjdk.java.net/browse/JDK-8088420?filter=-1)
>>
>> 1. Calling remove event listener does not free jni global references.
>> 2. When WebView goes out of scope (disposed from app) , its Event Listeners
>> are not being garbage collected.
>>
>> Solution:
>> 1. Detached the jni global reference from JavaEventListener.
>> 2. Create scoped ref counted wrapper class JavaObjectWrapperHandler for jni
>> global reference.
>> 3. Create unique JavaObjectWrapperHandler object for each JavaEventListener.
>> 4. EventListenerManager is a singleton class , which stores the
>> JavaObjectWrapperHandler mapped with JavaEventListener.
>> 5. EventListenerManager also stores the JavaEventListener mapped with
>> DOMWindow.
>> 6. When Event listener explicitly removed , JavaEventListener is being
>> forwarded to EventListenerManager to clear the listener.
>> 7. When WebView goes out of scope, EventListenerManager will de-registered
>> all the event listeners based on the ref counts attached with WebView
>> DOMWindow.
>
> Jay Bhaskar has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Adding JGObject in plave of raw jni object
modules/javafx.web/src/test/java/test/javafx/scene/web/EventListenerLeakTest.java
line 764:
> 762: Thread.sleep(100);
> 763: assertEquals("Click count", 6,
> listeners.get(1).get().getClickCount() +
> listeners.get(0).get().getClickCount());
> 764:
The above code should be restored.
modules/javafx.web/src/test/java/test/javafx/scene/web/EventListenerLeakTest.java
line 789:
> 787: Thread.sleep(100);
> 788: assertEquals("Click count", 6,
> listeners.get(1).get().getClickCount() +
> listeners.get(0).get().getClickCount());
> 789:
I think the above code should be restored.
-
PR: https://git.openjdk.java.net/jfx/pull/799
Re: RFR: 8088420: JavaFX WebView memory leak via EventListener [v5]
On Wed, 25 May 2022 10:10:08 GMT, Jay Bhaskar wrote: >> This PR is new implementation of JavaEvent listener memory management. >> Issue >> [JDK-8088420](https://bugs.openjdk.java.net/browse/JDK-8088420?filter=-1) >> >> 1. Calling remove event listener does not free jni global references. >> 2. When WebView goes out of scope (disposed from app) , its Event Listeners >> are not being garbage collected. >> >> Solution: >> 1. Detached the jni global reference from JavaEventListener. >> 2. Create scoped ref counted wrapper class JavaObjectWrapperHandler for jni >> global reference. >> 3. Create unique JavaObjectWrapperHandler object for each JavaEventListener. >> 4. EventListenerManager is a singleton class , which stores the >> JavaObjectWrapperHandler mapped with JavaEventListener. >> 5. EventListenerManager also stores the JavaEventListener mapped with >> DOMWindow. >> 6. When Event listener explicitly removed , JavaEventListener is being >> forwarded to EventListenerManager to clear the listener. >> 7. When WebView goes out of scope, EventListenerManager will de-registered >> all the event listeners based on the ref counts attached with WebView >> DOMWindow. > > Jay Bhaskar has updated the pull request incrementally with one additional > commit since the last revision: > > Review comments applied I left a couple comments on one of your test changes. I think the only remaining question (aside from the few minor comments that Ambarish left) is around the logic in `EventListenerManager::resetDOMWindow`. - PR: https://git.openjdk.java.net/jfx/pull/799
Re: RFR: 8274771: Map, FlatMap and OrElse fluent bindings for ObservableValue [v14]
On Tue, 22 Mar 2022 07:46:40 GMT, John Hendrikx wrote:
>> This is an implementation of the proposal in
>> https://bugs.openjdk.java.net/browse/JDK-8274771 that me and Nir Lisker
>> (@nlisker) have been working on. It's a complete implementation including
>> good test coverage.
>>
>> This was based on https://github.com/openjdk/jfx/pull/434 but with a smaller
>> API footprint. Compared to the PoC this is lacking public API for
>> subscriptions, and does not include `orElseGet` or the `conditionOn`
>> conditional mapping.
>>
>> **Flexible Mappings**
>> Map the contents of a property any way you like with `map`, or map nested
>> properties with `flatMap`.
>>
>> **Lazy**
>> The bindings created are lazy, which means they are always _invalid_ when
>> not themselves observed. This allows for easier garbage collection (once the
>> last observer is removed, a chain of bindings will stop observing their
>> parents) and less listener management when dealing with nested properties.
>> Furthermore, this allows inclusion of such bindings in classes such as
>> `Node` without listeners being created when the binding itself is not used
>> (this would allow for the inclusion of a `treeShowingProperty` in `Node`
>> without creating excessive listeners, see this fix I did in an earlier PR:
>> https://github.com/openjdk/jfx/pull/185)
>>
>> **Null Safe**
>> The `map` and `flatMap` methods are skipped, similar to `java.util.Optional`
>> when the value they would be mapping is `null`. This makes mapping nested
>> properties with `flatMap` trivial as the `null` case does not need to be
>> taken into account in a chain like this:
>> `node.sceneProperty().flatMap(Scene::windowProperty).flatMap(Window::showingProperty)`.
>> Instead a default can be provided with `orElse`.
>>
>> Some examples:
>>
>> void mapProperty() {
>> // Standard JavaFX:
>> label.textProperty().bind(Bindings.createStringBinding(() ->
>> text.getValueSafe().toUpperCase(), text));
>>
>> // Fluent: much more compact, no need to handle null
>> label.textProperty().bind(text.map(String::toUpperCase));
>> }
>>
>> void calculateCharactersLeft() {
>> // Standard JavaFX:
>>
>> label.textProperty().bind(text.length().negate().add(100).asString().concat("
>> characters left"));
>>
>> // Fluent: slightly more compact and more clear (no negate needed)
>> label.textProperty().bind(text.orElse("").map(v -> 100 - v.length() +
>> " characters left"));
>> }
>>
>> void mapNestedValue() {
>> // Standard JavaFX:
>> label.textProperty().bind(Bindings.createStringBinding(
>> () -> employee.get() == null ? ""
>> : employee.get().getCompany() == null ? ""
>> : employee.get().getCompany().getName(),
>> employee
>> ));
>>
>> // Standard JavaFX + Optional:
>> label.textProperty().bind(Bindings.createStringBinding(
>> () -> Optinal.ofNullable(employee.get())
>> .map(Employee::getCompany)
>> .map(Company::getName)
>> .orElse(""),
>> employee
>> ));
>>
>> // Fluent: no need to handle nulls everywhere
>> label.textProperty().bind(
>> employee.map(Employee::getCompany)
>> .map(Company::getName)
>> .orElse("")
>> );
>> }
>>
>> void mapNestedProperty() {
>> // Standard JavaFX:
>> label.textProperty().bind(
>> Bindings.when(Bindings.selectBoolean(label.sceneProperty(),
>> "window", "showing"))
>> .then("Visible")
>> .otherwise("Not Visible")
>> );
>>
>> // Fluent: type safe
>> label.textProperty().bind(label.sceneProperty()
>> .flatMap(Scene::windowProperty)
>> .flatMap(Window::showingProperty)
>> .orElse(false)
>> .map(showing -> showing ? "Visible" : "Not Visible")
>> );
>> }
>>
>> Note that this is based on ideas in ReactFX and my own experiments in
>> https://github.com/hjohn/hs.jfx.eventstream. I've come to the conclusion
>> that this is much better directly integrated into JavaFX, and I'm hoping
>> this proof of concept will be able to move such an effort forward.
>
> John Hendrikx has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Fix wording
Not yet, but it's on my list to look at this week.
-
PR: https://git.openjdk.java.net/jfx/pull/675
[jfx17u] Integrated: 8283869: Update attribution in webkit.md file
On Tue, 24 May 2022 17:34:25 GMT, Kevin Rushforth wrote: > Clean backport to `jfx17u`. This pull request has now been integrated. Changeset: 1835d1f5 Author: Kevin Rushforth URL: https://git.openjdk.java.net/jfx17u/commit/1835d1f59104136d6cb34090bd69091433ddac00 Stats: 5484 lines in 1 file changed: 5483 ins; 0 del; 1 mod 8283869: Update attribution in webkit.md file Backport-of: 7bb4819409dd617ba2e3658ee66f23b94dc0bec1 - PR: https://git.openjdk.java.net/jfx17u/pull/62
[jfx17u] Integrated: 8286256: Update libxml2 to 2.9.14
On Tue, 24 May 2022 17:33:57 GMT, Kevin Rushforth wrote: > Clean backport to `jfx17u`. This pull request has now been integrated. Changeset: 7b1637f8 Author: Kevin Rushforth URL: https://git.openjdk.java.net/jfx17u/commit/7b1637f8f617639ddc0c93e15589824c6a0d71b1 Stats: 21664 lines in 81 files changed: 4429 ins; 4294 del; 12941 mod 8286256: Update libxml2 to 2.9.14 8286257: Update libxslt to 1.1.35 Backport-of: d6770034a66a294b8740875c28871f2b4677aef2 - PR: https://git.openjdk.java.net/jfx17u/pull/61
[jfx11u] Integrated: 8283869: Update attribution in webkit.md file
On Tue, 24 May 2022 17:31:58 GMT, Kevin Rushforth wrote: > Clean backport to `jfx11u`. This pull request has now been integrated. Changeset: f89a5139 Author: Kevin Rushforth URL: https://git.openjdk.java.net/jfx11u/commit/f89a513980c2c2637c4f3d2b80f0708fd6f9e252 Stats: 5484 lines in 1 file changed: 5483 ins; 0 del; 1 mod 8283869: Update attribution in webkit.md file Backport-of: 7bb4819409dd617ba2e3658ee66f23b94dc0bec1 - PR: https://git.openjdk.java.net/jfx11u/pull/102
[jfx11u] Integrated: 8283869: Update attribution in webkit.md file
Clean backport to `jfx11u`. - Commit messages: - 8283869: Update attribution in webkit.md file Changes: https://git.openjdk.java.net/jfx11u/pull/102/files Webrev: https://webrevs.openjdk.java.net/?repo=jfx11u=102=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8283869 Stats: 5484 lines in 1 file changed: 5483 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jfx11u/pull/102.diff Fetch: git fetch https://git.openjdk.java.net/jfx11u pull/102/head:pull/102 PR: https://git.openjdk.java.net/jfx11u/pull/102
[jfx11u] Integrated: 8286256: Update libxml2 to 2.9.14
Clean backport to `jfx11u`. - Commit messages: - 8286256: Update libxml2 to 2.9.14 Changes: https://git.openjdk.java.net/jfx11u/pull/101/files Webrev: https://webrevs.openjdk.java.net/?repo=jfx11u=101=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8286256 Stats: 21664 lines in 81 files changed: 4429 ins; 4294 del; 12941 mod Patch: https://git.openjdk.java.net/jfx11u/pull/101.diff Fetch: git fetch https://git.openjdk.java.net/jfx11u pull/101/head:pull/101 PR: https://git.openjdk.java.net/jfx11u/pull/101
[jfx11u] Integrated: 8286256: Update libxml2 to 2.9.14
On Tue, 24 May 2022 17:31:27 GMT, Kevin Rushforth wrote: > Clean backport to `jfx11u`. This pull request has now been integrated. Changeset: 8504dad2 Author: Kevin Rushforth URL: https://git.openjdk.java.net/jfx11u/commit/8504dad26ac0f0dcbf7cb62ac58642dc8a91f02e Stats: 21664 lines in 81 files changed: 4429 ins; 4294 del; 12941 mod 8286256: Update libxml2 to 2.9.14 8286257: Update libxslt to 1.1.35 Backport-of: d6770034a66a294b8740875c28871f2b4677aef2 - PR: https://git.openjdk.java.net/jfx11u/pull/101
[jfx17u] RFR: 8283869: Update attribution in webkit.md file
Clean backport to `jfx17u`. - Commit messages: - 8283869: Update attribution in webkit.md file Changes: https://git.openjdk.java.net/jfx17u/pull/62/files Webrev: https://webrevs.openjdk.java.net/?repo=jfx17u=62=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8283869 Stats: 5484 lines in 1 file changed: 5483 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jfx17u/pull/62.diff Fetch: git fetch https://git.openjdk.java.net/jfx17u pull/62/head:pull/62 PR: https://git.openjdk.java.net/jfx17u/pull/62
[jfx17u] RFR: 8286256: Update libxml2 to 2.9.14
Clean backport to `jfx17u`. - Commit messages: - 8286256: Update libxml2 to 2.9.14 Changes: https://git.openjdk.java.net/jfx17u/pull/61/files Webrev: https://webrevs.openjdk.java.net/?repo=jfx17u=61=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8286256 Stats: 21664 lines in 81 files changed: 4429 ins; 4294 del; 12941 mod Patch: https://git.openjdk.java.net/jfx17u/pull/61.diff Fetch: git fetch https://git.openjdk.java.net/jfx17u pull/61/head:pull/61 PR: https://git.openjdk.java.net/jfx17u/pull/61
Re: RFR: 8088420: JavaFX WebView memory leak via EventListener [v4]
On Sun, 22 May 2022 14:06:41 GMT, Jay Bhaskar wrote:
>> This PR is new implementation of JavaEvent listener memory management.
>> Issue
>> [JDK-8088420](https://bugs.openjdk.java.net/browse/JDK-8088420?filter=-1)
>>
>> 1. Calling remove event listener does not free jni global references.
>> 2. When WebView goes out of scope (disposed from app) , its Event Listeners
>> are not being garbage collected.
>>
>> Solution:
>> 1. Detached the jni global reference from JavaEventListener.
>> 2. Create scoped ref counted wrapper class JavaObjectWrapperHandler for jni
>> global reference.
>> 3. Create unique JavaObjectWrapperHandler object for each JavaEventListener.
>> 4. EventListenerManager is a singleton class , which stores the
>> JavaObjectWrapperHandler mapped with JavaEventListener.
>> 5. EventListenerManager also stores the JavaEventListener mapped with
>> DOMWindow.
>> 6. When Event listener explicitly removed , JavaEventListener is being
>> forwarded to EventListenerManager to clear the listener.
>> 7. When WebView goes out of scope, EventListenerManager will de-registered
>> all the event listeners based on the ref counts attached with WebView
>> DOMWindow.
>
> Jay Bhaskar has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Adding JGObject in plave of raw jni object
I left a couple comments and one question on the changes to the fix.
The new tests look good. I left a few comments and suggestions on the test, but
overall they are good additions to verify the fix.
modules/javafx.web/src/main/native/Source/WebCore/bindings/java/EventListenerManager.cpp
line 29:
> 27: #include "JavaEventListener.h"
> 28: #include "DOMWindow.h"
> 29: #include
You probably don't need to include `stdio.h`
modules/javafx.web/src/main/native/Source/WebCore/bindings/java/EventListenerManager.cpp
line 99:
> 97: }
> 98:
> 99: void EventListenerManager::resetDOMWindow(DOMWindow* window)
Have you validated this logic? If I understand correctly, the
`isReferringToOtherListener` var will be true if any listener in the list of
listeners for this DOM window has a ref count > 1. Is my understanding correct?
This doesn't seem quite right to me, but I may be missing something.
modules/javafx.web/src/main/native/Source/WebCore/bindings/java/EventListenerManager.h
line 53:
> 51: JavaObjectWrapperHandler(const JLObject& handler) {
> 52: JNIEnv *env = nullptr;
> 53: env = JavaScriptCore_GetJavaEnv();
You can remove these two lines, since `env` is now unused here.
modules/javafx.web/src/main/native/Source/WebCore/bindings/java/EventListenerManager.h
line 60:
> 58: JNIEnv *env = nullptr;
> 59: env = JavaScriptCore_GetJavaEnv();
> 60: env->DeleteGlobalRef(handler_);
I think you can replace these three lines with: `handler_.clear();`
modules/javafx.web/src/test/java/test/javafx/scene/web/EventListenerLeakTest.java
line 96:
> 94: }
> 95:
> 96: static class MyListener1 implements EventListener {
There is no need for another subclass of `EventListener`. It is unused and can
be removed.
modules/javafx.web/src/test/java/test/javafx/scene/web/EventListenerLeakTest.java
line 630:
> 628: */
> 629: @Test
> 630: public void TestStrongRefNewContentLoad() throws Exception {
You should set `webView2 = null` here
modules/javafx.web/src/test/java/test/javafx/scene/web/EventListenerLeakTest.java
line 657:
> 655: });
> 656:
> 657: // Verify that all listeners have not been released
This comment is not right. It should be something like:
// Verify that the click event is not delivered to the event handler.
modules/javafx.web/src/test/java/test/javafx/scene/web/EventListenerLeakTest.java
line 664:
> 662: listeners1.clear();
> 663: domNodes1.clear();
> 664: webViewRefs.clear();
This makes the subsequent call to `assertNumActive` ineffective. There should
never be a need for any test method to explicitly modify the global refs lists.
modules/javafx.web/src/test/java/test/javafx/scene/web/EventListenerLeakTest.java
line 672:
> 670: // Verify that no listeners are strongly held
> 671: assertNumActive("MyListener", listenerRefs, 0);
> 672: listenerRefs.clear();
It is not necessary to clear the `listenerRefs` list. There is never a need for
a test to explicitly modify this list.
modules/javafx.web/src/test/java/test/javafx/scene/web/EventListenerLeakTest.java
line 676:
> 674:
> 675: /**
> 676: * Test that the listener ref cont increase on addevent and decrease
> on remove event
Typo: cont -> count
modules/javafx.web/src/test/java/test/javafx/scene/web/EventListenerLeakTest.java
line 706:
> 704: });
> 705:
> 706: // Verify that the events are delivered to the listeners (0 and
> 2 are same)
Actually all three refer to the same listener.
Re: RFR: 8088420: JavaFX WebView memory leak via EventListener [v2]
On Thu, 19 May 2022 22:01:31 GMT, Kevin Rushforth wrote:
>> Jay Bhaskar has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Platform.exit() , removing code block, as it is causing other test fail
>
> modules/javafx.web/src/test/java/test/javafx/scene/web/EventListenerLeakTest.java
> line 637:
>
>> 635: });
>> 636:
>> 637: assertEquals("Click count", 1,
>> listeners1.get(0).getClickCount());
>
> You should add a comment that this check is testing that the immediately
> previous click does _not_ get delivered since the associated DOM node is not
> part of the page any more. This is why the count remains at 1 (from the first
> click on the original page).
>
>
> Also, I think it would be useful here to clear the references to the
> listeners and WebView and make sure that the listener attached to the
> previously loaded page for this WebView gets released. Something like this as
> the final statements of the method:
>
>
> // Clear strong reference to listener and WebView
> listeners1.clear();
> webView1 = null;
>
> // Verify that there is no strong reference to the WebView
> assertNumActive("WebView", webViewRefs, 0);
>
> // Verify that no listeners are strongly held
> assertNumActive("MyListener", listenerRefs, 0);
You will also need to clear the references to the DOM nodes, and set `webView2
= null;` for this to work.
-
PR: https://git.openjdk.java.net/jfx/pull/799
Re: RFR: 8088420: JavaFX WebView memory leak via EventListener [v2]
On Thu, 19 May 2022 15:57:46 GMT, Jay Bhaskar wrote:
>> This PR is new implementation of JavaEvent listener memory management.
>> Issue
>> [JDK-8088420](https://bugs.openjdk.java.net/browse/JDK-8088420?filter=-1)
>>
>> 1. Calling remove event listener does not free jni global references.
>> 2. When WebView goes out of scope (disposed from app) , its Event Listeners
>> are not being garbage collected.
>>
>> Solution:
>> 1. Detached the jni global reference from JavaEventListener.
>> 2. Create scoped ref counted wrapper class JavaObjectWrapperHandler for jni
>> global reference.
>> 3. Create unique JavaObjectWrapperHandler object for each JavaEventListener.
>> 4. EventListenerManager is a singleton class , which stores the
>> JavaObjectWrapperHandler mapped with JavaEventListener.
>> 5. EventListenerManager also stores the JavaEventListener mapped with
>> DOMWindow.
>> 6. When Event listener explicitly removed , JavaEventListener is being
>> forwarded to EventListenerManager to clear the listener.
>> 7. When WebView goes out of scope, EventListenerManager will de-registered
>> all the event listeners based on the ref counts attached with WebView
>> DOMWindow.
>
> Jay Bhaskar has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Platform.exit() , removing code block, as it is causing other test fail
The fix looks good with a couple questions about removing entries from the map
when they are done and some cleanup comments.
In addition to the inline comments I left about the test, I think the following
scenarios should be added to the automated test:
1. Multiple listeners single webiew implicit release (i.e., WebView goes out of
scope)
2. Multiple listeners multiple webiew with explicit release of one webview and
implicit release of the other (this one is basically a combination of the
others)
3. Ref count test (single webview is fine) with adding and removing listeners.
It should handle the cases of both increasing and decreasing the ref count, and
adding the listener to another DOM node after its ref count has gone to zero to
make sure that case works.
modules/javafx.web/src/main/native/Source/WebCore/bindings/java/EventListenerManager.cpp
line 57:
> 55: if (it->second && it->second->use_count() == 1) {
> 56: delete it->second;
> 57: it->second = nullptr;
Do you need to remove the item from the list in this case?
modules/javafx.web/src/main/native/Source/WebCore/bindings/java/EventListenerManager.cpp
line 91:
> 89: for (win_it = windowHasEvent.begin(); win_it != windowHasEvent.end();
> win_it++) {
> 90: // de register associated event listeners with window
> 91: if(window == win_it->second) {
Minor: add space after the `if`.
modules/javafx.web/src/main/native/Source/WebCore/bindings/java/EventListenerManager.cpp
line 92:
> 90: // de register associated event listeners with window
> 91: if(window == win_it->second) {
> 92: unregisterListener(win_it->first);
Same question as earlier: do you need to also remove the entries matching the
`window` from the list here?
modules/javafx.web/src/main/native/Source/WebCore/bindings/java/EventListenerManager.h
line 48:
> 46:
> 47: class JavaObjectWrapperHandler {
> 48: jobject handler_;
Have you considered using `JGObject` here instead of raw `jobject`?
modules/javafx.web/src/main/native/Source/WebCore/bindings/java/JavaEventListener.cpp
line 50:
> 48: ? static_cast JavaEventListener*>()
> 49: : nullptr;
> 50: return jother && ( this == jother);
Minor: you can remove the space after `(`
modules/javafx.web/src/main/native/Source/WebCore/bindings/java/JavaEventListener.h
line 32:
> 30: #include "Node.h"
> 31:
> 32: #if PLATFORM(JAVA)
This is a java platform-specific class, so you don't need the `#if
PLATFORM(JAVA)` in this file.
modules/javafx.web/src/test/java/test/javafx/scene/web/EventListenerLeakTest.java
line 43:
> 41: import javafx.scene.web.WebEngine;
> 42: import javafx.scene.web.WebView;
> 43: import org.junit.AfterClass;
This import is no longer used.
modules/javafx.web/src/test/java/test/javafx/scene/web/EventListenerLeakTest.java
line 53:
> 51: import org.junit.Before;
> 52: import org.w3c.dom.Document;
> 53: import org.w3c.dom.NodeList;
Minor: maybe move these up to the previous block (with the ordinary imports?
modules/javafx.web/src/test/java/test/javafx/scene/web/EventListenerLeakTest.java
line 631:
> 629:
> 630: // Verify that all listeners have not been released
> 631: Thread.sleep(100);
The sleep should be right before the call to `getClickCount` (as in other cases
in the test).
modules/javafx.web/src/test/java/test/javafx/scene/web/EventListenerLeakTest.java
line 637:
> 635: });
> 636:
> 637: assertEquals("Click count", 1,
>
Re: RFR: 8088420: JavaFX WebView memory leak via EventListener
On Thu, 19 May 2022 13:13:01 GMT, Jay Bhaskar wrote:
> This PR is new implementation of JavaEvent listener memory management.
> Issue
> [JDK-8088420](https://bugs.openjdk.java.net/browse/JDK-8088420?filter=-1)
>
> 1. Calling remove event listener does not free jni global references.
> 2. When WebView goes out of scope (disposed from app) , its Event Listeners
> are not being garbage collected.
>
> Solution:
> 1. Detached the jni global reference from JavaEventListener.
> 2. Create scoped ref counted wrapper class JavaObjectWrapperHandler for jni
> global reference.
> 3. Create unique JavaObjectWrapperHandler object for each JavaEventListener.
> 4. EventListenerManager is a singleton class , which stores the
> JavaObjectWrapperHandler mapped with JavaEventListener.
> 5. EventListenerManager also stores the JavaEventListener mapped with
> DOMWindow.
> 6. When Event listener explicitly removed , JavaEventListener is being
> forwarded to EventListenerManager to clear the listener.
> 7. When WebView goes out of scope, EventListenerManager will de-registered
> all the event listeners based on the ref counts attached with WebView
> DOMWindow.
While testing, I noticed one problem in that the new unit tests calls
`Platform.exit()` (this was my fault for adding it when sending you the test
case), causing any subsequent web test to fail.
modules/javafx.web/src/test/java/test/javafx/scene/web/EventListenerLeakTest.java
line 113:
> 111: public static void cleanupOnce() {
> 112: Platform.exit();
> 113: }
I just noticed that this will cause subsequent tests to fail. Unit tests should
not call `Platform.exit()` (as opposed to system tests, which should). You can
remove the entire method.
-
PR: https://git.openjdk.java.net/jfx/pull/799
Re: RFR: 8090522: Allow support for disabling stage iconification (minimization)
On Thu, 19 May 2022 12:44:44 GMT, Paweł Kruszczyński wrote: > `Stage` minimalizing (iconified state) cannot be disabled for the user. The > same for `Stage` closable state. > PR adds these features without breaking backward compatibility. Marking this as "Draft" since it is not ready for review. - PR: https://git.openjdk.java.net/jfx/pull/798
Re: RFR: 8090522: Allow support for disabling stage iconification (minimization)
On Thu, 19 May 2022 12:44:44 GMT, Paweł Kruszczyński wrote: > `Stage` minimalizing (iconified state) cannot be disabled for the user. The > same for `Stage` closable state. > PR adds these features without breaking backward compatibility. @xardif It is premature to review this PR. Please read the section on [New features / API additions](https://github.com/openjdk/jfx/blob/master/CONTRIBUTING.md#new-features--api-additions) in the [CONTRIBUTING](https://github.com/openjdk/jfx/blob/master/CONTRIBUTING.md) guide for necessary steps, which involves discussing any new feature on the openjfx-dev mailing list to see if there is general support for the idea. - PR: https://git.openjdk.java.net/jfx/pull/798
Re: RFR: 8286867: Update #getGlyphCode return a negative number
On Tue, 17 May 2022 13:08:10 GMT, Tomator wrote: > Hello,i have signed oca ,and pull /signed command ,so I'm just wait for OCA > to pass？ Yes, at this point you wait for your OCA to be processed. - PR: https://git.openjdk.java.net/jfx/pull/795
Re: RFR: 8286867: Update #getGlyphCode return a negative number
On Fri, 13 May 2022 12:21:32 GMT, Kevin Rushforth wrote: > * Submit a bug report with a test case at > [bugreport.java.com](https://bugreport.java.com/) I see that you already have submitted a bug report. You should get an email when it is made public. - PR: https://git.openjdk.java.net/jfx/pull/795
Re: RFR: 8286867: Update #getGlyphCode return a negative number
On Fri, 13 May 2022 05:36:03 GMT, Tomator wrote:
>> When I used BlueJ, I found a problem with Chinese display.
>> /javafx.graphics/com/sun/javafx/font/CompositeGlyphMapper.java#getGlyphCode
>> may return a negative number when no font library is specified in Linux,and
>> this could cause java.lang.ArrayIndexOutOfBoundsException error.So
>> javafx.graphics/com/sun/prism/impl/GlyphCache.java#getCachedGlyph shou
>> check the glyphCode.
>> The crash demo like this:
>> `import javafx.application.Application;
>> import javafx.scene.Scene;
>> import javafx.scene.control.Menu;
>> import javafx.scene.control.MenuBar;
>> import javafx.scene.control.MenuItem;
>> import javafx.scene.layout.BorderPane;
>> import javafx.stage.Stage;
>>
>> public class MenuExample extends Application {
>> public static void main(String[] args) {
>> launch(args);
>> }
>>
>> @Override
>> public void start(Stage primaryStage) throws Exception {
>> BorderPane root = new BorderPane();
>> Scene scene = new Scene(root,200,300);
>> MenuBar menubar = new MenuBar();
>> Menu FileMenu = new Menu("文本");
>> MenuItem filemenu1=new MenuItem("新建");
>> MenuItem filemenu2=new MenuItem("Save");
>> MenuItem filemenu3=new MenuItem("Exit");
>> Menu EditMenu=new Menu("Edit");
>> MenuItem EditMenu1=new MenuItem("Cut");
>> MenuItem EditMenu2=new MenuItem("Copy");
>> MenuItem EditMenu3=new MenuItem("Paste");
>> EditMenu.getItems().addAll(EditMenu1,EditMenu2,EditMenu3);
>> root.setTop(menubar);
>> FileMenu.getItems().addAll(filemenu1,filemenu2,filemenu3);
>> menubar.getMenus().addAll(FileMenu,EditMenu);
>> primaryStage.setScene(scene);
>> primaryStage.setTitle("TEST");
>> primaryStage.show();
>>
>> }
>> } `
>>
>> the error:
>> `java.lang.ArrayIndexOutOfBoundsException: Index -25 out of bounds for
>> length 32
>> at com.sun.prism.impl.GlyphCache.getCachedGlyph(GlyphCache.java:332)
>> at com.sun.prism.impl.GlyphCache.render(GlyphCache.java:148)
>> at
>> com.sun.prism.impl.ps.BaseShaderGraphics.drawString(BaseShaderGraphics.java:2101)
>> at com.sun.javafx.sg.prism.NGText.renderText(NGText.java:312)
>> at com.sun.javafx.sg.prism.NGText.renderContent2D(NGText.java:270)
>> at com.sun.javafx.sg.prism.NGShape.renderContent(NGShape.java:261)
>> at com.sun.javafx.sg.prism.NGNode.doRender(NGNode.java:2072)
>> at com.sun.javafx.sg.prism.NGNode.render(NGNode.java:1964)
>> at com.sun.javafx.sg.prism.NGGroup.renderContent(NGGroup.java:270)
>> at com.sun.javafx.sg.prism.NGRegion.renderContent(NGRegion.java:578)
>> at com.sun.javafx.sg.prism.NGNode.doRender(NGNode.java:2072)
>> at com.sun.javafx.sg.prism.NGNode.render(NGNode.java:1964)
>> at com.sun.javafx.sg.prism.NGGroup.renderContent(NGGroup.java:270)
>> at com.sun.javafx.sg.prism.NGRegion.renderContent(NGRegion.java:578)
>> at com.sun.javafx.sg.prism.NGNode.doRender(NGNode.java:2072)
>> at com.sun.javafx.sg.prism.NGNode.render(NGNode.java:1964)
>> at com.sun.javafx.sg.prism.NGGroup.renderContent(NGGroup.java:270)
>> at com.sun.javafx.sg.prism.NGRegion.renderContent(NGRegion.java:578)
>> at com.sun.javafx.sg.prism.NGNode.doRender(NGNode.java:2072)
>> at com.sun.javafx.sg.prism.NGNode.render(NGNode.java:1964)
>> at com.sun.javafx.sg.prism.NGGroup.renderContent(NGGroup.java:270)
>> at com.sun.javafx.sg.prism.NGRegion.renderContent(NGRegion.java:578)
>> at com.sun.javafx.sg.prism.NGNode.doRender(NGNode.java:2072)
>> at com.sun.javafx.sg.prism.NGNode.render(NGNode.java:1964)
>> at com.sun.javafx.sg.prism.NGGroup.renderContent(NGGroup.java:270)
>> at com.sun.javafx.sg.prism.NGRegion.renderContent(NGRegion.java:578)
>> at com.sun.javafx.sg.prism.NGNode.doRender(NGNode.java:2072)
>> at com.sun.javafx.sg.prism.NGNode.render(NGNode.java:1964)
>> at
>> com.sun.javafx.tk.quantum.ViewPainter.doPaint(ViewPainter.java:479)
>> at
>> com.sun.javafx.tk.quantum.ViewPainter.paintImpl(ViewPainter.java:328)
>> at
>> com.sun.javafx.tk.quantum.PresentingPainter.run(PresentingPainter.java:91)
>> `
>
> The crash demo like this:
> `import javafx.application.Application;
> import javafx.scene.Scene;
> import javafx.scene.control.Menu;
> import javafx.scene.control.MenuBar;
> import javafx.scene.control.MenuItem;
> import javafx.scene.layout.BorderPane;
> import javafx.stage.Stage;
>
> public class MenuExample extends Application {
> public static void main(String[] args) {
> launch(args);
> }
>
> @Override
> public void start(Stage primaryStage) throws Exception {
> BorderPane root = new BorderPane();
> Scene scene = new
Re: RFR: JDK-8286256 : Update libxml2 to 2.9.14 [v4]
On Wed, 18 May 2022 13:39:12 GMT, Hima Bindu Meda wrote: >> Updated libxml to version 2.9.14.As mentioned in UPDATING.txt, configured >> for windows , linux and Mac platforms and updated the files accordingly. >> >> Updated libxslt to version 1.1.35. Generated the config.h files for windows >> , Mac and linux platforms and updated accordingly. >> >> Verified the build on all the platforms and sanity testing looks fine.No >> regressions observed. > > Hima Bindu Meda has updated the pull request incrementally with one > additional commit since the last revision: > > Space correction Marked as reviewed by kcr (Lead). - PR: https://git.openjdk.java.net/jfx/pull/797
Re: RFR: JDK-8286256 : Update libxml2 to 2.9.14 [v3]
On Wed, 18 May 2022 07:26:56 GMT, Hima Bindu Meda wrote: >> Updated libxml to version 2.9.14.As mentioned in UPDATING.txt, configured >> for windows , linux and Mac platforms and updated the files accordingly. >> >> Updated libxslt to version 1.1.35. Generated the config.h files for windows >> , Mac and linux platforms and updated accordingly. >> >> Verified the build on all the platforms and sanity testing looks fine.No >> regressions observed. > > Hima Bindu Meda has updated the pull request incrementally with one > additional commit since the last revision: > > Update version inof related steps @johanvos or @tiainen Can one of you be the second reviewer on this? - PR: https://git.openjdk.java.net/jfx/pull/797
Re: RFR: JDK-8286256 : Update libxml2 to 2.9.14 [v2]
On Tue, 17 May 2022 17:25:16 GMT, Kevin Rushforth wrote: >> Hima Bindu Meda has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Add UPDATING.txt for libxslt update > > modules/javafx.web/src/main/native/Source/ThirdParty/libxslt/linux/libexslt/exsltconfig.h > line 21: > >> 19: * the version string like "1.2.3" >> 20: */ >> 21: #define LIBEXSLT_DOTTED_VERSION "0.8.20" > > Shouldn't this be `1.1.35`? Actually, now that I see it, it looks correct as is, so you can ignore this question. - PR: https://git.openjdk.java.net/jfx/pull/797
Re: RFR: JDK-8286256 : Update libxml2 to 2.9.14 [v3]
On Wed, 18 May 2022 07:26:56 GMT, Hima Bindu Meda wrote: >> Updated libxml to version 2.9.14.As mentioned in UPDATING.txt, configured >> for windows , linux and Mac platforms and updated the files accordingly. >> >> Updated libxslt to version 1.1.35. Generated the config.h files for windows >> , Mac and linux platforms and updated accordingly. >> >> Verified the build on all the platforms and sanity testing looks fine.No >> regressions observed. > > Hima Bindu Meda has updated the pull request incrementally with one > additional commit since the last revision: > > Update version inof related steps The updated instructions look good with one minor issue (a missing space between a `.` and the start of the next sentence). modules/javafx.web/src/main/native/Source/ThirdParty/libxml/UPDATING.txt line 44: > 42: 9.2 Copy libxml\src\config.h to libxml\linux\config.h > 43: > 44: 10. Update version info in > 'modules/javafx.web/src/main/legal/libxml.md'.Also, update copyright if any > new files are added. Minor: missing space before `Also`. modules/javafx.web/src/main/native/Source/ThirdParty/libxslt/UPDATING.txt line 37: > 35: 9.1 Copy `libxslt/src/config.h` to `libxslt/linux/config.h` and follow > same guidelines as Windows to retain changes from our repo. > 36: > 37: 10. Update version info in > 'modules/javafx.web/src/main/legal/libxslt.md'.Also, update copyright if any > new files are added. Minor: missing space before `Also`. - Marked as reviewed by kcr (Lead). PR: https://git.openjdk.java.net/jfx/pull/797
Re: Looked-up color fails for -fx-background-color in JavaFX CSS file
Hi Davide, Are you referring to https://bugs.openjdk.java.net/browse/JDK-8268657 ? I just tried it again, and it works fine for me. -- Kevin On 5/18/2022 4:12 AM, Davide Perini wrote: Hi all, someone else opened an issue in past on this problem. I have the exact same problem: https://mail.openjdk.java.net/pipermail/openjfx-dev/2021-June/030723.html Kevin closed the issue because it was not able to reproduce but I can reproduce it on JavaFX 18.0.1 Can you double check it please? Thanks Davide
Re: RFR: JDK-8286256 : Update libxml2 to 2.9.14 [v2]
On Tue, 17 May 2022 17:06:51 GMT, Hima Bindu Meda wrote: >> Updated libxml to version 2.9.14.As mentioned in UPDATING.txt, configured >> for windows , linux and Mac platforms and updated the files accordingly. >> >> Updated libxslt to version 1.1.35. Generated the config.h files for windows >> , Mac and linux platforms and updated accordingly. >> >> Verified the build on all the platforms and sanity testing looks fine.No >> regressions observed. > > Hima Bindu Meda has updated the pull request incrementally with one > additional commit since the last revision: > > Add UPDATING.txt for libxslt update The code changes look good. I tested it on 3 platforms. All good. I left a few inline comments, including a couple questions about whitespace diffs. I notice that there are 27 files that only have whitespace changes and no other diffs, as well as one more where the whitespace diffs are the majority of the changes. Is this intentional? modules/javafx.web/src/main/native/Source/ThirdParty/libxml/src/valid.c line 30: > 28: > 29: static xmlElementPtr xmlGetDtdElementDesc2(xmlDtdPtr dtd, const xmlChar > *name, > 30:int create); This file has many whitespace-only changes (more than 2,800 lines). There is only one actual diff for this file. Can you double check the tab conversion? modules/javafx.web/src/main/native/Source/ThirdParty/libxslt/UPDATING.txt line 15: > 13: > cscript configure.js compiler=msvc xslt_debug=no iconv=no > mem_debug=no modules=no zlib=no profiler=no trio=no > 14: - Above command generates a header file 'src/config.h' file (on > all platforms) and libxslt\src\libxslt\xsltconfig.h. > 15: 4.1 Copy `libxslt\src\config.h` to `libxslt\win32\config.h'. config.h > file defines several macros to control libxslt features. We do not require > all of the features to be enabled. Minor: trailing whitespace (since this is a text file, jcheck won't complain, but since you will be editing this file anyway, maybe you can fix this). modules/javafx.web/src/main/native/Source/ThirdParty/libxslt/UPDATING.txt line 36: > 34: 9.1 Copy `libxslt/src/config.h` to `libxslt/linux/config.h` and follow > same guidelines as Windows to retain changes from our repo. > 35: > 36: 10. Remove tabs and trailing whitespaces from source files(.h and .c). Can you add a step to update the version in `modules/javafx.web/src/main/legal/libxslt.md`. Please also add a similar step to `modules/javafx.web/src/main/native/Source/ThirdParty/libxml/UPDATING.txt` (which is otherwise unmodified). modules/javafx.web/src/main/native/Source/ThirdParty/libxslt/linux/libexslt/exsltconfig.h line 21: > 19: * the version string like "1.2.3" > 20: */ > 21: #define LIBEXSLT_DOTTED_VERSION "0.8.20" Shouldn't this be `1.1.35`? modules/javafx.web/src/main/native/Source/ThirdParty/libxslt/src/libxslt/attributes.c line 67: > 65: ((c) == 0x0D)) > 66: > 67: #define IS_BLANK_NODE(n)\ This file has only whitespace changes and no other changes. other than whitespace changes. Is this intentional? - PR: https://git.openjdk.java.net/jfx/pull/797
Re: RFR: 8284665: First selected item of a TreeItem multiple selection gets removed if new items are constantly added to the TreeTableView
On Thu, 5 May 2022 16:21:45 GMT, Jose Pereda wrote: > This PR fixes an issue with selection of multiple items in TableView and > TreeTableView controls that gets moved unexpectedly when new items are added > even way below the selected items. > > A couple of tests have been added. They fail without this PR (first selected > item gets deselected, and table anchor gets moved), and pass with this PR > (first selected item keeps selected, and table anchor remains at the same > place). This looks simple enough that a single reviewer is sufficient. - PR: https://git.openjdk.java.net/jfx/pull/790
Re: RFR: 8286552: TextFormatter: UpdateValue/UpdateText is called, when no ValueConverter is set [v2]
On Fri, 13 May 2022 08:28:50 GMT, Marius Hanl wrote:
>> A common reason for using the `TextFormatter` is the need for a `filter`.
>> Therefore, the following constructor is typically used:
>> `public TextFormatter(@NamedArg("filter") UnaryOperator filter) {
>> ... }`
>>
>> With that, no `valueConverter` is set in the `TextFormatter`.
>>
>> When a `TextField` will commit his value, `TextFormatter.updateValue(...)`
>> is called.
>> Since `valueConverter` is null, an NPE is thrown and catched inside it and
>> `TextFormatter.updateText()` is called (which will call
>> `TextInputControl.updateText(...)`), which won't do anything either since
>> `valueConverter` is still not set (=null).
>>
>> A minor improvement here is to just skip `TextFormatter.updateValue(...)`
>> and therefore `TextFormatter.updateText()` (->
>> `TextInputControl.updateText(...)`), since this methods won't do anything
>> without a `valueConverter`.
>> With that change, no exception (NPE) is thrown and catched, and therefore
>> also exception breakpoints are not called (can be annoying ;)).
>> This will also slightly increase the performance, as throwing exceptions is
>> a (minor) bottleneck.
>>
>> Added tests for all `TextFormatter` functionalities, which pass before and
>> after.
>
> Marius Hanl has updated the pull request incrementally with one additional
> commit since the last revision:
>
> 8286552: Added space and revert typo fix
LGTM
-
Marked as reviewed by kcr (Lead).
PR: https://git.openjdk.java.net/jfx/pull/794
Re: RFR: 8181084: JavaFX show big icons in system menu on macOS with Retina display [v3]
On Sun, 8 May 2022 06:31:49 GMT, Paul wrote:
>> I hit on JDK-8181084 while making some changes to Scene Builder, so I
>> decided to investigate.
>>
>> Please note: I have not done any Objective-C or MacOS development before. So
>> I would really like some feedback from someone else who knows this stuff
>> better.
>>
>> Anyway, after some googling, I discovered that MacOS uses points values for
>> measurements and not pixels, so the actual fix for this issue was this block
>> in `GlassMenu.m`:-
>>
>>
>> if ((sx > 1) && (sy > 1) && (width > 1) && (height > 1)) {
>> NSSize imgSize = {width / sx, height / sy};
>> [image setSize: imgSize];
>> }
>>
>>
>> The rest of the changes were needed in order to get the `scaleX` and
>> `scaleY` values down from `PixelUtils.java` into `GlassMenu.m`.
>>
>> Before this fix:-
>>
>> > src="https://user-images.githubusercontent.com/437990/155880981-92163087-696b-4442-b047-845c276deb27.png;>
>>
>> After this fix:-
>>
>> > src="https://user-images.githubusercontent.com/437990/155880985-6bff0a06-9aee-4db2-b696-64730b0a6feb.png;>
>
> Paul has updated the pull request with a new target base due to a merge or a
> rebase. The incremental webrev excludes the unrelated changes brought in by
> the merge/rebase. The pull request contains five additional commits since the
> last revision:
>
> - Updated variable names to be a bit more consistent
> - Merge branch 'master' into JDK-8181084
> - Merge branch 'master' into JDK-8181084
> - Removing trailing whitespace.
> - Correctly scales hi-res icons in MacOS system menu items
I'll look at this in more detail later. You will likely need some error
checking when calling the JNI methods (even though they should not ever return
an error), and you might want to cache the class and field IDs it in `initIDs`.
This will need a testing on all platforms, since the change to create a Pixel
object using the scale is in shared code. Two things you could do to help with
this.
1. Enable GitHub actions runs in your personal fork of the jfx repo. You will
need to push some change (and empty commit is fine) to trigger it, once you do.
This will run the headless tests on all platforms, so is really just a "smoke
test".
2. Run the full set of tests on at least macOS by doing this:
gradle --continue --info -PFULL_TEST=true -PUSE_ROBOT=true test -x :web:test
-
PR: https://git.openjdk.java.net/jfx/pull/743
Re: RFR: 8286552: TextFormatter: UpdateValue/UpdateText is called, when no ValueConverter is set
On Tue, 10 May 2022 20:03:08 GMT, Marius Hanl wrote:
> A common reason for using the `TextFormatter` is the need for a `filter`.
> Therefore, the following constructor is typically used:
> `public TextFormatter(@NamedArg("filter") UnaryOperator filter) { ...
> }`
>
> With that, no `valueConverter` is set in the `TextFormatter`.
>
> When a `TextField` will commit his value, `TextFormatter.updateValue(...)` is
> called.
> Since `valueConverter` is null, an NPE is thrown and catched inside it and
> `TextFormatter.updateText()` is called (which will call
> `TextInputControl.updateText(...)`), which won't do anything either since
> `valueConverter` is still not set (=null).
>
> A minor improvement here is to just skip `TextFormatter.updateValue(...)` and
> therefore `TextFormatter.updateText()` (->
> `TextInputControl.updateText(...)`), since this methods won't do anything
> without a `valueConverter`.
> With that change, no exception (NPE) is thrown and catched, and therefore
> also exception breakpoints are not called (can be annoying ;)).
> This will also slightly increase the performance, as throwing exceptions is a
> (minor) bottleneck.
>
> Added tests for all `TextFormatter` functionalities, which pass before and
> after.
The fix and tests look good, with a couple requested changes.
modules/javafx.controls/src/main/java/javafx/scene/control/TextFormatter.java
line 40:
> 38: *
> 39: * A filter ({@link #getFilter()}) that can intercept and modify
> user input. This helps to keep the text
> 40: * in the desired format. A default text supplier can be used to
> provide the initial text.
I know this is a simple typo, but it is unrelated to your bug fix, and is in
public API docs, so I'd like to see it go in separately under a "Fix mistakes
in docs" bug. I filed
[JDK-8286678](https://bugs.openjdk.java.net/browse/JDK-8286678) to track this
and any other such issues that arise (as we've done for most recent releases).
modules/javafx.controls/src/main/java/javafx/scene/control/TextFormatter.java
line 202:
> 200:
> 201: void updateValue(String text) {
> 202: if (valueConverter != null &&!value.isBound()) {
Minor: please add a space between the `&&` and `!` operators.
-
PR: https://git.openjdk.java.net/jfx/pull/794
[jfx17u] Integrated: 8283218: Update GStreamer to 1.20.1
On Mon, 9 May 2022 21:07:39 GMT, Kevin Rushforth wrote: > Almost clean backport to `jfx17u`. Tested in connection with libffi update in > the `test-kcr-17.0.4` branch. > > The only difference from the mainline patch is that the following file is not > present in `jfx17u`, so that part of the patch was skipped: > > > modules/javafx.media/src/main/native/gstreamer/gstreamer-lite/gst-plugins-good/gst/audioparsers/gstaacparse.c This pull request has now been integrated. Changeset: 103cac04 Author:Kevin Rushforth URL: https://git.openjdk.java.net/jfx17u/commit/103cac04472d3eddb1f28a6a0b4f8d60f5ce3d7f Stats: 38921 lines in 412 files changed: 22007 ins; 5981 del; 10933 mod 8283218: Update GStreamer to 1.20.1 8283403: Update Glib to 2.72.0 Reviewed-by: jvos Backport-of: c4b1a72cc4d9253d1320d83281d50fb1f3bb6145 - PR: https://git.openjdk.java.net/jfx17u/pull/54
[jfx17u] Integrated: 8280840: Update libFFI to 3.4.2
On Mon, 9 May 2022 21:05:17 GMT, Kevin Rushforth wrote: > Clean backport to `jfx17u`. Tested in connection with gstreamer / glib update > in the `test-kcr-17.0.4` branch. This pull request has now been integrated. Changeset: 1c376ebd Author: Kevin Rushforth URL: https://git.openjdk.java.net/jfx17u/commit/1c376ebd51d59ae7b617cb84c5f1e04a80d87b0a Stats: 3475 lines in 34 files changed: 927 ins; 38 del; 2510 mod 8280840: Update libFFI to 3.4.2 Backport-of: 1beb3235223452929ec951ee18dd30a5345307cf - PR: https://git.openjdk.java.net/jfx17u/pull/53
Re: What can I do with the GStreamer bundled in JavaFX? Can I record the screen?
GStreamer is used by the JavaFX media API. It is an implementation detail, and is not exposed. There are no media capture APIs, although that is something we have considered providing in the future. -- Kevin On 5/10/2022 1:02 PM, Davide Perini wrote: Hi all, I haven't understood what part of GStreamer is included in JavaFX. Can I record the screen with it using d3d11screencapturesrc or ximagesrc? Thanks Davide
Integrated: 8283869: Update attribution in webkit.md file
On Tue, 10 May 2022 16:37:57 GMT, Kevin Rushforth wrote: > Doc-only change to update the license attribution in our `webkit.md` file. This pull request has now been integrated. Changeset: 7bb48194 Author: Kevin Rushforth URL: https://git.openjdk.java.net/jfx/commit/7bb4819409dd617ba2e3658ee66f23b94dc0bec1 Stats: 5484 lines in 1 file changed: 5483 ins; 0 del; 1 mod 8283869: Update attribution in webkit.md file Reviewed-by: arapte - PR: https://git.openjdk.java.net/jfx/pull/793
RFR: 8283869: Update attribution in webkit.md file
Doc-only change to update the license attribution in our `webkit.md` file. - Commit messages: - 8283869: Update attribution in webkit.md file Changes: https://git.openjdk.java.net/jfx/pull/793/files Webrev: https://webrevs.openjdk.java.net/?repo=jfx=793=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8283869 Stats: 5484 lines in 1 file changed: 5483 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jfx/pull/793.diff Fetch: git fetch https://git.openjdk.java.net/jfx pull/793/head:pull/793 PR: https://git.openjdk.java.net/jfx/pull/793
Re: [jfx17u] RFR: 8283218: Update GStreamer to 1.20.1
On Mon, 9 May 2022 21:07:39 GMT, Kevin Rushforth wrote: > Almost clean backport to `jfx17u`. Tested in connection with libffi update in > the `test-kcr-17.0.4` branch. > > The only difference from the mainline patch is that the following file is not > present in `jfx17u`, so that part of the patch was skipped: > > > modules/javafx.media/src/main/native/gstreamer/gstreamer-lite/gst-plugins-good/gst/audioparsers/gstaacparse.c Reviewer: @johanvos - PR: https://git.openjdk.java.net/jfx17u/pull/54
[jfx17u] Integrated: 8280275: JUnit5 tests using Assumptions API fail to compile in some cases
On Mon, 9 May 2022 21:20:29 GMT, Kevin Rushforth wrote: > This PR is dependent on #58 so it is in Draft for now. Once #58 is > integrated, I will rebase this and submit it, at which time it will be a > clean backport, and the diffs will only show the updated test and build > changes associated with just this fix. This pull request has now been integrated. Changeset: c8eafd82 Author:Kevin Rushforth URL: https://git.openjdk.java.net/jfx17u/commit/c8eafd827b5aff73e4fb14b2721bdf45c6f3dc0d Stats: 6 lines in 2 files changed: 4 ins; 1 del; 1 mod 8280275: JUnit5 tests using Assumptions API fail to compile in some cases Backport-of: 94807b6edfb9af55be353cab237e8e64007c61dc - PR: https://git.openjdk.java.net/jfx17u/pull/59
[jfx17u] RFR: 8280275: JUnit5 tests using Assumptions API fail to compile in some cases
This PR is dependent on #58 so it is in Draft for now. Once #58 is integrated, I will rebase this and submit it, at which time it will be a clean backport, and the diffs will only show the updated test and build changes associated with just this fix. - Commit messages: - 8280275: JUnit5 tests using Assumptions API fail to compile in some cases Changes: https://git.openjdk.java.net/jfx17u/pull/59/files Webrev: https://webrevs.openjdk.java.net/?repo=jfx17u=59=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8280275 Stats: 6 lines in 2 files changed: 4 ins; 1 del; 1 mod Patch: https://git.openjdk.java.net/jfx17u/pull/59.diff Fetch: git fetch https://git.openjdk.java.net/jfx17u pull/59/head:pull/59 PR: https://git.openjdk.java.net/jfx17u/pull/59
[jfx17u] Integrated: 8276142: Update gradle to version 7.3
On Mon, 9 May 2022 21:14:23 GMT, Kevin Rushforth wrote: > Clean backport to `jfx17u`. This pull request has now been integrated. Changeset: be207ddb Author: Kevin Rushforth URL: https://git.openjdk.java.net/jfx17u/commit/be207ddb7249b369a028bc6fb6985adda7841648 Stats: 227 lines in 7 files changed: 102 ins; 49 del; 76 mod 8276142: Update gradle to version 7.3 Backport-of: 13c24d22463436bc953ec5159beec7a78017019f - PR: https://git.openjdk.java.net/jfx17u/pull/56
[jfx17u] Integrated: 8276174: JavaFX build fails on macOS aarch64
On Mon, 9 May 2022 21:22:04 GMT, Kevin Rushforth wrote: > Clean backport to `jfx17u` so we can build on macOS / aarch64 without needing > to specify `-PCOMPILE_TARGET=arm64`. This pull request has now been integrated. Changeset: a5edae13 Author: Kevin Rushforth URL: https://git.openjdk.java.net/jfx17u/commit/a5edae1392699e7380a0b528eb1118889377cc02 Stats: 3 lines in 1 file changed: 2 ins; 0 del; 1 mod 8276174: JavaFX build fails on macOS aarch64 Backport-of: d289db94d15e49ed28f797b516ffccf023fce9c9 - PR: https://git.openjdk.java.net/jfx17u/pull/60
[jfx17u] Integrated: 8273998: Clarify specification for Window properties controlled by the window manager
On Mon, 9 May 2022 21:15:34 GMT, Kevin Rushforth wrote: > Clean backport to `jfx17u`. This pull request has now been integrated. Changeset: bdb8d28e Author: Kevin Rushforth URL: https://git.openjdk.java.net/jfx17u/commit/bdb8d28ecc7bb10278a9a8385da79b5f5652bb09 Stats: 84 lines in 2 files changed: 59 ins; 4 del; 21 mod 8273998: Clarify specification for Window properties controlled by the window manager Backport-of: 7a1a19c098e21572077c9c3d75cc2141fadc99f6 - PR: https://git.openjdk.java.net/jfx17u/pull/57
[jfx17u] Integrated: 8274274: Update JUnit to version 5.8.1
On Mon, 9 May 2022 21:18:07 GMT, Kevin Rushforth wrote: > Clean backport to `jfx17u`. This pull request has now been integrated. Changeset: 2ab0e82e Author: Kevin Rushforth URL: https://git.openjdk.java.net/jfx17u/commit/2ab0e82ee9cf2516b32914e2df4725becf8a2664 Stats: 161 lines in 5 files changed: 151 ins; 3 del; 7 mod 8274274: Update JUnit to version 5.8.1 Backport-of: ff6e8d50badd57549811391b1380707bb94ac55b - PR: https://git.openjdk.java.net/jfx17u/pull/58
[jfx17u] Integrated: 8281564: Update cmake to 3.22.3
On Mon, 9 May 2022 21:13:07 GMT, Kevin Rushforth wrote: > Clean backport to `jfx17u`. Tested along with the rest of the fixes in the > `test-kcr-17.0.4` branch. This pull request has now been integrated. Changeset: 684c80fc Author: Kevin Rushforth URL: https://git.openjdk.java.net/jfx17u/commit/684c80fc7c7149d77509936867e7a42dc88ed9ed Stats: 10 lines in 2 files changed: 0 ins; 0 del; 10 mod 8281564: Update cmake to 3.22.3 Backport-of: d960d639dc6e37de0cdb69075a31a17090b83a3d - PR: https://git.openjdk.java.net/jfx17u/pull/55
Proposed schedule for JavaFX 19
Here is the proposed schedule for JavaFX 19. RDP1: Jul 14, 2022 (aka “feature freeze”) RDP2: Aug 4, 2022 Freeze: Aug 25, 2022 GA: Sep 13, 2022 We plan to fork a jfx19 stabilization branch at RDP1. The GA date is one week ahead of JDK 19, matching what we did for 18. The start of RDP1, the start of RDP2, and the code freeze will be 16:00 UTC on the respective dates. Please let Johan or me know if you have any questions. -- Kevin
[jfx17u] RFR: 8276174: JavaFX build fails on macOS aarch64
Clean backport to `jfx17u` so we can build on macOS / aarch64 without needing to specify `-PCOMPILE_TARGET=arm64`. - Commit messages: - 8276174: JavaFX build fails on macOS aarch64 Changes: https://git.openjdk.java.net/jfx17u/pull/60/files Webrev: https://webrevs.openjdk.java.net/?repo=jfx17u=60=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8276174 Stats: 3 lines in 1 file changed: 2 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jfx17u/pull/60.diff Fetch: git fetch https://git.openjdk.java.net/jfx17u pull/60/head:pull/60 PR: https://git.openjdk.java.net/jfx17u/pull/60
[jfx17u] RFR: 8274274: Update JUnit to version 5.8.1
Clean backport to `jfx17u`. - Commit messages: - 8274274: Update JUnit to version 5.8.1 Changes: https://git.openjdk.java.net/jfx17u/pull/58/files Webrev: https://webrevs.openjdk.java.net/?repo=jfx17u=58=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8274274 Stats: 161 lines in 5 files changed: 151 ins; 3 del; 7 mod Patch: https://git.openjdk.java.net/jfx17u/pull/58.diff Fetch: git fetch https://git.openjdk.java.net/jfx17u pull/58/head:pull/58 PR: https://git.openjdk.java.net/jfx17u/pull/58
[jfx17u] RFR: 8273998: Clarify specification for Window properties controlled by the window manager
Clean backport to `jfx17u`. - Commit messages: - 8273998: Clarify specification for Window properties controlled by the window manager Changes: https://git.openjdk.java.net/jfx17u/pull/57/files Webrev: https://webrevs.openjdk.java.net/?repo=jfx17u=57=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8273998 Stats: 84 lines in 2 files changed: 59 ins; 4 del; 21 mod Patch: https://git.openjdk.java.net/jfx17u/pull/57.diff Fetch: git fetch https://git.openjdk.java.net/jfx17u pull/57/head:pull/57 PR: https://git.openjdk.java.net/jfx17u/pull/57
[jfx17u] RFR: 8276142: Update gradle to version 7.3
Clean backport to `jfx17u`. - Commit messages: - 8276142: Update gradle to version 7.3 Changes: https://git.openjdk.java.net/jfx17u/pull/56/files Webrev: https://webrevs.openjdk.java.net/?repo=jfx17u=56=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8276142 Stats: 227 lines in 7 files changed: 102 ins; 49 del; 76 mod Patch: https://git.openjdk.java.net/jfx17u/pull/56.diff Fetch: git fetch https://git.openjdk.java.net/jfx17u pull/56/head:pull/56 PR: https://git.openjdk.java.net/jfx17u/pull/56
Re: [jfx17u] RFR: 8281564: Update cmake to 3.22.3
On Mon, 9 May 2022 21:15:57 GMT, Kevin Rushforth wrote: > Reviewer: @arapte Since this is clean, a review is optional. - PR: https://git.openjdk.java.net/jfx17u/pull/55
[jfx17u] RFR: 8281564: Update cmake to 3.22.3
Clean backport to `jfx17u`. Tested along with the rest of the fixes in the `test-kcr-17.0.4` branch. - Commit messages: - 8281564: Update cmake to 3.22.3 Changes: https://git.openjdk.java.net/jfx17u/pull/55/files Webrev: https://webrevs.openjdk.java.net/?repo=jfx17u=55=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8281564 Stats: 10 lines in 2 files changed: 0 ins; 0 del; 10 mod Patch: https://git.openjdk.java.net/jfx17u/pull/55.diff Fetch: git fetch https://git.openjdk.java.net/jfx17u pull/55/head:pull/55 PR: https://git.openjdk.java.net/jfx17u/pull/55
Re: [jfx17u] RFR: 8281564: Update cmake to 3.22.3
On Mon, 9 May 2022 21:13:07 GMT, Kevin Rushforth wrote: > Clean backport to `jfx17u`. Tested along with the rest of the fixes in the > `test-kcr-17.0.4` branch. Reviewer: @arapte - PR: https://git.openjdk.java.net/jfx17u/pull/55
[jfx17u] RFR: 8283218: Update GStreamer to 1.20.1
Almost clean backport to `jfx17u`. Tested in connection with libffi update in the `test-kcr-17.0.4` branch. The only difference from the mainline patch is that the following file is not present in `jfx17u`, so that part of the patch was skipped: modules/javafx.media/src/main/native/gstreamer/gstreamer-lite/gst-plugins-good/gst/audioparsers/gstaacparse.c - Commit messages: - 8283218: Update GStreamer to 1.20.1 Changes: https://git.openjdk.java.net/jfx17u/pull/54/files Webrev: https://webrevs.openjdk.java.net/?repo=jfx17u=54=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8283218 Stats: 38921 lines in 412 files changed: 22007 ins; 5981 del; 10933 mod Patch: https://git.openjdk.java.net/jfx17u/pull/54.diff Fetch: git fetch https://git.openjdk.java.net/jfx17u pull/54/head:pull/54 PR: https://git.openjdk.java.net/jfx17u/pull/54
[jfx17u] RFR: 8280840: Update libFFI to 3.4.2
Clean backport to `jfx17u`. Tested in connection with gstreamer / glib update in the `test-kcr-17.0.4` branch. - Commit messages: - 8280840: Update libFFI to 3.4.2 Changes: https://git.openjdk.java.net/jfx17u/pull/53/files Webrev: https://webrevs.openjdk.java.net/?repo=jfx17u=53=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8280840 Stats: 3475 lines in 34 files changed: 927 ins; 38 del; 2510 mod Patch: https://git.openjdk.java.net/jfx17u/pull/53.diff Fetch: git fetch https://git.openjdk.java.net/jfx17u pull/53/head:pull/53 PR: https://git.openjdk.java.net/jfx17u/pull/53
Re: RFR: 8285534: Update the 3D lighting test sample [v3]
On Thu, 28 Apr 2022 17:28:34 GMT, Nir Lisker wrote: >> Update the the test utility. Includes: >> * Refactoring since there is no more need the split pre- and >> post-attenuation and light types. >> * Added customizable material to the `Boxes` to test the interaction between >> lights and materials.. >> * Light colors can now be changed. >> * Added ambient lights, >> >> Note that GitHub decided to count the removal of the `AttenLightingSample` >> and addition of the `Controls` file as renaming. The sample is now run now >> only through `LightingSample`. > > Nir Lisker has updated the pull request incrementally with one additional > commit since the last revision: > > Added ambient lights This looks like a nice improvement to the test app. - Marked as reviewed by kcr (Lead). PR: https://git.openjdk.java.net/jfx/pull/787
Re: RFR: 8285217: [Android] Window's screen is not updated after native screen was disposed [v3]
On Wed, 4 May 2022 16:55:30 GMT, Jose Pereda wrote: >> This PR updates the screen for each window even for the case where the old >> screen has been disposed but there is a new screen instance found for such >> window. >> >> This is the case of Android, where the lifecycle of the application allows >> destroying the native screen when the app goes to the background, and >> providing a new native screen, in case it comes back to the foreground. >> Before this PR, the screen for the window wasn't updated after returning >> from the background, and if orientation changes happened, the dimensions >> were wrong. > > Jose Pereda has updated the pull request incrementally with one additional > commit since the last revision: > > Scale screen dimensions I like this approach better. Since it just touches Monocle, and most changes are in the android port, perhaps @johanvos can review it? - PR: https://git.openjdk.java.net/jfx/pull/778
Re: RFR: 8283318: Videos with unusual sizes cannot be played on windows
On Tue, 19 Apr 2022 22:38:13 GMT, Alexander Matveev wrote: > Fixed by removing check which enables dynamic format change, since it > requires for portrait videos, not standard resolutions and anything that has > width > 1920 or/and height > 1080. Looks good. I confirm that it fixes the problem. This is a simple enough change that a single reviewer is sufficient. - Marked as reviewed by kcr (Lead). PR: https://git.openjdk.java.net/jfx/pull/775
Re: RFR: 8285217: [Android] Window's screen is not updated after native screen was disposed
On Wed, 20 Apr 2022 18:26:06 GMT, Jose Pereda wrote: > This PR updates the screen for each window even for the case where the old > screen has been disposed but there is a new screen instance found for such > window. > > This is the case of Android, where the lifecycle of the application allows > destroying the native screen when the app goes to the background, and > providing a new native screen, in case it comes back to the foreground. > Before this PR, the screen for the window wasn't updated after returning from > the background, and if orientation changes happened, the dimensions were > wrong. Since this fix is in shared code, have you tested this using multiple screens on all platforms? Are there new tests that could be written for this? - PR: https://git.openjdk.java.net/jfx/pull/778
[jfx11u] Integrated: 8280275: JUnit5 tests using Assumptions API fail to compile in some cases
On Sat, 30 Apr 2022 13:48:01 GMT, Kevin Rushforth wrote: > This PR is dependent on #97 so it is in Draft for now. Once #97 is > integrated, I will rebase this and submit it, at which time it will be a > clean backport, and the diffs will only show the updated test. This pull request has now been integrated. Changeset: f01fc732 Author: Kevin Rushforth URL: https://git.openjdk.java.net/jfx11u/commit/f01fc732ac0972a79c1c5b397b23df38ba3a2004 Stats: 6 lines in 2 files changed: 4 ins; 1 del; 1 mod 8280275: JUnit5 tests using Assumptions API fail to compile in some cases Backport-of: 94807b6edfb9af55be353cab237e8e64007c61dc - PR: https://git.openjdk.java.net/jfx11u/pull/98
[jfx11u] RFR: 8280275: JUnit5 tests using Assumptions API fail to compile in some cases
This PR is dependent on #97 so it is in Draft for now. Once #97 is integrated, I will rebase this and submit it, at which time it will be a clean backport, and the diffs will only show the updated test. - Commit messages: - 8280275: JUnit5 tests using Assumptions API fail to compile in some cases Changes: https://git.openjdk.java.net/jfx11u/pull/98/files Webrev: https://webrevs.openjdk.java.net/?repo=jfx11u=98=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8280275 Stats: 6 lines in 2 files changed: 4 ins; 1 del; 1 mod Patch: https://git.openjdk.java.net/jfx11u/pull/98.diff Fetch: git fetch https://git.openjdk.java.net/jfx11u pull/98/head:pull/98 PR: https://git.openjdk.java.net/jfx11u/pull/98
[jfx11u] Integrated: 8276174: JavaFX build fails on macOS aarch64
On Sat, 30 Apr 2022 13:51:23 GMT, Kevin Rushforth wrote: > Backport to `jfx11u` so we can build on macOS / aarch64 without needing to > specify `-PCOMPILE_TARGET=arm64`. It isn't a clean backport, since I also had > to include the definition of `IS_AARCH64` which is present in mainline, but > not in `jfx11u` (it was added as part of another unrelated fix that isn't > backported to `jfx11u`). This pull request has now been integrated. Changeset: ac52af6e Author:Kevin Rushforth URL: https://git.openjdk.java.net/jfx11u/commit/ac52af6ed3535f687e2026d2afd398aac4111e84 Stats: 4 lines in 1 file changed: 3 ins; 0 del; 1 mod 8276174: JavaFX build fails on macOS aarch64 Reviewed-by: jvos Backport-of: d289db94d15e49ed28f797b516ffccf023fce9c9 - PR: https://git.openjdk.java.net/jfx11u/pull/100
[jfx11u] Integrated: 8280840: Update libFFI to 3.4.2
On Sat, 30 Apr 2022 13:30:49 GMT, Kevin Rushforth wrote: > Clean backport to `jfx11u`. Tested in connection with gstreamer / glib update > in the `test-kcr-11.0.16` branch. This pull request has now been integrated. Changeset: 1e4862f4 Author: Kevin Rushforth URL: https://git.openjdk.java.net/jfx11u/commit/1e4862f436d17c409f0a33645a413acc771b731e Stats: 3475 lines in 34 files changed: 927 ins; 38 del; 2510 mod 8280840: Update libFFI to 3.4.2 Backport-of: 1beb3235223452929ec951ee18dd30a5345307cf - PR: https://git.openjdk.java.net/jfx11u/pull/93
[jfx11u] Integrated: 8283218: Update GStreamer to 1.20.1
On Sat, 30 Apr 2022 13:37:08 GMT, Kevin Rushforth wrote: > Backport to `jfx11u`. Tested in connection with libffi update in the > `test-kcr-11.0.16` branch. > > This was not a clean backport, but the merge conflicts were trivial to > resolve. Here is a summary of the changes. The jfx mainline patch applied > cleanly to all other files. > > 1. The following file is not present in jfx11u, so that part of the patch was > skipped: > > > modules/javafx.media/src/main/native/gstreamer/gstreamer-lite/gst-plugins-good/gst/audioparsers/gstaacparse.c > > > 2. The following files had minor merge conflicts, the first was a diff in > surrounding context and the rest were in copyright header blocks: > > > modules/javafx.media/src/main/native/gstreamer/projects/linux/fxplugins/Makefile > modules/javafx.media/src/main/native/jfxmedia/projects/linux/Makefile > modules/javafx.media/src/tools/native/def-glib.pl > modules/javafx.media/src/tools/native/def-gstlite.pl This pull request has now been integrated. Changeset: d0df8471 Author:Kevin Rushforth URL: https://git.openjdk.java.net/jfx11u/commit/d0df8471c4e38e0b0dfae0e83848316694033002 Stats: 38984 lines in 412 files changed: 22060 ins; 5959 del; 10965 mod 8283218: Update GStreamer to 1.20.1 8283403: Update Glib to 2.72.0 Reviewed-by: jvos Backport-of: c4b1a72cc4d9253d1320d83281d50fb1f3bb6145 - PR: https://git.openjdk.java.net/jfx11u/pull/94
[jfx11u] Integrated: 8274274: Update JUnit to version 5.8.1
On Sat, 30 Apr 2022 13:45:39 GMT, Kevin Rushforth wrote: > Nearly clean backport to `jfx11u`. It isn't a clean backport since the > mainline patch updated `gradle/verification-metadata.xml`, which doesn't > exist in 11. This pull request has now been integrated. Changeset: 44e691e4 Author: Kevin Rushforth URL: https://git.openjdk.java.net/jfx11u/commit/44e691e4811bac2202e2b8ee992f35937ec06f54 Stats: 63 lines in 4 files changed: 58 ins; 3 del; 2 mod 8274274: Update JUnit to version 5.8.1 Reviewed-by: arapte Backport-of: ff6e8d50badd57549811391b1380707bb94ac55b - PR: https://git.openjdk.java.net/jfx11u/pull/97
[jfx11u] Integrated: 8281564: Update cmake to 3.22.3
On Sat, 30 Apr 2022 13:39:32 GMT, Kevin Rushforth wrote: > Simple backport to `jfx11u`. It isn't a clean backport since the mainline > patch had an update to `gradle/verification-metadata.xml`, which doesn't > exist in 11. This pull request has now been integrated. Changeset: e877f305 Author: Kevin Rushforth URL: https://git.openjdk.java.net/jfx11u/commit/e877f305edbb9c00cee091470049f7873c4f208e Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod 8281564: Update cmake to 3.22.3 Reviewed-by: arapte Backport-of: d960d639dc6e37de0cdb69075a31a17090b83a3d - PR: https://git.openjdk.java.net/jfx11u/pull/95
[jfx11u] Integrated: 8276142: Update gradle to version 7.3
On Sat, 30 Apr 2022 13:43:09 GMT, Kevin Rushforth wrote: > Clean backport to `jfx11u`. This pull request has now been integrated. Changeset: e92f7fcd Author: Kevin Rushforth URL: https://git.openjdk.java.net/jfx11u/commit/e92f7fcd4f9e9bc89c91b06b1c7fa8296c4902bb Stats: 227 lines in 7 files changed: 102 ins; 49 del; 76 mod 8276142: Update gradle to version 7.3 Backport-of: 13c24d22463436bc953ec5159beec7a78017019f - PR: https://git.openjdk.java.net/jfx11u/pull/96
[jfx11u] Integrated: 8273998: Clarify specification for Window properties controlled by the window manager
On Sat, 30 Apr 2022 13:48:57 GMT, Kevin Rushforth wrote: > Clean backport to `jfx11u`. This pull request has now been integrated. Changeset: 1e62db92 Author: Kevin Rushforth URL: https://git.openjdk.java.net/jfx11u/commit/1e62db92944c545cc7dd333652819be1c50896ff Stats: 84 lines in 2 files changed: 59 ins; 4 del; 21 mod 8273998: Clarify specification for Window properties controlled by the window manager Backport-of: 7a1a19c098e21572077c9c3d75cc2141fadc99f6 - PR: https://git.openjdk.java.net/jfx11u/pull/99
Re: [jfx11u] RFR: 8276174: JavaFX build fails on macOS aarch64
On Sat, 30 Apr 2022 13:51:23 GMT, Kevin Rushforth wrote: > Backport to `jfx11u` so we can build on macOS / aarch64 without needing to > specify `-PCOMPILE_TARGET=arm64`. It isn't a clean backport, since I also had > to include the definition of `IS_AARCH64` which is present in mainline, but > not in `jfx11u` (it was added as part of another unrelated fix that isn't > backported to `jfx11u`). Reviewer: @johanvos - PR: https://git.openjdk.java.net/jfx11u/pull/100
[jfx11u] RFR: 8276174: JavaFX build fails on macOS aarch64
Backport to `jfx11u` so we can build on macOS / aarch64 without needing to specify `-PCOMPILE_TARGET=arm64`. It isn't a clean backport, since I also had to include the definition of `IS_AARCH64` which is present in mainline, but not in `jfx11u` (it was added as part of another unrelated fix that isn't backported to `jfx11u`). - Commit messages: - 8276174: JavaFX build fails on macOS aarch64 Changes: https://git.openjdk.java.net/jfx11u/pull/100/files Webrev: https://webrevs.openjdk.java.net/?repo=jfx11u=100=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8276174 Stats: 4 lines in 1 file changed: 3 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jfx11u/pull/100.diff Fetch: git fetch https://git.openjdk.java.net/jfx11u pull/100/head:pull/100 PR: https://git.openjdk.java.net/jfx11u/pull/100
[jfx11u] RFR: 8273998: Clarify specification for Window properties controlled by the window manager
Clean backport to `jfx11u`. - Commit messages: - 8273998: Clarify specification for Window properties controlled by the window manager Changes: https://git.openjdk.java.net/jfx11u/pull/99/files Webrev: https://webrevs.openjdk.java.net/?repo=jfx11u=99=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8273998 Stats: 84 lines in 2 files changed: 59 ins; 4 del; 21 mod Patch: https://git.openjdk.java.net/jfx11u/pull/99.diff Fetch: git fetch https://git.openjdk.java.net/jfx11u pull/99/head:pull/99 PR: https://git.openjdk.java.net/jfx11u/pull/99
Re: [jfx11u] RFR: 8274274: Update JUnit to version 5.8.1
On Sat, 30 Apr 2022 13:45:39 GMT, Kevin Rushforth wrote: > Nearly clean backport to `jfx11u`. It isn't a clean backport since the > mainline patch updated `gradle/verification-metadata.xml`, which doesn't > exist in 11. Reviewer: @arapte - PR: https://git.openjdk.java.net/jfx11u/pull/97
[jfx11u] RFR: 8274274: Update JUnit to version 5.8.1
Nearly clean backport to `jfx11u`. It isn't a clean backport since the mainline patch updated `gradle/verification-metadata.xml`, which doesn't exist in 11. - Commit messages: - 8274274: Update JUnit to version 5.8.1 Changes: https://git.openjdk.java.net/jfx11u/pull/97/files Webrev: https://webrevs.openjdk.java.net/?repo=jfx11u=97=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8274274 Stats: 63 lines in 4 files changed: 58 ins; 3 del; 2 mod Patch: https://git.openjdk.java.net/jfx11u/pull/97.diff Fetch: git fetch https://git.openjdk.java.net/jfx11u pull/97/head:pull/97 PR: https://git.openjdk.java.net/jfx11u/pull/97
[jfx11u] RFR: 8276142: Update gradle to version 7.3
Clean backport to `jfx11u`. - Commit messages: - 8276142: Update gradle to version 7.3 Changes: https://git.openjdk.java.net/jfx11u/pull/96/files Webrev: https://webrevs.openjdk.java.net/?repo=jfx11u=96=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8276142 Stats: 227 lines in 7 files changed: 102 ins; 49 del; 76 mod Patch: https://git.openjdk.java.net/jfx11u/pull/96.diff Fetch: git fetch https://git.openjdk.java.net/jfx11u pull/96/head:pull/96 PR: https://git.openjdk.java.net/jfx11u/pull/96
Re: [jfx11u] RFR: 8281564: Update cmake to 3.22.3
On Sat, 30 Apr 2022 13:39:32 GMT, Kevin Rushforth wrote: > Simple backport to `jfx11u`. It isn't a clean backport since the mainline > patch had an update to `gradle/verification-metadata.xml`, which doesn't > exist in 11. Reviewer: @arapte - PR: https://git.openjdk.java.net/jfx11u/pull/95
[jfx11u] RFR: 8281564: Update cmake to 3.22.3
Simple backport to `jfx11u`. It isn't a clean backport since the mainline patch had an update to `gradle/verification-metadata.xml`, which doesn't exist in 11. - Commit messages: - 8281564: Update cmake to 3.22.3 Changes: https://git.openjdk.java.net/jfx11u/pull/95/files Webrev: https://webrevs.openjdk.java.net/?repo=jfx11u=95=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8281564 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jfx11u/pull/95.diff Fetch: git fetch https://git.openjdk.java.net/jfx11u pull/95/head:pull/95 PR: https://git.openjdk.java.net/jfx11u/pull/95
Re: [jfx11u] RFR: 8283218: Update GStreamer to 1.20.1
On Sat, 30 Apr 2022 13:37:08 GMT, Kevin Rushforth wrote: > Backport to `jfx11u`. Tested in connection with libffi update in the > `test-kcr-11.0.16` branch. > > This was not a clean backport, but the merge conflicts were trivial to > resolve. Here is a summary of the changes. The jfx mainline patch applied > cleanly to all other files. > > 1. The following file is not present in jfx11u, so that part of the patch was > skipped: > > > modules/javafx.media/src/main/native/gstreamer/gstreamer-lite/gst-plugins-good/gst/audioparsers/gstaacparse.c > > > 2. The following files had minor merge conflicts, the first was a diff in > surrounding context and the rest were in copyright header blocks: > > > modules/javafx.media/src/main/native/gstreamer/projects/linux/fxplugins/Makefile > modules/javafx.media/src/main/native/jfxmedia/projects/linux/Makefile > modules/javafx.media/src/tools/native/def-glib.pl > modules/javafx.media/src/tools/native/def-gstlite.pl Reviewer: @johanvos - PR: https://git.openjdk.java.net/jfx11u/pull/94
[jfx11u] RFR: 8283218: Update GStreamer to 1.20.1
Backport to `jfx11u`. Tested in connection with libffi update in the `test-kcr-11.0.16` branch. This was not a clean backport, but the merge conflicts were trivial to resolve. Here is a summary of the changes. The jfx mainline patch applied cleanly to all other files. 1. The following file is not present in jfx11u, so that part of the patch was skipped: modules/javafx.media/src/main/native/gstreamer/gstreamer-lite/gst-plugins-good/gst/audioparsers/gstaacparse.c 2. The following files had minor merge conflicts, the first was a diff in surrounding context and the rest were in copyright header blocks: modules/javafx.media/src/main/native/gstreamer/projects/linux/fxplugins/Makefile modules/javafx.media/src/main/native/jfxmedia/projects/linux/Makefile modules/javafx.media/src/tools/native/def-glib.pl modules/javafx.media/src/tools/native/def-gstlite.pl - Commit messages: - 8283218: Update GStreamer to 1.20.1 Changes: https://git.openjdk.java.net/jfx11u/pull/94/files Webrev: https://webrevs.openjdk.java.net/?repo=jfx11u=94=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8283218 Stats: 38984 lines in 412 files changed: 22060 ins; 5959 del; 10965 mod Patch: https://git.openjdk.java.net/jfx11u/pull/94.diff Fetch: git fetch https://git.openjdk.java.net/jfx11u pull/94/head:pull/94 PR: https://git.openjdk.java.net/jfx11u/pull/94
[jfx11u] RFR: 8280840: Update libFFI to 3.4.2
Clean backport to `jfx11u`. Tested in connection with gstreamer / glib update in the `test-kcr-11.0.16` branch. - Commit messages: - 8280840: Update libFFI to 3.4.2 Changes: https://git.openjdk.java.net/jfx11u/pull/93/files Webrev: https://webrevs.openjdk.java.net/?repo=jfx11u=93=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8280840 Stats: 3475 lines in 34 files changed: 927 ins; 38 del; 2510 mod Patch: https://git.openjdk.java.net/jfx11u/pull/93.diff Fetch: git fetch https://git.openjdk.java.net/jfx11u pull/93/head:pull/93 PR: https://git.openjdk.java.net/jfx11u/pull/93
Re: RFR: 8285725: Wrong link to JBS in README.md
On Wed, 27 Apr 2022 14:01:06 GMT, Nir Lisker wrote: > Updated the README link to match the CONTRIBUTING link. Looks good. Thanks for fixing it. - Marked as reviewed by kcr (Lead). PR: https://git.openjdk.java.net/jfx/pull/788
Re: Wrong link in README.md?
Yes, this seems like a bug. I agree that it would be better for the "issues list" link to use the same filtered list of issues that CONTRIBUTING.md links to. -- Kevin On 4/26/2022 8:54 PM, Nir Lisker wrote: In the README.md, under Issue Tracking, the link to "issues list" leads to the JBS homepage. In CONTRIBUTING.md under Bug Report, the (almost) same paragraph links to the JavaFX filter in JBS, which is a lot more helpful. Shouldn't the link in README also link to the filtered issues list? - Nir
Re: D3D pipeline possible inconsistencies
As you note, there are a few different issues here. To answer your questions as best I can: 1 & 3. We should document that self-illum maps and specular only use the rgb components and that alpha should be ignored. It's possible (although I don't know for sure) that the image is being treated as a non- premultiplied color format, and is subsequently converted to a premultiplied format; if so, this could explain the color darkening. 2. This also needs to be documented. The diffuse component should have an alpha that applies whether from the diffuse color or from a diffuse map. I agree with you that the pixel fragment should not be discarded just because the diffuse component is transparent. A specular highlight should be possible on a fully transparent object. 4. It does seem that the result should be the same regardless of whether the color comes from a specular map or color, but I'd need to dig further. Do all of the above issues happen with the OpenGL shaders, too? -- Kevin On 4/26/2022 11:41 AM, Nir Lisker wrote: I found a comment [1] on JBS stating that specular and self-Illumination alphas should be ignored, so it seems like there's at least 2 bugs here already. https://bugs.openjdk.java.net/browse/JDK-8090548?focusedCommentId=13771150=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13771150 On Tue, Apr 26, 2022 at 4:25 AM Nir Lisker wrote: Hi, Using the updated lighting test sample [1], I found some odd behavior with regards to PhongMaterial: 1. The effect of the opacity (alpha channel) of a self-illumination map is not documented, but lowering its value makes the object darker. I looked at the pixel shader [2] and only the rgb components are sampled, so I'm a bit confused here. What is the intended behavior? 2. The opacity of the object is controlled in the shader by both the diffuse color and diffuse map. This is also not documented (although it might be obvious for some). In the shader, the pixel (fragment) is discarded only if the map is fully transparent (line 55), but not the color. This leads to a situation where the object completely disappears when the map is transparent, but not when the color is. In the shader, the pixel should be transparent because of the multiplication of the alpha, but it's not, so this is also confusing. Should they both have the same contribution? Shouldn't it be valid to have a transparent diffuse but still have specular reflections? 3. The specular map and color behave differently in regards to the opacity. There is no documented behavior here. The alpha on the color is ignored (it's used for the specular power), but not on the map - it controls the reflection's strength, probably by making its color darker. In the shader, lines 76-84 indeed ignore the alpha of the color, but take the alpha of the map, although later in line 93 it's not used, so again I'm confused. What's the intended behavior? 4. The specular map and color also behave differently in regards to the reflection's strength. In the shader, this happens in line 78: the specular power is corrected with NTSC_Gray if there is a map (with or without color), but not if there's only a color. Shouldn't the contributions be the same? Is the NTSC_Gray correction correct in this case? Thanks, Nir [1] https://github.com/openjdk/jfx/pull/787 [2] https://github.com/openjdk/jfx/blob/master/modules/javafx.graphics/src/main/native-prism-d3d/hlsl/Mtl1PS.hlsl
Re: [jfx18] RFR: 8285475: Create release notes for 18.0.1
On Mon, 25 Apr 2022 06:50:07 GMT, Abhinay Agarwal wrote: > Add release notes for JavaFX 18.0.1 to the repository Marked as reviewed by kcr (Lead). - PR: https://git.openjdk.java.net/jfx/pull/785
Re: [jfx18] RFR: 8285475: Create release notes for 18.0.1
On Mon, 25 Apr 2022 06:50:07 GMT, Abhinay Agarwal wrote: > Add release notes for JavaFX 18.0.1 to the repository The following two non-public (security) fixes are also included in 18.0.1: JDK-8276371 (not public) | Better long buffering | web JDK-8277465 (not public) | Additional fix for JDK-8276371 | web - PR: https://git.openjdk.java.net/jfx/pull/785
Re: RFR: 8284654: Modal behavior returns to wrong stage
On Fri, 22 Apr 2022 19:26:36 GMT, Thiago Milczarek Sayao wrote: > When there's an APPLICATION_MODAL window, all other windows are disabled and > re-enabled when the APPLICATION_MODAL window closes. This causes > `requestToFront()` to be called on every window, and it does not guarantee > order. > > The fix also works for: > https://bugs.openjdk.java.net/browse/JDK-8269429 > > But this one might need another fix: > https://bugs.openjdk.java.net/browse/JDK-8240640 I'll take a look at it, and also ask @arapte to review. As you know from my comment in the bug report, I especially want to ensure that this causes no regressions, so need to test [JDK-8240640](https://bugs.openjdk.java.net/browse/JDK-8240640) on macOS. - PR: https://git.openjdk.java.net/jfx/pull/784
Re: RFR: 8193442: Removing TreeItem from a TreeTableView sometime changes selectedItem [v3]
On Thu, 21 Apr 2022 08:37:11 GMT, Jose Pereda wrote: >> This PR fixes >> JDK-[8193442](https://bugs.openjdk.java.net/browse/JDK-8193442), but also >> [JDK-8187596](https://bugs.openjdk.java.net/browse/JDK-8187596), and >> verifies that the tests mentioned in >> [JDK-8088157](https://bugs.openjdk.java.net/browse/JDK-8088157) are working >> (with a minor fix). >> >> When removing an item that is below the selected item from TreeTableView or >> TreeView controls the selection and/or focus was wrongly changed in some >> occasions, because a shift in the selection was applied. >> >> This PR adds a method to ControlUtils to get the index of the sibling that >> is selected/focused or contains the descendant item with the current >> selection/focus. >> >> This index is required to compare properly if the selected/focus item is >> above or below the item that was removed, by comparing the indices of >> siblings. >> >> Tests have been added to TreeViewTest and TreeTableViewTest based on the >> existing tests on JDK-8193442 and JDK-8187596. The four tests fail without >> this PR, pass with it. >> >> In the process, I noticed that the ignored tests referred from JDK-8088157 >> were already passing, after removing some obsolete asserts, even without >> this PR. > > Jose Pereda has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains four commits: > > - Fix spacing in tests > - Merge branch 'master' into 8193442-treeitemselection > - Merge branch 'master' into 8193442-treeitemselection > - Don't shift selection/focus if item is below removed element Looks good. > In the process, I noticed that the ignored tests referred from JDK-8088157 > were already passing, after removing some obsolete asserts, even without this > PR. After this is integrated, can you close JDK-8088157 as "Cannot reproduce"? - Marked as reviewed by kcr (Lead). PR: https://git.openjdk.java.net/jfx/pull/753
Re: RFR: 8278430 : Several tests use terminally deprecated System.runFinalization method
On Thu, 21 Apr 2022 15:03:04 GMT, Hima Bindu Meda wrote: > This PR is to remove calls to deprecated method System.runFinalization. > Here is the list of module wise test cases affected by the removal: > > **web**: > test.javafx.scene.web.LeakTest > > **systemTests**: > test.memoryleak.JSCallbackMemoryTest > test.javafx.embed.swing.SwingNodeDnDMemoryLeakTest > test.javafx.embed.swing.SwingNodeMemoryLeakTest > test.javafx.scene.control.AccordionTitlePaneLeakTest > test.javafx.scene.control.TabPaneHeaderLeakTest > test.javafx.scene.shape.ShapeViewOrderLeakTest > test.javafx.scene.shape.meshmanagercacheleaktest.MeshManagerCacheLeakApp > test.javafx.stage.FocusedWindowTestBase --tests > test.memoryleak.JSCallbackMemoryTest > test.robot.javafx.web.TooltipFXTest > > **graphics**: > test.javafx.scene.SceneTest > > **controls**: > test.javafx.scene.control.ToggleButtonTest > test.javafx.scene.control.ControlAcceleratorSupportTest > > **base**: > > Executed above tests for 50 iterations on windows and Mac. > No regressions observed.All tests run fine. Looks good. I did a sanity test run on Mac and found no new issues. - Marked as reviewed by kcr (Lead). PR: https://git.openjdk.java.net/jfx/pull/781
Re: RFR: 8282449: Intermittent OOM error in PredefinedMeshManagerTest
On Thu, 21 Apr 2022 15:07:38 GMT, Ambarish Rapte wrote: > This is a test only fix. The tests used to create Sphere and Cylinder of > large divisions, which could fail intermittently with OOM. > The tests were added along with the fix for > [JDK-8180151](https://bugs.openjdk.java.net/browse/JDK-8180151). > The attributes of Sphere and Cylinder were chosen such that their hash code > would collide. The large number of divisions results in creating large sized > triangle mesh and sometimes cause OOM. > > The fix: is to use different combination of hashcode colliding attributes > with smaller number of divisions to avoid OOM. > > There are two commits in the PR: > 1. [First > commit](https://github.com/openjdk/jfx/commit/1ff6054503f5042f9ede54faac6fa00ec4369612) > contains the source changes with which test should fail. Only > `sphereCacheTest` fails with this commit but not the `cylinderCacheTest`. The > earlier combination of Cylinder attributes also does NOT fail. > 2. [Second > commit](https://github.com/openjdk/jfx/commit/0c8649abe94ef2b526007c37fab6910315a98654) > reverts the source code changes. Looks good. Thanks for the explanation regarding the replacement values. - Marked as reviewed by kcr (Lead). PR: https://git.openjdk.java.net/jfx/pull/782
Re: RFR: 8285034: Skip ServiceTest.testManyServicesRunConcurrently on Windows
On Thu, 21 Apr 2022 15:44:57 GMT, Ambarish Rapte wrote: > The test ServiceTest.testManyServicesRunConcurrently fails intermittently on > Windows platform with a time out error. > Test needs to be skipped until fixed. LGTM - Marked as reviewed by kcr (Lead). PR: https://git.openjdk.java.net/jfx/pull/783
Re: RFR: 8283218: Update GStreamer to 1.20.1 [v5]
On Wed, 20 Apr 2022 23:08:00 GMT, Alexander Matveev wrote: >> - GStreamer updated to 1.20.1 and GLib updated to 2.72.0. >> - No changes to our code, except in GstAudioSpectrum.cpp >> g_atomic_pointer_compare_and_exchange() was changed to >> g_atomic_pointer_set(). For some reason I was not able to get code compiled >> with g_atomic_pointer_compare_and_exchange() used from C++ code. Also, I do >> not see a need to use g_atomic_pointer_compare_and_exchange(), since >> m_pHolder always equals to old_holder. >> - Tested on all platforms with all supported media streams. > > Alexander Matveev has updated the pull request incrementally with one > additional commit since the last revision: > > 8283218: Update GStreamer to 1.20.1 [v5] Answering my own question from above, I was able to take a `gstreamer-lite` binary built on Oracle Linux 7.x and run it on my Ubuntu 16.04 system, so it looks like we are good to go. Alexander or I will file a follow-on bug to build and ship `glib-lite` on Linux. It will allow some of the custom code to be reverted, and will avoid this problem the next time we update GLib. It also matches what we do for the other platforms (and what we do for Linux on Oracle's JDK 8u releases, so we have a data point that shows it to be a feasible approach). - Marked as reviewed by kcr (Lead). PR: https://git.openjdk.java.net/jfx/pull/768
Re: RFR: 8283218: Update GStreamer to 1.20.1 [v4]
On Mon, 18 Apr 2022 18:38:03 GMT, Alexander Matveev wrote: >> - GStreamer updated to 1.20.1 and GLib updated to 2.72.0. >> - No changes to our code, except in GstAudioSpectrum.cpp >> g_atomic_pointer_compare_and_exchange() was changed to >> g_atomic_pointer_set(). For some reason I was not able to get code compiled >> with g_atomic_pointer_compare_and_exchange() used from C++ code. Also, I do >> not see a need to use g_atomic_pointer_compare_and_exchange(), since >> m_pHolder always equals to old_holder. >> - Tested on all platforms with all supported media streams. > > Alexander Matveev has updated the pull request incrementally with one > additional commit since the last revision: > > 8283218: Update GStreamer to 1.20.1 [v4] The latest patch builds and runs for me now on Ubuntu 16.04. I do want to make sure that regardless of whatever system we build it on, the binary is able to run on the oldest version we are targeting. Otherwise, a binary built on, say, Oracle Linux 7.x or Ubuntu 20.04 might not run on an Ubuntu 16.04 system. Basically what we don't want is to be dependent on a particular version of libraries at compile time. The more I think about it, the best long term solution (not this time) is probably to build and deliver glib-lite.so on Linux. - PR: https://git.openjdk.java.net/jfx/pull/768
Re: [jfx18] RFR: 8283328: Update libxml2 to 2.9.13
On Wed, 20 Apr 2022 13:29:36 GMT, Johan Vos wrote: > Reviewed-by: jvos, kcr, arapte I confirm that this is a clean backport, but Skara failed to detect that it was clean due to [SKARA-1332](https://bugs.openjdk.java.net/browse/SKARA-1332). - PR: https://git.openjdk.java.net/jfx/pull/777
Re: [jfx11u] RFR: 8283328: Update libxml2 to 2.9.13
On Wed, 20 Apr 2022 13:21:39 GMT, Johan Vos wrote: > Reviewed-by: jvos, kcr, arapte I confirm that this is a clean backport, but Skara failed to detect that it was clean due to [SKARA-1332](https://bugs.openjdk.java.net/browse/SKARA-1332). - PR: https://git.openjdk.java.net/jfx11u/pull/84
Re: [jfx17u] RFR: 8283328: Update libxml2 to 2.9.13
On Wed, 20 Apr 2022 13:24:53 GMT, Johan Vos wrote: > Reviewed-by: jvos, kcr, arapte I confirm that this is a clean backport, but Skara failed to detect that it was clean due to [SKARA-1332](https://bugs.openjdk.java.net/browse/SKARA-1332). - PR: https://git.openjdk.java.net/jfx17u/pull/41
Re: [External] : Backport requests for JavaFX 11, 17, 18
+1 On 4/20/2022 5:59 AM, Johan Vos wrote: Hi Kevin, I request permission to backport the following issue to JavaFX 11.0.16, JavaFX 17.0.4 and JavaFX 18.0.2 https://bugs.openjdk.java.net/browse/JDK-8283328 Apart from whitespace warnings, the patch applies clean - Johan
