<Swing Dev> [9] Review request for 8156217 Selected text is shifted on HiDPI display
Alexandr Scherbatiy
alexandr.scherbatiy at oracle.com
Wed Sep 21 13:06:25 UTC 2016
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.
>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/swing-dev/attachments/20160921/cc8d4bb7/attachment.html>
More information about the swing-dev
mailing list