Re: RFR: 8281953: NullPointer in InputMethod components in JFXPanel

2022-02-20 Thread eduardsdv
On Sat, 19 Feb 2022 17:40:54 GMT, delvh  wrote:

>> If the InputMethod's node is not in the scene, the default text location 
>> point is returned.
>
> modules/javafx.controls/src/main/java/javafx/scene/control/skin/TextInputControlSkin.java
>  line 340:
> 
>> 338: if (window == null) {
>> 339: return new Point2D(0, 0);
>> 340: }
> 
> As long as each scene needs to be located in a window, the following will 
> simplify the code:
> Suggestion:
> 
> if (scene == null) {
> return new Point2D(0, 0);
> }
> Window window = scene.getWindow();
> 
> Is it possible to have a scene without an assigned window?

Since the NullPointer occurs here because the synchronization between JavaFx 
and AWT threads is not really possible in this case, we cannot ensure that this 
method is only called when the scene is assigned to a window.
I would check here for null both the scene and the window.

-

PR: https://git.openjdk.java.net/jfx/pull/735


Re: RFR: 8280020: Underline and line-through not straight in WebView [v4]

2022-02-20 Thread Jay Bhaskar
> 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:

  Improve style and width iterartion logic

-

Changes:
  - all: https://git.openjdk.java.net/jfx/pull/731/files
  - new: https://git.openjdk.java.net/jfx/pull/731/files/135a073b..3246375d

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jfx&pr=731&range=03
 - incr: https://webrevs.openjdk.java.net/?repo=jfx&pr=731&range=02-03

  Stats: 13 lines in 1 file changed: 4 ins; 1 del; 8 mod
  Patch: https://git.openjdk.java.net/jfx/pull/731.diff
  Fetch: git fetch https://git.openjdk.java.net/jfx pull/731/head:pull/731

PR: https://git.openjdk.java.net/jfx/pull/731


Re: RFR: 8280020: Underline and line-through not straight in WebView [v3]

2022-02-20 Thread Jay Bhaskar
On Sat, 19 Feb 2022 15:03:38 GMT, Kevin Rushforth  wrote:

>> Jay Bhaskar has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Update test case for line straightness check
>
> 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;

I think it should be likefor (int i = line_start_y; i <= (width - 
line_start_y -6); i++) , as we need to iterate in x direction

-

PR: https://git.openjdk.java.net/jfx/pull/731


Re: RFR: 8280020: Underline and line-through not straight in WebView [v3]

2022-02-20 Thread Jay Bhaskar
On Mon, 21 Feb 2022 04:19:29 GMT, Jay Bhaskar  wrote:

>> 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;`
>
> The bounding rect.x rect.y is top left corner, and line is being drawn below 
> the bottom, so height and thickness need to be considered.

int i = line_start_y; i < (line_start_y - 6); i++ , this meets the condition of 
sampling near the right edge

-

PR: https://git.openjdk.java.net/jfx/pull/731


Re: RFR: 8280020: Underline and line-through not straight in WebView [v3]

2022-02-20 Thread Jay Bhaskar
On Sat, 19 Feb 2022 14:55:30 GMT, Kevin Rushforth  wrote:

>> Jay Bhaskar has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Update test case for line straightness check
>
> 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;`

The bounding rect.x rect.y is top left corner, and line is being drawn below 
the bottom, so height and thickness need to be considered.

-

PR: https://git.openjdk.java.net/jfx/pull/731


Re: RFR: 8280020: Underline and line-through not straight in WebView [v3]

2022-02-20 Thread Jay Bhaskar
On Sat, 19 Feb 2022 14:45:53 GMT, Kevin Rushforth  wrote:

>> Jay Bhaskar has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Update test case for line straightness check
>
> 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.

This would be remain same , i would not set size later

-

PR: https://git.openjdk.java.net/jfx/pull/731


Re: RFR: 8280020: Underline and line-through not straight in WebView [v3]

2022-02-20 Thread Jay Bhaskar
On Sat, 19 Feb 2022 16:51:52 GMT, Kevin Rushforth  wrote:

>> Jay Bhaskar has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Update test case for line straightness check
>
> 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)

done

-

PR: https://git.openjdk.java.net/jfx/pull/731


Re: RFR: 8280020: Underline and line-through not straight in WebView [v3]

2022-02-20 Thread Jay Bhaskar
On Sat, 19 Feb 2022 14:32:46 GMT, Kevin Rushforth  wrote:

>> Jay Bhaskar has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Update test case for line straightness check
>
> tests/system/src/test/java/test/javafx/scene/web/StraightLineTest.java line 
> 78:
> 
>> 76: launchLatch.countDown();
>> 77: }
>> 78: }
> 
> Add a blank after this.

done

> tests/system/src/test/java/test/javafx/scene/web/StraightLineTest.java line 
> 87:
> 
>> 85: }
>> 86: 
>> 87: 
> 
> Minor: the extra blank line is unnecessary.

done

> 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

space added, i will look this observation on windows

-

PR: https://git.openjdk.java.net/jfx/pull/731