Re: RFR: 8264449: Enable reproducible builds with SOURCE_DATE_EPOCH [v2]

2021-04-02 Thread John Neffenger
On Fri, 2 Apr 2021 17:45:38 GMT, John Neffenger  wrote:

>> Media makefiles look fine.
>
>> Silly question, is the difference with Windows just the nature of the native 
>> support on each platform (Unix based vs Windows) and libraries used as part 
>> of that?
> 
> That's a good question. I'm hoping the answer is no. So far, the difference 
> with Windows is the linker. The other systems use the GNU linker where it's 
> enough to sort the input object files. The Microsoft linker, though, creates 
> reproducible builds with the flag `/experimental:deterministic` (or 
> `-experimental:deterministic`). So on Windows, we just need to figure out 
> where to put the flag. I'm finding that to be a challenge for the WebKit 
> library.

> /contributor add ...

@bmwiedemann I added you as co-author of pull request #422, which is my 
continuation of your pull request #99 from last year. This pull request #446 
temporarily includes our co-authored commit, but those changes will disappear 
when #422 is integrated and I merge the master branch into this one. Sorry for 
the confusion.

-

PR: https://git.openjdk.java.net/jfx/pull/446


Re: RFR: 8262366: Update glib to version 2.66.7

2021-04-02 Thread Alexander Matveev
On Thu, 1 Apr 2021 23:47:15 GMT, Kevin Rushforth  wrote:

>> All my testing looks good on all three platforms. I'll take a look at the 
>> diffs next.
>> 
>> One thing I did spot is that you need to update `gstreamer.md` and `glib.md` 
>> to bump the version numbers.
>
> It now builds for me on my old Linux box. And the changes to the /legal/ 
> files looks good.
> 
> One more thing I just thought of: we should do a test build on macOS aarch64. 
> I have no reason to believe that there will be a problem, but worth 
> double-checking.

It builds fine on macOS aarch64 and I did minor testing as well. No issues.

-

PR: https://git.openjdk.java.net/jfx/pull/447


RFR: 8137323: Incorrect parsing of mnemonic in controls text

2021-04-02 Thread mstr2
This PR fixes incorrect parsing of mnemonic symbols in the following cases:
1. an escaped double underscore incorrectly shows up as a double underscore in 
displayed text
2. an extended mnemonic incorrectly removes a part of adjacent text

As a side effect, this PR also fixes another undocumented issue for multiline 
text where text metrics calculations incorrectly operate on text for which 
mnemonic symbols have not been processed.

This can lead to a glitch where the mnemonic underscore makes a text just long 
enough to wrap to a new line, but when the text is rendered, the second line 
shows up entirely empty. The reason for this is that for rendering, the text is 
laid out _after_ mnemonic symbols have been removed.

The solution is to _always_ use processed text ("_clean text_") when laying out 
text.

-

Commit messages:
 - Additional test
 - Fix mnemonic processing in LabeledSkinBase
 - Add failing tests

Changes: https://git.openjdk.java.net/jfx/pull/453/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jfx=453=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8137323
  Stats: 251 lines in 3 files changed: 120 ins; 51 del; 80 mod
  Patch: https://git.openjdk.java.net/jfx/pull/453.diff
  Fetch: git fetch https://git.openjdk.java.net/jfx pull/453/head:pull/453

PR: https://git.openjdk.java.net/jfx/pull/453


Integrated: 8259555: Webkit crashes on Apple Silicon

2021-04-02 Thread Arun Joseph
On Fri, 2 Apr 2021 15:37:38 GMT, Arun Joseph  wrote:

> WebKit crashes during JavaScriptCore initialization code for Apple Silicon.

This pull request has now been integrated.

Changeset: b2f842de
Author:Arun Joseph 
URL:   https://git.openjdk.java.net/jfx/commit/b2f842de
Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod

8259555: Webkit crashes on Apple Silicon

