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

Alexandr Scherbatiy alexandr.scherbatiy at oracle.com
Mon Sep 12 12:19:44 UTC 2016


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/20160912/3fca4add/attachment.html>


More information about the swing-dev mailing list