Re: Having trouble setting up the JFX project in IntelliJ

2021-08-19 Thread Jonathan Vusich
Thanks for the link to the IDE instructions, Nir. I did not see that
before. I was trying to import the project as a Gradle project which was
not working very well. I have imported the project as an IntelliJ project
and seem to be having more success, but I am still getting these module
warnings that are preventing me from building the project or running any of
the apps. https://imgur.com/a/G6r32HG If anyone else who uses IntelliJ
knows how to resolve this, please let me know.

On Thu, Aug 19, 2021 at 1:25 PM Nir Lisker  wrote:

> There are IDE instructions here:
> https://wiki.openjdk.java.net/display/OpenJFX/Using+an+IDE
>
> I don't know about IntelliJ since I use Eclipse. The project files there
> were pending an update at https://github.com/openjdk/jfx/pull/514, but
> since then projects were removed and more changes are planned, so I closed
> the PR. Still, you can test the changes there and see if they solve at
> least some of the issues. I can create a new PR if you are willing to serve
> as a reviewer because lack of reviewers is what stops the PRs for updating
> the Eclipse files.
>
> Some tests, like ones that use the Robot API, require additional command
> line arguments to work, so just clicking run on the unit test won't work. I
> tend to run those from the command line, but it should be possible to make
> them run from Eclipse somehow. Regular unit tests run for me.
>
> On Wed, Aug 18, 2021 at 10:46 PM Jonathan Vusich 
> wrote:
>
>> Hello all,
>>
>>
>>
>> As the title suggests, I am struggling to figure out how to properly set
>> up
>> the JavaFX project in IntelliJ or Eclipse. I can run the tests for JavaFX
>> through Gradle on the command line, so I am sure that I have all of the
>> needed tools and environment variables set correctly. However, my IntelliJ
>> and Eclipse IDEs both show many compile and build errors, and do not allow
>> me to run any of the test apps due to these compile errors. In order to
>> address feedback on a pull request for JavaFX, I need to be able to run
>> the
>> Ensemble8 test application, and I cannot figure out how to do this from
>> the
>> command line alone. Is there something that I am missing that would cause
>> IntelliJ to be able to resolve these compile errors? Or am I just supposed
>> to run everything through the command line? If the latter is true, how do
>> I
>> run the Ensemble8 application through Gradle from the command line? Many
>> thanks to anyone who can provide some answers.
>>
>>
>>
>> Jonathan Vusich
>>
>


Having trouble setting up the JFX project in IntelliJ

2021-08-18 Thread Jonathan Vusich
Hello all,



As the title suggests, I am struggling to figure out how to properly set up
the JavaFX project in IntelliJ or Eclipse. I can run the tests for JavaFX
through Gradle on the command line, so I am sure that I have all of the
needed tools and environment variables set correctly. However, my IntelliJ
and Eclipse IDEs both show many compile and build errors, and do not allow
me to run any of the test apps due to these compile errors. In order to
address feedback on a pull request for JavaFX, I need to be able to run the
Ensemble8 test application, and I cannot figure out how to do this from the
command line alone. Is there something that I am missing that would cause
IntelliJ to be able to resolve these compile errors? Or am I just supposed
to run everything through the command line? If the latter is true, how do I
run the Ensemble8 application through Gradle from the command line? Many
thanks to anyone who can provide some answers.



Jonathan Vusich


Re: RFR: 8255572: Axis does not compute preferred height properly when autoRanging is off [v8]

2021-08-04 Thread Jonathan Vusich
> As noted in the corresponding JBS issue, `Axis` does not properly compute its 
> preferred height when `autoRanging` is turned off. The simplest fix seems to 
> be changing `CategoryAxis` so that `tickLabelRotation` is set to 90 degrees 
> if there is not enough room for the category labels to layout horizontally. 
> This fixes the fact that the axis labels are truncated and also ensures that 
> the chart does not leave unused space from the layout calculations. 
> `CategoryAxis` is already setting the `categorySpacing` property when 
> `autoRanging` is off, so this seems to be an appropriate fix.