Co-authored-by: Peter Zhelezniakov 
Reviewed-by: kcr, jvos

-

PR: https://git.openjdk.java.net/jfx/pull/452


Re: RFR: 8259555: Webkit crashes on Apple Silicon

2021-04-02 Thread Johan Vos
On Fri, 2 Apr 2021 15:37:38 GMT, Arun Joseph  wrote:

> WebKit crashes during JavaScriptCore initialization code for Apple Silicon.

The other required changes required to build for M1 can easily be passed as 
build parameters using gradlew -P 
Hence, this change contains all that is needed to be changed in the repository.

-

Marked as reviewed by jvos (Reviewer).

PR: https://git.openjdk.java.net/jfx/pull/452


Re: RFR: 8264449: Enable reproducible builds with SOURCE_DATE_EPOCH [v2]

2021-04-02 Thread John Neffenger
On Thu, 1 Apr 2021 23:21:55 GMT, Alexander Matveev  wrote:

>> John Neffenger has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Include media shared libraries for Windows
>>   
>>   Enable reproducible builds of the native media shared libraries for
>>   Windows when SOURCE_DATE_EPOCH is defined. The libraries are:
>>   
>> fxplugins.dll
>> glib-lite.dll
>> gstreamer-lite.dll
>> jfxmedia.dll
>
> Media makefiles look fine.

> Silly question, is the difference with Windows just the nature of the native 
> support on each platform (Unix based vs Windows) and libraries used as part 
> of that?

That's a good question. I'm hoping the answer is no. So far, the difference 
with Windows is the linker. The other systems use the GNU linker where it's 
enough to sort the input object files. The Microsoft linker, though, creates 
reproducible builds with the flag `/experimental:deterministic` (or 
`-experimental:deterministic`). So on Windows, we just need to figure out where 
to put the flag. I'm finding that to be a challenge for the WebKit library.

-

PR: https://git.openjdk.java.net/jfx/pull/446


Re: RFR: 8259555: Webkit crashes on Apple Silicon

2021-04-02 Thread Kevin Rushforth
On Fri, 2 Apr 2021 15:37:38 GMT, Arun Joseph  wrote:

> WebKit crashes during JavaScriptCore initialization code for Apple Silicon.

Marked as reviewed by kcr (Lead).

-

PR: https://git.openjdk.java.net/jfx/pull/452


Re: RFR: 8259555: Webkit crashes on Apple Silicon

2021-04-02 Thread Kevin Rushforth
On Fri, 2 Apr 2021 17:32:41 GMT, Johan Vos  wrote:

>> WebKit crashes during JavaScriptCore initialization code for Apple Silicon.
>
> This works, as long as MACOSX_MIN_VERSION is set to 11. I guess we don't want 
> to commit that in the build.gradle, as we only require that for a build on 
> Apple Silicon.

Any changes to minimum OS version in `mac.gradle` would need to be qualified by 
a check for the target architecture. It might make sense to do that in 
connection with other needed build changes.

-

PR: https://git.openjdk.java.net/jfx/pull/452


Re: RFR: 8259555: Webkit crashes on Apple Silicon

2021-04-02 Thread Johan Vos
On Fri, 2 Apr 2021 15:37:38 GMT, Arun Joseph  wrote:

> WebKit crashes during JavaScriptCore initialization code for Apple Silicon.

This works, as long as MACOSX_MIN_VERSION is set to 11. I guess we don't want 
to commit that in the build.gradle, as we only require that for a build on 
Apple Silicon.

-

PR: https://git.openjdk.java.net/jfx/pull/452


RFR: 8259555: Webkit crashes on Apple Silicon

2021-04-02 Thread Arun Joseph
WebKit crashes during JavaScriptCore initialization code for Apple Silicon.

-

Commit messages:
 - 8259555: Webkit crashes on Apple Silicon

