-
Notifications
You must be signed in to change notification settings - Fork 506
8227425: Add support for e-paper displays on i.MX6 devices #60
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
Conversation
Hi jgneff, welcome to this OpenJDK project and thanks for contributing! We do not recognize you as Contributor and need to ensure you have signed the Oracle Contributor Agreement (OCA). If you have not signed the OCA, please follow the instructions. Please fill in your GitHub username in the "Username" field of the application. Once you have signed the OCA, please let us know by writing If you already are an OpenJDK Author, Committer or Reviewer, please click here to open a new issue so that we can record that fact. Please use "Add GitHub user jgneff" as summary for the issue. If you are contributing this work on behalf of your employer and your employer has signed the OCA, please let us know by writing |
/signed |
Thank you! Please allow for up to two weeks to process your OCA, although it is usually done within one to two business days. Also, please note that pull requests that are pending an OCA check will not usually be evaluated, so your patience is appreciated! |
Webrevs
|
/reviewers 2 |
@kevinrushforth |
@kevinrushforth Did I merge the upstream master branch correctly? The last time I did this on the old openjdk-jfx repository, the pull request showed only the single merge commit. This time all 87 commits from the upstream master branch are shown as part of this pull request, even though the code I propose has not changed since the pull request was created on December 4, 2019. Let me know if you would like me to rebase my changes and force push a cleaner update. While I have your attention ... would you be able to be my second reviewer? This pull request is a small update that completes the large project begun by JDK-8217605 over a year ago. Your comments on that first part were very helpful, so I'm hoping you can bring that experience to this second and final part of the project. |
/reviewers 2 |
@kevinrushforth |
Yes, this is what you will need to do. More than just being needed for a cleaner update, the PR lists 6,151 changed files! Either the merge went badly or it has somehow managed to confuse GitHub (my money is on the latter based on what I see in the diffs). Either way, rebasing and force-pushing your branch is the way to go. You might need to export your patch and reset your branch to master if git has trouble rebasing. |
I just checked and there is nothing wrong with your merge. But somehow it managed to confuse GitHub. |
f7028e8
to
ba668b5
Compare
Thanks, Kevin. A simple I hope it's okay for me to add some advocacy to the conversation here. My initial comment makes it easy to miss the general context of this update. This pull request completes the project of adding support for e-paper displays that we started back in JavaFX 13. I would love to see these last few changes merged in time for JavaFX 15. Below are five reasons to merge this pull request into the next release:
To my potential reviewers, @johanvos and @kevinrushforth, is there anything I can do to make your code review easier? |
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.
Looks good to me.
@jgneff This change now passes all automated pre-integration checks. When the change also fulfills all project specific requirements, type
Since the source branch of this PR was last updated there have been 26 commits pushed to the
As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid automatic rebasing, please merge As you are not a known OpenJDK Author, an existing Committer must agree to sponsor your change. Possible candidates are the reviewers of this PR (@johanvos, @kevinrushforth) but any other Committer may sponsor as well. ➡️ To flag this PR as ready for integration with the above commit message, type |
/integrate |
/sponsor |
@kevinrushforth @jgneff The following commits have been pushed to master since your change was applied:
Your commit was automatically rebased without conflicts. Pushed as commit 66c3b38. |
Update the home page that the second part of the support for e-paper displays will be in JavaFX 15.
This is just a note to follow up on that performance problem I saw in December. I think I found the cause of the problem, and it appears to be fixed in JDK 14 and 15. The chart below shows the drastic drop in performance before the fix. The following flame graph, created by async-profiler, led quickly to an existing Java bug report. See the description at JDK 13 Performance for details. Thank you for the quick fix, @voitylov! |
This pull request adds support for e-paper displays with the i.MX 6 Series of Applications Processors and implements Issue #521 in the obsolete javafxports/openjdk-jfx repository. Some of the changes were made to support the new device models, while others are minor changes to the existing support in JavaFX 13.
The following changes were made to support the new device models.
Work around problems on the Kobo Glo HD Model N437.
Ignore the error ENOTTY (25), "Inappropriate ioctl for device," when setting the waveform modes. The IOCTL call is ignored by the driver on the Kobo Glo HD where the error occurs, anyway.
Use the visible y-resolution (
yres
), not the virtual y-resolution (yres_virtual
), when calculating the capacity of the off-screen byte buffer and the length of the frame buffer mapping. The virtual y-resolution reported by the frame buffer device driver can be an incorrect value.Work around problems on the Kobo Clara HD Model N249.
The Kobo Clara HD requires the native screen width to be set to the visible x-resolution (
xres
), instead than the normal virtual x-resolution (xres_virtual
), when using an unrotated and uninverted 8-bit grayscale frame buffer. The work-around is provided through a new Boolean system property called monocle.epd.fixWidthY8UR.The following changes were made to the existing code that supports e-paper displays in JavaFX 13.
Use the correct constant for the number of bytes per pixel.
Use the number of bytes per pixel (
Integer.BYTES
), not the number of bits per pixel (Integer.SIZE
), when calculating the capacity of the 32-bit off-screen byte buffer.Do not round the luma value to the nearest integer.
Use the value of luma rounded toward zero automatically by Java when converting a
float
to anint
instead of rounding to the nearest integer. The additional rounding operation can decrease performance anywhere from zero to 10 percent and doesn't seem worth it for a display with just 16 levels of gray.Change camel case of system property y8inverted.
Change the camel case of the system property monocle.epd.y8inverted to the form monocle.epd.Y8Inverted so that it is consistent with the other system properties containing Y1, Y4, and Y8 in their names.
Improve error handling when mapping the frame buffer.
Log the mapping and unmapping of the frame buffer. Log any errors that occur on either of the system calls. Handle a
null
return value fromEPDFrameBuffer.getMappedBuffer
.Replace non-ASCII characters with their ASCII equivalent.
Replace non-ASCII characters in comments and log messages as follows:
Add descriptions to Monocle EPD system properties.
Add Javadoc comments to each constant that defines a Monocle EPD system property.
Rename
IntStructure.getInteger
toIntStructure.get
.Rename the methods
getInteger
andsetInteger
inIntStructure
to avoid confusion with the Java methodInteger.getInteger
, which gets the integer value of a system property.Below are some of the more interesting test results.
The Kobo Glo HD and Kobo Clara HD implement a true GC4 waveform mode that displays only pixels with the four gray values
0x00
,0x55
,0xAA
, and0xFF
. On the older Kobo Touch models, the GC4 waveform mode is the same as GC16.When the forceMonochrome update flag is enabled, the Kobo Clara HD uses a zero-percent threshold that displays black for pixels with the value
0x00
and white for all other values. The other models use a 50-percent threshold that displays black for values0x7F
or less and white for values0x80
or greater.The OpenJDK 11 package in Ubuntu 18.04 LTS runs twice as fast as the AdoptOpenJDK build of OpenJDK 13 for some of the tests. The speed difference is greatest for the tests that require pixel conversion into an 8-bit or 16-bit frame buffer. I plan to investigate the cause of the performance difference.
In general, the faster processor and memory bus of the newer models does not fully compensate for the threefold increase in the number of pixels in their displays. The table below shows the animation speed of each model in milliseconds per frame and frames per second.
Progress
Issue
Reviewers
Download
$ git fetch https://git.openjdk.java.net/jfx pull/60/head:pull/60
$ git checkout pull/60