<Swing Dev> [9] Review request for 8156217 Selected text is shifted on HiDPI display
Sergey Bylokhov
Sergey.Bylokhov at oracle.com
Tue Sep 27 18:13:10 UTC 2016
The public API Looks fine.
On 21.09.16 16:06, Alexandr Scherbatiy wrote:
>
> Hello,
>
> Could you review the updated fix:
> http://cr.openjdk.java.net/~alexsch/8156217/webrev.07/all_01
> The public API remains unchanged:
> http://cr.openjdk.java.net/~alexsch/8156217/webrev.07/public-api
>
> ParagraphView and GlyphPainter2 are updated to return Recatngel2D shape
> for ParagraphView.modelToView(...) call.
> However, the returned rectangle has int coordinates when "i18n" is set
> just because TextLayout.getCaretInfo(hit) method in class GlyphPainter2
> returns int caret info.
>
> Thanks,
> Alexandr.
>
> On 9/20/2016 2:41 PM, Semyon Sadetsky wrote:
>>
>> Yet another question: Why the floating point API doesn't work for
>> JTextPane?
>>
>> Also it doesn't work for JTextArea, JTextField and JTextPane if i18n
>> is on.
>>
>> --Semyon
>>
>>
>> On 12.09.2016 15:19, Alexandr Scherbatiy wrote:
>>>
>>> Hello,
>>>
>>> Could you review the updated fix:
>>> all changes: http://cr.openjdk.java.net/~alexsch/8156217/webrev.07/all
>>> public API changes:
>>> http://cr.openjdk.java.net/~alexsch/8156217/webrev.07/public-api
>>>
>>> - @since 9 tag is added to new methods.
>>>
>>> Thanks,
>>> Alexandr.
>>>
>>> On 9/10/2016 2:36 AM, Philip Race wrote:
>>>> - * Returns the tab size set for the document, defaulting to 8.
>>>> - *
>>>> - * @implSpec This implementation calls {@link #getTabSize()
>>>> getTabSize()}.
>>>> - *
>>>> - * @return the tab size
>>>> - */
>>>> - protected float getFractionalTabSize() {
>>>> - return getTabSize();
>>>> - }
>>>> -
>>>>
>>>>
>>>> It seems this was added only in 9.
>>>> Since I think I remember that asking a question about it.
>>>> I note it has no @since. Moot if you are really removing it but
>>>> what has it to do with the rest of this change ?
>>>>
>>>> -phil.
>>>>
>>>> On 9/9/16, 11:51 AM, Alexandr Scherbatiy wrote:
>>>>>
>>>>> Hello,
>>>>>
>>>>> Could you review the updated fix:
>>>>> all changes: http://cr.openjdk.java.net/~alexsch/8156217/webrev.06/all
>>>>> public API changes:
>>>>> http://cr.openjdk.java.net/~alexsch/8156217/webrev.06/public-api
>>>>>
>>>>> - reflection is used to detect do methods with floating point API
>>>>> need to be called.
>>>>>
>>>>> Thanks,
>>>>> Alexandr.
>>>>>
>>>>> On 9/1/2016 9:17 PM, Semyon Sadetsky wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> On 9/1/2016 8:26 PM, Alexandr Scherbatiy wrote:
>>>>>>> On 9/1/2016 7:27 PM, Alexandr Scherbatiy wrote:
>>>>>>>> On 9/1/2016 6:49 PM, Semyon Sadetsky wrote:
>>>>>>>>>
>>>>>>>>> Alexander, did you consider possibility to check if method is
>>>>>>>>> really over-riden then to use the old API?
>>>>>>>>>
>>>>>>>> Could you give a sample how it can be done?
>>>>>>> I think it is possible to use a reflection to found the latest
>>>>>>> overridden method which uses int coordinates and check does it
>>>>>>> has a corresponding overridden method with floating point
>>>>>>> arguments. But I doubt that it is a good solution.
>>>>>> yes. You could use:
>>>>>> useFloatingPointAPI =
>>>>>> PlainView.class.equals(getClass().getMethod("drawUnselectedText",
>>>>>> ...).getDeclaringClass());
>>>>>>
>>>>>> Otherwise, with high probability your new API will never be used.
>>>>>>
>>>>>> --Semyon
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Alexandr.
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Alexandr.
>>>>>>>>
>>>>>>>>> --Semyon
>>>>>>>>>
>>>>>>>>> On 9/1/2016 3:07 PM, Alexandr Scherbatiy wrote:
>>>>>>>>>> On 9/1/2016 11:31 AM, Semyon Sadetsky wrote:
>>>>>>>>>>>
>>>>>>>>>>> Hi Alexander,
>>>>>>>>>>>
>>>>>>>>>>> It is a good style to add a note recommending what to use
>>>>>>>>>>> instead of the method which is being deprecated.
>>>>>>>>>>>
>>>>>>>>>> Could you review the updated public API there "replaced by"
>>>>>>>>>> notes are added to the deprecated methods:
>>>>>>>>>>
>>>>>>>>>> http://cr.openjdk.java.net/~alexsch/8156217/webrev.05/public-api.02
>>>>>>>>>>>
>>>>>>>>>>> I did not get for what the useFloatingPointAPI property was
>>>>>>>>>>> introduced and moreover is set to false by default. If the
>>>>>>>>>>> old API is used then it doesn't matter which value it has
>>>>>>>>>>> because the float values will receive ints. And for the new
>>>>>>>>>>> API I would expect everything having the float precision, and
>>>>>>>>>>> it is unclear what may be the reason to switch it off back to
>>>>>>>>>>> integer. Especially if
>>>>>>>>>>>
>>>>>>>>>>> " This allows to draw text properly using graphics with
>>>>>>>>>>> scaled transform."
>>>>>>>>>>>
>>>>>>>>>>> so an improper mode is the default?
>>>>>>>>>>>
>>>>>>>>>> This is has been discussed below. For example new
>>>>>>>>>> drawSelectedText(Graphics2D g, float x, float y, int p0, int
>>>>>>>>>> p1) with floating point coordinates is added to the PlainView
>>>>>>>>>> which has the same method with int coordinates. Suppose
>>>>>>>>>> someone has a custom password component which uses and old
>>>>>>>>>> methods with int coordinates.
>>>>>>>>>> --------
>>>>>>>>>> public class CustomPasswordField extends FieldView {
>>>>>>>>>>
>>>>>>>>>> @Override
>>>>>>>>>> protected int drawSelectedText(Graphics g, int x, int y,
>>>>>>>>>> int p0, int p1) throws BadLocationException {
>>>>>>>>>> // draw echo chars
>>>>>>>>>> }
>>>>>>>>>> }
>>>>>>>>>> --------
>>>>>>>>>>
>>>>>>>>>> If we start to call drawSelectedText() with floating point
>>>>>>>>>> values the customization of old components will not be used
>>>>>>>>>> and the CustomPasswordField from the example starts to show
>>>>>>>>>> real text instead of echo chars. This is incompatible change
>>>>>>>>>> with previous JDK releases.
>>>>>>>>>>
>>>>>>>>>> The solution is to switch to new floating point API only when
>>>>>>>>>> it is known that a component properly overrides new methods
>>>>>>>>>> with floating point arguments. After that the
>>>>>>>>>> PlainView.useFloatingPointAPI flag can be set to true.
>>>>>>>>>>
>>>>>>>>>> For example, BasicPasswordFieldUI sets the
>>>>>>>>>> PasswordView.useFloatingPointAPI flag to true because it is
>>>>>>>>>> sure that drawSelectedText() methods with floating point
>>>>>>>>>> arguments is overridden. So Swing standard text components are
>>>>>>>>>> switched to use new floating point API.
>>>>>>>>>>
>>>>>>>>>> Thanks,
>>>>>>>>>> Alexandr.
>>>>>>>>>>
>>>>>>>>>>> --Semyon
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> On 19.08.2016 11:03, Alexandr Scherbatiy wrote:
>>>>>>>>>>>> On 8/19/2016 2:25 AM, Philip Race wrote:
>>>>>>>>>>>>> OK .. I do not know enough about how modelToView is used by
>>>>>>>>>>>>> Swing
>>>>>>>>>>>>> to know what is really needed here. Someone with a bit more
>>>>>>>>>>>>> Swing
>>>>>>>>>>>>> background needs to chime in. I was encouraged that the
>>>>>>>>>>>>> *API* surface
>>>>>>>>>>>>> of the changes was much smaller than it had seemed from the
>>>>>>>>>>>>> webrev
>>>>>>>>>>>>> but maybe that is because you did not include everything.
>>>>>>>>>>>>> For example
>>>>>>>>>>>>> although they are just subclassing the method overrides in
>>>>>>>>>>>>> PasswordView
>>>>>>>>>>>>> since that is a public class would become part of the spec
>>>>>>>>>>>>> .. would they not ?
>>>>>>>>>>>>> Just like the "int" counterparts today :-
>>>>>>>>>>>>> https://docs.oracle.com/javase/8/docs/api/javax/swing/text/PasswordView.html
>>>>>>>>>>>>>
>>>>>>>>>>>>> Put another way I was looking for what the content of the
>>>>>>>>>>>>> CCC would be.
>>>>>>>>>>>> Here is the updated version of the public API change
>>>>>>>>>>>> which includes overridden deprecated methods:
>>>>>>>>>>>>
>>>>>>>>>>>> http://cr.openjdk.java.net/~alexsch/8156217/webrev.05/public-api.01
>>>>>>>>>>>>
>>>>>>>>>>>> Thanks,
>>>>>>>>>>>> Alexandr.
>>>>>>>>>>>>>
>>>>>>>>>>>>> -phil.
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> On 8/15/16, 11:48 AM, Alexander Scherbatiy wrote:
>>>>>>>>>>>>>> On 15/08/16 21:43, Phil Race wrote:
>>>>>>>>>>>>>>> Why is the caret support added in here ? Same for the
>>>>>>>>>>>>>>> modelToView
>>>>>>>>>>>>>>> That will just hold this up as the reasoning behind
>>>>>>>>>>>>>>> needing those changes is not something
>>>>>>>>>>>>>>> I have yet been able to convince myself about - even
>>>>>>>>>>>>>>> after reading your last email.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> The main change for the Caret public API (methods
>>>>>>>>>>>>>> Caret.getMagicCaretPosition2D()/setMagicCaretPosition2D(Point2D
>>>>>>>>>>>>>> p)) is not included in the current fix. I only moved the
>>>>>>>>>>>>>> new methods JTextComponent.modelToView2D(int
>>>>>>>>>>>>>> pos)/viewToModel2D(Point2D pt) from the fix for the Caret
>>>>>>>>>>>>>> to this fix. These methods are used not only for caret but
>>>>>>>>>>>>>> in other cases like mouse handling, text dragging and others.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>> Alexandr.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> -phil.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> On 08/15/2016 04:13 AM, Alexandr Scherbatiy wrote:
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Hello,
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Could you review the updated fix?
>>>>>>>>>>>>>>>> webrev which contains only change in public API:
>>>>>>>>>>>>>>>> http://cr.openjdk.java.net/~alexsch/8156217/webrev.05/public-api
>>>>>>>>>>>>>>>> webrev with contains all changes:
>>>>>>>>>>>>>>>> http://cr.openjdk.java.net/~alexsch/8156217/webrev.05/all
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> - methods with int coordinates are deprecated
>>>>>>>>>>>>>>>> - public
>>>>>>>>>>>>>>>> isUseFloatingPointAPI()/setUseFloatingPointAPI() methods
>>>>>>>>>>>>>>>> are added to the PlainView and WrappedPlainView classes
>>>>>>>>>>>>>>>> - JTextComponent.modelToView2D(int
>>>>>>>>>>>>>>>> pos)/viewToModel2D(Point2D pt) public methods from fix
>>>>>>>>>>>>>>>> JDK-8163124 Add floating point API support to
>>>>>>>>>>>>>>>> javax.swing.text.Caret
>>>>>>>>>>>>>>>> are added
>>>>>>>>>>>>>>>> - some @implSpec descriptions are removed from the new
>>>>>>>>>>>>>>>> text drawing methods with floating point arguments
>>>>>>>>>>>>>>>> - Built-in L&Fs are updated to use floating point API
>>>>>>>>>>>>>>>> in standard Java text components
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>>>> Alexandr.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> On 7/28/2016 5:38 PM, Alexandr Scherbatiy wrote:
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> 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.
>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>
>>
>
--
Best regards, Sergey.
More information about the swing-dev
mailing list