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

[Alexander Scherbatiy]

Hello,

Could you review the updated fix:
   http://cr.openjdk.java.net/~alexsch/8132119/webrev.03/

- methods description is updated to mention used text properties and 
anti-aliasing hints from the provided component
- the drawStringUnderlineCharAt method description is updated
- @since tag is added for the drawString() method
- the description that some parameters may/must not be null is added
- the test is updated to call the methods on EDT
- the test is updated to check passed null arguments

On 09/12/15 14:40, Alexey Ivanov wrote:
> 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).

  I updated it to "The underlined index points to a char value (Unicode 
code unit) in the given string."
  The 'refers' word was used for a value at the given index.
  However, I am not sure that the new variant is better.
>
>
> * 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()}.
  Updated.
>
>
> For consistency, please remove the full stop in @return tag in 
> description of getClippedString.

Updated.

Thanks,
Alexandr.

>
> 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.
>>>>>>
>>>>>
>>>>
>>>
>>
>