Skip to content

Improve calculations of total requested height by caching cell heights #413

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
johanvos wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Improve calculations of total requested height by caching cell heights #413

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a894724
Improve calculations of total requested height by caching cell heights
johanvos Nov 28, 2025
f463e7a
Merge branch 'main' into 412-height
johanvos Dec 8, 2025
1723817
Set height to non-0 when we don't have any info yet
johanvos Dec 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions rta/src/main/java/com/gluonhq/richtextarea/RichListCell.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -219,10 +219,14 @@ protected void updateItem(Paragraph item, boolean empty) {
setGraphic(paragraphTile);
// required: update caret and selection
paragraphTile.updateLayout();
item.setPreferredHeight(this.getHeight());
Copy link

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Calling this.getHeight() immediately after updateLayout() may not return the correct height. The cell's height is typically computed during the layout pass, which happens asynchronously. This could result in caching an incorrect (possibly 0 or outdated) height value.

Consider using prefHeight() instead, which computes the preferred height based on the current content:

item.setPreferredHeight(this.prefHeight(-1));

Alternatively, if you need the actual rendered height, consider caching it in a later phase of the layout cycle using layoutBoundsProperty() listener or overriding layoutChildren().

Suggested change
item.setPreferredHeight(this.getHeight());
item.setPreferredHeight(this.prefHeight(-1));

Copilot uses AI. Check for mistakes.
} else {
// clean up listeners
paragraphTile.setParagraph(null, null, null, null);
setGraphic(null);
if (item != null) {
item.setPreferredHeight(-1);
}
}
}

Expand Down
29 changes: 15 additions & 14 deletions rta/src/main/java/com/gluonhq/richtextarea/RichTextAreaSkin.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -989,21 +989,22 @@ void keyTypedListener(KeyEvent e) {
}

private void computeFullHeight() {
RichListCell cell = new RichListCell(RichTextAreaSkin.this);
double rtaWidth = getSkinnable().prefWidth(-1);
Scene scene = new Scene(cell, rtaWidth, 1000);
if (getSkinnable().getScene() != null) {
scene.getStylesheets().setAll(getSkinnable().getScene().getStylesheets());
ObservableList<Paragraph> items = paragraphListView.getItems();
int psize = items.size();
int unknown = 0;
double total = 0.;
for (Paragraph p : items) {
double ph = p.getPreferredHeight();
if (ph < 0) {
unknown++;
} else {
total = total + ph;
Copy link

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The expression total = total + ph can be simplified to use the compound assignment operator for better readability:

total += ph;
Suggested change
total = total + ph;
total += ph;

Copilot uses AI. Check for mistakes.
}
}
if (unknown > 0) {
total = total * psize / (psize - unknown);
Copy link

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Division by zero will occur when all paragraphs have unknown heights (i.e., psize == unknown). In this case, (psize - unknown) becomes 0, resulting in a division by zero error.

Add a check to handle this edge case:

if (unknown > 0) {
    if (unknown == psize) {
        // All heights are unknown, cannot estimate
        total = 0.;
    } else {
        total = total * psize / (psize - unknown);
    }
}
Suggested change
total = total * psize / (psize - unknown);
if (unknown == psize) {
// All heights are unknown, cannot estimate
total = 0.0;
} else {
total = total * psize / (psize - unknown);
}

Copilot uses AI. Check for mistakes.
}
double fullHeight = paragraphListView.getItems().stream()
.mapToDouble(item -> {
cell.updateItem(item, false);
cell.applyCss();
cell.layout();
return cell.prefHeight(rtaWidth);
})
.sum();
fullHeightProperty.set(fullHeight);
fullHeightProperty.set(total);
}

private void populateContextMenu(boolean isEditable) {
Expand Down
10 changes: 10 additions & 0 deletions rta/src/main/java/com/gluonhq/richtextarea/model/Paragraph.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ public class Paragraph {

private ParagraphDecoration decoration;

private double preferredHeight = -1.; // cache the expected height
Copy link

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The inline comment should be more descriptive about the purpose and invalidation strategy of the cache. Consider using a JavaDoc comment:

/**
 * Cached preferred height of the paragraph cell. A value of -1 indicates
 * the height has not been computed or is invalid and needs recalculation.
 */
private double preferredHeight = -1.;
Suggested change
private double preferredHeight = -1.; // cache the expected height
/**
* Cached preferred height of the paragraph cell.
* <p>
* A value of -1 indicates that the height has not been computed or is invalid,
* and needs to be recalculated. The cache should be invalidated and reset to -1
* whenever the paragraph's content or decoration changes in a way that affects its height.
*/
private double preferredHeight = -1.;

Copilot uses AI. Check for mistakes.

public Paragraph(int start, int end, ParagraphDecoration decoration) {
this.start = start;
this.end = end;
Expand Down Expand Up @@ -74,6 +76,14 @@ public void setDecoration(ParagraphDecoration decoration) {
this.decoration = decoration;
}

public double getPreferredHeight() {
return preferredHeight;
}

public void setPreferredHeight(double preferredHeight) {
this.preferredHeight = preferredHeight;
}

@Override
public boolean equals(Object o) {
if (this == o) return true;
Expand Down