Re: RFR: 8197991: Selecting many items in a TableView is very slow [v5]
On Thu, 6 Jan 2022 21:01:11 GMT, Kevin Rushforth wrote: >> Abhinay Agarwal has updated the pull request incrementally with two >> additional commits since the last revision: >> >> - Remove commented out method. Document constructors. >> - Add tests > > modules/javafx.controls/src/test/java/test/javafx/scene/control/MultipleSelectionModelImplTest.java > line 1436: > >> 1434: model.clearSelection(); >> 1435: >> 1436: assertTrue(model.getSelectedIndices().isEmpty()); > > I recommend to also check the size. Isn't checking the size redundant in this case? `model.getSelectedIndices().isEmpty()` indicates that `model.getSelectedIndices().size()` is `0`. - PR: https://git.openjdk.java.net/jfx/pull/673
Re: RFR: 8279328: CssParser uses default charset instead of UTF-8
On Wed, 29 Dec 2021 15:39:39 GMT, Michael Strauß wrote: > `CssParser.parse(URL)` is specified to assume UTF-8 file encoding, but > (implicitly) uses the default charset to read the file, potentially resulting > in incorrect interpretation of the file content. > > This can be fixed by explicitly specifying the UTF-8 charset for > `InputStreamReader`. Marked as reviewed by aghaisas (Reviewer). - PR: https://git.openjdk.java.net/jfx/pull/705
Re: RFR: 8277572: ImageStorage should correctly handle MIME types for images encoded in data URIs [v4]
On Wed, 24 Nov 2021 03:43:42 GMT, Michael Strauß wrote:
>> `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 either be empty or a subtype of the
>> - 96* {@code image} type.
>> + 95* and the MIME type must be a subtype of the {@code image} type.
>> + 96* The MIME type must match the image format of the data
>> contained in
>> + 97* the URL. In case of a mismatch between MIME type and image
>> format,
>> + 98* the image will be loaded if the image format can be determined
>> by
>> + 99* JavaFX, and a warning will be logged.
>
> Michael Strauß has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Added test for image format without signature
The API doc change looks good to me. Go ahead and finalize the CSR.
The fix and test look good with one question about what looks like an unrelated
change.
modules/javafx.graphics/src/main/java/com/sun/javafx/iio/ImageStorage.java line
490:
> 488: if (stream.read(header) <= 0) {
> 489: return null;
> 490: }
What was the reason for this change? The former would work even if the stream
returned less data that the size of the header in a single read, whereas the
latter would fail.
-
PR: https://git.openjdk.java.net/jfx/pull/676
Re: RFR: 8197991: Selecting many items in a TableView is very slow [v5]
On Thu, 6 Jan 2022 08:17:53 GMT, Abhinay Agarwal 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.
>
> Abhinay Agarwal has updated the pull request incrementally with two
> additional commits since the last revision:
>
> - Remove commented out method. Document constructors.
> - Add tests
The fix looks good. I left a few comments on the tests.
modules/javafx.controls/src/test/java/test/javafx/scene/control/MultipleSelectionModelImplTest.java
line 1427:
> 1425: assertTrue(model.isSelected(2));
> 1426: assertTrue(model.isSelected(5));
> 1427: assertFalse(model.isSelected(0));
I recommend to also check the size.
modules/javafx.controls/src/test/java/test/javafx/scene/control/MultipleSelectionModelImplTest.java
line 1436:
> 1434: model.clearSelection();
> 1435:
> 1436: assertTrue(model.getSelectedIndices().isEmpty());
I recommend to also check the size.
tests/manual/controls/SelectTableViewTest.java line 40:
> 38: public class SelectTableViewTest extends Application {
> 39:
> 40: final int ROW_COUNT = 700_000;
This count is too large, even with the fix. I recommend changing it to
something smaller (no more than `70_000`, which matches what you did for
`SelectListViewTest`).
tests/manual/controls/SelectTableViewTest.java line 101:
> 99: long t = System.currentTimeMillis();
> 100: tableView.getSelectionModel().selectAll();
> 101: System.out.println("time:"+ (System.currentTimeMillis() - t));
Minor: space before the `+` here and a few places below (also in the other test
as noted).
-
PR: https://git.openjdk.java.net/jfx/pull/673
Re: RFR: 8197991: Selecting many items in a TableView is very slow [v3]
On Fri, 17 Dec 2021 20:38:56 GMT, Kevin Rushforth wrote:
>> Abhinay Agarwal has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Update ROW_COUNT to 700_000
>
> tests/manual/controls/SelectListViewTest.java line 66:
>
>> 64: long t = System.currentTimeMillis();
>> 65: listView.getSelectionModel().clearSelection();
>> 66: System.out.println("time:"+ (System.currentTimeMillis() - t));
>
> MInor: space before the `+` here and a few places below.
This is still pending.
-
PR: https://git.openjdk.java.net/jfx/pull/673
Re: RFR: 8279228 Leak in ScrollPaneSkin, related to touch events
On Thu, 23 Dec 2021 17:43:19 GMT, Florian Kirmaier wrote: > Fixing memoryleak, related to touch events in ScrollPaneWhen touchDetected or > mouseDown is true, the sbTouch animation is running, > and the node is removed from the Scene, then the animation will never stop, > causing a memory leak. > A simple fix is to also check, whether the Node is visible, by checking the > "isTreeShowing" property. Two quick questions: 1. The fix checks whether or not the node is treeShowing at the time the `startSBReleasedAnimation` method is called. Is this sufficient? If a node's tree showing state changes, is it guaranteed that this method will be called? 2. Can you provide an automated test for this? - PR: https://git.openjdk.java.net/jfx/pull/701
Re: RFR: 8274771: Map, FlatMap and OrElse fluent bindings for ObservableValue [v4]
On Wed, 5 Jan 2022 12:29:01 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
>> ));
>>
>> // 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:
>
> Apply changes suggested in review and updated copyright years to 2022
The changes look good. I added some minor grammar comments. I think that the
API is in a good spot.
modules/javafx.base/src/main/java/javafx/beans/value/ObservableValue.java line
145:
> 143: /**
> 144: * Creates an {@code ObservableValue} that holds the result of
> applying a
> 145: * mapping on the value held by this {@code ObservableValue}. The
> result is
After one of your suggestions, another phrasing for `on the value held by this
{@code ObservableValue}` is `on this {@code ObservableValue}'s value`, which is
a bit shorter. If you think it helps, you can make this change here and in
other places. Fine to leave as it.
modules/javafx.base/src/main/java/javafx/beans/value/ObservableValue.java line
164:
> 162: * @param mapper a {@code Function} which converts a given value to
> a new value, cannot
Re: RFR: 8274771: Map, FlatMap and OrElse fluent bindings for ObservableValue [v3]
On Wed, 5 Jan 2022 10:51:28 GMT, John Hendrikx wrote:
>> modules/javafx.base/src/main/java/javafx/beans/value/ObservableValue.java
>> line 152:
>>
>>> 150: * @return an {@link ObservableValue} which provides a mapping of
>>> the value
>>> 151: * held by this {@code ObservableValue}, and provides {@code
>>> null} when
>>> 152: * this {@code ObservableValue} holds {@code null}, never null
>>
>> No need for `@link`.
>>
>> Since we already explained how the mapping works, maybe we can be more brief
>> here:
>>
>> an {@code ObservableValue} that holds the result of the mapping of the value
>> held by this {@code ObservableValue}; never {@code null} itself
>
> I think that `@return` should mention that the returned observable can hold
> `null`, how about:
>
> an {@code ObservableValue} that holds a mapping of this {@code
> ObservableValue}'s value
> or holds {@code null} when the value is {@code null}; never returns
> {@code null}
Good idea to mention that it can hold `null`.
I slightly prefer to say that the returned `ObservableValue` holds the result
of the mapping rather than holds the mapping. I don't really mind it, but it's
the phrasing used in the method description "holds the result of applying a
mapping". "The mapping" itself could be mistaken for the mapping `Function` in
my opinion. If you think it's clear, you can change it to that phrasing, it's
also fine.
-
PR: https://git.openjdk.java.net/jfx/pull/675
Re: RFR: 8274771: Map, FlatMap and OrElse fluent bindings for ObservableValue [v4]
On Wed, 5 Jan 2022 12:29:01 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
>> ));
>>
>> // 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:
>
> Apply changes suggested in review and updated copyright years to 2022
modules/javafx.base/src/main/java/javafx/beans/binding/ObjectBinding.java line
194:
> 192: /**
> 193: * Checks if the binding has at least one listener registered on it.
> This
> 194: * is useful for subclasses which want to conserve resources when
> not observed.
Nitpick: "which" -> "that"
modules/javafx.base/src/main/java/javafx/beans/binding/ObjectBinding.java line
205:
> 203: /**
> 204: * Checks if the binding is allowed to become valid. Overriding
> classes can
> 205: * prevent a binding from becoming valid. This is useful in
> subclasses which
Nitpick: "which" -> "that"
modules/javafx.base/src/main/java/javafx/beans/value/ObservableValue.java line
162:
> 160: *
> 161: * @param the type of values held by the resulting {@code
> ObservableValue}
> 162: * @param mapper a
Re: RFR: 8279328: CssParser uses default charset instead of UTF-8
On Thu, 6 Jan 2022 12:35:56 GMT, Ajit Ghaisas wrote: > The added test passes on my Mac without changing CssParser.java. It may be > due to the default charset being UTF-8. Which platform did you test on? Yes, the default charset on macOS is UTF-8. I’ve tested this on Windows 11, which uses Windows-1252 as the default charset. - PR: https://git.openjdk.java.net/jfx/pull/705
Integrated: 8234921: Add DirectionalLight to the selection of 3D light types
On Mon, 28 Jun 2021 12:13:44 GMT, Nir Lisker wrote: > Adds a directional light as a subclass of `LightBase`. I think that this is > the correct hierarchy for it. > > I tried to simulate a directional light by putting a point light far away, > but I got artifacts when the distance was large. Instead, I added an on/off > attenuation flag as the 4th component of the attenuation 4-vector. When it is > 0, a simpler computation is used in the pixel/fragment shader that calculates > the illumination based on the light direction only (making the position > variables meaningless). When it is 1, the point/spot light computation is > used. It's possible that the vertex shader can also be simplified in this > case since it does not need to transform the position vectors, but I left > this optimization avenue for another time. > > I noticed a drop of ~1 fps in the stress test of 5000 meshes. > > I added a system test that verifies the correct color result from a few > directions. I also updated the lighting sample application to include 3 > directional lights and tested them on all the models visually. The lights > seem to behave the way I would expect. This pull request has now been integrated. Changeset: 32f21ffd Author:Nir Lisker URL: https://git.openjdk.java.net/jfx/commit/32f21ffda9ca21285aac7119458efa35e9b44418 Stats: 543 lines in 29 files changed: 482 ins; 11 del; 50 mod 8234921: Add DirectionalLight to the selection of 3D light types Reviewed-by: kcr, arapte - PR: https://git.openjdk.java.net/jfx/pull/548
Re: RFR: 8234921: Add DirectionalLight to the selection of 3D light types [v6]
On Mon, 20 Dec 2021 13:13:09 GMT, Nir Lisker wrote: >> Adds a directional light as a subclass of `LightBase`. I think that this is >> the correct hierarchy for it. >> >> I tried to simulate a directional light by putting a point light far away, >> but I got artifacts when the distance was large. Instead, I added an on/off >> attenuation flag as the 4th component of the attenuation 4-vector. When it >> is 0, a simpler computation is used in the pixel/fragment shader that >> calculates the illumination based on the light direction only (making the >> position variables meaningless). When it is 1, the point/spot light >> computation is used. It's possible that the vertex shader can also be >> simplified in this case since it does not need to transform the position >> vectors, but I left this optimization avenue for another time. >> >> I noticed a drop of ~1 fps in the stress test of 5000 meshes. >> >> I added a system test that verifies the correct color result from a few >> directions. I also updated the lighting sample application to include 3 >> directional lights and tested them on all the models visually. The lights >> seem to behave the way I would expect. > > Nir Lisker has updated the pull request incrementally with one additional > commit since the last revision: > > Added import The Skara bot needed something to wake it up, and your comment did just that. - PR: https://git.openjdk.java.net/jfx/pull/548
Re: RFR: 8234921: Add DirectionalLight to the selection of 3D light types [v6]
On Mon, 20 Dec 2021 13:13:09 GMT, Nir Lisker wrote: >> Adds a directional light as a subclass of `LightBase`. I think that this is >> the correct hierarchy for it. >> >> I tried to simulate a directional light by putting a point light far away, >> but I got artifacts when the distance was large. Instead, I added an on/off >> attenuation flag as the 4th component of the attenuation 4-vector. When it >> is 0, a simpler computation is used in the pixel/fragment shader that >> calculates the illumination based on the light direction only (making the >> position variables meaningless). When it is 1, the point/spot light >> computation is used. It's possible that the vertex shader can also be >> simplified in this case since it does not need to transform the position >> vectors, but I left this optimization avenue for another time. >> >> I noticed a drop of ~1 fps in the stress test of 5000 meshes. >> >> I added a system test that verifies the correct color result from a few >> directions. I also updated the lighting sample application to include 3 >> directional lights and tested them on all the models visually. The lights >> seem to behave the way I would expect. > > Nir Lisker has updated the pull request incrementally with one additional > commit since the last revision: > > Added import The CSR has been approved, but I don't see it being registered here. - PR: https://git.openjdk.java.net/jfx/pull/548
Re: RFR: 8279328: CssParser uses default charset instead of UTF-8
On Wed, 29 Dec 2021 15:39:39 GMT, Michael Strauß wrote: > `CssParser.parse(URL)` is specified to assume UTF-8 file encoding, but > (implicitly) uses the default charset to read the file, potentially resulting > in incorrect interpretation of the file content. > > This can be fixed by explicitly specifying the UTF-8 charset for > `InputStreamReader`. The added test passes on my Mac without changing CssParser.java. It may be due to the default charset being UTF-8. Which platform did you test on? - PR: https://git.openjdk.java.net/jfx/pull/705
Re: RFR: 8197991: Selecting many items in a TableView is very slow [v3]
On Wed, 22 Dec 2021 13:32:06 GMT, Kevin Rushforth wrote: >> A cache that's manually invalidated and then validated when needed is a form >> of lazy evaluation. The main point, regardless of how you name it, is to >> ensure that every call that modifies the underlying BitSet invalidates the >> size. If it does, and it can be proven to do so, then that's sufficient. > > I checked, and I think that `set(int index, int... indices)` is fine, since > it doesn't modify `bitset` directly, and all of the methods it calls that do > modify it, correctly invalidate `size`. > > One more thing I spotted that needs to be checked, is the > `SelectedIndicesList(BitSet)` constructor. It saves a reference to the source > `BitSet` without making a copy. If a caller ever modified the source > `BitSet`, it would change the underlying `BitSet` seen by the > `SelectedIndicesList` class, and the size would therefore be wrong. Even if > the current code doesn't do this, it's somewhat fragile. I recommend adding a > comment to the constructor, and to the one place it's called, noting that the > source `BitSet` must not be modified after passing it to the constructor. > > Finally, there is a commented out `clearAndSelect` method that would break > the new contract of invalidating size whenever the bitmap is modified, if > that method were ever uncommented. Either it should be deleted as part of > this PR or it should be modified with a (commented out, of course) `size = > -1` in the right place(s). I have added some tests which calls public methods in `MultipleSelectionModelBase` and in turn calls methods from `SelectedIndicesList`. - PR: https://git.openjdk.java.net/jfx/pull/673
Re: RFR: 8197991: Selecting many items in a TableView is very slow [v5]
> 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. Abhinay Agarwal has updated the pull request incrementally with two additional commits since the last revision: - Remove commented out method. Document constructors. - Add tests - Changes: - all: https://git.openjdk.java.net/jfx/pull/673/files - new: https://git.openjdk.java.net/jfx/pull/673/files/2d321087..741aa92f Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jfx=673=04 - incr: https://webrevs.openjdk.java.net/?repo=jfx=673=03-04 Stats: 98 lines in 2 files changed: 58 ins; 40 del; 0 mod Patch: https://git.openjdk.java.net/jfx/pull/673.diff Fetch: git fetch https://git.openjdk.java.net/jfx pull/673/head:pull/673 PR: https://git.openjdk.java.net/jfx/pull/673
