Improve calculations of total requested height by caching cell heights

[johanvos]

fix #412

[Copilot]

Pull request overview

This PR improves the performance of height calculations in the rich text area by implementing a caching mechanism for paragraph cell heights. Instead of creating temporary cells and computing heights on-demand, the implementation now caches the preferred height in the Paragraph model and uses these cached values with estimation for unknown heights.

Key Changes

• Added preferredHeight field to Paragraph class to cache cell heights
• Refactored computeFullHeight() to use cached heights instead of creating temporary cells and computing heights on-the-fly
• Updated RichListCell to cache heights during cell updates

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.

File	Description
rta/src/main/java/com/gluonhq/richtextarea/model/Paragraph.java	Added preferredHeight field with getter/setter for caching paragraph cell heights
rta/src/main/java/com/gluonhq/richtextarea/RichTextAreaSkin.java	Replaced expensive height computation with cached values and estimation logic for unknown heights
rta/src/main/java/com/gluonhq/richtextarea/RichListCell.java	Added logic to cache cell heights in paragraph model during updateItem()

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.

[Copilot]

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]

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]

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]

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]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.