8167268: StandardGlyphVector.getGlyphMetrics creates metrics with erroneous bounds for characters with no outline (e.g., the space character ' ') #27580
Conversation
|
👋 Welcome back dgredler! A progress list of the required criteria for merging this PR into |
|
❗ This change is not yet ready to be integrated. |
Webrevs
|
| } | ||
| } | ||
|
|
||
| private static void assertEqual(double d1, double d2, double variance, String scenario, int index) { |
There was a problem hiding this comment.
please split the long line to have 80 chars
There was a problem hiding this comment.
Done, let me know if that looks good.
| vb.getMinY() - pt.getY(), | ||
| vb.getWidth(), | ||
| vb.getHeight()); | ||
| if (!vb.isEmpty()) { |
There was a problem hiding this comment.
Just to double check: do we want to skip glyphs only when the bounds are empty, or we also want to skip them when the bounds are flipped (negative width/height)?
There was a problem hiding this comment.
Do you know when that might happen? This code gets its values (after a few layers of abstraction) from StandardGlyphVector$GlyphStrike.getGlyphOutlineBounds(int, float, float), which has a similar isEmpty check.
There was a problem hiding this comment.
I am not sure if it is possible, but I would like to make sure we did not introduce any issues, since isEmpty() will skip ‘flipped’ bounds.
GlyphMetricsobjects returned byStandardGlyphVector.getGlyphMetrics(int)contain bounds that are calculated by taking the glyph’s visual bounds and normalizing them by subtracting the glyph’s position.However, some glyphs (e.g., the glyph for the space character) do not have visual bounds. Their outline is an empty shape. In such a case the bounds in the metrics should not be normalized by the glyph’s position. The code erroneously ignores this special case, thus producing bounds with inconsistent negative x-positions.
Progress
Issue
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/27580/head:pull/27580$ git checkout pull/27580Update a local copy of the PR:
$ git checkout pull/27580$ git pull https://git.openjdk.org/jdk.git pull/27580/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 27580View PR using the GUI difftool:
$ git pr show -t 27580Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/27580.diff
Using Webrev
Link to Webrev Comment