<Swing Dev> [13] RFR JDK-8214702:Wrong text position for whitespaced string in printing Swing text

[Phil Race]

The current fix confused me for a while as I could not see how it was
at all different than the existing code, since I can't imagine when we'd
ever take the "else" branch here :

533 TextLayout layout;
534 if (!isFontRenderContextPrintCompatible(frc, deviceFRC)) {
535 layout = createTextLayout(c, text, g2d.getFont(), deviceFRC);
536 } else {
537 layout = createTextLayout(c, text, g2d.getFont(), frc);
538 } Eventually when walking through it I noticed this 531 FontMetrics 
fm = g2d.getFontMetrics();
532 float screenWidth = SwingUtilities2.stringWidth(c, fm ,trimmedText);


"fm" from line 532 is getting a FontMetrics from the PRINTER - ie the 
scaled FontRenderContext.
It then uses this to calculate the advance width for such a case - ie 
the printer
but then *assigns it to a variable called screenWidth*.

Previously we were using DEFAULT_FRC to make it a screen width which except
for maybe needing to be updated for hi-dpi screens is what we want.

So in the updated proposed fix the wrong width is passed to  
getJustifiedLayout().

This may not matter here because there is plenty of space, but in other 
cases
Swing printing will be clipped as a result. And there were many, many, 
bug reports about
that. Which is why the code is laying out to the screenwidth because 
that is where the
UI component size available came from. Buttons & Label text are the 
typical cases where
this showed up.

There maybe other things to change, as well but the incorrect 
screenWidth is the
main problem I see here.

-phil.

On 2/25/19 12:05 AM, Prasanta Sadhukhan wrote:
>
>
> On 21-Feb-19 4:50 AM, Sergey Bylokhov wrote:
>> On 13/02/2019 22:53, Prasanta Sadhukhan wrote:
>>> Hi Sergey,
>>>
>>> I believe drawChars() also has same printing issue [and should be 
>>> changed like modified drawString()] but I am not able to test it as 
>>> reproducer testcase uses JLabel whose constructor can only accept 
>>> "String" and not char[] so I can only test drawString(). Using 
>>> drawChars() implementation in drawString() still reproduces the issue.
>>
>> Is it possible temporary replace the call to drawString() by the 
>> drawChars(), to check how drawChars() will work?
> As I told, it behaves similarly to unmodified drawString and the issue 
> can still be seen. I think we should commit this drawString() change 
> in this fix and I can open another bug to investigate drawChars() impl 
> and reproducer. Will that be fine?
>
> Regards
> Prasanta
>> or probably we can implement drawChars() on top of drawString()?
>>
>>>
>>> Regards
>>> Prasanta
>>> On 14-Feb-19 4:12 AM, Sergey Bylokhov wrote:
>>>> Hi, Prasanta.
>>>>
>>>>> I modified the fix to use deviceFRC if not compatible and in sync 
>>>>> with the comment which says "obtain a TextLayout with advances for 
>>>>> the printer graphics FRC"
>>>>> I used SwingUtilies2.getStringWidth() which calculates the 
>>>>> advances of the string if text layouting is used.
>>>>>
>>>>> http://cr.openjdk.java.net/~psadhukhan/8214702/webrev.2/
>>>>
>>>> Can you please take a look to the existed drawChars() method, which 
>>>> is implemented in the similar way as drawStringImpl() and new 
>>>> version of drawString(), but they have some small difference. Why 
>>>> we cannot use the same logic?
>>>>
>>>> For example in the drawChars:
>>>> =========
>>>>             FontRenderContext deviceFontRenderContext = g2d.
>>>>                 getFontRenderContext();
>>>>             FontRenderContext frc = getFontRenderContext(c);
>>>>             if (frc != null &&
>>>>                 !isFontRenderContextPrintCompatible
>>>>                 (deviceFontRenderContext, frc)) {
>>>>                  String text = new String(data, offset, length);
>>>>                  TextLayout layout = new TextLayout(text, 
>>>> g2d.getFont(),
>>>>                                deviceFontRenderContext);
>>>>                  String trimmedText = trimTrailingSpaces(text);
>>>>                  if (!trimmedText.isEmpty()) {
>>>>                      float screenWidth = (float)g2d.getFont().
>>>>                          getStringBounds(trimmedText, frc).getWidth();
>>>>                      layout = layout.getJustifiedLayout(screenWidth);
>>>>
>>>> ==========
>>>> Similar but not the same logic in the fix:
>>>>
>>>>  524                 FontRenderContext frc = getFontRenderContext(c);
>>>>  525                 if (frc.isAntiAliased() || 
>>>> frc.usesFractionalMetrics()) {
>>>>  526                     frc = new 
>>>> FontRenderContext(frc.getTransform(), false, false);
>>>>  527                 }
>>>>  528                 FontRenderContext deviceFRC = 
>>>> g2d.getFontRenderContext();
>>>>  529                 String trimmedText = trimTrailingSpaces(text);
>>>>  530                 if (!trimmedText.isEmpty()) {
>>>>  531                     FontMetrics fm = g2d.getFontMetrics();
>>>>  532                     float screenWidth = 
>>>> SwingUtilities2.stringWidth(c, fm ,trimmedText);
>>>>  533                     TextLayout layout;
>>>>  534                     if 
>>>> (!isFontRenderContextPrintCompatible(frc, deviceFRC)) {
>>>>  535                         layout = createTextLayout(c, text, 
>>>> g2d.getFont(), deviceFRC);
>>>>  536                     } else {
>>>>  537                         layout = createTextLayout(c, text, 
>>>> g2d.getFont(), frc);
>>>>  538                     }
>>>>  540                     layout = 
>>>> layout.getJustifiedLayout(screenWidth);
>>>>
>>>>
>>>>
>>>
>>
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/swing-dev/attachments/20190225/f0e51427/attachment.html>