On Sat, 19 Feb 2022 03:52:41 GMT, Jay Bhaskar <d...@openjdk.java.net> wrote:
>> Issue: The end point of line in drawLinesForText , add thickness to the
>> endPoint.y(). In this case origin which is start point and the end point
>> would not be same, and line would be drawn not straight.
>> Solution: Do not add thickness to the y position of end point of line.
>> Start Point(x,y) ----------End Point(x + width, 0)
>
> Jay Bhaskar has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Update test case for line straightness check
The test is a good start, but I left several comments inline.
tests/system/src/test/java/test/javafx/scene/web/StraightLineTest.java line 75:
> 73: this.primaryStage = primaryStage;
> 74: this.primaryStage.setWidth(80);
> 75: this.primaryStage.setHeight(60);
Minor: Since you set the size of the Scene later on, you don't need to set it
here.
tests/system/src/test/java/test/javafx/scene/web/StraightLineTest.java line 78:
> 76: launchLatch.countDown();
> 77: }
> 78: }
Add a blank after this.
tests/system/src/test/java/test/javafx/scene/web/StraightLineTest.java line 87:
> 85: }
> 86:
> 87:
Minor: the extra blank line is unnecessary.
tests/system/src/test/java/test/javafx/scene/web/StraightLineTest.java line 105:
> 103: Platform.runLater(() -> {
> 104: webView = new WebView();
> 105: Scene scene = new Scene(webView,80,60);
This test passes on my Windows system even without the fix. Based on what I see
on the screen, I think its because the scene size is too small. I would make it
larger (at least 150x100).
Minor: add a space after the commas to separate the args
tests/system/src/test/java/test/javafx/scene/web/StraightLineTest.java line 163:
> 161:
> 162: // buttom start x position of underline ( startx + font size
> + thickness -1)
> 163: int line_start_x = start_x + height + 20 - 1;
The x value shouldn't be affected by thickness or the height. This should be
something like `start_x + 3` (the `+3` is so you don't sample anywhere near the
edge, to avoid boundary problems).
I also recommend sampling near the right edge to catch the problem of a slanted
line, so: `int line_end_x = start_x + width - 3;`
tests/system/src/test/java/test/javafx/scene/web/StraightLineTest.java line 165:
> 163: int line_start_x = start_x + height + 20 - 1;
> 164: // buttom start y position of underline ( startx + height)
> 165: int line_start_y = start_y + height;
As mentioned above, I recommend adding 3 pixels to avoid sampling near the top
edge of the thick line, so `start_y + height + 3`
tests/system/src/test/java/test/javafx/scene/web/StraightLineTest.java line 167:
> 165: int line_start_y = start_y + height;
> 166: String line_color = "rgba(0,0,0,255)"; // color of line
> 167: for (int i = line_start_y; i < snapshot.getHeight(); i++) {
The snapshot height is irrelevant. You should use thickness minus 6 (3 pixels
on each of the top and bottom):
i < line_start_y + thickness - 6;
tests/system/src/test/java/test/javafx/scene/web/StraightLineTest.java line 172:
> 170: continue;
> 171: else
> 172: fail("Each pixel color of line should be" +
> line_color + " but was:" + expected);
The name `expected` is misleading. It isn't the expected value, but rather the
"actual" value that you just read.
Also, there are two formatting issues:
1. There should be a space after the `if`
2. The target statements of the `if` and `else` must be surrounded by curly
braces (unless on the same line as the `if` which would look odd in this case)
-------------
PR: https://git.openjdk.java.net/jfx/pull/731