Changes: https://git.openjdk.java.net/jfx/pull/452/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jfx=452=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8259555
  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jfx/pull/452.diff
  Fetch: git fetch https://git.openjdk.java.net/jfx pull/452/head:pull/452

PR: https://git.openjdk.java.net/jfx/pull/452


Re: [External] : Re: Consistent baseline layout algorithm

2021-04-02 Thread Scott Palmer
I haven’t done much in terms of custom controls, but I do see this issue a lot 
in my primary application.  It would be nice to have it fixed as the layout 
looks rather sloppy without it.  Any time I see the layout “correct” itself 
when I interact with a control or tweak the size of a window it is also 
distracting.
When I saw this fix proposed here my first thought was, “finally!”  So I for 
one am looking forward to this.  It seems that the number of iterations will be 
very low for most cases, so I’m not terribly concerned about any sort of 
performance impact.  But that is my only worry, as I have had issues with 
layout/CSS taking a lot of time in the past, so in some cases even a single 
extra iteration may lead to a perceptible delay.  (I was mostly working with FX 
8 when I had those issues, so hopefully things are better in recent versions, I 
haven’t measured.)
I agree that the default limit could probably be reduced, 10 iterations would 
likely be plenty more than needed.  As you say, something is probably wrong 
with your control if it goes that far.

Regards,

Scott


> On Apr 2, 2021, at 10:51 AM, Kevin Rushforth  
> wrote:
> 
> Circling back to this: Do any JavaFX application developers on this list 
> have any thoughts? I think this is a good idea, and it seems worth moving 
> this forward, but it will require a fair bit of effort. I'd like to hear 
> buy-in from other developers -- particularly those who develop or use custom 
> controls.
> 
> A few additional comments:
> 
> * Regarding the threshold, I like the idea of logging at a fine (or finer) 
> level if it goes beyond a smaller number, say 3 or 4, passes, but continuing 
> further to iterate. I wonder if 100 might be too large for the default limit, 
> though. It seems likely that the control is unstable and will never converge 
> if it gets to that many layout passes. If there were a way to analyze what a 
> reasonable limit is that would be better (if not, we can probably live with a 
> fairly high value, if it really is an unlikely pathological case). Assuming a 
> reasonably high threshold, logging at a warning level would be good if it 
> doesn't converge.
> 
> * Documentation: I do think we will want to document the overall concept of 
> layout needing multiple passes to converge in some cases, along with 
> documenting the threshold. I also think that Node::getBaselineOffset should 
> mention the preference of text nodes. The concept of "text" nodes needs to be 
> on Node, if for no other reason than that Text doesn't inherit from Parent.
> 
> * This will need to be very well tested, both to ensure no regressions in 
> behavior, and with many new tests added to validate the new functionality, 
> including testing the threshold logic.
> 
> -- Kevin
> 
> 
>> On 3/20/2021 6:42 AM, Michael Strauß wrote:
>> 1) Pathological cases: So far, I haven't been able to produce
>> pathological cases that exceed 3 layout passes by using standard
>> layout containers and controls. If we need to have a threshold
>> substantially higher than that depends on whether or not there are
>> indeed legitimate cases where some kind of layout requires more passes
>> to converge. Maybe we could log at a very fine level when a control
>> takes more than 3 passes to converge, but still continue processing up
>> to a substantially higher number of passes. This would allow
>> developers to debug and optimize their custom controls, but still
>> produce a valid solution if a particular control is generally stable,
>> but just very poorly optimized.
>> 
>> 2) I believe that the multi-pass layout algorithm shouldn't need
>> additional documentation since it could be considered an
>> implementation detail. There is no 1:1 correspondence between pulses
>> and layout passes currently, and there won't be with the new
>> algorithm. Also, any layout pass with the current algorithm can
>> potentially trigger another layout pass, so that's not fundamentally
>> different either. For control authors, we might need to document that
>> it is generally okay to rely on circular dependencies between size
>> hints, baseline offset and child nodes, as long as those dependencies
>> allow the layout to settle. The fact that text nodes are preferred
>> with respect to baseline computation is documented in
>> Parent.getBaselineOffset(), maybe Node.getBaselineOffset() should also
>> mention this. We might also add a section on baseline computation to
>> the javafx.scene.layout package documentation, where I think it is
>> most relevant (and currently entirely undocumented).
>> 
>> 3) New API on Node: in order for this idea to work, there needs to be
>> an abstract concept of "text node". Since the "baseline" concept is
>> introduced on Node, I thought this would also be the appropriate place
>> to introduce the "text node" concept; however, it could also be
>> introduced on Parent instead. Note that the "text node" concept is
>> introduced at a lower level than actual text 

