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

Tue Jan 12 19:42:39 UTC 2016

@since needs to be just "9" as of the updated verona versioning scheme.

-phil.

On 1/12/16, 4:25 AM, Semyon Sadetsky wrote:
> Hi Alexander,
>
> I see that you've added the next clarifications to the methods specs:
>
> > draws string/returns width ... using text properties and 
> anti-aliasing hints from the provided component
>
> It still seems too brief and even incorrect for getStringWidth().
>
> For drawString():
>
> For non-printing case I would write:
>
> Sets the anti-aliasing rendering hints to the Graphics object if those 
> are specified in the component properties. Then the string is drawn 
> using the provided Graphics object with the numeric shaper taken into 
> account if it is defined for the component.
>
> Please consider the printing case...
>
> For getStringWidth():
>
> Did not get which anti-aliasing hints it takes into account while 
> there is no Graphics in the params list...
> On the contrary, it does not take into account the rendering context.
> My suggestion:
>
> Returns string pixel width by the provided font metrics and according 
> to the numeric shaper if it is defined for the component.
>
> --Semyon
>
> For
> On 12/10/2015 5:06 PM, Alexander Scherbatiy wrote:
>>
>>  Hello,
>>
>>  Could you review the updated fix:
>> http://cr.openjdk.java.net/~alexsch/8132119/webrev.04/
>>
>> On 12/10/2015 2:39 PM, Alexey Ivanov wrote:
>>> Hi Alexandr,
>>>
>>> I suggest using {@code underlinedIndex} in this sentence:
>>>
>>> * The {@code underlinedIndex} parameter points to a char value 
>>> (Unicode code unit) in the
>>> * given string.
>>>
>>> in the Javadoc for drawStringUnderlineCharAt().
>>>
>>>
>>> And I suggest using "fits" instead of "can fit" in @return for 
>>> getClippedString() and rephrasing the conditions where empty string 
>>> is returned:
>>>
>>> * @return the clipped string that fits in the provided space, an empty
>>> *         string if the given string argument is {@code null} or empty.
>>
>>    The fix is updated according to the provided comments.
>>
>>    Thanks,
>>    Alexandr.
>>>
>>>
>>> Regards,
>>> Alexey
>>>
>>> On 10.12.2015 5:23, Alexander Scherbatiy wrote:
>>>> 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.
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/swing-dev/attachments/20160112/1b3e4a0b/attachment.html>