<Swing Dev> [11] RFR: JDK-8199441: Wrong caret position in multiline text components on Windows with a screen resolution higher than 100%
Prasanta Sadhukhan
prasanta.sadhukhan at oracle.com
Sun Jun 10 15:42:53 UTC 2018
On 6/9/2018 3:53 AM, Philip Race wrote:
> 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.
>
Seems logical.
Modified webrev
http://cr.openjdk.java.net/~psadhukhan/8199441/webrev.03/
Regards
Prasanta
> -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