<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