openjdk-jfx
openjdk-jfx copied to clipboard
javafxports
8207932 : Wrong rendering of variation sequences
This is separated from http://mail.openjdk.java.net/pipermail/openjfx-dev/2018-June/022005.html and modified to simple patch for Win(VISTA or later) and MacOS(10.6 or later).
I checked on Windows7 and Windows10. But I could not check on VISTA and MacOSX because of no having these OS.
Especially, I take care about MacOS code version. I have no idea that following code is correct or not.
MAC_10_6_OR_LATER = MAC && versionNumberGreaterThanOrEqualTo(10.6f);
As mentioned in PR #125 we will need a signed OCA from you before we can consider this.
Also, you will need to file a new bug in JBS, since you have referenced a closed bug for another component (client-libs/2d) and we need a unique bug ID.
Finally, you have included a commit from another PR in addition to the one you did for this bug. At some point (before we start an actual review of this issue) you will need to remove it.
As a meta comment about the patch itself, there is no need for the platform check, since the current minimum platform on which we will run is Windows 7 SP1 and macOS 10.10, respectively.
@nakajima-akira Please file a new issue in JBS, via bugreport.java.com, to track this when you are ready for the review to proceed (along with removing this commit).
I just now reported to bugreport.java.com. Maybe 2-3days later Bug ID will be published. I removed old commit and pushed new commit.
SWING was fixed by using harfbuzz. (harfbuzz supports VS) https://bugs.openjdk.java.net/browse/JDK-8187100
But JavaFX is using OS native pango library on Linux. (pango does not supports VS, and maybe pango never supports)
It is better to implement HBGlyphLayout and prioritize to use harfbuzz over pango.
- modules/javafx.graphics/src/main/java/com/sun/javafx/font/freetype/HBGlyphLayout.java
But it's big changes. I can not do it.
I already made patch to process VS within JavaFX. http://mail.openjdk.java.net/pipermail/openjfx-dev/2018-June/022005.html changes
- javafx.graphics/src/main/java/com/sun/javafx/font/CMap.java
- javafx.graphics/src/main/java/com/sun/javafx/font/CharToGlyphMapper.java
- javafx.graphics/src/main/java/com/sun/javafx/font/CompositeGlyphMapper.java
- javafx.graphics/src/main/java/com/sun/javafx/font/OpenTypeGlyphMapper.java
Is it possible to apply above patch for Linux? (I will modify above patch for Linux)
It is better to implement HBGlyphLayout and prioritize to use harfbuzz over pango.
As you say, that would be a large project, and out of scope for the near term.
Is it possible to apply above patch for Linux? (I will modify above patch for Linux)
Yes, you should fix this on Linux as well, if possible.
Assigned to @prrace in JBS. He will need to review this proposed enhancement.
I pushed patch for Linux. rendering variation sequences (Linux) https://github.com/javafxports/openjdk-jfx/commit/805207ad71dac2759b6a100c05ee6446b82dba9c
Outline of patch for Linux is as follows.
- CMap.java
- implement Format14
- generate instance of CMap14 at first request of displaying VS
- CMap14 reads partial table from the font
There are not many opportunities to display VS, but VS fonts are large (over 10MB). Conventional CMap(Format 0 to 12) reads all tables at CMap instance generation. For saving memory and speed up
- Instance of CMap14 is generated when first display request of VS.
- CMap14 reads partial of table. (For example, if request is VS17, read only VS17 table from the font)
-
OpenTypeGlyphMapper.java If hiding Format14 inside CMap.java, I was worried that it will be difficult to deal with new formats coming out in the future (perhaps like Format16). In anyway, since it is necessary to write the code for VS in OpenTypeGlyphMapper side, I modified to controll cmap and cmap14 in OpenTypeGlyphMapper.
-
CharToGlyphMapper.java I delegated the judgment of surrogate pair to java.lang.Character. From the basis that IVS is part of Surrogate Pair, I modified charsToGlyphs().
-
CompositeGlyphMapper.java HashMap for VS has been added and implemented of get/put.
I pushed patch for Linux. rendering variation sequences (Linux) 805207a
The above commit is not referenced by this PR. It looks like you pushed it to a different branch than the one which this PR references. We cannot review it in its current state.
The above commit is not referenced by this PR. It looks like you pushed it to a different branch than the > one which this PR references. We cannot review it in its current state.
Sorry. I pushed to different branch, since I don't know whether it is OK to mix two different patches (for Win/Mac and for Linux). Now I pushed Linux patch to this branch. Would this be right?
Since this is all part of the same enhancement request, we would want to review your proposed changes for the three supported desktop platforms at the same time. In any case, it will be a while before we have time to look at this enhancement request.
Targeted to openjfx13, but @prrace will need to evaluate whether we want to implement it.
As announced in this message, the official jfx repository is moving from hg.openjdk.java.net/openjfx/jfx-dev/rt to github.com/openjdk/jfx.
This sandbox repository is being retired on 1-Oct-2019. You will need to migrate your pull request to the openjdk/jfx repo in order for this review to continue. Here are instructions for migrating your pull request. The updated CONTRIBUTING.md doc has additional information on how to proceed.
Once you have done this, it would be helpful to add a comment with a pointer to the new PR.
The new openjdk/jfx repo will be open for pull requests on Wednesday, 2-Oct-2019. I will send email to the openjfx-dev mailing list announcing this.