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

[anton nashatyrev]

Hello, Sergey

     swing regression tests are all passed,
     some of swing JCK tests failed but the fail set is the same before 
and after fix.

Regards,
Anton.

On 21.06.2013 17:26, Sergey Bylokhov wrote:
> Hi, Anton.
> Can you run swing's tests from the jdk and jck and check that there is 
> no new regressions.
> Thanks.
>
> On 21.06.2013 16:59, 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.
>>>>>
>>>>>
>>>>>
>>>>
>>>
>>>
>>>
>>
>
>