Re: [Rev 01] RFR: 8236259: MemoryLeak in ProgressIndicator

2019-12-19 Thread Florian Kirmaier
On Thu, 19 Dec 2019 13:47:14 GMT, Jeanette Winzenburg wrote: >> no way, afaik - there had been debates when the api was added somewhere in >> jbs, don't recall exactly where > > here's where it was added: https://bugs.openjdk.java.net/browse/JDK-8151617 I don't see an API in the discussion,

Re: [Rev 01] RFR: 8236259: MemoryLeak in ProgressIndicator

2019-12-19 Thread Jeanette Winzenburg
On Thu, 19 Dec 2019 13:56:39 GMT, Florian Kirmaier wrote: >> here's where it was added: https://bugs.openjdk.java.net/browse/JDK-8151617 > > I don't see an API in the discussion, on how to remove a specific listener. > Did i miss something? It seems to me, that the API is written to support

Re: [Rev 01] RFR: 8236259: MemoryLeak in ProgressIndicator

2019-12-19 Thread Jeanette Winzenburg
On Thu, 19 Dec 2019 15:27:59 GMT, Jeanette Winzenburg wrote: >> As mentioned, I could copy the class of the library into the JavaFX-Project. >> Alternatively, I could remove the unit-test entirely. >> >> But to be honest, the current state related to memory-leaks in the JavaFX >> project is

Re: [Rev 01] RFR: 8236259: MemoryLeak in ProgressIndicator

