+1
On 10/12/15 22:33, Alexey Ivanov wrote:
Hi Alexandr,
The updated code looks fine to me.
Thanks,
Alexey
On 10.12.2015 17:06, 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.
--
Best regards, Sergey.
