Re: [8] Review request for 8023392: Swing text components printed with spaces between chars

[Alexander Potochkin]

Hello Anton

Looks good

alexp

Hello,

here is the new webrev (including manual regression test and proposed changes): http://cr.openjdk.java.net/~dmarkov/8023392/webrev.01/ <http://cr.openjdk.java.net/%7Edmarkov/8023392/webrev.01/>. Could you please review it?

Thanks!
Anton.

On 26.08.2013 17:27, Alexander Scherbatiy wrote:

On 8/26/2013 4:41 PM, anton nashatyrev wrote:

Hello Alexander,

On 26.08.2013 16:05, Alexander Scherbatiy wrote:

I see that SwingUtilities2.drawString(..) and SwingUtilities2.drawChars(...) are used a lot in the Swing but the issue is only about printing. Could removing trailing spaces in these methods affect other non-printed components?

All the changes are made under isPrinting() condition, so the behavior when rendering on screen/buffer should remain the same.

    I see now.

  Is it possible to add a test for the fix?

Sure, but I believe only manual is suitable for this case.
I will add one...

I would suggest to rename the 'AttributedCharacterIterator trimTrailingSpaces(AttributedCharacterIterator iterator)' method to something like getTrimmedTrailingSpacesIterator.

Could you also split long lines added in the fix so they fit to a page?

   Otherwise the fix looks good for me.

   Thanks,
   Alexandr.

Thanks!
Anton.

  Thanks,
  Alexandr.


On 8/22/2013 3:57 PM, anton nashatyrev wrote:

Hello,
    could you please review the following fix:

fix: http://cr.openjdk.java.net/~vkarnauk/8023392/jdk8/webrev.00/ <http://cr.openjdk.java.net/%7Evkarnauk/8023392/jdk8/webrev.00/> bug: http://bugs.sun.com/view_bug.do?bug_id=80223392 (the bug is still not replicated here, so internal link might be used if available)

The problem description: While printing the text output is handled a bit differently: the text layout is justified for the text width (in theory on the screen this text should look identically as without justification), which helps printer to output text more evenly (https://bugs.openjdk.java.net/browse/JDK-6488219). But the problem is the TextLayout.getJustifiedLayout() trim the trailing spaces from the string before laying it out. In the case when the string contains trailing spaces it is justified for the width greater than required.

The fix description: as far as we likely don't want to change TextLayout.getJustifiedLayout() behavior, we should respect that implementation feature and justify the text for the width of the string without trailing spaces. I.e. trim trailing spaces before TextLayout.getAdvance().

Thanks!
Anton.