2019-12-19 Thread Jeanette Winzenburg
On Thu, 19 Dec 2019 15:44:37 GMT, Florian Kirmaier wrote: >> for now, you could have a look at ProgressIndicatorTest (or >> ProgressSkinTest) attemptGC - might not be optimal (don't know enough about >> the dirty details of gc :) but gets the test failing/passing before/after >> fixing a

Re: RFR: 8236259: MemoryLeak in ProgressIndicator

2019-12-19 Thread Jeanette Winzenburg
On Thu, 19 Dec 2019 13:35:51 GMT, Florian Kirmaier wrote: >> modules/javafx.controls/src/main/java/javafx/scene/control/skin/ProgressIndicatorSkin.java >> line 555: >> >>> 554: } >>> 555: >>> 556: private void setFillOverride(Paint fillOverride) { >> >> just a side-note:

Re: [Rev 01] RFR: 8236259: MemoryLeak in ProgressIndicator

2019-12-19 Thread Florian Kirmaier
> Hi everyone, > > ticket: https://bugs.openjdk.java.net/browse/JDK-8236259 > > The fix itself is quite straight forward. > It basically just removed the listener which causes the leak. > > The unit-test for the fix is a bit more complicated. > > I added a library JMemoryBuddy

Re: [Rev 01] RFR: 8236259: MemoryLeak in ProgressIndicator

2019-12-19 Thread Kevin Rushforth
On Thu, 19 Dec 2019 15:12:12 GMT, Florian Kirmaier wrote: >> Hi everyone, >> >> ticket: https://bugs.openjdk.java.net/browse/JDK-8236259 >> >> The fix itself is quite straight forward. >> It basically just removed the listener which causes the leak. >> >> The unit-test for the fix is a bit

Re: RFR: 8236259: MemoryLeak in ProgressIndicator

2019-12-19 Thread Jeanette Winzenburg
On Thu, 19 Dec 2019 13:39:07 GMT, Jeanette Winzenburg wrote: >> Yes, do you know an API for registerChangeListener which allows me to only >> remove a specific listener? > > no way, afaik - there had been debates when the api was added somewhere in > jbs, don't recall exactly where here's

Re: [Rev 03] RFR: 8130738: Add tabSize property to Text and TextFlow

2019-12-19 Thread Scott Palmer
On Thu, 12 Dec 2019 21:51:56 GMT, Nir Lisker wrote: >> The pull request has been updated with 1 additional commit. > > I was thinking of deferring the `apps/toys` demo to avoid any delays in getting the new API into JavaFX 14. I have something for it, I just don't want any feedback on it

Re: [Rev 01] RFR: 8236259: MemoryLeak in ProgressIndicator

2019-12-19 Thread Jeanette Winzenburg
On Thu, 19 Dec 2019 14:19:57 GMT, Florian Kirmaier wrote: >> just thinking aloud: actually, it's rather whacky that the indicator >> (mis-)uses the skin's multiplePropertyListener - it's api isn't meant for >> frequent register/unregisters (the unregisters is quite a heavy measure). >> Maybe

Re: [Rev 01] RFR: 8236259: MemoryLeak in ProgressIndicator

2019-12-19 Thread Florian Kirmaier
On Thu, 19 Dec 2019 14:01:08 GMT, Jeanette Winzenburg wrote: >> Yes, that doesn't make a lot of sense. It's probably an artifact because I >> expected a different API do unregister the listener. I'm gonna change it >> back! > > just thinking aloud: actually, it's rather whacky that the

Re: [Rev 01] RFR: 8236259: MemoryLeak in ProgressIndicator

2019-12-19 Thread Florian Kirmaier
On Thu, 19 Dec 2019 15:33:09 GMT, Jeanette Winzenburg wrote: >> Florian, basically it's a single class isn't it? If so, it might be >> acceptable to add that class to the test.xx.infrastructure package (don't >> know if doing so would require a jbs issue) > > for now, you could have a look

Re: RFR: 8236259: MemoryLeak in ProgressIndicator

2019-12-19 Thread Jeanette Winzenburg
On Thu, 19 Dec 2019 12:35:56 GMT, Florian Kirmaier wrote: > Hi everyone, > > ticket: https://bugs.openjdk.java.net/browse/JDK-8236259 > > The fix itself is quite straight forward. > It basically just removed the listener which causes the leak. > > The unit-test for the fix is a bit more

Re: RFR: 8236259: MemoryLeak in ProgressIndicator

2019-12-19 Thread Florian Kirmaier
On Thu, 19 Dec 2019 13:23:25 GMT, Jeanette Winzenburg wrote: >> Hi everyone, >> >> ticket: https://bugs.openjdk.java.net/browse/JDK-8236259 >> >> The fix itself is quite straight forward. >> It basically just removed the listener which causes the leak. >> >> The unit-test for the fix is a

Re: RFR: 8236259: MemoryLeak in ProgressIndicator

2019-12-19 Thread Florian Kirmaier
On Thu, 19 Dec 2019 13:28:07 GMT, Jeanette Winzenburg wrote: >> Hi everyone, >> >> ticket: https://bugs.openjdk.java.net/browse/JDK-8236259 >> >> The fix itself is quite straight forward. >> It basically just removed the listener which causes the leak. >> >> The unit-test for the fix is a

Re: [Rev 01] RFR: 8236259: MemoryLeak in ProgressIndicator

2019-12-19 Thread Jeanette Winzenburg
On Thu, 19 Dec 2019 13:35:27 GMT, Florian Kirmaier wrote: >> modules/javafx.controls/src/main/java/javafx/scene/control/skin/ProgressIndicatorSkin.java >> line 521: >> >>> 520: registerChangeListener(text.fontProperty(), >>> (Consumer>) fontListener); >>> 521: >>> 522:

Re: [Rev 02] RFR: 8232811: Dialog's preferred size no longer accommodates multi-line strings

2019-12-19 Thread Ambarish Rapte
On Thu, 19 Dec 2019 14:32:32 GMT, Thiago Milczarek Sayao wrote: >> https://bugs.openjdk.java.net/browse/JDK-8232811 >> >> This one was hard to tackle. >> >> ./gradlew -PFULL_TEST=true -PUSE_ROBOT=true :systemTests:test --tests >> test.robot.javafx.scene.dialog.DialogWithOwnerSizingTest > >

Re: [Rev 01] RFR: 8236259: MemoryLeak in ProgressIndicator

2019-12-19 Thread Jeanette Winzenburg
On Thu, 19 Dec 2019 15:24:48 GMT, Florian Kirmaier wrote: >> I haven't looked at the fix itself, but there are two things that _must_ be >> fixed before this PR can be considered: >> 1. You need to remove the additional 3rd-party test library dependency. >> 2. You need to revert the change

Re: [Rev 01] RFR: 8236259: MemoryLeak in ProgressIndicator

2019-12-19 Thread Kevin Rushforth
On Thu, 19 Dec 2019 15:48:17 GMT, Jeanette Winzenburg wrote: >> The point of having this GC-magic in a library is, that it's verified by the >> library that it's implementation is stable for all common JavaVersions. The >> attemptGC is basically bad practice. I think in the JavaFX-Codebase

Re: [Rev 01] RFR: 8236259: MemoryLeak in ProgressIndicator

2019-12-19 Thread Florian Kirmaier
On Thu, 19 Dec 2019 15:12:01 GMT, Kevin Rushforth wrote: >> The pull request has been updated with 1 additional commit. > > I haven't looked at the fix itself, but there are two things that _must_ be > fixed before this PR can be considered: > 1. You need to remove the additional 3rd-party

Re: Skara - bot sending can-be-integrated message prematurely?

2019-12-19 Thread Kevin Rushforth
FYI, for anyone interested, the Skara team submitted the following PR to modify the "ready for integration" message: https://github.com/openjdk/skara/pull/343 We can file a follow-up RFE to have them consider adding "/reviewers" and "/csr" commands. -- Kevin On 12/18/2019 7:17 PM, Phil

apps/toys HelloTextFlow renders incorrectly on macOS 10.15

2019-12-19 Thread Scott Palmer
As follow up to JDK-8130738 I was thinking about adding a new panel to HelloTextFlow to show adjusting the tabSize rather than making a new ‘toy’. However I noticed rendering issues right away. When you select text, parts of the text seem to paint white-on-white .. proportional to the amount

Re: RFR: 8235627: Fixed blank stage when running in macOS guest VM

2019-12-19 Thread Kevin Rushforth
On Thu, 19 Dec 2019 19:17:34 GMT, Kevin Rushforth wrote: >> https://bugs.openjdk.java.net/browse/JDK-8235627 > > While this looks simple enough, I will also request @arapte to review this, > in addition to reviewing it myself. The fix looks fine to me, but I want to run a couple quick tests.

Re: [Rev 01] RFR: 8236259: MemoryLeak in ProgressIndicator

2019-12-19 Thread David Grieve
On Thu, 19 Dec 2019 16:16:28 GMT, Kevin Rushforth wrote: >> probably not invented so often - just c'd from searches of occurances of >> System.gc ;-) > > Yes, exactly. > > @FlorianKirmaier Proposing to add test class to the test infrastructure would > be a reasonable alternative to pulling

RFR: 8088198: Exception thrown from snapshot if dimensions are larger than max texture size

2019-12-19 Thread Frederic Thevenet
This PR aims to address the following issue: JDK-8088198 Exception thrown from snapshot if dimensions are larger than max texture size In order to do that, it simply captures snapshots in multiple tiles of maxTextureSize^2 dimensions (or less, as needed), and then recomposes all the tiles into

Re: [Rev 02] RFR: 8232811: Dialog's preferred size no longer accommodates multi-line strings

2019-12-19 Thread Thiago Milczarek Sayao
On Tue, 17 Dec 2019 21:37:34 GMT, Kevin Rushforth wrote: >> The pull request has been updated with 1 additional commit. > > tests/system/src/test/java/test/robot/javafx/scene/dialog/DialogWithOwnerSizingTest.java > line 63: > >> 62: @Test(timeout = 15000) >> 63: public void

RFR: 8235364: Update copyright header for files modified in 2019

2019-12-19 Thread Kevin Rushforth
Update the last-modified year in the copyright header to 2019 for files that were modified this year without updating the year in the copyright header. - Commits: - 9c6e0a64: 8235364: Update copyright header for files modified in 2019 Changes:

Re: RFR: 8235364: Update copyright header for files modified in 2019

2019-12-19 Thread Kevin Rushforth
On Thu, 19 Dec 2019 22:47:07 GMT, Kevin Rushforth wrote: > Update the last-modified year in the copyright header to 2019 for files that > were modified this year without updating the year in the copyright header. Reviewer: @arapte (this only needs a single reviewer) - PR:

Re: RFR: 8235627: Fixed blank stage when running in macOS guest VM

2019-12-19 Thread Kevin Rushforth
On Tue, 10 Dec 2019 13:12:37 GMT, Frederic Thevenet wrote: > https://bugs.openjdk.java.net/browse/JDK-8235627 While this looks simple enough, I will also request @arapte to review this, in addition to reviewing it myself. - PR: https://git.openjdk.java.net/jfx/pull/65

Re: [Rev 01] RFR: 8236259: MemoryLeak in ProgressIndicator

2019-12-19 Thread David Grieve
Github failed me. My 'remove' comment was applied to the System.out.println, not the assert. > -Original Message- > From: openjfx-dev On Behalf Of > David Grieve > Sent: Thursday, December 19, 2019 2:29 PM > To: openjfx-dev@openjdk.java.net > Subject: [EXTERNAL] Re: [Rev 01] RFR:

RFR: 8234474: [macos 10.15] Crash in file dialog in sandbox mode

2019-12-19 Thread Kevin Rushforth
This PR is a fix for [JDK-8234474](https://bugs.openjdk.java.net/browse/JDK-8234474), a crash in the code that shows a file open or save dialog. In order to provide additional support for Copy (CMD-C), Cut (CMD-X), and Paste (CMD-V), the Glass implementation for displaying a file open or save

Re: [Rev 02] RFR: 8232811: Dialog's preferred size no longer accommodates multi-line strings

2019-12-19 Thread Thiago Milczarek Sayao
On Thu, 19 Dec 2019 14:29:39 GMT, Ambarish Rapte wrote: >> The pull request has been updated with 1 additional commit. > > modules/javafx.graphics/src/main/native-glass/gtk/glass_window.cpp line 1062: > >> 1061: geometry.final_height.type = BOUNDSTYPE_CONTENT; >> 1062: } >> 1063:

Re: [Rev 02] RFR: 8232811: Dialog's preferred size no longer accommodates multi-line strings

2019-12-19 Thread Thiago Milczarek Sayao
On Thu, 19 Dec 2019 14:13:47 GMT, Ambarish Rapte wrote: >> The pull request has been updated with 1 additional commit. > > tests/system/src/test/java/test/robot/javafx/scene/dialog/DialogWithOwnerSizingTest.java > line 64: > >> 63: public void dialogWithOwnerSizingTest() throws Exception

Re: [Rev 03] RFR: 8232811: Dialog's preferred size no longer accommodates multi-line strings

2019-12-19 Thread Thiago Milczarek Sayao
> https://bugs.openjdk.java.net/browse/JDK-8232811 > > This one was hard to tackle. > > ./gradlew -PFULL_TEST=true -PUSE_ROBOT=true :systemTests:test --tests > test.robot.javafx.scene.dialog.DialogWithOwnerSizingTest The pull request has been updated with 2 additional commits. -

Re: [Rev 02] RFR: 8232811: Dialog's preferred size no longer accommodates multi-line strings

2019-12-19 Thread Thiago Milczarek Sayao
On Tue, 17 Dec 2019 23:01:51 GMT, Kevin Rushforth wrote: >> The pull request has been updated with 1 additional commit. > > tests/system/src/test/java/test/robot/javafx/scene/dialog/DialogWithOwnerSizingTest.java > line 124: > >> 123: >> 124:

Re: Difference in CSS styling / rendering.

2019-12-19 Thread Kevin Rushforth
Hi Dirk, Any update on this? We'd love to take a look at it if you have a test case. Thanks. -- Kevin On 12/9/2019 1:13 PM, Dirk Lemmermann wrote: Hi, I just noticed that the timeline in FlexGanttFX does not get rendered correctly anymore after updating to 14-ea+4. The borders around the

RFR: 8235627: Fixed blank stage when running in macOS guest VM

2019-12-19 Thread Frederic Thevenet
https://bugs.openjdk.java.net/browse/JDK-8235627 - Commits: - 73523680: 8235627: Fixed blank stage when running in macOS guest VM Changes: https://git.openjdk.java.net/jfx/pull/65/files Webrev: https://webrevs.openjdk.java.net/jfx/65/webrev.00 Issue:

Re: [Rev 03] RFR: 8130738: Add tabSize property to Text and TextFlow

2019-12-19 Thread Kevin Rushforth
On Thu, 19 Dec 2019 13:48:11 GMT, Scott Palmer wrote: >> > > I was thinking of deferring the `apps/toys` demo to avoid any delays in > getting the new API into JavaFX 14. I have something for it, I just don't > want any feedback on it to hold up the review of this issue. Is there > anything

Re: RFR: 8234474: [macos 10.15] Crash in file dialog in sandbox mode

2019-12-19 Thread Kevin Rushforth
On Thu, 19 Dec 2019 00:08:06 GMT, Kevin Rushforth wrote: > This PR is a fix for > [JDK-8234474](https://bugs.openjdk.java.net/browse/JDK-8234474), a crash in > the code that shows a file open or save dialog. > > In order to provide additional support for Copy (CMD-C), Cut (CMD-X), and >

Re: [Rev 01] RFR: 8236259: MemoryLeak in ProgressIndicator

2019-12-19 Thread Florian Kirmaier
On Thu, 19 Dec 2019 19:59:07 GMT, David Grieve wrote: >> Yes, exactly. >> >> @FlorianKirmaier Proposing to add test class to the test infrastructure >> would be a reasonable alternative to pulling it in as a third-party >> dependency. I recommend separating it out into its own enhancement,

Re: apps/toys HelloTextFlow renders incorrectly on macOS 10.15

2019-12-19 Thread David Grieve
Because the toy itself handles the highlighting, I believe this is a problem in the toy, not in the library.

Re: [Rev 01] RFR: 8236259: MemoryLeak in ProgressIndicator

2019-12-19 Thread David Grieve
On Thu, 19 Dec 2019 19:28:34 GMT, Florian Kirmaier wrote: >> Hi everyone, >> >> ticket: https://bugs.openjdk.java.net/browse/JDK-8236259 >> >> The fix itself is quite straight forward. >> It basically just removed the listener which causes the leak. >> >> The unit-test for the fix is a bit

Re: Re: [Rev 01] RFR: 8236259: MemoryLeak in ProgressIndicator

2019-12-19 Thread Eric Bresie
Would it just be better to run some code analysis tools on the code base to hunt for these? Like maybe sonarqube integratedas part of CI build analysis? Eric Bresie ebre...@gmail.com > On December 19, 2019 at 4:23:14 PM CST, Florian Kirmaier > wrote: > On Thu, 19 Dec 2019 19:59:07 GMT, David

RFR: JDK-8236259: MemoryLeak in ProgressIndicator - DeterminateIndicator doesn't get collected

2019-12-19 Thread Florian Kirmaier
Hi everyone, Here is my PR for the memory-leak I've reported some minutes ago. PR: https://github.com/openjdk/jfx/pull/71 Ticket: https://bugs.openjdk.java.net/browse/JDK-8236259 The fix itself is quite straight forward. It basically just removed the listener which causes the leak. The unit-test

RFR: 8236259: MemoryLeak in ProgressIndicator

2019-12-19 Thread Florian Kirmaier
Hi everyone, ticket: https://bugs.openjdk.java.net/browse/JDK-8236259 The fix itself is quite straight forward. It basically just removed the listener which causes the leak. The unit-test for the fix is a bit more complicated. I added a library JMemoryBuddy