<Swing Dev> [9] Review request for 8156217 Selected text is shifted on HiDPI display
Alexandr Scherbatiy
alexandr.scherbatiy at oracle.com
Thu Jul 28 14:38:43 UTC 2016
See comments inline.
On 7/26/2016 11:57 PM, Phil Race wrote:
> I have a lot of doubts about this as well as trouble getting my head
> around all of it.
>
> Given that apps need to 'buy in' to the floating point I am not sure
> what we are gaining
> but I need to make sure I understand the problem.
>
> It affects only the methods that the 3rd party code can over-ride
> in subclasses and that are called by the JDK internal code.
>
> There are just two protected methods that matter :-
> PlainView.drawSelectedText(..)
> and
> PlainView.drawUnselectedText(..)
>
> The hidpi precison matters since they are drawing a sub-range of the text.
> Is there any other method that matters / is used in this way ?
I have found the following methods which relate to text drawing, can
be overridden and could have floating point coordinates:
javax.swing.text.PlainView.drawLine(...)
javax.swing.text.PlainView.lineToRect(...)
javax.swing.text.PasswordView.drawEchoCharacter(...)
javax.swing.plaf.TextUI.modelToView(...)
javax.swing.plaf.TextUI.viewToModel(...)
javax.swing.plaf.TextUI.getToolTipText(...)
There is also a method which relates to a caret position in a text:
javax.swing.text.DefaultCaret.setMagicCaretPosition(Point p)
This requires additional investigation because DefaultCaret extends
Rectangle and so its coordinates can't be float.
>
> Since 3rd party code is not over-riding these they will get the JDK
> super-class version, thus losing any customisation they might have done
> in the no-longer-called int version.
>
> Assuming that is correct, what customisation would be lost and how
> much does it matter?
The example is javax.swing.text.PasswordView class which overrides
drawSelectedText(...)/drawUnselectedText(...) methods and draws echo
chars instead of text.
The similar can be done in a custom component:
--------
public class CustomPasswordField extends FieldView {
@Override
protected int drawSelectedText(Graphics g, int x, int y, int p0,
int p1) throws BadLocationException {
// draw echo chars
}
}
--------
Switching to support new methods with floating point coordinates will
lead that real text will be shown for old applications in password fields.
>
> My prefernce is to deprecate the int versions and always call the
> float versions
> rather than the opt-in approach.
>
> Actually my real preference would be to come up with something that does
> not involve drawing the text in chunks like this.
>
> ie Swing should use AttributedCharacterIterator .. it looks like the
> code to
> do this might already be there !
>
> 106 private float drawElement(int lineIndex, Element elem, Graphics g,
> 107 float x, float y, boolean fractionalCharBounds)
> 108 throws BadLocationException
> 109 {
> 110 int p0 = elem.getStartOffset();
> 111 int p1 = elem.getEndOffset();
> 112 p1 = Math.min(getDocument().getLength(), p1);
> 113
> 114 if (lineIndex == 0) {
> 115 x += firstLineOffset;
> 116 }
> 117 AttributeSet attr = elem.getAttributes();
> 118 if (Utilities.isComposedTextAttributeDefined(attr)) {
> 119 g.setColor(unselected);
> 120 x = Utilities.drawComposedText(this, attr, g, x, y,
> 121 p0-elem.getStartOffset(),
> 122 p1-elem.getStartOffset());
> 123 } else {
>
> In fact what *that* illustrates is that applications already cannot expect
> their over-ridden methods to be called, so this fix is trying to fix
> something
> that can't be fixed.
The javadoc for the "protected PlainView.drawLine(...)" method is:
---------
/**
* Renders a line of text, suppressing whitespace at the end
* and expanding any tabs. This is implemented to make calls
* to the methods {@code drawUnselectedText} and
* {@code drawSelectedText} so that the way selected and
* unselected text are rendered can be customized.
---------
Applications can rely on this behaviour and stopping to call the
drawSelectedText(...)/drawUnselectedText(...) methods with int
coordinates will be incompatible change.
>
> So why can't we do that ? Just deprecate those int methods, don't add
> the float methods and use ACI ..
New float methods allow to easily migrate on new API for applications
without significant changes.
> BTW getTabSize() is supposed to be a character count isn't it ? Not a
> pixel
> count. So why does it need a float version.
Could you review the updated fix:
http://cr.openjdk.java.net/~alexsch/8156217/webrev.04
- methods with int coordinates which can be overridden are deprecated
- getFractionalTabSize() method is removed
Thanks,
Alexandr.
>
> -phil
>
> On 06/30/2016 08:50 AM, Alexandr Scherbatiy wrote:
>> On 6/28/2016 8:14 PM, Alan Snyder wrote:
>>> Suppose an application is only partially fixed to use/override the
>>> floating point methods. Perhaps it uses a library that has not been
>>> fixed.
>>>
>>> Is there a more fine grained way to detect programmer awareness or
>>> lack of awareness of the new methods?
>>
>> Here is a slightly updated version which adds public
>> isUseFloatingPointAPI()/setUseFloatingPointAPI() methods to the
>> PlainView and WrappedPlainView classes:
>> http://cr.openjdk.java.net/~alexsch/8156217/webrev.02
>>
>> Using the floating point API is disabled by default and enabled for
>> standard Swing text component classes. This has advantage that
>> selection will work for text component in users applications on HiDPI
>> display.
>>
>> But it still has the same problem. Applications which use custom
>> View classes needs to updated them to implement corresponding text
>> drawing methods with floating point arguments and enable the floating
>> point API usage.
>>
>> Thanks,
>> Alexandr.
>>
>>>
>>> Alan
>>>
>>>
>>>> On Jun 28, 2016, at 9:59 AM, Alexandr Scherbatiy
>>>> <alexandr.scherbatiy at oracle.com
>>>> <mailto:alexandr.scherbatiy at oracle.com>> wrote:
>>>>
>>>>
>>>> Hello,
>>>>
>>>> I tried to merge this fix with the 8132119 Provide public API for
>>>> text related methods in SwingUtilities2
>>>> and found a flow in the used algorithm.
>>>>
>>>> For each method that uses integer coordinates the fix adds a pair
>>>> with floating point arguments.
>>>> The fix 8156217 uses only methods with floating point values to
>>>> correctly handle a selected text.
>>>> This leads that overridden method with integer arguments in user
>>>> code is not called anymore.
>>>>
>>>> I think that this can be handled in the following way:
>>>> - Add a property that enables to use methods with floating point
>>>> arguments in Swing.
>>>> By default it is false and all work as before. The issue with
>>>> selected text is reproduced.
>>>> An application with enabled property does not have issue with
>>>> the selected text but a user should override
>>>> all methods with floating point values if he uses corresponding
>>>> methods with integer values.
>>>>
>>>> Here is a proposed solution where new public system property
>>>> "javax.swing.floatingPoints.enabled" is added:
>>>> http://cr.openjdk.java.net/~alexsch/8156217/webrev.01
>>>>
>>>> - Fix the enhancement JDK-8157461 Glyph image rendering for HiDPI
>>>> displays
>>>>
>>>> Thanks,
>>>> Alexandr.
>>>>
>>>> On 6/16/2016 6:07 PM, Alexandr Scherbatiy wrote:
>>>>> On 6/16/2016 4:47 PM, Alexandr Scherbatiy wrote:
>>>>>>
>>>>>> I tried to look deeper in the code and it seems there is a
>>>>>> rounding issue when float values are summed up.
>>>>>>
>>>>>> Suppose a transform with scale 1.5 is used and the 'a' char
>>>>>> advance is 10 in a dev space.
>>>>>> The 'a' char has advance 10 / 1.5 = 6.666666666666667 as double
>>>>>> value and 6.6666665 when it is cast to float in user space.
>>>>>> The width of a string which consists of 15 'a' chars is 15 *
>>>>>> 6.6666665 = 100.000000.
>>>>>> But the same width calculated as sum of each glyph advance in
>>>>>> StandardGlyphVector.initPositions() method is 99.999992.
>>>>>> --------------
>>>>>> double scale = 1.5;
>>>>>> float advance = (float) (10 / scale);
>>>>>> int N = 15;
>>>>>> System.out.printf("%d * %f = %f\n", N, advance, N * advance);
>>>>>> float sum = 0;
>>>>>> for (int i = 0; i < N; i++) {
>>>>>> sum += advance;
>>>>>> }
>>>>>> System.out.printf("sum: %f\n", sum);
>>>>>> --------------
>>>>>>
>>>>>> Because of this a string drawn from float position 99.999998 is
>>>>>> shifted one pixel left which affects the text selection code in
>>>>>> Swing:
>>>>>> ------------------------
>>>>>> g.scale(1.5, 1.5);
>>>>>> String TEXT = "aaaaaaaaaaaaaaaa";
>>>>>> Rectangle2D rect = font.getStringBounds(TEXT, 0, index,
>>>>>> g.getFontMetrics().getFontRenderContext());
>>>>>> float selectedTextPosition = (float) rect.getWidth();
>>>>>> // 99.999992
>>>>>> g.drawString(TEXT.substring(0, index), x, y); //
>>>>>> non-selected text
>>>>>> g.drawString(TEXT.substring(index, TEXT.length()), x +
>>>>>> selectedTextPosition, y); // selected text is shifted to one
>>>>>> pixel left
>>>>>> ------------------------
>>>>> The last step is how coordinates are scaled in
>>>>> Graphics2D.drawString() method.
>>>>> If the graphics has scale 1.5 and zero translate the
>>>>> transformed coordinates are:
>>>>> (99.999992 + 0) * 1.5 = 149.999985
>>>>> (100.000000 + 0) * 1.5 = 150.000000
>>>>>
>>>>> Both of them are rounded to the same value.
>>>>>
>>>>> If the translate is set to integer 1 value:
>>>>> (99.999992 + 1) * 1.5 = 151.499989 // shifted to one pixel left
>>>>> (100.000000 + 1) * 1.5 = 151.500000
>>>>>
>>>>> A position 99.999992 in user space is rounded to 151 in dev space.
>>>>> A position 100.000000 in user space is rounded to 152 in dev space.
>>>>>
>>>>> And this difference can depend on the translate even it has
>>>>> integer value in user space because it is multiplied on the
>>>>> graphics scale.
>>>>>
>>>>> Thanks,
>>>>> Alexandr.
>>>>>
>>>>>>
>>>>>> Thanks,
>>>>>> Alexandr.
>>>>>>
>>>>>> On 6/2/2016 11:41 PM, Alexandr Scherbatiy wrote:
>>>>>>> On 5/31/2016 10:40 PM, Phil Race wrote:
>>>>>>>>
>>>>>>>> I applied this and it is *much* better but there still seem to
>>>>>>>> be some tiny quirks.
>>>>>>>> When I drag the mouse to select text down and then up again, as
>>>>>>>> I pass the
>>>>>>>> original mouse click point vertically, repaint seem to jiggle
>>>>>>>> vertically by a pixel.
>>>>>>>> Perhaps a rounding issue in the repaint code's calculation of
>>>>>>>> the location of
>>>>>>>> the target y. I think I may see the same in left/right dragging
>>>>>>>> along a line too.
>>>>>>>> So I think this is repaint and not text related. Can you take a
>>>>>>>> look.
>>>>>>>
>>>>>>> I am able to reproduce this only using a floating point scale.
>>>>>>> It looks like 2d issue. I used a test which draws a text in
>>>>>>> two pieces. The second piece of the text is shifted from the
>>>>>>> first piece by the floating point size of the the first piece of
>>>>>>> the text.
>>>>>>> -----------
>>>>>>> Rectangle2D rect = font.getStringBounds(TEXT, 0, index,
>>>>>>> g.getFontMetrics().getFontRenderContext());
>>>>>>> float selectedTextPosition = (float) rect.getWidth();
>>>>>>> g.drawString(TEXT.substring(0, index), x, y);
>>>>>>> g.drawString(TEXT.substring(index, TEXT.length()), x +
>>>>>>> selectedTextPosition, y);
>>>>>>> -----------
>>>>>>>
>>>>>>> The second piece of the text can be shifted in the 2 cases:
>>>>>>> a) graphics scale is 1.5 and translation is 1.
>>>>>>> b) graphics scale is 2.25 without applied translation
>>>>>>>
>>>>>>> I have filed an issue on it:
>>>>>>> JDK-8158370 Text drawn from float pointing position and with
>>>>>>> float pointing scale is shifted
>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8158370
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Alexandr.
>>>>>>>
>>>>>>>>
>>>>>>>> -phil.
>>>>>>>>
>>>>>>>>
>>>>>>>> On 05/06/2016 12:31 PM, Alexandr Scherbatiy wrote:
>>>>>>>>> Hello,
>>>>>>>>>
>>>>>>>>> Could you review the fix:
>>>>>>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8156217
>>>>>>>>> webrev: http://cr.openjdk.java.net/~alexsch/8156217/webrev.00
>>>>>>>>>
>>>>>>>>> This is the second part of the fix related to the fact that
>>>>>>>>> char width can be fractional in user space.
>>>>>>>>> http://mail.openjdk.java.net/pipermail/swing-dev/2016-May/005814.html
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> The Font.getStringBounds(...) method is used for the
>>>>>>>>> fractional string width calculation by Swing in user space.
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>> Alexandr.
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/swing-dev/attachments/20160728/c3bcabdd/attachment.html>
More information about the swing-dev
mailing list