Jonathan Vusich has updated the pull request with a new target base due to a 
merge or a rebase. The incremental webrev excludes the unrelated changes 
brought in by the merge/rebase. The pull request contains eight additional 
commits since the last revision:

 - Merge branch 'master' into jonathan/fix-chart-axis-labels
 - Updated per review comments
 - Add tests for vertical axis as well
 - Improve layout calculations for rotated text
 - Remove unused import
 - Unrotate labels if there is enough space for them
 - Added copyright header
 - Added fix and test

-

Changes:
  - all: https://git.openjdk.java.net/jfx/pull/342/files
  - new: https://git.openjdk.java.net/jfx/pull/342/files/9d2c8ac9..cc73f8dc

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jfx=342=07
 - incr: https://webrevs.openjdk.java.net/?repo=jfx=342=06-07

  Stats: 442103 lines in 7940 files changed: 245930 ins; 125567 del; 70606 mod
  Patch: https://git.openjdk.java.net/jfx/pull/342.diff
  Fetch: git fetch https://git.openjdk.java.net/jfx pull/342/head:pull/342

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


Re: RFR: 8255572: Axis does not compute preferred height properly when autoRanging is off [v6]

2021-04-30 Thread Jonathan Vusich
On Mon, 26 Apr 2021 21:06:41 GMT, Kevin Rushforth  wrote:

>> Well, it's complicated. The algorithm that calculates the required space for 
>> the tick labels does not take into account the tick label rotation. I did 
>> not look into adding this at this time because that opens up a lot of other 
>> potential pathways that seem broader in scope. With these changes, if the 
>> application detects that there is not enough space to display the text at 
>> the current rotation (which is assumed to be either 90 or 0 degrees), we 
>> rotate it by 90 degrees and remeasure. If that second remeasuring shows that 
>> the tick labels can be fully displayed at that rotation, the tick label 
>> rotation is overwritten to either 0 or 90 degrees. Theoretically if a user 
>> defines a set tick label rotation, it can be overwritten under these 
>> circumstances. However, I think this is reasonable given that when 
>> `autoRanging` is disabled a number of other properties are also overwritten 
>> in a similar manner.
>
> Yeah, it is complicated, and I agree doesn't work entirely correctly today, 
> but this change breaks setting the axis tick label rotation completely, even 
> in case where will fit without forcing the rotation to 90.
> 
> To reproduce this problem, run the Ensemble8 app with your fix, select the 
> Bar Chart sample, and attempt to change the X axis `tickLabelRotation`, 
> either using the slider or entering a number in the text field. You will be 
> unable to change the value since the code you added will overwrite it.
> 
> If you run it without your fix, you can change the X axis 
> `tickLabelRotation`. It won't take effect right away (that's a bug), but if 
> you uncheck and then recheck `tickLabelsVisible` it will re-layout the chart 
> and the axis labels will be drawn rotated.
> 
> With auto-ranging selected, if you run with your fix, you can change the 
> axis, but it won't actually do anything, even in the case where the label 
> fits.
> 
> Without your fix, you can rotate the labels with the slider just fine.
> 
> So while I agree that improving the situation when a rotation is set could be 
> done as a follow-up bug, we need a solution that won't break existing use 
> cases that are working.

@kevinrushforth Thank you for pointing this out, I was not aware of the 
Ensemble8 app that provided a testbed for this functionality. I will dig into 
this again and figure out a better solution. 

On a somewhat unrelated note to this PR, are there any instructions for how to 
get the JavaFX repository set up in an IDE? I have had a terrible time dealing 
with compile warnings in my IntelliJ IDE even though I can run everything 
successfully through Gradle on the command line.

-

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


Re: RFR: 8255572: Axis does not compute preferred height properly when autoRanging is off [v7]

