Re: RFR: 8252446: Screen.getScreens() is empty sometimes
On Wed, 16 Sep 2020 18:31:51 GMT, Pankaj Bansal wrote: >> As noted in the bug report, we get a pair of change events every time the >> list of screens changes. First, a change is >> sent with an empty list of screens and then a change is sent with the new >> list of screens. This happens whenever a >> monitor is plugged in or unplugged. It also happens on Mac at application >> startup. As noted in the bug the reason for >> this is because the `updateConfiguration` method makes two separate calls on >> the list of screens, `clear` and `addAll`, >> rather than calling `setAll`. The latter ensures that only a single change >> event is delivered. I verified that before >> this fix, the example program attached to the bug works correctly after the >> fix. >> I wrote a unit test. It ends up being skipped on Windows and Linux since we >> don't get an initial change event. On Mac >> the test fails without the fix and passes with the fix. > > I tried this on Mac and Ubuntu 20.04. I could not reproduce the issue without > the fix and test passes with/without the > fix. But the changes make sense, so approving the changes. > Reviewers: > ... > Pankaj Bansal (@pankaj-bansal - no project role) This is a case where the Skara `jcheck` bot is more restrictive than `hg jcheck` was. What it means is that even though Pankaj is a "R"eviewer in another project, and has several commits in the `jfx` project, the Skara tooling doesn't consider him as an Author in the `openjfx` project, so doesn't count that review towards the required 2. Given that Pankaj has several `jfx` commits, I will reflect the intent of that by lowering the requirement to 1 reviewer + 1 contributor to satisfy the tooling. - PR: https://git.openjdk.java.net/jfx/pull/295
Re: RFR: 8252446: Screen.getScreens() is empty sometimes
On Thu, 3 Sep 2020 15:18:06 GMT, Kevin Rushforth wrote: > As noted in the bug report, we get a pair of change events every time the > list of screens changes. First, a change is > sent with an empty list of screens and then a change is sent with the new > list of screens. This happens whenever a > monitor is plugged in or unplugged. It also happens on Mac at application > startup. As noted in the bug the reason for > this is because the `updateConfiguration` method makes two separate calls on > the list of screens, `clear` and `addAll`, > rather than calling `setAll`. The latter ensures that only a single change > event is delivered. I verified that before > this fix, the example program attached to the bug works correctly after the > fix. > I wrote a unit test. It ends up being skipped on Windows and Linux since we > don't get an initial change event. On Mac > the test fails without the fix and passes with the fix. I tried this on Mac and Ubuntu 20.04. I could not reproduce the issue without the fix and test passes with/without the fix. But the changes make sense, so approving the changes. - Marked as reviewed by pbansal (no project role). PR: https://git.openjdk.java.net/jfx/pull/295
Re: RFR: 8252446: Screen.getScreens() is empty sometimes
On Thu, 3 Sep 2020 15:18:06 GMT, Kevin Rushforth wrote: > As noted in the bug report, we get a pair of change events every time the > list of screens changes. First, a change is > sent with an empty list of screens and then a change is sent with the new > list of screens. This happens whenever a > monitor is plugged in or unplugged. It also happens on Mac at application > startup. As noted in the bug the reason for > this is because the `updateConfiguration` method makes two separate calls on > the list of screens, `clear` and `addAll`, > rather than calling `setAll`. The latter ensures that only a single change > event is delivered. I verified that before > this fix, the example program attached to the bug works correctly after the > fix. > I wrote a unit test. It ends up being skipped on Windows and Linux since we > don't get an initial change event. On Mac > the test fails without the fix and passes with the fix. Looks good to me. As you mentioned in the comments, On my Mac the test does not fail without this change. - Marked as reviewed by arapte (Reviewer). PR: https://git.openjdk.java.net/jfx/pull/295
