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

Semyon Sadetsky semyon.sadetsky at oracle.com
Tue Jan 19 06:30:12 UTC 2016 


On 1/18/2016 5:37 PM, Sergey Bylokhov wrote:
> On 18/01/16 10:09, Semyon Sadetsky wrote:
>>
>>
>> On 1/16/2016 12:37 AM, Sergey Bylokhov wrote:
>>> On 15/01/16 19:58, Semyon Sadetsky wrote:
>>>>>> getStringWidth(): It is worth to add a note that the returning value
>>>>>> is not the exact visual string width because rendering hints are not
>>>>>> taken into account.
>>>
>>> It depends, the current description is correct. Default font metrics
>>> of the component should take into account KEY_TEXT_ANTIALIASING 
>>> property.
>>> The width of the string will be calculated by the passed font metrics.
>>> If the font metrics differs from the metrics of the graphics object
>>> then results will be different, if the metrics will be the same then
>>> results will be the same.
>>>
>>> If these methods will produce inconsistent result when we will get a
>>> bugs when the size of the component(which depends from the font) will
>>> differ from their appearance on the screen.
>> Then, this:
>> https://docs.oracle.com/javase/tutorial/2d/text/measuringtext.html is
>> incorrect?
>
> No, information in this link is correct. Note that example uses 
> FontMetrics from the graphics object, this is possible when we draw 
> the text, but in case of Swing the layout of the components occurs 
> before we try to draw a component.
>
Then the stringWidth() is not precise. We should note that in the javadoc.
>>>
>>>
>>>>>     I thought that rendering hints can affect a string quaility and
>>>>> drawing performance. Do they really change the visual string width?
>>>> Please look how the font anti-aliasing works:
>>>> https://upload.wikimedia.org/wikipedia/commons/3/34/A_aliased_and_simple.jpg 
>>>>
>>>>
>>>>
>>>> For example if user cut this width from a drawing artifacts may be
>>>> remained.
>>>>>>
>>>>>> What was the reason for renaming clipStringIfNecessary() and
>>>>>> stringWidth()?
>>>>>>
>>>>>> stringWidth() is a standard method name to get the advance in all
>>>>>> other JDK interfaces.
>>>>>     The 'get' prefix is used here not because it is JavaBeans
>>>>> requirements but because of the method name conventions.
>>>>>     For example, the same Utilities class contains
>>>>> getTabbedTextWidth() method which also has the 'get' prefix.
>>>>>
>>>>>> clipStringIfNecessary method name better reflects the method
>>>>>> operation than the proposed one.
>>>>>     The 'necessary' word is vague in this case. Is there any need to
>>>>> clip a string if it is not necessary?
>>>>>     The method description gives the explanation in which case the
>>>>> string will be clipped.
>>>>>
>>>>>  Thanks,
>>>>>  Alexandr.
>>>>>
>>>>>> This is an utility static API and we don't need to follow JavaBeans
>>>>>> naming conventions here.
>>>>>>
>>>>>> --Semyon
>>>>>>
>>>>>>
>>>>>> On 1/13/2016 6:03 PM, Alexander Scherbatiy wrote:
>>>>>>>
>>>>>>>   Hello,
>>>>>>>
>>>>>>>   Could you review the updated fix:
>>>>>>> http://cr.openjdk.java.net/~alexsch/8132119/webrev.05/
>>>>>>>
>>>>>>>   - the methods description are updated to mention a component
>>>>>>> numeric shaper and non-print graphics context
>>>>>>>   - @since tag value is updated to 9
>>>>>>>
>>>>>>>  Thanks,
>>>>>>>  Alexandr.
>>>>>>>
>>>>>>> On 1/12/2016 10:42 PM, Philip Race wrote:
>>>>>>>> @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.
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>>
>>
>
>

More information about the swing-dev mailing list