Re: RFR: 8246357: Allow static build of webkit library on linux
On Tue, 2 Jun 2020 17:52:37 GMT, Johan Vos wrote: > add changes to build a static version of libjfxwebkit.a > Fox for JDK-8246357 Changes looks good to me. - Marked as reviewed by ajoseph (Committer). PR: https://git.openjdk.java.net/jfx/pull/245
Re: RFR: 8246204: No 3D support for newer Intel graphics drivers on Linux
On Mon, 1 Jun 2020 17:52:46 GMT, Michael Paus wrote:
> It seems to be sufficient to add "intel" as an additional vendor to the list
> in the X11GLFactory class. Tests pass and
> my own application also works with the new build.
modules/javafx.graphics/src/main/java/com/sun/prism/es2/X11GLFactory.java line
46:
> 45: new GLGPUInfo("intel open source technology center", null),
> 46: new GLGPUInfo("intel", null),
> 47: new GLGPUInfo("nvidia", null),
Since the qualification check is `vendor.startsWith(gi.vendor)` you can replace
the previous `"intel open source
technology center"` entry with the new one.
-
PR: https://git.openjdk.java.net/jfx/pull/243
Re: RFR: 8246357: Allow static build of webkit library on linux
On Tue, 2 Jun 2020 17:52:37 GMT, Johan Vos wrote: > add changes to build a static version of libjfxwebkit.a > Fox for JDK-8246357 Marked as reviewed by kcr (Lead). - PR: https://git.openjdk.java.net/jfx/pull/245
Re: CFV: New OpenJFX Reviewer: Nir Lisker
Vote: yes. -phil.
Re: [Rev 01] RFR: 8193800: TreeTableView selection changes on sorting
On Wed, 3 Jun 2020 05:15:34 GMT, Ambarish Rapte wrote: > I have tested with a small TreeTableView of 15 rows of 3 levels and 3 columns. > Would it be enough to test with TreeTableView of ~500 rows of 5 levels, 10 > columns ? > > Update: I am testing with 7 level of nested rows, with 10, 9, 7, 6, 5, 4, 3 > number of children in each level > respectively. The fix works fine till level 3. But can observe issue with > level 4 and further. Shall debug this more > and update. OK, thanks. In addition to making sure that insertion, deletion, and sorting work, are you also checking that the events being sent are as expected? - PR: https://git.openjdk.java.net/jfx/pull/244
Re: RFR: 8246357: Allow static build of webkit library on linux
On Tue, 2 Jun 2020 18:01:40 GMT, Kevin Rushforth wrote: >> add changes to build a static version of libjfxwebkit.a >> Fox for JDK-8246357 > > @arun-Joseph @guruhb can one of you also review? The diffs look good. I'm doing test builds now. - PR: https://git.openjdk.java.net/jfx/pull/245
Re: CFV: New OpenJFX Reviewer: Nir Lisker
Vote: YES On 6/3/2020 6:28 AM, Kevin Rushforth wrote: I hereby nominate Nir Lisker [1] to OpenJFX Reviewer.
CFV: New OpenJFX Reviewer: Nir Lisker
I hereby nominate Nir Lisker [1] to OpenJFX Reviewer. Nir is an OpenJFX community member, who has contributed 29 changesets [2][3] to OpenJFX over the course of the past 2 years. Nir consistently participates in code reviews of various OpenJFX pull requests, and has become proficient in the areas of: scene graph, animation, graphics, and bindings. Votes are due by June 17, 2020. Only current OpenJFX Reviewers [4] are eligible to vote on this nomination. Votes must be cast in the open by replying to this mailing list. For Three-Vote Consensus voting instructions, see [5]. Nomination to a project Reviewer is described in [6]. Thanks. -- Kevin [1] https://openjdk.java.net/census#nlisker [2] https://github.com/openjdk/jfx/search?q=author-email%3Anlisker%40openjdk.org=author-date=Commits [3] https://github.com/openjdk/jfx/search?q=author-email%3Anlisker%40gmail.com=author-date=Commits [4] https://openjdk.java.net/census#openjfx [5] https://openjdk.java.net/bylaws#three-vote-consensus [6] https://openjdk.java.net/projects#project-reviewer
Re: [Rev 01] RFR: 8193800: TreeTableView selection changes on sorting
On Wed, 3 Jun 2020 12:03:43 GMT, Jeanette Winzenburg wrote: >>> The algorithm looks correct to me for sorting. How much regression testing >>> have you done for cases where rows or >>> columns are inserted? >> >> I have tested with a small TreeTableView of 15 rows of 3 levels and 3 >> columns. >> Would it be enough to test with TreeTableView of ~500 rows of 5 levels, 10 >> columns ? >> >> Update: I am testing with 7 level of nested rows, with 10, 9, 7, 6, 5, 4, 3 >> number of children in each level >> respectively. The fix works fine till level 3. But can observe issue with >> level 4 and further. Shall debug this more >> and update. > > hmm .. TreeModificationEvent seems to have a different interpretation (than a > list change) of _permutated_: its > wasPermutated may return true even on a replace - that's probably why some > tests are throwing a IllegalStateException > before the fix. The other way round: do we really want to introduce a > singularity in selection handling after > replace? The other selectionModels for virtualized controls try to keep the > selectedItem/Index only. need a break ;) Will come back later with an example comparing the behavior of multiple selection state across virtualized controls - preliminary results Sort: - ListView/TableView keep selection (corrrectly) on all selectedItems, adjust indices as needed - TreeView: ?? - TreeTableView: throws IllegalState before this fix, keeps selection (correctly) on all selectedItems, adjusts indices as needed after this fix Replace with same items in different sequence - ListView/TableView: keep only selectedItem, adjust selectedIndex as needed - TreeView: keeps selectedIndices (?) - TreeTableView: throws IllegalState before this fix, keeps selectedItems, adjusts selectedIndices as needed after this fix - PR: https://git.openjdk.java.net/jfx/pull/244
Re: [Rev 01] RFR: 8193800: TreeTableView selection changes on sorting
On Wed, 3 Jun 2020 12:03:43 GMT, Jeanette Winzenburg wrote: >>> The algorithm looks correct to me for sorting. How much regression testing >>> have you done for cases where rows or >>> columns are inserted? >> >> I have tested with a small TreeTableView of 15 rows of 3 levels and 3 >> columns. >> Would it be enough to test with TreeTableView of ~500 rows of 5 levels, 10 >> columns ? >> >> Update: I am testing with 7 level of nested rows, with 10, 9, 7, 6, 5, 4, 3 >> number of children in each level >> respectively. The fix works fine till level 3. But can observe issue with >> level 4 and further. Shall debug this more >> and update. > > hmm .. TreeModificationEvent seems to have a different interpretation (than a > list change) of _permutated_: its > wasPermutated may return true even on a replace - that's why some tests are > throwing a IllegalStateException before the > fix. The other way round: do we really want to introduce a singularity in > selection handling after replace? The other > selectionModels for virtualized controls try to keep the selectedItem/Index > only. ahh .. was wrong (note to self: run examples for all controls before commenting ;) - TableView also tries to keep all selected state on replace with the same items in changed sequence, ListView doesn't (which then is the singularity which might need a fix?). - PR: https://git.openjdk.java.net/jfx/pull/244
Re: [Rev 01] RFR: 8193800: TreeTableView selection changes on sorting
On Wed, 3 Jun 2020 05:15:34 GMT, Ambarish Rapte wrote: >> The algorithm looks correct to me for sorting. How much regression testing >> have you done for cases where rows or >> columns are inserted? >> I left a few comments, and will complete my testing in parallel. > >> The algorithm looks correct to me for sorting. How much regression testing >> have you done for cases where rows or >> columns are inserted? > > I have tested with a small TreeTableView of 15 rows of 3 levels and 3 columns. > Would it be enough to test with TreeTableView of ~500 rows of 5 levels, 10 > columns ? > > Update: I am testing with 7 level of nested rows, with 10, 9, 7, 6, 5, 4, 3 > number of children in each level > respectively. The fix works fine till level 3. But can observe issue with > level 4 and further. Shall debug this more > and update. hmm .. TreeModificationEvent seems to have a different interpretation (than a list change) of _permutated_: its wasPermutated may return true even on a replace - that's why some tests are throwing a IllegalStateException before the fix. The other way round: do we really want to introduce a singularity in selection handling after replace? The other selectionModels for virtualized controls try to keep the selectedItem/Index only. - PR: https://git.openjdk.java.net/jfx/pull/244
Re: [Rev 02] RFR: 8193800: TreeTableView selection changes on sorting
On Tue, 2 Jun 2020 17:31:52 GMT, Kevin Rushforth wrote:
>> Ambarish Rapte has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> kcr review comment changes
>
> modules/javafx.controls/src/main/java/javafx/scene/control/TreeTableView.java
> line 1808:
>
>> 1807: boolean sortingInProgress;
>> 1808: protected boolean isSortingInProgress() {
>> 1809: return sortingInProgress;
>
> This is an implementation detail. By making it `protected` it becomes public
> API. You will need to make it
> package-scope instead
Updated in next commit
-
PR: https://git.openjdk.java.net/jfx/pull/244