Re: [External] : Re: Consistent baseline layout algorithm

2021-04-02 Thread Kevin Rushforth
Circling back to this: Do any JavaFX application developers on this list 
have any thoughts? I think this is a good idea, and it seems worth 
moving this forward, but it will require a fair bit of effort. I'd like 
to hear buy-in from other developers -- particularly those who develop 
or use custom controls.


A few additional comments:

* Regarding the threshold, I like the idea of logging at a fine (or 
finer) level if it goes beyond a smaller number, say 3 or 4, passes, but 
continuing further to iterate. I wonder if 100 might be too large for 
the default limit, though. It seems likely that the control is unstable 
and will never converge if it gets to that many layout passes. If there 
were a way to analyze what a reasonable limit is that would be better 
(if not, we can probably live with a fairly high value, if it really is 
an unlikely pathological case). Assuming a reasonably high threshold, 
logging at a warning level would be good if it doesn't converge.


* Documentation: I do think we will want to document the overall concept 
of layout needing multiple passes to converge in some cases, along with 
documenting the threshold. I also think that Node::getBaselineOffset 
should mention the preference of text nodes. The concept of "text" nodes 
needs to be on Node, if for no other reason than that Text doesn't 
inherit from Parent.


* This will need to be very well tested, both to ensure no regressions 
in behavior, and with many new tests added to validate the new 
functionality, including testing the threshold logic.


-- Kevin


On 3/20/2021 6:42 AM, Michael Strauß wrote:

1) Pathological cases: So far, I haven't been able to produce
pathological cases that exceed 3 layout passes by using standard
layout containers and controls. If we need to have a threshold
substantially higher than that depends on whether or not there are
indeed legitimate cases where some kind of layout requires more passes
to converge. Maybe we could log at a very fine level when a control
takes more than 3 passes to converge, but still continue processing up
to a substantially higher number of passes. This would allow
developers to debug and optimize their custom controls, but still
produce a valid solution if a particular control is generally stable,
but just very poorly optimized.

2) I believe that the multi-pass layout algorithm shouldn't need
additional documentation since it could be considered an
implementation detail. There is no 1:1 correspondence between pulses
and layout passes currently, and there won't be with the new
algorithm. Also, any layout pass with the current algorithm can
potentially trigger another layout pass, so that's not fundamentally
different either. For control authors, we might need to document that
it is generally okay to rely on circular dependencies between size
hints, baseline offset and child nodes, as long as those dependencies
allow the layout to settle. The fact that text nodes are preferred
with respect to baseline computation is documented in
Parent.getBaselineOffset(), maybe Node.getBaselineOffset() should also
mention this. We might also add a section on baseline computation to
the javafx.scene.layout package documentation, where I think it is
most relevant (and currently entirely undocumented).

3) New API on Node: in order for this idea to work, there needs to be
an abstract concept of "text node". Since the "baseline" concept is
introduced on Node, I thought this would also be the appropriate place
to introduce the "text node" concept; however, it could also be
introduced on Parent instead. Note that the "text node" concept is
introduced at a lower level than actual text controls like Labeled,
because layout containers (which are not controls) need this
information. Also, the definition of what constitutes "text" is left
to control authors. In a very contrived example, a control might
display an image, but declare itself to be "text".