2021-04-26 Thread Jonathan Vusich
> As noted in the corresponding JBS issue, `Axis` does not properly compute its 
> preferred height when `autoRanging` is turned off. The simplest fix seems to 
> be changing `CategoryAxis` so that `tickLabelRotation` is set to 90 degrees 
> if there is not enough room for the category labels to layout horizontally. 
> This fixes the fact that the axis labels are truncated and also ensures that 
> the chart does not leave unused space from the layout calculations. 
> `CategoryAxis` is already setting the `categorySpacing` property when 
> `autoRanging` is off, so this seems to be an appropriate fix.

Jonathan Vusich has updated the pull request incrementally with one additional 
commit since the last revision:

  Updated per review comments

-

Changes:
  - all: https://git.openjdk.java.net/jfx/pull/342/files
  - new: https://git.openjdk.java.net/jfx/pull/342/files/d5a3fae9..9d2c8ac9

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jfx=342=06
 - incr: https://webrevs.openjdk.java.net/?repo=jfx=342=05-06

  Stats: 6 lines in 3 files changed: 4 ins; 0 del; 2 mod
  Patch: https://git.openjdk.java.net/jfx/pull/342.diff
  Fetch: git fetch https://git.openjdk.java.net/jfx pull/342/head:pull/342

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


Re: RFR: 8255572: Axis does not compute preferred height properly when autoRanging is off [v6]

2021-04-26 Thread Jonathan Vusich
On Fri, 23 Apr 2021 23:28:49 GMT, Kevin Rushforth  wrote:

>> Jonathan Vusich has updated the pull request incrementally with two 
>> additional commits since the last revision:
>> 
>>  - Add tests for vertical axis as well
>>  - Improve layout calculations for rotated text
>
> modules/javafx.controls/src/main/java/javafx/scene/chart/CategoryAxis.java 
> line 365:
> 
>> 363: double requiredLengthToDisplay = 
>> calculateRequiredSize(side.isVertical(), tickLabelRotation);
>> 364: if (requiredLengthToDisplay > length) {
>> 365: if (tickLabelRotation != 90) {
> 
> One difference between the new algorithm and the old is that the new doesn't 
> take into account which side you are on. If the only two values are 0 or 90, 
> then it wouldn't matter. But what happens when someone sets a 45 degree 
> rotation (or 30)?

Well, it's complicated. The algorithm that calculates the required space for 
the tick labels does not take into account the tick label rotation. I did not 
look into adding this at this time because that opens up a lot of other 
potential pathways that seem broader in scope. Currently, if the application 
detects that there is not enough space to display the text at the current 
rotation (which is assumed to be either 90 or 0 degrees), we rotate it by 90 
degrees and remeasure. If that second remeasuring shows that the tick labels 
can be fully displayed at that rotation, the tick label rotation is overwritten 
to either 0 or 90 degrees. Theoretically if a user defines a set tick label 
rotation, it can be overwritten under these circumstances. However, I think 
this is reasonable given that when `autoRanging` is disabled a number of other 
properties are also overwritten in a similar manner.

-

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


Re: RFR: 8255572: Axis does not compute preferred height properly when autoRanging is off [v6]

2021-03-29 Thread Jonathan Vusich
On Wed, 3 Feb 2021 17:08:17 GMT, Jeanette Winzenburg  
wrote:

>> wondering about the system test - what stands in the way to add a simple 
>> "normal" unit test?
>
>> 
>> 
>> wondering about the system test - what stands in the way to add a simple 
>> "normal" unit test?
> 
> looks like there might be a bug in StubFontLoader that makes a normal unit 
> test fail w/out the fix: Axis uses a Text field (measure) for measuring size 
> requirements for the tick labels - its font has no nativeFont set in the stub 
> context such that all fields of the bounds of text are always 0, consequently 
> the change in autoRange never kicks in. Hacking around with explicitly 
> setting a tickLabelFont gives us a test that fails before and passes after 
> the fix:
> 
> @Test
> public void testRotatedStandAlone() {
> Pane root = new Pane();
> Scene scene = new Scene(root);
> Stage stage = new Stage();
> stage.setScene(scene);
> CategoryAxis xAxis = new 
> CategoryAxis(FXCollections.observableArrayList(categories));
> // hack around stubFontLoader bug (?feature)
> xAxis.setTickLabelFont(new Font(8));
> BarChart chart = new BarChart<>(xAxis, new 
> NumberAxis());
> chart.setPrefWidth(400);
> root.getChildren().add(chart);
> stage.show();
> assertEquals(90, xAxis.getTickLabelRotation(), 0.1);
> }
> 
> Question is why the stubFontLoader fails: its load implementation looks like 
> it should always set the nativeFont fiel to a fitting test font, but it 
> doesn't if the name is a plain "Amble" (vs. a more specialized like "Amble 
> Regular").

@kleopatra @kevinrushforth Are the system tests that I have provided sufficient 
for this PR to move forward or do they need to be rewritten as unit tests?

-

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


Re: RFR: 8255572: Axis does not compute preferred height properly when autoRanging is off [v6]

2021-02-01 Thread Jonathan Vusich
On Mon, 1 Feb 2021 21:37:44 GMT, Kevin Rushforth  wrote:

>> @JonathanVusich Once a review is in progress, please don't force-push your 
>> branch without a compelling reason, since that makes it harder to do 
>> incremental reviews. In the future, we recommend `git merge master` rather 
>> than `git rebase master`, since the former doesn't require force-pushing.
>
> I do see that there are no changes from last time, which is good, so it will 
> be fine for this time.
> 
> When ready, you can add the new test by pushing a new commit.

@kevinrushforth I was not aware of that, thank you for letting me know! 
I wrote a new test for vertical axes and discovered that there were more issues 
with axis layout calculations, which I also addressed.

I also found out that the layout tests sometimes fail without a short pause 
using `Util.sleep()`. I don't really like having to add sleeps, but I did find 
that it got rid of any test flakiness.

-

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


Re: RFR: 8255572: Axis does not compute preferred height properly when autoRanging is off [v6]

2021-02-01 Thread Jonathan Vusich
> As noted in the corresponding JBS issue, `Axis` does not properly compute its 
> preferred height when `autoRanging` is turned off. The simplest fix seems to 
> be changing `CategoryAxis` so that `tickLabelRotation` is set to 90 degrees 
> if there is not enough room for the category labels to layout horizontally. 
> This fixes the fact that the axis labels are truncated and also ensures that 
> the chart does not leave unused space from the layout calculations. 
> `CategoryAxis` is already setting the `categorySpacing` property when 
> `autoRanging` is off, so this seems to be an appropriate fix.

Jonathan Vusich has updated the pull request incrementally with two additional 
commits since the last revision:

 - Add tests for vertical axis as well
 - Improve layout calculations for rotated text

-

Changes:
  - all: https://git.openjdk.java.net/jfx/pull/342/files
  - new: https://git.openjdk.java.net/jfx/pull/342/files/dff9ee17..d5a3fae9

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jfx=342=05
 - incr: https://webrevs.openjdk.java.net/?repo=jfx=342=04-05

  Stats: 134 lines in 3 files changed: 120 ins; 1 del; 13 mod
  Patch: https://git.openjdk.java.net/jfx/pull/342.diff
  Fetch: git fetch https://git.openjdk.java.net/jfx pull/342/head:pull/342

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


Re: RFR: 8255572: Axis does not compute preferred height properly when autoRanging is off [v5]

2021-02-01 Thread Jonathan Vusich
> As noted in the corresponding JBS issue, `Axis` does not properly compute its 
> preferred height when `autoRanging` is turned off. The simplest fix seems to 
> be changing `CategoryAxis` so that `tickLabelRotation` is set to 90 degrees 
> if there is not enough room for the category labels to layout horizontally. 
> This fixes the fact that the axis labels are truncated and also ensures that 
> the chart does not leave unused space from the layout calculations. 
> `CategoryAxis` is already setting the `categorySpacing` property when 
> `autoRanging` is off, so this seems to be an appropriate fix.

Jonathan Vusich has updated the pull request with a new target base due to a 
merge or a rebase. The pull request now contains four commits:

 - Remove unused import
 - Unrotate labels if there is enough space for them
 - Added copyright header
 - Added fix and test

-

Changes: https://git.openjdk.java.net/jfx/pull/342/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jfx=342=04
  Stats: 126 lines in 2 files changed: 126 ins; 0 del; 0 mod
  Patch: https://git.openjdk.java.net/jfx/pull/342.diff
  Fetch: git fetch https://git.openjdk.java.net/jfx pull/342/head:pull/342

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


Re: RFR: 8255572: Axis does not compute preferred height properly when autoRanging is off [v4]

2020-12-22 Thread Jonathan Vusich
> As noted in the corresponding JBS issue, `Axis` does not properly compute its 
> preferred height when `autoRanging` is turned off. The simplest fix seems to 
> be changing `CategoryAxis` so that `tickLabelRotation` is set to 90 degrees 
> if there is not enough room for the category labels to layout horizontally. 
> This fixes the fact that the axis labels are truncated and also ensures that 
> the chart does not leave unused space from the layout calculations. 
> `CategoryAxis` is already setting the `categorySpacing` property when 
> `autoRanging` is off, so this seems to be an appropriate fix.

Jonathan Vusich has updated the pull request incrementally with one additional 
commit since the last revision:

  Remove unused import

-

Changes:
  - all: https://git.openjdk.java.net/jfx/pull/342/files
  - new: https://git.openjdk.java.net/jfx/pull/342/files/6d83f79c..0d11d341

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jfx=342=03
 - incr: https://webrevs.openjdk.java.net/?repo=jfx=342=02-03

  Stats: 1 line in 1 file changed: 0 ins; 1 del; 0 mod
  Patch: https://git.openjdk.java.net/jfx/pull/342.diff
  Fetch: git fetch https://git.openjdk.java.net/jfx pull/342/head:pull/342

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


Re: RFR: 8255572: Axis does not compute preferred height properly when autoRanging is off [v3]

2020-11-25 Thread Jonathan Vusich
> As noted in the corresponding JBS issue, `Axis` does not properly compute its 
> preferred height when `autoRanging` is turned off. The simplest fix seems to 
> be changing `CategoryAxis` so that `tickLabelRotation` is set to 90 degrees 
> if there is not enough room for the category labels to layout horizontally. 
> This fixes the fact that the axis labels are truncated and also ensures that 
> the chart does not leave unused space from the layout calculations. 
> `CategoryAxis` is already setting the `categorySpacing` property when 
> `autoRanging` is off, so this seems to be an appropriate fix.

Jonathan Vusich has updated the pull request incrementally with one additional 
commit since the last revision:

  Unrotate labels if there is enough space for them

-

Changes:
  - all: https://git.openjdk.java.net/jfx/pull/342/files
  - new: https://git.openjdk.java.net/jfx/pull/342/files/49a5e1f7..6d83f79c

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jfx=342=02
 - incr: https://webrevs.openjdk.java.net/?repo=jfx=342=01-02

  Stats: 19 lines in 2 files changed: 14 ins; 2 del; 3 mod
  Patch: https://git.openjdk.java.net/jfx/pull/342.diff
  Fetch: git fetch https://git.openjdk.java.net/jfx pull/342/head:pull/342

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


Re: RFR: 8255572: Axis does not compute preferred height properly when autoRanging is off [v2]

2020-11-25 Thread Jonathan Vusich
On Tue, 24 Nov 2020 22:21:04 GMT, Kevin Rushforth  wrote:

>> Can you merge in the latest `jfx/master`? That way the GitHub actions 
>> build/test will be run (although it won't run the new headful test). You 
>> also need a copyright header as noted below.
>
> While this does fix the specific problem, it introduces a new one. If the 
> labels are initially too big, but after resizing the window would now fit, it 
> does not recompute the orientation. This means that you are left with labels 
> that are rotated even when they don't need to be.

@kevinrushforth Thank you for catching that. Do you think it would be 
acceptable to simply rotate the labels back to zero if the user expands the 
window?

-

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


Re: RFR: 8255572: Axis does not compute preferred height properly when autoRanging is off [v2]

2020-11-11 Thread Jonathan Vusich
> As noted in the corresponding JBS issue, `Axis` does not properly compute its 
> preferred height when `autoRanging` is turned off. The simplest fix seems to 
> be changing `CategoryAxis` so that `tickLabelRotation` is set to 90 degrees 
> if there is not enough room for the category labels to layout horizontally. 
> This fixes the fact that the axis labels are truncated and also ensures that 
> the chart does not leave unused space from the layout calculations. 
> `CategoryAxis` is already setting the `categorySpacing` property when 
> `autoRanging` is off, so this seems to be an appropriate fix.

Jonathan Vusich has updated the pull request with a new target base due to a 
merge or a rebase. The incremental webrev excludes the unrelated changes 
brought in by the merge/rebase. The pull request contains three additional 
commits since the last revision:

 - Added copyright header
 - Merge branch 'master' into jonathan/fix-chart-axis-labels
 - Added fix and test

-

Changes:
  - all: https://git.openjdk.java.net/jfx/pull/342/files
  - new: https://git.openjdk.java.net/jfx/pull/342/files/e3eb791e..49a5e1f7

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jfx=342=01
 - incr: https://webrevs.openjdk.java.net/?repo=jfx=342=00-01

  Stats: 1552 lines in 40 files changed: 1343 ins; 136 del; 73 mod
  Patch: https://git.openjdk.java.net/jfx/pull/342.diff
  Fetch: git fetch https://git.openjdk.java.net/jfx pull/342/head:pull/342

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


Re: RFR: 8255572: Axis does not compute preferred height properly when autoRanging is off [v2]

2020-11-11 Thread Jonathan Vusich
On Wed, 11 Nov 2020 16:08:20 GMT, Kevin Rushforth  wrote:

>> Jonathan Vusich has updated the pull request with a new target base due to a 
>> merge or a rebase. The incremental webrev excludes the unrelated changes 
>> brought in by the merge/rebase. The pull request contains three additional 
>> commits since the last revision:
>> 
>>  - Added copyright header
>>  - Merge branch 'master' into jonathan/fix-chart-axis-labels
>>  - Added fix and test
>
> tests/system/src/test/java/test/javafx/scene/chart/ChartAxisLayoutTest.java 
> line 1:
> 
>> 1: package test.javafx.scene.chart;
> 
> You need a standard copyright header here. See any of the other test classes 
> for an example (should be just a single `2020,` for the year).

I'll get right on it.

-

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


RFR: 8255572: Axis does not compute preferred height properly when autoRanging is off

2020-11-11 Thread Jonathan Vusich
As noted in the corresponding JBS issue, `Axis` does not properly compute its 
preferred height when `autoRanging` is turned off. The simplest fix seems to be 
changing `CategoryAxis` so that `tickLabelRotation` is set to 90 degrees if 
there is not enough room for the category labels to layout horizontally. This 
fixes the fact that the axis labels are truncated and also ensures that the 
chart does not leave unused space from the layout calculations. `CategoryAxis` 
is already setting the `categorySpacing` property when `autoRanging` is off, so 
this seems to be an appropriate fix.

-

Commit messages:
 - Added fix and test

Changes: https://git.openjdk.java.net/jfx/pull/342/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jfx=342=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8255572
  Stats: 90 lines in 2 files changed: 90 ins; 0 del; 0 mod
  Patch: https://git.openjdk.java.net/jfx/pull/342.diff
  Fetch: git fetch https://git.openjdk.java.net/jfx pull/342/head:pull/342

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