Skip to content
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.

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

8341670: [Text,TextFlow] Public API for Text Layout Info #1596

Open
wants to merge 47 commits into
base: master
Choose a base branch
from
Open
Changes from 42 commits
Commits
Show all changes
47 commits
Select commit Hold shift + click to select a range
285db28
draft api
andy-goryachev-oracle Oct 8, 2024
43ac0e1
bounds
andy-goryachev-oracle Oct 8, 2024
21dbe6c
review comments
andy-goryachev-oracle Oct 9, 2024
04c4abb
javadoc
andy-goryachev-oracle Oct 9, 2024
6869c76
clarify
andy-goryachev-oracle Oct 9, 2024
2009a7e
convert to wrapper
andy-goryachev-oracle Oct 9, 2024
6b7b079
Merge remote-tracking branch 'origin/master' into ag.text.layout.api
andy-goryachev-oracle Oct 11, 2024
c2e26d5
text line info
andy-goryachev-oracle Oct 11, 2024
88e9ee1
caret info
andy-goryachev-oracle Oct 14, 2024
17dcdd4
whitespace
andy-goryachev-oracle Oct 14, 2024
fd84f30
tests
andy-goryachev-oracle Oct 14, 2024
466bba7
remove line spacing
andy-goryachev-oracle Oct 14, 2024
5ab4f47
Merge remote-tracking branch 'origin/master' into ag.text.layout.api
andy-goryachev-oracle Oct 14, 2024
eb99008
Merge remote-tracking branch 'origin/master' into ag.text.layout.api
andy-goryachev-oracle Oct 24, 2024
6776d97
review comments
andy-goryachev-oracle Oct 25, 2024
9b1f99a
Merge remote-tracking branch 'origin/master' into ag.text.layout.api
andy-goryachev-oracle Oct 30, 2024
4f4e659
include line spacing
andy-goryachev-oracle Oct 31, 2024
dd34810
Merge remote-tracking branch 'origin/master' into ag.text.layout.api
andy-goryachev-oracle Oct 31, 2024
a716e57
Merge remote-tracking branch 'origin/master' into ag.text.layout.api
andy-goryachev-oracle Nov 1, 2024
75b171f
caret and range
andy-goryachev-oracle Nov 1, 2024
029bbe6
for text
andy-goryachev-oracle Nov 1, 2024
3925c02
Merge remote-tracking branch 'origin/master' into ag.text.layout.api
andy-goryachev-oracle Nov 4, 2024
e139484
Merge remote-tracking branch 'origin/master' into ag.text.layout.api
andy-goryachev-oracle Nov 6, 2024
8a5b904
Merge remote-tracking branch 'origin/master' into ag.text.layout.api
andy-goryachev-oracle Nov 8, 2024
a0e59fb
Merge remote-tracking branch 'origin/master' into ag.text.layout.api
andy-goryachev-oracle Nov 15, 2024
3c2976b
line spacing
andy-goryachev-oracle Nov 15, 2024
5076f36
shorter array
andy-goryachev-oracle Nov 18, 2024
4479f0b
text flow test
andy-goryachev-oracle Nov 19, 2024
c05c790
text layout test
andy-goryachev-oracle Nov 19, 2024
7c86978
note
andy-goryachev-oracle Nov 19, 2024
366ae4b
coordinates
andy-goryachev-oracle Nov 19, 2024
d31eb2b
Merge remote-tracking branch 'origin/master' into ag.text.layout.api
andy-goryachev-oracle Nov 19, 2024
7ea4e3b
Merge remote-tracking branch 'origin/master' into ag.text.layout.api
andy-goryachev-oracle Nov 22, 2024
944bd0d
segments
andy-goryachev-oracle Nov 22, 2024
1dd2b0c
Merge branch 'master' into ag.text.layout.api
andy-goryachev-oracle Dec 4, 2024
2a3a754
Merge remote-tracking branch 'origin/master' into ag.text.layout.api
andy-goryachev-oracle Dec 10, 2024
c1c46ab
Merge remote-tracking branch 'origin/master' into ag.text.layout.api
andy-goryachev-oracle Dec 20, 2024
7d656df
Merge branch 'master' into ag.text.layout.api
andy-goryachev-oracle Jan 16, 2025
0ab5890
25 25
andy-goryachev-oracle Jan 16, 2025
d81f65d
Merge remote-tracking branch 'origin/master' into ag.text.layout.api
andy-goryachev-oracle Feb 5, 2025
d688d74
review comments
andy-goryachev-oracle Feb 5, 2025
0f94efd
Merge remote-tracking branch 'origin/master' into ag.text.layout.api
andy-goryachev-oracle Mar 5, 2025
3260b8e
Merge remote-tracking branch 'origin/master' into ag.text.layout.api
andy-goryachev-oracle Mar 10, 2025
0ad6f66
review comments
andy-goryachev-oracle Mar 10, 2025
9d499c1
tests
andy-goryachev-oracle Mar 10, 2025
b7033cf
twice
andy-goryachev-oracle Mar 10, 2025
9cee2dc
Merge remote-tracking branch 'origin/master' into ag.text.layout.api
andy-goryachev-oracle Mar 26, 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
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2012, 2024, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2012, 2025, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -25,12 +25,10 @@