Am Fr., 19. März 2021 um 23:26 Uhr schrieb Kevin Rushforth
:

I'd be interested to hear from app developers on this list.

Here are a few quick thoughts I have.

As you note, this is a long-standing problem with layout in FX. You
mention in the performance considerations that for most cases this will
iterate quickly. It would be interesting to know what some of the corner
cases are, so we can see how bad the pathological case will be. I see
that you propose to iterate up to 100 times. Maybe a lower threshold
would be better? We already run layout passes 3 times in many cases. So
also running 3 (or maybe 4 or 5) times seems reasonable, especially when
your testing shows that most of the time it converges in <= 2 passes. So
a smaller threshold than 100 would seem to make sense. If a control
doesn't converge, you might consider logging it (in case an app
developer wants to debug it) perhaps at a "fine" level so it isn't shown
by default.

How do you propose to document this behavioral change -- not so much the
fact that it will (usually) get 

Re: Re: RFR: 8264449: Enable reproducible builds with SOURCE_DATE_EPOCH

2021-04-02 Thread Eric Bresie
Silly question, is the difference with Windows just the nature of the native 
support on each platform (Unix based vs Windows) and libraries used as part of 
that?

Eric Bresie
ebre...@gmail.com (mailto:ebre...@gmail.com)

> On April 1, 2021 at 4:29:09 PM CDT, Kevin Rushforth  (mailto:k...@openjdk.java.net)> wrote:
> On Thu, 1 Apr 2021 20:18:03 GMT, John Neffenger  (mailto:jgn...@openjdk.org)> wrote:
>
> > > IMHO, it would make sense to merge any clear improvement of the 
> > > status-quo and debug+fix more in a later PR. "Perfect is the enemy of 
> > > good."
> > >
> > > Regarding Webkit, I know of
> > > https://bugs.webkit.org/show_bug.cgi?id=174540
> > > https://bugs.webkit.org/show_bug.cgi?id=188738
> > > https://build.opensuse.org/request/show/667729
> > > but your version is probably not *that* old.
> >
> > > IMHO, it would make sense to merge any clear improvement of the 
> > > status-quo and debug+fix more in a later PR.
> >
> > I agree that we will have to draw the line somewhere, but right now I'm 
> > down to just one file on one operating system out of 10,633 files in the 
> > build output! (It's just `jfxwebkit.dll` on Windows.) It would be a shame 
> > to give up now.
> >
> > I would, though, like to postpone tests of static builds and builds for 
> > Android and iOS. My priorities for this pull request are:
> >
> > 1. Linux distributions building JavaFX
> > 2. Developers building JavaFX from the repository source with the build 
> > defaults
> > 3. Oracle, Gluon, and BellSoft building JavaFX for their customers and for 
> > downloading
> >
> > I would be happy to satisfy just the first group, but we might be close to 
> > getting all three.
>
> @sashamatveev Can you look at the changes to the media Makefiles?
>
> -
>
> PR: https://git.openjdk.java.net/jfx/pull/446


Integrated: JDK-8258381 : [macos] Exception when input emoji using Chinese input method

2021-04-02 Thread Hsiafan
On Sun, 21 Mar 2021 04:13:48 GMT, Hsiafan 
 wrote:

> The convertNSStringToJString function convert NSString to Java String using 
> NewStringUTF,NewStringUTF accept modified-UTF8 encoded str。This fix using 
> UTF-16 encoding for converting.

This pull request has now been integrated.

Changeset: 052e9f7d
Author:dongliu 
Committer: Kevin Rushforth 
URL:   https://git.openjdk.java.net/jfx/commit/052e9f7d
Stats: 2 lines in 1 file changed: 1 ins; 0 del; 1 mod

8258381: [macos] Exception when input emoji using Chinese input method

Reviewed-by: jpereda, kcr

-

PR: https://git.openjdk.java.net/jfx/pull/436


Re: RFR: 8252936: Optimize removal of listeners from ExpressionHelper.Generic

2021-04-02 Thread yosbits
On Thu, 1 Apr 2021 23:50:12 GMT, Marius Hanl 
 wrote:

>> Because it is such a small correction
>> Problem from me I feel that it is not easy to register, but I will try to 
>> register. 
>> 
>> It has passed two existing tests for compatibility:
>> 
>> * gradle base:test 
>> * gradle controls:test 
>> 
>> I have just reported it as an enhancement proposal.
>
> Any news for this PR? Can I help somewhere?

It seems that performance improvement PR often needs to make up for the lack of 
existing unit tests. However, this addition of unit tests should not be 
included in the same PR as it needs to be tested to meet the specifications 
before and after the change.

**I think we need a step to fix the lack of unit tests before the performance 
improvement patch.**

And care must be taken not to rely on internal implementations for this unit 
test so as not to hinder performance improvement. Some existing unit test code 
depends on the internal implementation, so some test changes may be needed.

-

PR: https://git.openjdk.java.net/jfx/pull/108


Re: Proof of concept for fluent bindings for ObservableValue

2021-04-02 Thread Nir Lisker
Hi John,

I've had my eyes set on ReactFX enhancements for a while too, especially as
a replacement for the unsafe "select" mechanism. One of the things that
kept me from going forward with this is seeing what Valhalla will bring.
Generic specialization might save a lot of duplication work on something
like this, and Tomas touched another related issue [1], but since it could
be a long time before that happens, it's worth planning what we can extract
from ReactFX currently.

I think that we should break the enhancements into parts.
The first that I would advise to look at are the additions to
properties/observables. Tomas had to create Val and Var because he couldn't
change the core interfaces, but we can. Fitting them with the Optional
methods like `isPresent`, `isEmpty`, `ifPresent`, `map`. `flatMap` etc.;
and `select` and friends, is already a good start that will address many
common requirements.
The second part is related to listeners. The subscription model and event
streams try to solve the memory issues with hard and weak references, and
allow better composition.
The third part is for collections - things like transformation lists
(LiveList) and for other collections.

Since these share behavior under the hood, we need to look ahead, but in
terms of functionality, I think we should take smaller steps. It will also
be easier to propose these then.

- Nir

[1]
https://github.com/TomasMikula/ReactFX/wiki/Creating-a-Val-or-Var-Instance#the-javafx-propertynumber-implementation-issue

On Wed, Mar 24, 2021 at 11:49 PM John Hendrikx  wrote:

> I just wanted to draw some attention to a recent proof of concept I made
> in this pull request: https://github.com/openjdk/jfx/pull/434
>
> It is based on the work I did in
> https://github.com/hjohn/hs.jfx.eventstream which is in part based on
> work done in ReactFX by Tomas Mikula. The PR itself however shares no
> code with ReactFX and is
> completely written by me.
>
> If there is interest, I'm willing to invest more time in smoothing out
> the API and documentation, investigating further how this would interact
> with the primitive types and adding unit test coverage (I have extensive
> tests, but thesea are written in JUnit 5, so they would require
> conversion or JavaFX could move to support JUnit 5).
>
> What follows below is the text of the PR for easy reading. Feedback is
> appreciated.
>
> 
>
> This is a proof of concept of how fluent bindings could be introduced to
> JavaFX. The main benefit of fluent bindings are ease of use, type safety
> and less surprises. Features:
>
> 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: #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 or orElseGet.
>
> Conditional Bindings
> Bindings can be made conditional using the conditionOn method. A
> conditional binding retains its last value when its condition is false.
> Conditional bindings donot observe their source when the condition is
> false, allowing developers to automatically stop listening to properties
> when a certain condition is met. A major use of this feature is to have
> UI components that need to keep models updated which may outlive the UI
> conditionally update the long lived model only when the UI is showing.
>
> 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:
>