<Swing Dev> [9] Review request for 8156217 Selected text is shifted on HiDPI display

Alexandr Scherbatiy alexandr.scherbatiy at oracle.com
Tue Sep 20 12:18:28 UTC 2016


On 9/20/2016 1:56 PM, Semyon Sadetsky wrote:
>
> Hi Alexander,
>
> why in the TextUI class the new viewToModel2D and modelToView2D 
> methods implementations fall-back to the deprecated ones?
>
   TextUI is a public abstract class. There is no way to add a new 
abstract method to it and keep a backward compatibility with previous 
version. So new viewToModel2D/modelToView2D methods should have an 
implementation. New code which starts to use new 
TextUI.viewToModel2D()/modelToView2D() methods can get a reference to 
old TextUI class implementation which does not override new 
viewToModel2D()/modelToView2D() methods but it expects that it they 
return a meaningful result. The only way to keep compatibility with old 
classes which extend TextUI class is that new 
viewToModel2D()/modelToView2D() methods fall back to old ones.

   Thanks,
   Alexandr.

> --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.
>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/swing-dev/attachments/20160920/b93c5e64/attachment.html>


More information about the swing-dev mailing list