<Swing Dev> [11] RFR: JDK-8199441: Wrong caret position in multiline text components on Windows with a screen resolution higher than 100%
Philip Race
philip.race at oracle.com
Fri Jun 8 22:23:00 UTC 2018
The oddity about using round=true at line 845 is the other case seems to
be using round == false ?
I think round==false was intended for use with fractional metrics but it
is not clear.
-phil
On 6/7/18, 4:12 AM, Prasanta Sadhukhan wrote:
> Updated webrev (as you pointed) to modify WrappedPlainView to use FP
> version
>
> http://cr.openjdk.java.net/~psadhukhan/8199441/webrev.02/
>
> I have used "round" to be true as
> public static final int getTabbedTextOffset(Segment s, FontMetrics
> metrics,
> int x0, int x,
> TabExpander e,
> int startOffset) {
> uses "round" value as true.
> I did not find any other usage of this int version of the API other
> than WrappedPlainView.
>
> static final int getTabbedTextOffset(View view, Segment s, FontMetrics
> metrics,
> int x0, int x, TabExpander e,
> int startOffset,
> int[] justificationData) {}
> is used in GLyphPainter1#viewToModel but it was not needed for this
> particular issue so I have not changed it for now.
>
> Regards
> Prasanta
> On 6/7/2018 3:33 AM, Phil Race wrote:
>> This is changing the internals of getTabbedTextOffset and using FP
>> regardless
>> of whether useFPAPI is true. This is not what I said.
>>
>> I wrote that you should call the FP version directly.
>>
>> Look at this
>> ---
>> * @deprecated replaced by
>> * {@link #getTabbedTextOffset(Segment, FontMetrics, float,
>> float,
>> * TabExpander, int, boolean)}
>> */
>> @Deprecated(since = "9")
>> public static final int getTabbedTextOffset(Segment s,
>> FontMetrics metrics,
>> int x0, int x,
>> TabExpander e,
>> int startOffset) {
>> return getTabbedTextOffset(s, metrics, x0, x, e, startOffset,
>> true);
>> ----
>> If we had wanted what you proposed in the first webrev and I think
>> this one is effectively same, we'd have altered the behaviour to and not
>> deprecated and added new API
>>
>> So whatever code in our L&F is currently calling
>>
>> getTabbedTextOffset(Segment s,
>> FontMetrics metrics,
>> int x0, int x, TabExpander e,
>> int startOffset)
>>
>> should instead call
>>
>> getTabbedTextOffset(Segment s,
>> FontMetrics metrics,
>> float x0, float x, TabExpander e,
>> int startOffset,
>> boolean round)
>>
>> I am not sure if round should be true or false but calling the float
>> version is the key.
>>
>> This (and no other change needed) does the trick for me :
>>
>> ---
>> a/src/java.desktop/share/classes/javax/swing/text/WrappedPlainView.java
>> +++
>> b/src/java.desktop/share/classes/javax/swing/text/WrappedPlainView.java
>> @@ -360,11 +360,11 @@
>> int currentWidth = getWidth();
>> if (wordWrap) {
>> p = p0 + Utilities.getBreakLocation(segment, metrics,
>> - tabBase, tabBase +
>> currentWidth,
>> + (float)tabBase,
>> (float)(tabBase + currentWidth),
>> this, p0);
>> } else {
>> p = p0 + Utilities.getTabbedTextOffset(segment, metrics,
>> - tabBase, tabBase
>> + currentWidth,
>> + (float)tabBase, (float)(tabBase + currentWidth),
>> this, p0, false);
>> }
>> SegmentCache.releaseSharedSegment(segment);
>> @@ -847,8 +847,8 @@
>> Segment segment =
>> SegmentCache.getSharedSegment();
>> loadText(segment, p0, p1);
>> int n =
>> Utilities.getTabbedTextOffset(segment, metrics,
>> - alloc.x, x,
>> - WrappedPlainView.this, p0);
>> + (float)(alloc.x), (float)x,
>> + WrappedPlainView.this, p0, false);
>> SegmentCache.releaseSharedSegment(segment);
>> return Math.min(p0 + n, p1 - 1);
>> }
>>
>> I may have missed something else but to verify I downloaded and used
>> your test, which BTW has DOS line-endings !
>>
>> -phil.
>>
>> On 06/06/2018 12:04 AM, Prasanta Sadhukhan wrote:
>>>
>>>
>>> On 5/9/2018 12:41 AM, Phil Race wrote:
>>>> I am not sure this is the right fix.
>>>> If the intention had been to change all calls to
>>>> getTabbedTextOffset() to use the FP
>>>> API it would have just been changed .. rather than adding a new API
>>>> that provides
>>>> the option to specify whether to use FP.
>>>>
>>>> So perhaps the problem is that internal code needs to be updated to
>>>> call the FP
>>>> version directly ..
>>> I have updated the internal code to use the FP version directly
>>> http://cr.openjdk.java.net/~psadhukhan/8199441/webrev.01/
>>> There are no new regressions in jtreg and jck tests.
>>>
>>> Regards
>>> Prasanta
>>>> I'd like to read https://bugs.openjdk.java.net/browse/JDK-8168992
>>>> to see
>>>> what was said there but JBS just went down for 5 hours maintenance ..
>>>>
>>>> -phil.
>>>>
>>>> On 05/07/2018 11:54 PM, Prasanta Sadhukhan wrote:
>>>>> Hi Sergey,
>>>>>
>>>>> I have run javax/swing/text jtreg and jck tests and did not
>>>>> observe any regressions.
>>>>>
>>>>> Regards
>>>>> Prasanta
>>>>> On 4/28/2018 5:38 AM, Sergey Bylokhov wrote:
>>>>>> Hi, Prasanta.
>>>>>> Please confirm that you run related jck/reg tests.
>>>>>>
>>>>>> On 26/04/2018 23:52, Prasanta Sadhukhan wrote:
>>>>>>> Hi All,
>>>>>>>
>>>>>>> Please review a fix for an issue where it is seen that,
>>>>>>> with a screen resolution higher than 100% and then clicking in
>>>>>>> a JTextArea having setLineWrap(true) set, the caret (insertion
>>>>>>> point) is not aligned with the cursor.
>>>>>>>
>>>>>>> The issue seems to stem from the fact that caret position
>>>>>>> calculation in DefaultCaret class utilises API that uses integer
>>>>>>> calculation than floating point calculations.
>>>>>>> The code flow as seen is
>>>>>>> DefaultCaret#positionCaret=>BasicTextUI#viewToModel=>BoxView.viewToModel=>CompositeView#viewToModel=>WrappedPlainView#viewToModel=>Utilities.getTabbedOffset
>>>>>>>
>>>>>>> Now, getTabbedOffset utilises FontMetrics.charsWidth() which
>>>>>>> uses integer arithmetic to get the caret position.
>>>>>>> The same getTabbedOffset uses Font.getStringBounds() which uses
>>>>>>> floating point arithmetic via Rectangle2D.Float.
>>>>>>>
>>>>>>> Proposed fix is to make sure getTabbedOffset uses floating point
>>>>>>> calculations by using getStringBounds() instead of charsWidth()
>>>>>>> so that it calculates
>>>>>>> the character width(s) of text present in JTextArea in floating
>>>>>>> point to align the caret.
>>>>>>>
>>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8199441
>>>>>>> webrev: http://cr.openjdk.java.net/~psadhukhan/8199441/webrev.00/
>>>>>>>
>>>>>>> Regards
>>>>>>> Prasanta
>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>
More information about the swing-dev
mailing list