<Swing Dev> [8] Review request for 8016833: Underlines and strikethrough not rendering correctly

[Alexander Scherbatiy]

  The fix looks good for me.

   Thanks,
   Alexandr.

On 6/21/2013 4:59 PM, anton nashatyrev wrote:
> Alexander,
>
>     here is the separate issue for background: 
> bugs.sun.com/view_bug.do?bug_id=8017266
>
>     fixed webrev is here: 
> http://cr.openjdk.java.net/~vkarnauk/8016833/jdk8/webrev.01/
>
> Thanks!
> Anton.
>
> On 21.06.2013 15:16, Alexander Scherbatiy wrote:
>> On 6/21/2013 3:00 PM, anton nashatyrev wrote:
>>> Hello, Alexander,
>>>
>>>     thanks for your suggestion, though I'm not sure drawing a 
>>> background has the same semantics as drawing underline. The latter 
>>> may be though of as a part of glyph while for background it is ok to 
>>> 'highlight' the space _reserved_ for a View. Also the taller 
>>> highlight is almost not visible to a user comparing to incorrectly 
>>> painted underline.
>>>     I'd prefer either not to fix this at all or at least not to 
>>> include the fix to this customer issue.
>>       It could be filled as a separate issue.
>>>
>>>     Had fixed the EDT issue in the test.
>>    Could you send the updated webrev?
>>
>>    Thanks,
>>    Alexandr.
>>>
>>> Thanks,
>>> Anton.
>>>
>>> On 20.06.2013 18:57, Alexander Scherbatiy wrote:
>>>> On 6/20/2013 2:41 PM, anton nashatyrev wrote:
>>>>> Hello,
>>>>>     could you please review the following fix:
>>>>>
>>>>> fix: http://cr.openjdk.java.net/~vkarnauk/8016833/jdk8/webrev.00/ 
>>>>> <http://cr.openjdk.java.net/%7Evkarnauk/8016833/jdk8/webrev.00/>
>>>>> bug: http://bugs.sun.com/view_bug.do?bug_id=8016833
>>>>>
>>>>     It seem that there is the same problem with the background color:
>>>>         Style style = comp.addStyle("underlined superscript", null);
>>>>         StyleConstants.setBackground(style, Color.BLUE);
>>>>         ...
>>>>             doc.insertString(doc.getLength(), "hello", style);
>>>>
>>>>     because paint(Graphics g, Shape a) method in GlyphView uses the 
>>>> alloc.height in the same way.
>>>>
>>>>
>>>>   Could invoke all Swing methods on EDT and remove the comments 
>>>> from the test:
>>>>
>>>>  266 //        bug8016833 b = new bug8016833();
>>>>  267 //        b.demo();
>>>>
>>>> Thanks,
>>>>    Alexandr.
>>>>
>>>>> The reason of such behavior is that the superscripted GlyphView 
>>>>> requested
>>>>> increased vertical span, since the alignment of value 1.0
>>>>> (GlyphView.getAligment(View.Y_AXIS) returns 1.0) isn't enough to 
>>>>> paint it as
>>>>> high as required.
>>>>> (BTW, for subscripted text setting the alignment is enough)
>>>>>
>>>>> GlyphView.paintTextUsingColor() calculates the Y position of the 
>>>>> underscore
>>>>> as following:
>>>>>
>>>>>   int y = alloc.y + alloc.height - (int) painter.getDescent(this);
>>>>>
>>>>> but here alloc.height means the space _reserved_ for glyphs and 
>>>>> not the
>>>>> actual boundary of the painted text. To fix that we need to 
>>>>> substitute
>>>>> alloc.height with painter.getHeight(this).
>>>>>
>>>>> The fix looks safe since the GlyphView preferred vertical span is 
>>>>> initially
>>>>> requested from the GlyphPainter.
>>>>>
>>>>> Thanks!
>>>>> Anton.
>>>>
>>>>
>>>>
>>>
>>
>>
>>
>