Just small comment(for a discussion), about a performance of these
methods. How much faster these methods from the case when the
TextMeasure is created from scratch? I mean that if they are much faster
and the insertChar() method works already, then probably we can support
this in deleteChar as well?
On 01/01/2018 22:53, Jayathirth D V wrote:
Hi Prahalad,
Please find my inputs:
Since we are making changes to deleteChar() code and specification I think we
should make similar changes to insertChar() to make them behave in same way. I
observed Jane Wang comment in JBS and tested inserting multiple character using
TextMeasurer.insertChar() and it doesn't through ArrayIndexOutOfBoundsException
or any other exception, it just initializes the text parameters using the
newParagraph.
We can make changes corresponding to insertChar() in the same bug or we can
raise separate bug, but its better if we make changes to insertChar() also in
the same JBS as they are similar change corresponding to identical behavior in
same Class.
Regarding JCK test fail, since we are targeting it to JDK 11 I think we have
time to update the same without any risk. But we should get input from
Phil/Others regarding the same.
In the test case may be we can remove the System.out.println("Test passed."), since we
know that if we don't get any exception/errors the jtreg test is passed. I see that some test cases
have this "Test Passed" output but I feel it is redundant.
Thanks,
Jay
-----Original Message-----
From: Prahalad Kumar Narayanan
Sent: Friday, December 29, 2017 4:30 PM
To: 2d-dev
Subject: [OpenJDK 2D-Dev] [11] RFR: [JDK-5064835] TextMeasurer/deleteChar
function fails when deleting more than one characters
Hello Everyone
Good day to you.
Request your time in reviewing the fix for the bug:
JDK-5064835 TextMeasurer/deleteChar function fails when deleting more than
one characters.
Root Cause:
. The spec clearly mentions that the concerned method is to be used to delete a
single character.
. However, the spec does not mention the outcome when the method is used to
delete multiple characters (as reported in the bug)
Solution Approaches:
. Since the spec does not mention the outcome when multiple characters are
deleted, the result is left to the implementation.
. The solution can be approached in two perspectives-
1. Update the spec to explicitly mention the exception that would be
thrown in such cases or
2. Re-initialize the TextMeasurer with the new text as present in the
argument of the method.
Solution:
. I inspected feasibility/ risk of both the approaches and I 'm of the opinion
that approach 1. is better.
. Reason is that, with the second approach-
. The re-initialization would reset all text attributes, and state
variabes that were set on TextMeasurer (and internal StyledParagraph) object.
. If re-initialization is required, one could create a new TextMeasurer
using the modified text rather than invoking deleteChar method.
. Thus in the proposed solution- I 've added a throws clause that explicitly
mentions that IllegalArgumentException will be thrown when attempted to delete
multiple characters.
Other Info:
. The fix was tested with existing jtreg test cases- No regressions were seen.
. 2 JCK tests have been found to fail. They are-
. java_awt/Font/TextMeasurer/CharTest (TestCase5)
. java_awt/Font/LineBreakMeasurer/CharTest (TestCase4) . In both the
failures, incorrect arguments are passed to deleteChar method- newParagraph (with
multiple chars deleted) & beginIndex (-1) . While the test case expects index
out of bounds exception for -ve index, the code now throws
IllegalArgumentsException.
. A minor correction to JCK test will fix the issue. I shall raise a JCK bug once
the fix is approved & submitted.
Kindly review the changes at your convenience & share your feedback:
http://cr.openjdk.java.net/~pnarayanan/5064835/webrev.00/
Note: I 've not raised a CSR for this bug yet.
Based on review, I will create the CSR for change to the specification.
Thank you for your time in review &
Happy New Year
Prahalad N.
--
Best regards, Sergey.
