<Swing Dev> [9] Review request for 8132119 Provide public API for text related methods in SwingUtilities2

[Alexey Ivanov]

Hi Alexandr,

Shouldn't drawString() also have @since tag?


Could you please also clarify whether underlinedIndex in 
drawStringUnderlineCharAt refers to the char index in the string?

The statement
* The underlined index refers to char values (Unicode code units).
makes it unclear: underlinedIndex is an *index* or a *char* (Unicode 
code point).


* Nothing is drawn for null string. No character is underlined for the
* index {@code < 0}, {@code >=} than the string width or if the char value
* specified at the given index is in the low-surrogate range.

I think it will be better to spell comparison operators, I mean to use  
"greater than" rather than ">=". And "length" must be used instead of 
"width".

I propose the following text:

No character is underlined if the index is negative or greater than the 
string length or if the char value specified at the given index is in 
the low-surrogate range.

For the first part of condition, you can add clarification in 
parenthesis: {@code index < 0 || index >= string.length()}.


For consistency, please remove the full stop in @return tag in 
description of getClippedString.

Regards,
Alexey

On 25.11.2015 18:28, Alexandr Scherbatiy wrote:
>
>
> Hello,
>
> Could you review the updated fix:
> http://cr.openjdk.java.net/~alexsch/8132119/webrev.02
>
> The javadoc references for the #drawStringUnderlineCharAt and 
> #getClippedString methods are moved after parameters description.
>
> Thanks,
> Alexandr.
>
>
> 14.09.2015 17:39, Alexander Scherbatiy пишет:
>>
>> Hello,
>>
>> Could you review the updated fix:
>> http://cr.openjdk.java.net/~alexsch/8132119/webrev.01/
>>
>> I tried to use Utilities.drawStringUnderlineCharAt(...) with chars 
>> that have
>> - 1 character:2 glyphs mapping (U+00E1) and ligature (U+FB01)
>> The whole glyph is underlined.
>> - 2 characters: 1 glyph mapping (supplementary char U+10400)
>>
>> The char value specified the the underlined index should point to the 
>> high-surrogate range of a supplementary character.
>> I updated the javadoc for the 
>> Utilities.drawStringUnderlineCharAt(...) method to:
>> -----------------------------
>> /**
>> * Draws the given string at the specified location underlining
>> * the specified character.
>> * <p>
>> * The underlined index refers to char values (Unicode code units).
>> * If the char value specified at the given underlined index is in
>> * the high-surrogate range and the char value at the following index 
>> is in
>> * the low-surrogate range then the supplementary character corresponding
>> * to this surrogate pair is underlined.
>> * <p>
>> * Nothing is drawn for null string. No character is underlined for the
>> * index {@code < 0}, {@code >=} than the string width or if the char 
>> value
>> * specified at the given index is in the low-surrogate range.
>> -----------------------------
>>
>> Thanks,
>> Alexandr.
>>
>> On 9/7/2015 12:27 PM, Alexander Scherbatiy wrote:
>>> On 9/2/2015 8:09 PM, Phil Race wrote:
>>>> I don't remember or know how Swing resolves this but the 
>>>> measurement ones
>>>> are not reliable since they do not take a Graphics context, so you 
>>>> cannot
>>>> measure the string properly. You need a FontRenderContext to measure.
>>>
>>> The provided methods use 
>>> SwingUtilities2.getFontRenderContext(JComponent) method which 
>>> returns the FontRenderContext associated with the component.
>>>
>>>>
>>>> So as it stands these APIs do not appear suitable to be made public 
>>>> as they
>>>> are not reliable.
>>>>
>>>> Whilst I could look at the code, if I instead just look at the API, 
>>>> I am scratching my
>>>> head over :-
>>>>
>>>> public static void drawString(JComponent c, Graphics g, String 
>>>> text, int x, int y)
>>>>
>>>> Here you provide the Graphics *and* the Component.
>>>> And it says the JComponent may be null.
>>>> So I am supposing that there is optional information that may be 
>>>> pulled from the
>>>> JComponent regarding rendering mode ?
>>>
>>> The optional information provided by the component is:
>>> - java.awt.font.NumericShaper
>>> - java.awt.font.FontRenderContext
>>> - antialiasing hints
>>>
>>>>
>>>> drawStringUnderlineCharAt(..) probably needs to explain if the 
>>>> index is code point
>>>> or UTF16 char index and what happens if there is not 1:1 code 
>>>> point:glyph mapping.
>>> I will update this.
>>>>
>>>> Are we sure that (any of) these really ought/need to be public - 
>>>> particularly given the
>>>> resolution of https://bugs.openjdk.java.net/browse/JDK-6302464
>>>
>>> These methods are used by JDK L&Fs to draw text. The initial request 
>>> was to provide public methods that can be used by a custom L&F to 
>>> draw strings consistently with other L&Fs.
>>>
>>> They are also designed to properly render text for printing. To do 
>>> that they use call to internal ProxyPrintGraphics class to obtain 
>>> the print graphics context.
>>>
>>> Even if printing staff will be public, these methods are just 
>>> utility methods (in the same way as other text methods in the 
>>> javax.swing.text.Utilities class) that help easily to draw and print 
>>> text in the same way as JDK L&Fs do that.
>>>
>>> Thanks,
>>> Alexandr.
>>>
>>>>
>>>> -phil.
>>>>
>>>> On 09/02/2015 08:28 AM, Alexander Scherbatiy wrote:
>>>>>
>>>>> Hello,
>>>>>
>>>>> Could you review the fix:
>>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8132119
>>>>> webrev: http://cr.openjdk.java.net/~alexsch/8132119/webrev.00
>>>>>
>>>>> The suggested drawString, drawStringUnderlineCharAt, 
>>>>> clipStringIfNecessary, and stringWidth methods are
>>>>> added to the javax.swing.text.Utilities class.
>>>>>
>>>>> Thanks,
>>>>> Alexandr.
>>>>>
>>>>
>>>
>>
>