New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
8285217: [Android] Window's screen is not updated after native screen was disposed #778
Conversation
|
Webrevs
|
Since this fix is in shared code, have you tested this using multiple screens on all platforms? Are there new tests that could be written for this? |
You are right, the proposed change was intended only for Android but was touching shared code for all platforms. Based on the comment "Note that if a window has moved to another screen...", and considering that this could apply to the Android case (the initial screen gets destroyed and a new screen is created when app resumes), I have now another solution that does the same but that is mostly on the Android side ( |
I like this approach better. Since it just touches Monocle, and most changes are in the android port, perhaps @johanvos can review it? |
0, 0, (jint) width, (jint) height, | ||
0, 0, (jint) width, (jint) height, | ||
0, 0, (jint) width, (jint) height, | ||
100, 100, (jfloat) 1, (jfloat) 1, androidDensity, androidDensity); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
might be good to replace the 100
with e.g. dpi
and assign a constant value (100) at the top of this file to it, in case we later want to retrieve that in a more dynamic way.
This looks now more in line with what the other platforms are doing. We expect the native layer to deal with low-level window/screen handling, and they should notify the java layer in case of relevant events. |
@jperedadnr This change now passes all automated pre-integration checks. After integration, the commit message for the final commit will be:
You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed. At the time when this comment was updated there had been 9 new commits pushed to the
Please see this link for an up-to-date comparison between the source branch of this pull request and the
|
/integrate |
Going to push as commit 6c6545f.
Your commit was automatically rebased without conflicts. |
@jperedadnr Pushed as commit 6c6545f. |
This PR updates the screen for each window even for the case where the old screen has been disposed but there is a new screen instance found for such window.
This is the case of Android, where the lifecycle of the application allows destroying the native screen when the app goes to the background, and providing a new native screen, in case it comes back to the foreground. Before this PR, the screen for the window wasn't updated after returning from the background, and if orientation changes happened, the dimensions were wrong.
Progress
Issue
Reviewers
Reviewing
Using
git
Checkout this PR locally:
$ git fetch https://git.openjdk.java.net/jfx pull/778/head:pull/778
$ git checkout pull/778
Update a local copy of the PR:
$ git checkout pull/778
$ git pull https://git.openjdk.java.net/jfx pull/778/head
Using Skara CLI tools
Checkout this PR locally:
$ git pr checkout 778
View PR using the GUI difftool:
$ git pr show -t 778
Using diff file
Download this PR as a diff file:
https://git.openjdk.java.net/jfx/pull/778.diff