<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