Hi all,
This pull request contains a clean backport of commit
[e2f42c5b](https://github.com/openjdk/jfx/commit/e2f42c5b71ef061c614540508fbac3fb610b1ae3)
from the [openjdk/jfx](https://git.openjdk.org/jfx) repository.
The commit being backported was authored by Robert Lichtenberger on 14 Feb 202
On Fri, 16 Feb 2024 21:12:21 GMT, Jose Pereda wrote:
> Hi all,
>
> This pull request contains a clean backport of commit
> [e2f42c5b](https://github.com/openjdk/jfx/commit/e2f42c5b71ef061c614540508fbac3fb610b1ae3)
> from the [openjdk/jfx](https://git.openjdk.org/jfx) repository.
>
> The commi
Hi all,
This pull request contains a clean backport of commit
[e2f42c5b](https://github.com/openjdk/jfx/commit/e2f42c5b71ef061c614540508fbac3fb610b1ae3)
from the [openjdk/jfx](https://git.openjdk.org/jfx) repository.
The commit being backported was authored by Robert Lichtenberger on 14 Feb 202
On Wed, 7 Feb 2024 08:14:26 GMT, Robert Lichtenberger
wrote:
> I hope this is the right approach for an RFE ...
thank you for filing an RFE, @effad
-
PR Comment: https://git.openjdk.org/jfx/pull/1358#issuecomment-1932319542
On Fri, 2 Feb 2024 06:55:36 GMT, Robert Lichtenberger
wrote:
> The PR simply moves column and view-updates outside the loop. Since the
> column or view never changes within the for-loop it is not necessary to call
> these again and again.
Looks good to me.
Checking `updateTableColumn` and `
On Wed, 7 Feb 2024 09:59:45 GMT, Robert Lichtenberger
wrote:
>> modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableColumnHeader.java
>> line 670:
>>
>>> 668: cell.updateTableRow(tableRow);
>>> 669: cell.updateIndex(row);
>>> 670:
>>
>> we could also
On Wed, 7 Feb 2024 08:52:32 GMT, Marius Hanl wrote:
>> The PR simply moves column and view-updates outside the loop. Since the
>> column or view never changes within the for-loop it is not necessary to call
>> these again and again.
>
> modules/javafx.controls/src/main/java/javafx/scene/control
On Fri, 2 Feb 2024 06:55:36 GMT, Robert Lichtenberger
wrote:
> The PR simply moves column and view-updates outside the loop. Since the
> column or view never changes within the for-loop it is not necessary to call
> these again and again.
modules/javafx.controls/src/main/java/javafx/scene/con
On Fri, 2 Feb 2024 06:55:36 GMT, Robert Lichtenberger
wrote:
> The PR simply moves column and view-updates outside the loop. Since the
> column or view never changes within the for-loop it is not necessary to call
> these again and again.
I've filed https://bugs.openjdk.org/browse/JDK-8325390
On Mon, 5 Feb 2024 09:45:25 GMT, Marius Hanl wrote:
> I will review this PR soon as well.
The discussion about a "bigger" solution aside I think the PR is still of value
to improve performance, so I look forward to your review.
-
PR Comment: https://git.openjdk.org/jfx/pull/1358#is
On Tue, 6 Feb 2024 15:53:07 GMT, Andy Goryachev wrote:
> > So maybe we should:
> >
> > * Provide an API that let's the client specify how many rows should be
> > taken into account
> > * Make resizeColumnToFitContent available for clients
>
> Let's create an RFE. What would be your recommendat
On Tue, 6 Feb 2024 06:24:18 GMT, Robert Lichtenberger
wrote:
> So maybe we should:
>
> * Provide an API that let's the client specify how many rows should be taken
> into account
> * Make resizeColumnToFitContent available for clients
Let's create an RFE.
What would be your recommendation for
On Mon, 5 Feb 2024 16:02:46 GMT, Andy Goryachev wrote:
> > , in order to "not break things" we would have to introduce an API to users
> > to be able to control the number of rows they want to be taken into account.
>
> I think the UI "breaking itself" is much worse than us changing the (buggy)
On Mon, 5 Feb 2024 12:51:15 GMT, Robert Lichtenberger
wrote:
>> You're right. I tried it out and it seems to work.
>
> I've tried out an improved version of the patch with
> cell.updateTableRow(tableRow) moved out of the loop. However, this makes
> overall performance a bit slower.
> This patc
On Mon, 5 Feb 2024 09:45:25 GMT, Marius Hanl wrote:
> A side comment: the default implementation of resizeColumnToFitContent() is
> to iterate over all the rows, and I think this is not right. The UI locks up
> if the table has, let's say, 10,000,000 rows. I think in this case it should
> inst
On Mon, 5 Feb 2024 09:31:37 GMT, Robert Lichtenberger
wrote:
> , in order to "not break things" we would have to introduce an API to users
> to be able to control the number of rows they want to be taken into account.
I think the UI "breaking itself" is much worse than us changing the (buggy)
On Fri, 2 Feb 2024 10:39:49 GMT, Robert Lichtenberger
wrote:
>> Well, the `tableRow` is still set (but before the loop), we just update the
>> index in this loop.
>> It looks like setting the table row in this loop is not needed, just once
>> above. I also don't see that `setTableRow` has any
Hi, just wanted to add my 2 cents,
In my opinion, no operation in List/Table/TreeTable should be doing such
measurements on all rows or columns (where cells need to be loaded, CSS
aplied, etc) -- it just won't scale. In my application many of my lists
contain images, and pre-loading all of th
On Mon, 5 Feb 2024 09:31:37 GMT, Robert Lichtenberger
wrote:
> A side comment: the default implementation of resizeColumnToFitContent() is
> to iterate over all the rows, and I think this is not right. The UI locks up
> if the table has, let's say, 10,000,000 rows. I think in this case it shou
On Fri, 2 Feb 2024 16:15:43 GMT, Andy Goryachev wrote:
> I see it working right in macOS 14.2.1, by double-clicking on the column
> divider in the table header.
>
> Question to @effad : could you provide the measurements for this fix
> (before/after) please?
Applying the patch improves perfor
On Fri, 2 Feb 2024 06:55:36 GMT, Robert Lichtenberger
wrote:
> The PR simply moves column and view-updates outside the loop. Since the
> column or view never changes within the for-loop it is not necessary to call
> these again and again.
I see it working right in macOS 14.2.1, by double-clic
On Fri, 2 Feb 2024 06:55:36 GMT, Robert Lichtenberger
wrote:
> The PR simply moves column and view-updates outside the loop. Since the
> column or view never changes within the for-loop it is not necessary to call
> these again and again.
@andy-goryachev-oracle Can you review this?
-
On Fri, 2 Feb 2024 08:44:45 GMT, Marius Hanl wrote:
>>> can this also be moved out? `cell.updateTableRow(tableRow);`
>>
>> I don't think so. Since tableRow gets an updated index above, the cell's
>> style could change (e.g. odd / even styling).
>
> Well, the `tableRow` is still set (but before
On Fri, 2 Feb 2024 08:20:09 GMT, Robert Lichtenberger
wrote:
>> modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableColumnHeader.java
>> line 668:
>>
>>> 666: tableRow.updateIndex(row);
>>> 667:
>>> 668: cell.updateTableRow(tableRow);
>>
>> can this a
On Fri, 2 Feb 2024 08:05:12 GMT, Marius Hanl wrote:
> can this also be moved out? `cell.updateTableRow(tableRow);`
I don't think so. Since tableRow gets an updated index above, the cell's style
could change (e.g. odd / even styling).
-
PR Review Comment: https://git.openjdk.org/jf
On Fri, 2 Feb 2024 06:55:36 GMT, Robert Lichtenberger
wrote:
> The PR simply moves column and view-updates outside the loop. Since the
> column or view never changes within the for-loop it is not necessary to call
> these again and again.
modules/javafx.controls/src/main/java/javafx/scene/con
The PR simply moves column and view-updates outside the loop. Since the column
or view never changes within the for-loop it is not necessary to call these
again and again.
-
Commit messages:
- 8325154: resizeColumnToFitContent is slower than it needs to be
Changes: https://git.ope
27 matches
Mail list logo
