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

Philip Race philip.race at oracle.com
Tue Sep 27 20:02:08 UTC 2016


I have looked at the full webrev and it seems fine.

-phil.

On 9/27/16, 11:13 AM, Sergey Bylokhov wrote:
> 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.
>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>
>>>
>>
>
>



More information about the swing-dev mailing list