RFR: JDK-8316781 Fix legal and monarch paper sizes
The sizes should be 8.5 wide for letter, and 3.875 wide for monarch. - Commit messages: - Fix legal and monarch paper sizes Changes: https://git.openjdk.org/jfx/pull/1248/files Webrev: https://webrevs.openjdk.org/?repo=jfx&pr=1248&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8316781 Stats: 4 lines in 1 file changed: 1 ins; 0 del; 3 mod Patch: https://git.openjdk.org/jfx/pull/1248.diff Fetch: git fetch https://git.openjdk.org/jfx.git pull/1248/head:pull/1248 PR: https://git.openjdk.org/jfx/pull/1248
Re: RFR: 8316518 javafx.print.Paper getWidth / getHeight rounds values, causing errors.
On Tue, 19 Sep 2023 11:56:39 GMT, Florian Kirmaier wrote: > The Method javafx.print.getWidth and getHeight returns a double of points. > But the values are always rounded to full integers. This causes especially > problems, > with mm based Papers. > > I asked in the mailing list, with the conclusion (me and John Hendrix) that > is best to just fix this behavior by removing the rounding. I did a fix for the incorrect legal paper size here: https://github.com/openjdk/jfx/pull/1248 - PR Comment: https://git.openjdk.org/jfx/pull/1244#issuecomment-1732394588
Re: RFR: 8316518 javafx.print.Paper getWidth / getHeight rounds values, causing errors.
On Sat, 23 Sep 2023 06:02:14 GMT, John Hendrikx wrote:
>> About the rounding; all these values are fixed values and could have been
>> entered in points as constants directly.
>>
>> Also realize that all papers measured in inches can be expressed exactly in
>> points (after the above errors are fixed) so no rounding is needed for those
>> at all; the same is not true for the papers specified in millimeters.
>>
>> Since the papers in inches never should need any rounding, the rounding can
>> therefore be removed for those. If there is a worry that the multiplication
>> (* 72) may cause floating point errors, then I suggest to specify all papers
>> in points immediately, like this:
>>
>> new Paper("Letter", 8.5 * 72, 11 * 72, POINT);
>>
>> As for the papers specified in millimeters, these can't be expressed in
>> whole points, as a millimeter is equivalent to `2,83465` points. Rounding
>> them to points will require some guess work to find out what the actual size
>> in millimeters was, for example, when it says 3 points it must have been 1
>> mm, while 1, 2, 4 or 5 points has no direct mm equivalent.
>
> It is also very odd that `Paper` would expose the name (like `A4`), but not
> the unit (`MM`) and the exact sizes for those. Representing these in a UI
> therefore becomes terribly complicated; if I want to show the user, I can
> show them the name, but then when I want to display its size, I have to show
> it in points (a useless measurement, for users, for both papers specific in
> inches and mm).
Typically paper sizes are displayed in whatever units are appropriate for the
locale. On my Mac all paper sizes are displayed in whole millimeters if the
locale is United Kingdom and inches (to 1/100 of an inch) if the locale is
United States.
-
PR Review Comment: https://git.openjdk.org/jfx/pull/1244#discussion_r1335047374
Re: RFR: JDK-8314215 Trailing Spaces before Line Breaks Affect the Center Alignment of Text
On Thu, 14 Sep 2023 22:41:20 GMT, Andy Goryachev wrote:
>> There are a number of tickets open related to text rendering:
>>
>> https://bugs.openjdk.org/browse/JDK-8314215
>>
>> https://bugs.openjdk.org/browse/JDK-8145496
>>
>> https://bugs.openjdk.org/browse/JDK-8129014
>>
>> They have in common that wrapped text is taking the trailing spaces on each
>> wrapped line into account when calculating where to wrap. This looks okay
>> for text that is left aligned (as the spaces will be trailing the lines and
>> generally aren't a problem, but looks weird with CENTER and RIGHT
>> alignments. Even with LEFT alignment there are artifacts of this behavior,
>> where a line like `AAA BBB CCC` (note the **double** spaces) gets split up
>> into `AAA `, `BBB ` and `CCC`, but if space reduces further, it will wrap
>> **too** early because the space is taken into account (ie. `AAA` may still
>> have fit just fine, but `AAA ` doesn't, so the engine wraps it to `AA` + `A
>> ` or something).
>>
>> The fix for this is two fold; first the individual lines of text should not
>> include any trailing spaces into their widths; second, the code that is
>> taking the trailing space into account when wrapping should ignore all
>> trailing spaces (currently it is ignoring all but one trailing space). With
>> these two fixes, the layout in LEFT/CENTER/RIGHT alignments all look great,
>> and there is no more early wrapping due to a space being taking into account
>> while the actual text still would have fit (this is annoying in tight
>> layouts, where a line can be wrapped early even though it looks like it
>> would have fit).
>>
>> If it were that simple, we'd be done, but there may be another issue here
>> that needs solving: wrapped aligned TextArea's.
>>
>> TextArea don't directly support text alignment (via a setTextAlignment
>> method like Label) but you can change it via CSS.
>>
>> For Left alignment + wrapping, TextArea will ignore any spaces typed before
>> a line that was wrapped. In other words, you can type spaces as much as you
>> want, and they won't show up and the cursor won't move. The spaces are all
>> getting appended to the previous line. When you cursor through these
>> spaces, the cursor can be rendered out of the control's bounds. To
>> illustrate, if you have the text `AAA BBB CCC`, and the text
>> gets wrapped to `AAA`, `BBB`, `CCC`, typing spaces before `BBB` will not
>> show up. If you cursor back, the cursor may be outside the control bounds
>> because so many spaces are trailing `AAA`.
>>
>> The above behavior has NOT changed, is pretty standard for wrapped text
>> controls,...
>
> modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java
> line 965:
>
>> 963:
>> 964: for (int i = length + startOffset - 1; i >= startOffset; i--) {
>> 965: if (chars[i] != ' ') {
>
> should `Character.isWhitespace()` be used instead (think of symbols like
> U+2001 that might break, see
> https://en.wikipedia.org/wiki/Whitespace_character)
I'm not entirely sure, perhaps Phil Race @prrace could answer that? There are
loops that just check for 0x20, but also more complicated loops that use
`Character.isWhitespace`.
-
PR Review Comment: https://git.openjdk.org/jfx/pull/1236#discussion_r1335019827
Re: RFR: JDK-8314215 Trailing Spaces before Line Breaks Affect the Center Alignment of Text
On Sun, 10 Sep 2023 20:50:18 GMT, John Hendrikx wrote:
> There are a number of tickets open related to text rendering:
>
> https://bugs.openjdk.org/browse/JDK-8314215
>
> https://bugs.openjdk.org/browse/JDK-8145496
>
> https://bugs.openjdk.org/browse/JDK-8129014
>
> They have in common that wrapped text is taking the trailing spaces on each
> wrapped line into account when calculating where to wrap. This looks okay
> for text that is left aligned (as the spaces will be trailing the lines and
> generally aren't a problem, but looks weird with CENTER and RIGHT alignments.
> Even with LEFT alignment there are artifacts of this behavior, where a line
> like `AAA BBB CCC` (note the **double** spaces) gets split up into `AAA `,
> `BBB ` and `CCC`, but if space reduces further, it will wrap **too** early
> because the space is taken into account (ie. `AAA` may still have fit just
> fine, but `AAA ` doesn't, so the engine wraps it to `AA` + `A ` or
> something).
>
> The fix for this is two fold; first the individual lines of text should not
> include any trailing spaces into their widths; second, the code that is
> taking the trailing space into account when wrapping should ignore all
> trailing spaces (currently it is ignoring all but one trailing space). With
> these two fixes, the layout in LEFT/CENTER/RIGHT alignments all look great,
> and there is no more early wrapping due to a space being taking into account
> while the actual text still would have fit (this is annoying in tight
> layouts, where a line can be wrapped early even though it looks like it would
> have fit).
>
> If it were that simple, we'd be done, but there may be another issue here
> that needs solving: wrapped aligned TextArea's.
>
> TextArea don't directly support text alignment (via a setTextAlignment method
> like Label) but you can change it via CSS.
>
> For Left alignment + wrapping, TextArea will ignore any spaces typed before a
> line that was wrapped. In other words, you can type spaces as much as you
> want, and they won't show up and the cursor won't move. The spaces are all
> getting appended to the previous line. When you cursor through these spaces,
> the cursor can be rendered out of the control's bounds. To illustrate, if
> you have the text `AAA BBB CCC`, and the text gets wrapped to
> `AAA`, `BBB`, `CCC`, typing spaces before `BBB` will not show up. If you
> cursor back, the cursor may be outside the control bounds because so many
> spaces are trailing `AAA`.
>
> The above behavior has NOT changed, is pretty standard for wrapped text
> controls, and IMHO does not need further attent...
@kevinrushforth I think this PR may be worth consideration, although I am not
100% sure that I haven't missed anything, the examples look a lot better with
this fix.
Here's the program I used to make the screenshots:
import javafx.application.Application;
import javafx.scene.Scene;
import javafx.scene.control.Label;
import javafx.scene.layout.GridPane;
import javafx.scene.text.Font;
import javafx.scene.text.TextAlignment;
import javafx.stage.Stage;
public class TextFlowProblem2 {
public static void main(String[] args) {
Application.launch(App.class);
}
public static class App extends Application {
/**
* @param args the command line arguments
*/
public static void start(String[] args) {
Application.launch(args);
}
@Override
public void start(Stage stage) {
Label label1 = new Label("AAA BBB CCC");
label1.setFont(Font.font("courier new", 60));
label1.setWrapText(true);
label1.setTextAlignment(TextAlignment.LEFT);
label1.setStyle("-fx-border-color: red;");
Label label2 = new Label("AAA BBB CCC");
label2.setFont(Font.font("courier new", 60));
label2.setWrapText(true);
label2.setTextAlignment(TextAlignment.CENTER);
label2.setStyle("-fx-border-color: red;");
Label label3 = new Label("AAA BBB CCC");
label3.setFont(Font.font("courier new", 60));
label3.setWrapText(true);
label3.setTextAlignment(TextAlignment.RIGHT);
label3.setStyle("-fx-border-color: red;");
GridPane gridPane = new GridPane();
Label alignmentLabel1 = new Label("Left Aligned");
Label alignmentLabel2 = new Label("Center Aligned");
Label alignmentLabel3 = new Label("Right Aligned");
alignmentLabel1.setMinWidth(100);
alignmentLabel2.setMinWidth(100);
alignmentLabel3.setMinWidth(100);
gridPane.addRow(0, alignmentLabel1, label1);
gridPane.addRow(1, alignmentLabel2, label2);
gridPane.addRow(2, alignmentLabel3, label3);
stage.setScene(new Scene(gridPane));
stage.setWidth(1024);
stage.setHeight(1024);
stage.show();
}
}
}
-
PR Comment: https://git.openjdk.org/jfx/pull/1236#issuecomment-1732332509
PR Comment: https://git.openjdk.org/jfx/pull/1236#issuecomment-