package com.sun.javafx.scene.text;

import javafx.scene.shape.PathElement;
import java.util.Objects;
import com.sun.javafx.geom.BaseBounds;
import com.sun.javafx.geom.Shape;

import java.util.Objects;

public interface TextLayout {

/* Internal flags Flags */
@@ -79,6 +77,12 @@ public interface TextLayout {

public static final int DEFAULT_TAB_SIZE = 8;

/** Callback to be called for each rectangular shape */
@FunctionalInterface
public static interface GeometryCallback {
public void addRectangle(float left, float top, float right, float bottom);
Copy link
Collaborator

Choose a reason for hiding this comment

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

I understand that you use float here because all the calculations in PrismTextLayout::getRange use floats (from TextRun).

However, the calculations down the line to generate the Rectangle2D mix floats and doubles without any casting (TextUtils::getRange with implicit casts from double to float, PrismLayoutInfo::getGeometry with float and double sums).

Do you think we could use doubles here instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the use of float in this interface is correct, but the code in TextUtils::getRange is bad, and i should feel bad. good catch!

}

public static class Hit {
int charIndex;
int insertionIndex;
@@ -233,8 +237,36 @@ public String toString() {
*/
public Hit getHitInfo(float x, float y);

public PathElement[] getCaretShape(int offset, boolean isLeading,
float x, float y);
public PathElement[] getRange(int start, int end, int type,
float x, float y);
/**
* Queries the caret geometry and associated information at the specified text position.
* <p>
* The geometry is encoded as a sequence of coordinates using two different formats,
* depending on whether the caret is drawn as a single vertical line or as two separate
* lines (a "split" caret).
* <ul>
* <li>{@code [x, y, h]} - corresponds to a single line from (x, y) to (x, y + h)
* <li>{@code [x, y, x2, h]} - corresponds to a split caret drawn as two lines, the first line
* drawn from (x, y) to (x, y + h/2), the second line drawn from (x2, y + h/2) to (x2, y + h).
* </ul>
*
* @param offset the character offset
* @param leading whether the caret is biased on the leading edge of the character
* @return the caret geometry
*/
public float[] getCaretGeometry(int offset, boolean leading);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does this method throw any exceptions? If so, please specify.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This method does not throw any exceptions, so none are specified.


/**
* Queries the range geometry of the range of text within the text layout for one of the three possible types:
* <ul>
* <li>{@link #TYPE_STRIKETHROUGH} - strike-through shape
* <li>{@link #TYPE_TEXT} - text selection shape
* <li>{@link #TYPE_UNDERLINE} - underline shape
* </ul>
*
* @param start the start offset
* @param end the end offset
* @param type the type of the geometry
* @param client the callback to invoke for each rectangular shape
*/
public void getRange(int start, int end, int type, GeometryCallback client);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does this method throw any exceptions?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no.

}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2012, 2024, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2012, 2025, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -37,11 +37,11 @@ public interface TextLine {
public GlyphList[] getRuns();

/**
* Returns metrics information about the line as follow:
* Returns metrics information about the line as follows:
*
* bounds().getWidth() - the width of the line.
* The width for the line is sum of all run's width in the line, it is not
* affect by any wrapping width but it will include any changes caused by
* affected by any wrapping width but it will include any changes caused by
* justification.
*
* bounds().getHeight() - the height of the line.
@@ -73,7 +73,7 @@ public interface TextLine {
public int getStart();

/**
* Returns the line length in character.
* Returns the line length in characters.
*/
public int getLength();
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
/*
* Copyright (c) 2024, 2025, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation. Oracle designates this
* particular file as subject to the "Classpath" exception as provided
* by Oracle in the LICENSE file that accompanied this code.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*/
package com.sun.javafx.text;

import javafx.geometry.Rectangle2D;
import javafx.scene.text.CaretInfo;

/**
* CaretInfo as reported by the PrismTextLayout.
*/
public final class PrismCaretInfo extends CaretInfo {

private final Rectangle2D[] parts;

public PrismCaretInfo(Rectangle2D[] parts) {
this.parts = parts;
}

@Override
public int getSegmentCount() {
return parts.length;
}

@Override
public Rectangle2D getSegmentAt(int index) {
return parts[index];
Copy link
Collaborator

Choose a reason for hiding this comment

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

do we need a bound check here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no special handling is needed here I think: an exception will be thrown

Copy link
Collaborator

Choose a reason for hiding this comment

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

While true, I think we should throw IndexOutOfBoundsException rather than ArrayIndexOutOfBoundsException (use Objects.checkIndex() for that). This should also be specified with a @throws javadoc tag in CaretInfo.

}

@Override
public String toString() {
StringBuilder sb = new StringBuilder();
sb.append("PrismCaretInfo{parts=[");
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think you shouldn't leak implementation details via the toString() method (the name of the implementing class, as well as parts=, which is not a term that appears in the API). Maybe just drop PrismCaretInfo{parts= entirely.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

makes sense.

On a related note, we don't seem to have a consistent standard for toString() output (probably no one does!), so maybe it's worth polling the devs about it.

Standardized log output might be useful when one has to go through masses of old/transient logs, or when scraping the logs for analytics. A JSON output might be useful since it offers a cheap structured form, although it suffers from two issues: verbosity and difficulty to encode binary data and object types easily.

Any ideas?

for (int i = 0; i < getSegmentCount(); i++) {
if (i > 0) {
sb.append(",");
}
sb.append(getSegmentAt(i));
}
sb.append("]}");
return sb.toString();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,156 @@
/*
* Copyright (c) 2025, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation. Oracle designates this
* particular file as subject to the "Classpath" exception as provided
* by Oracle in the LICENSE file that accompanied this code.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*/
package com.sun.javafx.text;

import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import javafx.geometry.Insets;
import javafx.geometry.Rectangle2D;
import javafx.scene.text.CaretInfo;
import javafx.scene.text.LayoutInfo;
import javafx.scene.text.TextLineInfo;
import com.sun.javafx.geom.BaseBounds;
import com.sun.javafx.scene.text.TextLayout;
import com.sun.javafx.scene.text.TextLine;

/**
* Layout information as reported by PrismLayout.
*/
public abstract class PrismLayoutInfo extends LayoutInfo {

protected abstract double lineSpacing();

protected abstract Insets insets();

private final TextLayout layout;

public PrismLayoutInfo(TextLayout layout) {
this.layout = layout;
}

@Override
public Rectangle2D getBounds(boolean includeLineSpacing) {
BaseBounds b = layout.getBounds();
Insets m = insets();
Copy link
Collaborator

Choose a reason for hiding this comment

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

See my comments below about adding the insets to the layout information. Maybe we could also add another boolean includeInsets for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good question! I don't think this is needed: the call returns the layout bounds (in the owning node coordinate system).

The insets only change the position of the layout within the owning node, unlike lineSpacing which actually changes the shape by adding or removing the last line spacing.

I think this method is ok as is.

double dx = m.getLeft(); // TODO rtl?
double dy = m.getTop();
double sp = includeLineSpacing ? lineSpacing() : 0.0;
return TextUtils.toRectangle2D(b, dx, dy, sp);
}

@Override
public int getTextLineCount() {
return layout.getLines().length;
}

@Override
public List<TextLineInfo> getTextLines(boolean includeLineSpacing) {
TextLine[] lines = layout.getLines();
Insets m = insets();
double dx = m.getLeft(); // TODO rtl?
double dy = m.getTop();
double sp = includeLineSpacing ? lineSpacing() : 0.0;
int sz = lines.length;

ArrayList<TextLineInfo> rv = new ArrayList<>(sz);
for (int i = 0; i < sz; i++) {
rv.add(TextUtils.toLineInfo(lines[i], dx, dy, sp));
}
return Collections.unmodifiableList(rv);
}

@Override
public TextLineInfo getTextLine(int index, boolean includeLineSpacing) {
Insets m = insets();
double dx = m.getLeft(); // TODO rtl?
double dy = m.getTop();
double sp = includeLineSpacing ? lineSpacing() : 0.0;
return TextUtils.toLineInfo(layout.getLines()[index], dx, dy, sp);
}

@Override
public List<Rectangle2D> selectionShape(int start, int end, boolean includeLineSpacing) {
double sp = includeLineSpacing ? lineSpacing() : 0.0;
return getGeometry(start, end, TextLayout.TYPE_TEXT, sp);
}

@Override
public List<Rectangle2D> strikeThroughShape(int start, int end) {
return getGeometry(start, end, TextLayout.TYPE_STRIKETHROUGH, 0.0);
}

@Override
public List<Rectangle2D> underlineShape(int start, int end) {
return getGeometry(start, end, TextLayout.TYPE_UNDERLINE, 0.0);
}

private List<Rectangle2D> getGeometry(int start, int end, int type, double lineSpacing) {
Insets m = insets();
double dx = m.getLeft(); // TODO RTL?
double dy = m.getTop();

ArrayList<Rectangle2D> rv = new ArrayList<>();
layout.getRange(start, end, type, (left, top, right, bottom) -> {
if (left < right) {
rv.add(new Rectangle2D(left + dx, top + dy, right - left, bottom - top + lineSpacing));
} else {
rv.add(new Rectangle2D(right + dx, top + dy, left - right, bottom - top + lineSpacing));
}
});
return Collections.unmodifiableList(rv);
}

@Override
public CaretInfo caretInfo(int charIndex, boolean leading) {
Insets m = insets();
double dx = m.getLeft(); // TODO RTL?
double dy = m.getTop();
float[] c = layout.getCaretGeometry(charIndex, leading);

Rectangle2D[] parts;
if (c.length == 3) {
// [x, y, h] - corresponds to a single line from (x, y) to (x, y + h)
double x = c[0] + dx;
double y = c[1] + dy;
double h = c[2];
parts = new Rectangle2D[] {
new Rectangle2D(x, y, 0.0, h)
};
} else {
// [x, y, x2, h] - corresponds to a split caret drawn as two lines, the first line
// drawn from (x, y) to (x, y + h/2), the second line drawn from (x2, y + h/2) to (x2, y + h).
double x = c[0] + dx;
double y = c[1] + dy;
double x2 = c[2] + dx;
double h2 = c[3] / 2.0;
parts = new Rectangle2D[] {
new Rectangle2D(x, y, 0.0, h2),
new Rectangle2D(x2, y + h2, 0.0, h2)
};
}
return new PrismCaretInfo(parts);
}
}
Loading