Re: RFR: 8277572: ImageStorage should correctly handle MIME types for images encoded in data URIs [v2]
> `com.sun.javafx.iio.ImageStorage` currently ignores the MIME image subtype
> specified for images encoded in data URIs. This should be improved as follows:
>
> 1. If the specified image subtype is not supported, an exception will be
> thrown.
> 2. If the specified image subtype is supported, but the data contained in the
> URI is of a different (but also supported) image format, the image will be
> loaded and a warning will be logged. For example, if the MIME type is
> "image/jpeg", but the image data is PNG, the following warning will be
> generated:
>
>
> Image format 'PNG' does not match MIME type 'image/jpeg' in URI
> 'data:image/jpeg;base64,iVBORw0KGgoAAA...AAAElFTkSuQmCC'
>
>
> Also, the javadoc of `javafx.scene.image.Image` incorrectly states:
>
> 94* If a URL uses the "data" scheme, the data must be base64-encoded
> 95* and the MIME type must either be empty or a subtype of the
> 96* {@code image} type.
>
> However, omitting the MIME type of a data URI is specified to imply
> "text/plain" (RFC 2397, section 2). Since the `com.sun.javafx.util.DataURI`
> class is implemented according to this specification, trying to load an image
> without MIME type correctly fails with an `ImageStorageException`:
> "Unexpected MIME type: text".
>
> The solution is to fix the documentation:
>
> 94* If a URL uses the "data" scheme, the data must be base64-encoded
> + 95* and the MIME type must be a subtype of the {@code image} type.
> - 95* and the MIME type must either be empty or a subtype of the
> - 96* {@code image} type.
Michael Strauß has updated the pull request incrementally with one additional
commit since the last revision:
ImageStorage correctly handles MIME types in data URIs
-
Changes:
- all: https://git.openjdk.java.net/jfx/pull/676/files
- new: https://git.openjdk.java.net/jfx/pull/676/files/4abb9b47..73f1bb13
Webrevs:
- full: https://webrevs.openjdk.java.net/?repo=jfx&pr=676&range=01
- incr: https://webrevs.openjdk.java.net/?repo=jfx&pr=676&range=00-01
Stats: 152 lines in 9 files changed: 85 ins; 8 del; 59 mod
Patch: https://git.openjdk.java.net/jfx/pull/676.diff
Fetch: git fetch https://git.openjdk.java.net/jfx pull/676/head:pull/676
PR: https://git.openjdk.java.net/jfx/pull/676
Re: RFR: 8264449: Enable reproducible builds with SOURCE_DATE_EPOCH [v6]
On Mon, 22 Nov 2021 06:43:30 GMT, John Neffenger wrote: >> This pull request allows for reproducible builds of JavaFX on Linux, macOS, >> and Windows by defining the `SOURCE_DATE_EPOCH` environment variable. For >> example, the following commands create a reproducible build: >> >> >> $ export SOURCE_DATE_EPOCH=$(git log -1 --pretty=%ct) >> $ bash gradlew sdk jmods javadoc >> $ strip-nondeterminism -v -T $SOURCE_DATE_EPOCH build/jmods/*.jmod >> >> >> The three commands: >> >> 1. set the build timestamp to the date of the latest source code change, >> 2. build the JavaFX SDK libraries, JMOD archives, and API documentation, and >> 3. recreate the JMOD files with stable file modification times and ordering. >> >> The third command won't be necessary once Gradle can build the JMOD archives >> or the `jmod` tool itself has the required support. For more information on >> the environment variable, see the [`SOURCE_DATE_EPOCH`][1] page. For more >> information on the command to recreate the JMOD files, see the >> [`strip-nondeterminism`][2] repository. I'd like to propose that we allow >> for reproducible builds in JavaFX 17 and consider making them the default in >> JavaFX 18. >> >> Fixes >> >> There are at least four sources of non-determinism in the JavaFX builds: >> >> 1. Build timestamp >> >> The class `com.sun.javafx.runtime.VersionInfo` in the JavaFX Base module >> stores the time of the build. Furthermore, for builds that don't run on the >> Hudson continuous integration tool, the class adds the build time to the >> system property `javafx.runtime.version`. >> >> 2. Modification times >> >> The JAR, JMOD, and ZIP archives store the modification time of each file. >> >> 3. File ordering >> >> The JAR, JMOD, and ZIP archives store their files in the order returned >> by the file system. The native shared libraries also store their object >> files in the order returned by the file system. Most file systems, though, >> do not guarantee the order of a directory's file listing. >> >> 4. Build path >> >> The class `com.sun.javafx.css.parser.Css2Bin` in the JavaFX Graphics >> module stores the absolute path of its `.css` input file in the >> corresponding `.bss` output file, which is then included in the JavaFX >> Controls module. >> >> This pull request modifies the Gradle and Groovy build files to fix the >> first three sources of non-determinism. A later pull request can modify the >> Java files to fix the fourth. >> >> [1]: https://reproducible-builds.org/docs/source-date-epoch/ >> [2]: https://salsa.debian.org/reproducible-builds/strip-nondeterminism > > John Neffenger has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains ten commits: > > - Get build timestamp in UTC and set ZIP timestamps > >Create the build timestamp as a zoned date and time in UTC instead >of a local date and time. If SOURCE_DATE_EPOCH is defined, set the >last modification time of ZIP and JAR entries to the local date and >time in UTC of the instant defined by SOURCE_DATE_EPOCH. > >Add a comment as a reminder to make JMOD files deterministic when >the following enhancements are complete: > >* Enable deterministic file content ordering for Jar and Jmod > https://bugs.openjdk.java.net/browse/JDK-8276764 > https://github.com/openjdk/jdk/pull/6395 > >* Enable jar and jmod to produce deterministic timestamped content > https://bugs.openjdk.java.net/browse/JDK-8276766 > https://github.com/openjdk/jdk/pull/6481 > - Merge branch 'master' into allow-reproducible-builds > - Make build of SDK ZIP bundle reproducible > - Merge branch 'master' into allow-reproducible-builds > - Merge branch 'master' into allow-reproducible-builds > - Include WebKit shared library for Windows > >Enable reproducible builds of the native WebKit shared library for >Windows (jfxwebkit.dll) when SOURCE_DATE_EPOCH is defined. > - 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 > - Enable reproducible builds with SOURCE_DATE_EPOCH > - 8238650: Allow to override buildDate with SOURCE_DATE_EPOCH The results of 33 builds are shown in the table below: | Builds | Linux | macOS | Win10 | | --- |:-:|:-:|:-:| | Develop | ✔ | ✔ | ✔ | | Actions | ✔ | ✔ | ✔ | | Release | ✔ | ✔ | ✔ | where the check mark (✔) means that the unit tests ran successfully and that the files in the `build` directory where identical between two builds on the same system except for the `libjfxwebkit.dylib` file on macOS and the JMOD files on all of the systems. The build types are shown below by their Gradle options and tasks: * **Develop:** `bash grad
Re: RFR: 8274274: Update JUnit to version 5.8.1 [v9]
On Mon, 22 Nov 2021 13:43:34 GMT, John Hendrikx wrote: >> I've added JUnit 5 as a test dependency and made sure that the JUnit 4 tests >> still work. Also added a single JUnit 5 tests, and confirmed it works. >> >> I've updated the Eclipse project file for the base module only. > > John Hendrikx has updated the pull request incrementally with two additional > commits since the last revision: > > - Remove remaining references to old JUnit 4.8.2 > - Upgrade to JUnit 4.13.2 @johanvos @tiainen Can one of you also review this to ensure it causes no problems with your builds? - PR: https://git.openjdk.java.net/jfx/pull/633
Re: RFR: 8274274: Update JUnit to version 5.8.1 [v9]
On Mon, 22 Nov 2021 13:43:34 GMT, John Hendrikx wrote: >> I've added JUnit 5 as a test dependency and made sure that the JUnit 4 tests >> still work. Also added a single JUnit 5 tests, and confirmed it works. >> >> I've updated the Eclipse project file for the base module only. > > John Hendrikx has updated the pull request incrementally with two additional > commits since the last revision: > > - Remove remaining references to old JUnit 4.8.2 > - Upgrade to JUnit 4.13.2 Looks good. I did a CI build with the latest changes. - Marked as reviewed by kcr (Lead). PR: https://git.openjdk.java.net/jfx/pull/633
Re: RFR: 8264449: Enable reproducible builds with SOURCE_DATE_EPOCH [v5]
On Sat, 18 Sep 2021 15:15:10 GMT, Kevin Rushforth wrote: > I did a CI build yesterday and again today on all three platforms and > compared the sdk between the two builds for each platform. On all three > platforms the results are the same: All files were identical except the > native jfxwebkit library. So there is something in the WebKit build that is > affected by an external input (perhaps the system date or similar). I can recreate this difference, but only on macOS. **Lesson learned:** When testing reproducible builds on a permanent system, never use the Gradle Daemon! I always get a different `libjfxwebkit.dylib` file when I run the build with `bash gradlew --no-daemon`. Shown below are the versions of the compilers I'm using: john@linux:~$ gcc --version gcc (Ubuntu 9.3.0-17ubuntu1~20.04) 9.3.0 john@macos:~$ gcc --version Apple clang version 13.0.0 (clang-1300.0.29.3) john@win10:~$ gcc --version gcc (GCC) 11.2.0 When I dump each of the shared libraries with the following script, I get a text file with 10,413,716 lines, yet there are only 108 lines different between the two output files. #!/bin/bash # Dumps macOS shared libraries # Diffoscope fails due to missing '=' after '--arch-name' option: # llvm-objdump: error: unknown argument '--arch-name' llvm-objdump --arch-name=x86_64 --section="__TEXT,__text" --macho \ --demangle --no-leading-addr --no-show-raw-insn "$@" The differences are shown below: diff --git a/libjfxwebkit.5.txt b/libjfxwebkit.6.txt index 9d6932a..63f2b70 100644 --- a/libjfxwebkit.5.txt +++ b/libjfxwebkit.6.txt @@ -1,4 +1,4 @@ -libjfxwebkit.5.dylib: +libjfxwebkit.6.dylib: Contents of (__TEXT,__text) section __ZN6WebKit15StorageAreaImplD2Ev: pushq %rbp @@ -881412,30 +881412,22 @@ __ZN7WebCore30isCSSPropertyEnabledBySettingsENS_13CSSPropertyIDEPKNS_8SettingsE: movq%rsp, %rbp movb$1, %al testq %rsi, %rsi - je 0x319ee0 + je 0x319ee7 leal-258(%rdi), %ecx cmpw$37, %cx - ja 0x319eb7 + ja 0x319eb2 movzwl %cx, %ecx - leaq79(%rip), %rdx + leaq75(%rip), %rdx movslq (%rdx,%rcx,4), %rcx addq%rdx, %rcx jmpq*%rcx movb466(%rsi), %al - andb$4, %al - shrb$2, %al - popq%rbp - retq + jmp 0x319ee2 + cmpw$43, %di + je 0x319edc movzwl %di, %ecx cmpl$366, %ecx - je 0x319ed5 - cmpw$43, %di - jne 0x319ee0 - movb451(%rsi), %al - andb$4, %al - shrb$2, %al - popq%rbp - retq + jne 0x319ee7 movb454(%rsi), %al andb$32, %al shrb$5, %al @@ -881446,46 +881438,52 @@ __ZN7WebCore30isCSSPropertyEnabledBySettingsENS_13CSSPropertyIDEPKNS_8SettingsE: shrb%al popq%rbp retq + movb451(%rsi), %al + andb$4, %al + shrb$2, %al + popq%rbp + retq + nopl(%rax) + .long 4294967230@ KIND_JUMP_TABLE32 + .long 4294967230@ KIND_JUMP_TABLE32 + .long 4294967230@ KIND_JUMP_TABLE32 + .long 4294967291@ KIND_JUMP_TABLE32 + .long 4294967291@ KIND_JUMP_TABLE32 + .long 4294967291@ KIND_JUMP_TABLE32 + .long 4294967291@ KIND_JUMP_TABLE32 + .long 4294967291@ KIND_JUMP_TABLE32 + .long 4294967291@ KIND_JUMP_TABLE32 + .long 4294967291@ KIND_JUMP_TABLE32 + .long 4294967291@ KIND_JUMP_TABLE32 + .long 4294967291@ KIND_JUMP_TABLE32 + .long 4294967291@ KIND_JUMP_TABLE32 + .long 4294967291@ KIND_JUMP_TABLE32 + .long 4294967291@ KIND_JUMP_TABLE32 + .long 4294967291@ KIND_JUMP_TABLE32 + .long 4294967291@ KIND_JUMP_TABLE32 + .long 4294967291@ KIND_JUMP_TABLE32 + .long 4294967291@ KIND_JUMP_TABLE32 + .long 4294967291@ KIND_JUMP_TABLE32 + .long 4294967291@ KIND_JUMP_TABLE32 + .long 4294967291@ KIND_JUMP_TABLE32 + .long 4294967291@ KIND_JUMP_TABLE32 + .long 4294967291@ KIND_JUMP_TABLE32 + .long 4294967291@ KIND_JUMP_TABLE32 + .long 4294967291@ KIND_JUMP_TABLE32 + .long 4294967291@ KIND_JUMP_TABLE32 + .long 4294967291@ KIND_JUMP_TABLE32 + .long 4294967291@ KIND_JUMP_TABLE32 + .long 4294967291@ KIND_JUMP_TABLE32 + .long 4294967291@ KIND_JUMP_TABLE32 + .long 4294967291@ KIND_JUMP_TABLE32 + .long 4294967255@ KIND_JUMP_TABLE32 + .long 4294967291@ KIND_JUMP_TABLE32 + .long 4294967291@ KIND_JUMP_TABLE32 + .long 4294967291@ KIND_JUMP_TABLE32 + .long 4294967255
Re: RFR: 8264449: Enable reproducible builds with SOURCE_DATE_EPOCH [v5]
On Fri, 17 Sep 2021 22:10:41 GMT, Kevin Rushforth wrote:
>> John Neffenger has updated the pull request with a new target base due to a
>> merge or a rebase. The pull request now contains seven commits:
>>
>> - Make build of SDK ZIP bundle reproducible
>> - Merge branch 'master' into allow-reproducible-builds
>> - Merge branch 'master' into allow-reproducible-builds
>> - Include WebKit shared library for Windows
>>
>>Enable reproducible builds of the native WebKit shared library for
>>Windows (jfxwebkit.dll) when SOURCE_DATE_EPOCH is defined.
>> - 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
>> - Enable reproducible builds with SOURCE_DATE_EPOCH
>> - 8238650: Allow to override buildDate with SOURCE_DATE_EPOCH
>
> build.gradle line 559:
>
>> 557: buildDate = new java.util.Date(ms)
>> 558: }
>> 559: def buildTimestamp = new
>> java.text.SimpleDateFormat("-MM-dd-HHmmss").format(buildDate)
>
> I think it would be useful to format `buildDate` using UTC as the time zone
> when `SOURCE_DATE_EPOCH` is set.
Indeed. The build now uses the new `java.time.Instant` class to get the instant
on the time-line, whether or not `SOURCE_DATE_EPOCH` is set, and creates the
timestamp in UTC using the ISO 8601 date and time format.
> build.gradle line 3518:
>
>> 3516: def lFlags =
>> webkitProperties.linkFlags?.join(' ') ?: ''
>> 3517: cmakeArgs = "$cmakeArgs
>> -DCMAKE_C_FLAGS='${cFlags}' -DCMAKE_CXX_FLAGS='${cFlags}'"
>> 3518: cmakeArgs = "$cmakeArgs
>> -DCMAKE_SHARED_LINKER_FLAGS='${lFlags}'"
>
> I presume you've tested this both with and without `SOURCE_DATE_EPOCH`?
Well, now I have! 😄 Thanks.
> build.gradle line 3914:
>
>> 3912: tasks.withType(AbstractArchiveTask) {
>> 3913: if (sourceDateEpoch != null) {
>> 3914: preserveFileTimestamps = false
>
> This is a problem given how gradle generates a zip archive when this is set.
> How hard would it be to force all time stamps to be `SOURCE_DATE_EPOCH`?
It was harder than I had hoped, but I think I found a good solution. The only
changes in the ZIP and JAR archives are shown in the `diff` extract below (from
`zipinfo -v`):
29c29
< minimum software version required to extract: 1.0
---
> minimum software version required to extract: 2.0
33,34c33,34
< extended local header: no
< file last modified on (DOS date/time): 1980 Feb 1 00:00:00
---
> extended local header: yes
> file last modified on (DOS date/time): 2021 Nov 16 17:55:42
44c44
< MS-DOS file attributes (10 hex):dir
---
> MS-DOS file attributes (00 hex):none
where:
* The minimum software version required to extract is now set to the correct
value of 2.0, which is the minimum for file entries compressed with DEFLATE.
* The extended local header is defined but with length zero.
* The timestamp is set to the local date and time in UTC of the instant of the
build.
* The MS-DOS file attribute for a directory is not set. It does not appear to
be useful, as an entry in a ZIP file is a directory if and only if its name
ends with a slash ("/").
These changes are due to Gradle using the Apache Ant
`org.apache.tools.zip.ZipEntry` class, while the build now uses the
`java.util.zip.ZipEntry` class in OpenJDK to create the archives.
-
PR: https://git.openjdk.java.net/jfx/pull/446
Re: RFR: 8150709: Mac OSX and German Keyboard Layout (Y/Z) [v4]
On Thu, 25 Mar 2021 17:41:40 GMT, Martin Fox wrote: >> This PR adds code to ensure that KeyCodeCombinations match KeyEvents as >> expected by more accurately mapping from a Mac key code to a Java key code >> based on the user’s active keyboard layout (the existing code assumes a US >> QWERTY layout). The new code first identifies a set of Mac keys which can >> produce different characters based on the user’s keyboard layout. A Mac key >> code outside that area is processed exactly as before. For a key inside the >> layout-sensitive area the code calls UCKeyTranslate to translate the key to >> an unshifted ASCII character based on the active keyboard and uses that to >> determine the Java key code. >> >> When performing the reverse mapping for the Robot the code first uses the >> old QWERTY mapping to find a candidate key. If it lies in the >> layout-sensitive area the code then scans the entire area calling >> UCKeyTranslate until it finds a match. If the key lies outside the >> layout-sensitive area it’s processed exactly as before. >> >> There are multiple duplicates of these bugs logged against Mac applications >> built with JavaFX. >> >> https://bugs.openjdk.java.net/browse/JDK-8090257 Mac: Inconsistent KeyEvents >> with alternative keyboard layouts >> https://bugs.openjdk.java.net/browse/JDK-8088120 [Accelerator, Mac] CMD + Z >> accelerator is not working with French keyboard >> https://bugs.openjdk.java.net/browse/JDK-8087915 Mac: accelerator doesn't >> take into account azerty keyboard layout >> https://bugs.openjdk.java.net/browse/JDK-8150709 Mac OSX and German Keyboard >> Layout (Y/Z) > > Martin Fox has updated the pull request incrementally with one additional > commit since the last revision: > > Fixed whitespace error. I’m not sure how to approach regression testing. MacOS 11 ships with 189 keyboard layouts and this PR changes the key assignments for 62 of them. On each layout there are just under 50 keys that might be affected. Some automated or manual sanity-checking tests would go a long way but the Robot code has bugs on every platform and I want to deal with the user-facing issues first (though this PR also fixes the Robot on Mac). For manual testing I run a JavaFX app that dumps out info on each key event. There’s one attached to this e-mail thread under the name KeyboardTest.txt. I use the Mac Keyboard Viewer app to visualize layouts that don’t match what’s printed on my keyboard. It’s also helpful to compare against a Windows box if you have one. When testing code changes I rely on a command line tool that runs through all the layouts and compares the results of the new algorithm against the old algorithm. This is useful for flagging issues that no amount of manual testing would be likely to find (ask me about Lithuanian digits some day) but I don’t really expect other testers to go that far. If you are testing this keep in mind that there are no key codes corresponding to accented characters or other characters that bear diacritic marks. Those keys will be assigned KeyCode.UNDEFINED. And stay away from dead keys, the way they behave on the Mac is, um, complicated. - PR: https://git.openjdk.java.net/jfx/pull/425
RFR: 8277572: Image class documentation incorrect for images encoded in data URIs
The javadoc of `javafx.scene.image.Image` incorrectly states:
94* If a URL uses the "data" scheme, the data must be base64-encoded
95* and the MIME type must either be empty or a subtype of the
96* {@code image} type.
However, omitting the MIME type of a data URI is specified to imply
"text/plain" (RFC 2397, section 2). Since the `com.sun.javafx.util.DataURI`
class is implemented according to this specification, trying to load an image
without MIME type correctly fails with an `ImageStorageException`: "Unexpected
MIME type: text".
The solution is to fix the documentation:
94* If a URL uses the "data" scheme, the data must be base64-encoded
+ 95* and the MIME type must be a subtype of the {@code image} type.
- 95* and the MIME type must either be empty or a subtype of the
- 96* {@code image} type.
-
Commit messages:
- Fixed incorrect documentation of image data URIs
Changes: https://git.openjdk.java.net/jfx/pull/676/files
Webrev: https://webrevs.openjdk.java.net/?repo=jfx&pr=676&range=00
Issue: https://bugs.openjdk.java.net/browse/JDK-8277572
Stats: 32 lines in 2 files changed: 30 ins; 1 del; 1 mod
Patch: https://git.openjdk.java.net/jfx/pull/676.diff
Fetch: git fetch https://git.openjdk.java.net/jfx pull/676/head:pull/676
PR: https://git.openjdk.java.net/jfx/pull/676
Re: RFR: 8197991: Selecting many items in a TableView is very slow
On Mon, 22 Nov 2021 15:55:03 GMT, Abhinay Agarwal wrote: >> the for-loop is certainly faster and would allocate less memory - i find the >> `for(int i = 0, max = size())`-style a bit odd > > I could move `int max = size();` outside the loop But why? The initialization block of a `for` statement is exactly where you'd put loop-scoped variables. - PR: https://git.openjdk.java.net/jfx/pull/673
Re: RFR: 8197991: Selecting many items in a TableView is very slow
On Thu, 18 Nov 2021 08:53:07 GMT, Marius Hanl wrote: >> This work improves the performance of `MultipleSelectionModel` over large >> data sets by caching some values and avoiding unnecessary calls to >> `SelectedIndicesList#size`. It further improves the performance by reducing >> the number of iterations required to find the index of an element in the >> BitSet. >> >> The work is based on [an abandoned >> patch](https://github.com/openjdk/jfx/pull/127) submitted by @yososs >> >> There are currently 2 manual tests for this fix. > > Just wondering, isn't it also possible to write some unit tests for the > MultipleSelectionModel(Base) ? @Maran23 I don't know what would be an apt unit test as the PR changes implementation detail. If you have suggestions, please let me know. There are 276 unit tests in `test.javafx.scene.control.MultipleSelectionModelImplTest` and all of them still pass with the changes made to this PR. - PR: https://git.openjdk.java.net/jfx/pull/673
Re: RFR: 8197991: Selecting many items in a TableView is very slow
On Thu, 18 Nov 2021 00:54:30 GMT, Nir Lisker wrote: >> This work improves the performance of `MultipleSelectionModel` over large >> data sets by caching some values and avoiding unnecessary calls to >> `SelectedIndicesList#size`. It further improves the performance by reducing >> the number of iterations required to find the index of an element in the >> BitSet. >> >> The work is based on [an abandoned >> patch](https://github.com/openjdk/jfx/pull/127) submitted by @yososs >> >> There are currently 2 manual tests for this fix. > > modules/javafx.controls/src/main/java/com/sun/javafx/scene/control/ReadOnlyUnbackedObservableList.java > line 193: > >> 191: arr[i] = get(i); >> 192: } >> 193: return arr; > > Have you tried `return stream().toArray();`? I haven't measured how `stream().toArray()` compare to a for..loop, but I inherently try to avoid streams in such scenarios. - PR: https://git.openjdk.java.net/jfx/pull/673
Re: RFR: 8197991: Selecting many items in a TableView is very slow
On Thu, 18 Nov 2021 09:06:08 GMT, Tom Schindl wrote: >> modules/javafx.controls/src/main/java/com/sun/javafx/scene/control/ReadOnlyUnbackedObservableList.java >> line 119: >> >>> 117: Object obj = get(i); >>> 118: if (o.equals(obj)) return i; >>> 119: } >> >> An alternative is >> >> return IntStream.range(0, size()) >> .filter(i -> o.equals(get(i))) >> .findFirst() >> .orElse(-1); >> >> I don't know if it helps. > > the for-loop is certainly faster and would allocate less memory - i find the > `for(int i = 0, max = size())`-style a bit odd I could move `int max = size();` outside the loop - PR: https://git.openjdk.java.net/jfx/pull/673
Re: RFR: 8277122: SplitPane divider drag can hang the layout [v2]
> When a divider is moved via drag or code it will call **requestLayout()** for > the **SplitPane**. > While this is fine, it is also called when the > **SplitPaneSkin#layoutChildren(..)** method is repositioning the divider. > > This makes no sense since we are currently layouting everything, so we don't > need to request it again. (The divider positioning is the first part of > **layoutChildren(..)**. In the second part the SplitPane content is layouted > based off those positions) > > -> With this behaviour the layout may hang under some conditions (check > attached source). The problem is that the **requestLayout()** will mark the > **SplitPane** dirty but won't propagate to the parent since the **SplitPane** > is currently doing a layout. > > This PR fixes this by not requesting layout when we are currently doing it > (and thus repositioning the dividers as part of it). > > Note: I also fixed a simple typo of a private method in SplitPaneSkin: > initializeDivderEventHandlers -> initializeDiv**i**derEventHandlers Marius Hanl has updated the pull request incrementally with one additional commit since the last revision: 8277122: Added test for setting a negative divider position + improved readability - Changes: - all: https://git.openjdk.java.net/jfx/pull/669/files - new: https://git.openjdk.java.net/jfx/pull/669/files/919f5db8..9db28ff0 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jfx&pr=669&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jfx&pr=669&range=00-01 Stats: 27 lines in 2 files changed: 14 ins; 1 del; 12 mod Patch: https://git.openjdk.java.net/jfx/pull/669.diff Fetch: git fetch https://git.openjdk.java.net/jfx pull/669/head:pull/669 PR: https://git.openjdk.java.net/jfx/pull/669
Re: RFR: 8276490: Incorrect path for duplicate x and y values, when path falls outside axis bound [v3]
> PathElements were skipped in AreaChart if the data point were outside axis > bounds and had duplicate value for either x or y. This is now fixed with this > PR. Abhinay Agarwal has updated the pull request incrementally with one additional commit since the last revision: Update line ending - Changes: - all: https://git.openjdk.java.net/jfx/pull/667/files - new: https://git.openjdk.java.net/jfx/pull/667/files/1381aaa4..c98cc976 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jfx&pr=667&range=02 - incr: https://webrevs.openjdk.java.net/?repo=jfx&pr=667&range=01-02 Stats: 535 lines in 1 file changed: 0 ins; 0 del; 535 mod Patch: https://git.openjdk.java.net/jfx/pull/667.diff Fetch: git fetch https://git.openjdk.java.net/jfx pull/667/head:pull/667 PR: https://git.openjdk.java.net/jfx/pull/667
Re: RFR: 8276490: Incorrect path for duplicate x and y values, when path falls outside axis bound [v2]
On Wed, 17 Nov 2021 10:07:37 GMT, Ajit Ghaisas wrote:
>> Abhinay Agarwal has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Fix condition and add more tests
>
> modules/javafx.controls/src/test/java/test/javafx/scene/chart/AreaChartTest.java
> line 187:
>
>> 185: }
>> 186:
>> 187: @Test public void testPathOutsideXBoundsWithDuplicateXAndLowerY() {
>
> I wonder whether we need another set of similar tests with X and Y
> interchanged?
> Something like - testPathOutsideYBoundsWithDuplicateYAndLowerX
I have added a couple of additional tests
-
PR: https://git.openjdk.java.net/jfx/pull/667
Re: RFR: 8274274: Update JUnit to version 5.8.1 [v9]
> I've added JUnit 5 as a test dependency and made sure that the JUnit 4 tests > still work. Also added a single JUnit 5 tests, and confirmed it works. > > I've updated the Eclipse project file for the base module only. John Hendrikx has updated the pull request incrementally with two additional commits since the last revision: - Remove remaining references to old JUnit 4.8.2 - Upgrade to JUnit 4.13.2 - Changes: - all: https://git.openjdk.java.net/jfx/pull/633/files - new: https://git.openjdk.java.net/jfx/pull/633/files/f556a1ae..5f4f695b Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jfx&pr=633&range=08 - incr: https://webrevs.openjdk.java.net/?repo=jfx&pr=633&range=07-08 Stats: 10 lines in 2 files changed: 1 ins; 8 del; 1 mod Patch: https://git.openjdk.java.net/jfx/pull/633.diff Fetch: git fetch https://git.openjdk.java.net/jfx pull/633/head:pull/633 PR: https://git.openjdk.java.net/jfx/pull/633
Re: RFR: 8276490: Incorrect path for duplicate x and y values, when path falls outside axis bound [v2]
> PathElements were skipped in AreaChart if the data point were outside axis > bounds and had duplicate value for either x or y. This is now fixed with this > PR. Abhinay Agarwal has updated the pull request incrementally with one additional commit since the last revision: Fix condition and add more tests - Changes: - all: https://git.openjdk.java.net/jfx/pull/667/files - new: https://git.openjdk.java.net/jfx/pull/667/files/07efa203..1381aaa4 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jfx&pr=667&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jfx&pr=667&range=00-01 Stats: 726 lines in 3 files changed: 534 ins; 11 del; 181 mod Patch: https://git.openjdk.java.net/jfx/pull/667.diff Fetch: git fetch https://git.openjdk.java.net/jfx pull/667/head:pull/667 PR: https://git.openjdk.java.net/jfx/pull/667
Re: RFR: 8274274: Update JUnit to version 5.8.1 [v7]
On Mon, 22 Nov 2021 12:52:10 GMT, Kevin Rushforth wrote: >> John Hendrikx has updated the pull request incrementally with two additional >> commits since the last revision: >> >> - Add trivial assert to JUnit5Test >> - Add explicit dependencies in build.gradle > > modules/javafx.base/src/test/java/test/JUnit5Test.java line 28: > >> 26: package test; >> 27: >> 28: import static org.junit.Assert.assertNotNull; > > For JUnit 5, shouldn't this be > `org.junit.jupiter.api.Assertions.assertNotNull`? Sorry, I totally missed that. Although you can mix these without ill effect, we should move towards the `jupiter` annotations if we ever want to get to the point where JUnit 4 can be phased out. - PR: https://git.openjdk.java.net/jfx/pull/633
Re: RFR: 8274274: Update JUnit to version 5.8.1 [v8]
> I've added JUnit 5 as a test dependency and made sure that the JUnit 4 tests > still work. Also added a single JUnit 5 tests, and confirmed it works. > > I've updated the Eclipse project file for the base module only. John Hendrikx has updated the pull request incrementally with one additional commit since the last revision: Fix import to use jupiter annotations - Changes: - all: https://git.openjdk.java.net/jfx/pull/633/files - new: https://git.openjdk.java.net/jfx/pull/633/files/d06d6570..f556a1ae Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jfx&pr=633&range=07 - incr: https://webrevs.openjdk.java.net/?repo=jfx&pr=633&range=06-07 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jfx/pull/633.diff Fetch: git fetch https://git.openjdk.java.net/jfx pull/633/head:pull/633 PR: https://git.openjdk.java.net/jfx/pull/633
Re: RFR: 8274274: Update JUnit to version 5.8.1 [v7]
On Sun, 21 Nov 2021 21:50:36 GMT, John Hendrikx wrote: >> I've added JUnit 5 as a test dependency and made sure that the JUnit 4 tests >> still work. Also added a single JUnit 5 tests, and confirmed it works. >> >> I've updated the Eclipse project file for the base module only. > > John Hendrikx has updated the pull request incrementally with two additional > commits since the last revision: > > - Add trivial assert to JUnit5Test > - Add explicit dependencies in build.gradle I was looking for places where JUnit 4.8.2 was still being referenced and found two more places that should be changed: 1. [`buildSrc/build.gradle`](https://github.com/openjdk/jfx/blob/d06d657007c3af86ba2ff908b546cae18e775bfb/buildSrc/build.gradle#L77) depends on JUnit 4.8.2 -- this should be changed to JUnit 4.13.2 (and you will need to add hamcrest) 2. Once the above is fixed, you should be able to remove the JUnit 4.8.2 artifacts from [`gradle/verification-metadata.xml`](https://github.com/openjdk/jfx/blob/d06d657007c3af86ba2ff908b546cae18e775bfb/gradle/verification-metadata.xml#L122) - PR: https://git.openjdk.java.net/jfx/pull/633
Re: RFR: 8274274: Update JUnit to version 5.8.1 [v7]
On Sun, 21 Nov 2021 21:50:36 GMT, John Hendrikx wrote: >> I've added JUnit 5 as a test dependency and made sure that the JUnit 4 tests >> still work. Also added a single JUnit 5 tests, and confirmed it works. >> >> I've updated the Eclipse project file for the base module only. > > John Hendrikx has updated the pull request incrementally with two additional > commits since the last revision: > > - Add trivial assert to JUnit5Test > - Add explicit dependencies in build.gradle Looks good with one comment inline. modules/javafx.base/src/test/java/test/JUnit5Test.java line 28: > 26: package test; > 27: > 28: import static org.junit.Assert.assertNotNull; For JUnit 5, shouldn't this be `org.junit.jupiter.api.Assertions.assertNotNull`? - PR: https://git.openjdk.java.net/jfx/pull/633
Integrated: 8276174: JavaFX build fails on macOS aarch64
On Thu, 11 Nov 2021 10:46:26 GMT, Andreas Heger wrote: > By changing the value for the clang -arch parameter to "arm64", the jfx > project compiles on an apple silicon system. Are there any side effects which > I might be missing in this simple solution? This pull request has now been integrated. Changeset: d289db94 Author:andreas-heger Committer: Kevin Rushforth URL: https://git.openjdk.java.net/jfx/commit/d289db94d15e49ed28f797b516ffccf023fce9c9 Stats: 3 lines in 1 file changed: 2 ins; 0 del; 1 mod 8276174: JavaFX build fails on macOS aarch64 Reviewed-by: kcr, jvos - PR: https://git.openjdk.java.net/jfx/pull/666
