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

Prasanta Sadhukhan prasanta.sadhukhan at oracle.com
Mon Jan 28 10:22:51 UTC 2019



On 28-Jan-19 10:34 AM, Philip Race wrote:
>
>
> On 1/27/19, 8:37 PM, Prasanta Sadhukhan wrote:
>>
>>
>>
>> On 28-Jan-19 12:23 AM, Philip Race wrote:
>>> This seems to invalidate the preceding comment as well as contradict 
>>> it's intent :
>>> /* The printed text must scale linearly with the UI.
>>>   519                  * Calculate the width on screen, obtain a TextLayout with
>>>   520                  * advances for the printer graphics FRC, and then justify
>>>   521                  * it to fit in the screen width. This distributes the spacing
>>>   522                  * more evenly than directly laying out to the screen advances.
>>>   523                  */
>>>
>>>
>>> I am not sure when
>>> 531 if (!isFontRenderContextPrintCompatible(frc, deviceFRC)) { would 
>>> return "false" since we are in an isPrinting() block and you are 
>>> comparing the screen and printer FRCs, so then you will inevitably 
>>> end up calling 532 layout = createTextLayout(c, text, g2d.getFont(), 
>>> frc);
>>> which will create spacing at screen resolution.
>>> In the other two places in this file where a test similar to that at 
>>> 531 is used,
>>> if the result is that they aren't compatible, then a TextLayout is 
>>> created
>>> using the deviceFRC .. the opposite of what you are doing here.
>> In drawStringImpl(), I guess it is doing the same as I am doing 
>> (calling with screen frc if not compatible)
> so ...
>
>> if (isPrinting) {
>>                 FontRenderContext deviceFRC = g2d.getFontRenderContext();
>>                if (!isFontRenderContextPrintCompatible(frc, deviceFRC)) {
>>                     layout = new TextLayout(iterator, deviceFRC);
>
> .. why do you say / think deviceFRC is the screen frc in the line above ?
> g2d is a printer graphics, isn't it ?
>
Sorry..my bad...I was looking into this line..
float screenWidth = new TextLayout(trimmedIt, frc)..

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/

Regards
Prasanta
> -phil.
>
>>                     AttributedCharacterIterator trimmedIt =
>> getTrimmedTrailingSpacesIterator(iterator);
>>                     if (trimmedIt != null) {
>>                         float screenWidth = new TextLayout(trimmedIt, 
>> frc).
>>
>> I am not sure if the comment in l519-523 is introduced in JDK-6488219
>> (as I am not able to see the fix in JBS)
>> but since it is claimed that this bug is regression from this fix 
>> onwards, maybe this comment is not sacrosanct.
>>
>>>
>>> So I think you will regress 
>>> https://bugs.openjdk.java.net/browse/JDK-6488219
>>>
>>> Are you running Swing printing tests when you make changes here ?
>>>
>>> The bug above is covered by java/awt/print/PrinterJob/SwingUIText.java
>>> but it is manual since pass/fail is a subjective manual verification ..
>>>
>> I ran with / without fix and this looks the same to me. Attached are 
>> the output.
>>
>> Regards
>> Prasanta
>>> -phil.
>>>
>>> On 1/25/19, 3:41 AM, Prasanta Sadhukhan wrote:
>>>>
>>>> Hi All,
>>>>
>>>> Please find an updated webrev using FontRenderContext compatible to 
>>>> printer device. With this, swing text with and without leading 
>>>> spaces are printed ok.
>>>> http://cr.openjdk.java.net/~psadhukhan/8214702/webrev.1/
>>>>
>>>> Regards
>>>> Prasanta
>>>> On 25-Jan-19 11:01 AM, Prasanta Sadhukhan wrote:
>>>>>
>>>>>
>>>>>
>>>>> On 25-Jan-19 10:57 AM, Philip Race wrote:
>>>>>>
>>>>>>
>>>>>> On 1/24/19, 8:19 PM, Prasanta Sadhukhan wrote:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On 25-Jan-19 2:00 AM, Phil Race wrote:
>>>>>>>> I can't work out what you are trying to say.
>>>>>>>> The changes are only in the printing code path.
>>>>>>>>
>>>>>>>> However what it looks like to me is that Prasanta is now using 
>>>>>>>> TextLayout for printing only under
>>>>>>>> the same conditions as we use it on screen -- which is when 
>>>>>>>> "international" text is being rendered.
>>>>>>>> This will reintroduce clipped text bugs which the existing code 
>>>>>>>> was meant to fix so I don't think it
>>>>>>>> can be right.
>>>>>>> OK. Thanks for the info. I didn't know that as I am not able to 
>>>>>>> get the fix for JDK-6760148 which is pre 7u23 where mercurial 
>>>>>>> tracking began.
>>>>>>>>
>>>>>>>> It may not be pretty or a complete solution but I think we need 
>>>>>>>> to look at breaking the text into
>>>>>>>> the leading spaces + the rest of the trimmed string and using 
>>>>>>>> the screen advance of the
>>>>>>>> leading spaces to position the rest of the justified text.
>>>>>>> Are you implying the leading spaces in text is making text 
>>>>>>> justifcation going astray?
>>>>>>
>>>>>> Yes.
>>>>>>
>>>>>>> I have seen even without leading spaces in swing text in console
>>>>>>>
>>>>>>> , the text justification during printing is wrong as I am getting
>>>>>>>
>>>>>>>
>>>>>> That is quite different from what you showed earlier.
>>>>> The earlier embedded image was "with" leading spaces inswing text. 
>>>>> The above was "without"
>>>>>
>>>>> Regards
>>>>> Prasanta
>>>>>> The first visible chars are all aligned properly. But all the 
>>>>>> adjustment for the width
>>>>>> is being assigned to one space.
>>>>>> For this you will need to look into the text layout justification 
>>>>>> code.
>>>>>>
>>>>>> -phil.
>>>>>>
>>>>>>> Regards
>>>>>>> Prasanta
>>>>>>>>
>>>>>>>> -phil.
>>>>>>>>
>>>>>>>> On 1/24/19 9:33 AM, Sergey Bylokhov wrote:
>>>>>>>>> Hi, Prasanta.
>>>>>>>>>
>>>>>>>>> On 24/01/2019 02:18, Prasanta Sadhukhan wrote:
>>>>>>>>>> Proposed fix is to check if we need text layouting for 
>>>>>>>>>> printing too as it is done for swing text drawing on console. 
>>>>>>>>>> If we need text layouting, then only use textlayout 
>>>>>>>>>> justification.
>>>>>>>>>
>>>>>>>>> For swing components we skip textlayout by default because of 
>>>>>>>>> performance reason(when we know that it should be safe to do), 
>>>>>>>>> are you sure that it is safe in your case? Will the text 
>>>>>>>>> scales in the same way as the components on which it drawn?
>>>>>>>>>
>>>>>>>>> BTW the new codepath missed the call:
>>>>>>>>> g2d.setColor(((PrintColorUIResource)col).getPrintColor());
>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>
>>>>
>>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/swing-dev/attachments/20190128/f2d04c0e/attachment-0001.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: dpjfejcclknicjhh.png
Type: image/png
Size: 1991 bytes
Desc: not available
URL: <https://mail.openjdk.java.net/pipermail/swing-dev/attachments/20190128/f2d04c0e/dpjfejcclknicjhh-0001.png>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: elbjfmmkdlnpffen.png
Type: image/png
Size: 7295 bytes
Desc: not available
URL: <https://mail.openjdk.java.net/pipermail/swing-dev/attachments/20190128/f2d04c0e/elbjfmmkdlnpffen-0001.png>


More information about the swing-dev mailing list