<Swing Dev> [9] Review request for 8132119 Provide public API for text related methods in SwingUtilities2
Alexander Scherbatiy
alexandr.scherbatiy at oracle.com
Tue Apr 26 20:31:34 UTC 2016
On 26/04/16 22:31, Phil Race wrote:
> On 04/26/2016 11:26 AM, Alexandr Scherbatiy wrote:
>> On 4/26/2016 8:38 PM, Phil Race wrote:
>>> On 04/19/2016 01:08 AM, Alexandr Scherbatiy wrote:
>>>> On 4/11/2016 4:29 PM, Philip Race wrote:
>>>>>
>>>>>
>>>>> On 4/6/16, 1:23 PM, Alexander Scherbatiy wrote:
>>>>>>
>>>>>> Hello,
>>>>>>
>>>>>> Could you review the updated fix:
>>>>>> http://cr.openjdk.java.net/~alexsch/8132119/webrev.09
>>>>>>
>>>>>> - TextUIDrawing interface and its default implementaion
>>>>>> BasicTextUIDrawing class are added
>>>>>> - font metrics argument description is updated
>>>>>>
>>>>>> On 31/03/16 23:23, Phil Race wrote:
>>>>>>> Another webrev where you have to slip past 40_ files to get to
>>>>>>> the two that really matter :-)
>>>>>>> I would have put SwingUtilities2.java and TextUIDrawing.java as
>>>>>>> the first files.
>>>>>> Updated.
>>>>>>>
>>>>>>>
>>>>>>> Some of what I have to say here is more along the lines of
>>>>>>> things to think
>>>>>>> about rather than things that are wrong .. but there are also
>>>>>>> maybe some things
>>>>>>> that need to be fixed.
>>>>>>>
>>>>>>> Is javax.swing.plaf really the right package for the new class ?
>>>>>>> I suppose it is for the use by the UI classes so maybe its right.
>>>>>>>
>>>>>>> Should the methods be taking "double" instead of "int" for
>>>>>>> location ?
>>>>>>> This means the measurement APIs too.
>>>>>>> None of the JDK 1.2 text APIs use ints. That is all 1.0 legacy.
>>>>>>> So if Swing internally wants to use ints that is OK but maybe
>>>>>>> the API
>>>>>>> should be floating point (double).
>>>>>> The provided methods use Graphics as argument which only has
>>>>>> drawString(String, int, int) method.
>>>>>> If it is possible it is better to add the methods with float
>>>>>> arguments and Graphics2D later.
>>>>>>
>>>>>>> Would that help hi-dpi at all ?
>>>>>> The hi-dpi support mostly does not require changes in Swing.
>>>>>> What it does just scales graphics using default transform from
>>>>>> graphics configuration.
>>>>>
>>>>> Yes, but in another bug you are dealing with a problem positioning
>>>>> the caret because of (somewhat) similar issues where coordinates
>>>>> have been
>>>>> rounded to an integral. A floating point value allows you to say that
>>>>> this is 25.5 in user-space, even it if is 51.0 in device space.
>>>>
>>>> It needs some more investigation.
>>>>
>>>> What I have now is Swing uses font metrics to calculate a string
>>>> width (FontMetrics.charsWidth(...)) which sums up float char values.
>>>> The difference between font metrics used by Swing and font
>>>> metrics from graphics passed to paint method is that the fist has
>>>> null frc.tx matrix and the second one has a matrix with scales 2 on
>>>> HiDPI display.
>>>> The returned char width by the font metrics with null transform
>>>> has value 7 for char 'a' (linear advance is 6.67 and xAdvance is 7).
>>>> The char width for the font metrics with scaled transform is 6.5
>>>> for the same font and char. FileFontStrike requests glyph metrics
>>>> and gets linear advance 13.35 (dev transform is taken into
>>>> account) xAdvance 13 - and apply the reverse transform. The
>>>> result is 13 / 2 = 6.5.
>>>>
>>>> And this bothers me because a result for applying the tx
>>>> transform and inverting it is different than just use the identity
>>>> transform. There are definitely problems with advance rounding but
>>>> it seems they are placed out of the Swing area.
>>>>
>>>
>>> I am not sure if you are implying a bug in the font code, but there
>>> is none that I see
>>> from the above.
>>>
>>> There are several distinct issues here
>>> 1) You must specify the device transform, unless you are requesting
>>> and using linear advances.
>>> 2) Linear advance is not generally used by Swing since it implies
>>> unhinted text
>>> 3) Rounding of advances back to user space is OK in the case of
>>> hinted advances and
>>> identity transform - ie the traditional Swing case - but in the
>>> case like that you describe
>>> where the device pixel advance is 13 for a (2.0,2.0) device
>>> scale then the translation
>>> back to user space can't express that accurately if it only has
>>> int to work with,
>>>
>>> If Swing is not using the same device transform in calculating the
>>> advance as it is when
>>> drawing then that is a bug . That was the point of the comment we
>>> added below about
>>> obtaining the correct FontMetrics .. our L&Fs should be doing that
>>> as well as admonishing
>>> others to do so.
>>
>> It is true that Swing uses FontRenderingContext which does not
>> take the graphics configuration transform into account.
>>
>> However, there are use case which should be considered:
>>
>> Using graphics configuration transform in Swing
>> FontRenderingContext leads that text component size will depend on
>> the graphics device.
>>
>> Let's take an a frame which has some text components placed in
>> one row. Moving a frame from a non-HiDPI display to HiDPI display can
>> lead that the size of text components can be changed and they will be
>> rearranged in two rows.
>> The same can be applicable for printing. Printing a text
>> component from HiDPI display can lead that text will be shorter or
>> longer than component bounds on a page.
>
> Indeed .. and there have been bugs on this where Swing text is clipped
> when printed.
> Sadly I don't believe the bug tail from that has been completely
> squashed.
>
>>
>> There is one more case. A text component before it is added to a
>> frame does not know about device where it will be shown.
>> So the following is possible:
>> -----------------
>> JTextField textField = new JTextField("ABC");
>> int width1 = textField.getWidth(); // no graphics
>> configuration is provided, frc Tx is identity
>> frame.add(textField);
>> int width2 = textField.getWidth(); // frc Tx is 2 on HiDPI
>> display
>> -----------------
>> Now it is possible that width1 is not equal to width2.
>
> Absolutely true. And it is something Swing needs to handle.
Even more, it is not will be possible to rely on the fact that width
of number of chars is just sum of each char width.
For example, on graphics configuration with transform 2 the glyph
xAdvance can be 13 and the char width in user space will be calculated
as 13 / 2 = 6.5.
FontMetrics. charWidth('a') will return 7 and charsWidth(array of 10
'a' ) will be 65 wich is not 7 * 10.
Thanks,
Alexandr.
>
> -phil.
>
>>
>> Thanks,
>> Alexandr.
>>
>>>
>>>
>>>>>
>>>>>>
>>>>>>> I suppose it would add over-head since all the existing code
>>>>>>> uses int
>>>>>>> and we are no worse off and can add double methods later if we
>>>>>>> want to.
>>>>>>>
>>>>>>>
>>>>>>> Regarding FontMetrics we need to add a caution that is must be a
>>>>>>> FontMetrics
>>>>>>> *obtained from the correct font and graphics*.
>>>>>> Updated.
>>>>>>>
>>>>>>>
>>>>>>> i.e what about attributes on the font such as "tracking" ?
>>>>>>> or on the graphics such as FRACTIONALMETRICS
>>>>>>> It looks like Swing might already fail if that were used.
>>>>>>>
>>>>>>> Look at this code :-
>>>>>>>
>>>>>>> public static int stringWidth(JComponent c, FontMetrics fm,
>>>>>>> String string){
>>>>>>> if (string == null || string.equals("")) {
>>>>>>> return 0;
>>>>>>> }
>>>>>>> boolean needsTextLayout = ((c != null) &&
>>>>>>> (c.getClientProperty(TextAttribute.NUMERIC_SHAPING) != null));
>>>>>>> if (needsTextLayout) {
>>>>>>> synchronized(charsBufferLock) {
>>>>>>> int length = syncCharsBuffer(string);
>>>>>>> needsTextLayout = isComplexLayout(charsBuffer,
>>>>>>> 0, length);
>>>>>>> }
>>>>>>> }
>>>>>>> if (needsTextLayout) {
>>>>>>> TextLayout layout = createTextLayout(c, string,
>>>>>>> fm.getFont(),
>>>>>>> fm.getFontRenderContext());
>>>>>>> return (int) layout.getAdvance();
>>>>>>> } else {
>>>>>>> return fm.stringWidth(string);
>>>>>>> }
>>>>>>> }
>>>>>>>
>>>>>>> The only thing Swing is looking at is one TextAttribute and
>>>>>>> whether we have complex text.
>>>>>>> That is not enough. This is an existing implementation issue but
>>>>>>> one we should fix here.
>>>>>>> You need to examine all the methods for similar issues.
>>>>>> I created an enhancement for this:
>>>>>> JDK-8153662
>>>>>> SwingUtilities2.drawString()/getStringWidth()/clipString() should
>>>>>> use more text attributes
>>>>>> https://bugs.openjdk.java.net/browse/JDK-8153662
>>>>>
>>>>> you mean bug ? I upgraded it to P3 because it matters a lot more now
>>>>> with this public API
>>>>
>>>> I do not think that it is a bug because the main request was to
>>>> have methods which draw strings in the same way as it is done by
>>>> Swing L&Fs.
>>>
>>> It seems like a bug to me
>>>
>>>> This will allow to have custom UI component which mimic to the
>>>> standard L&Fs.
>>>>
>>>> It is also can be considered from the following point of view: is
>>>> the proposed request to use more text attributes more important for
>>>> standard Swing L&F or for custom L&F.
>>>> It seems is not the first case because Swing lives with the
>>>> current implementation for the long time.
>>>> For the second case we provide the public TextUIDrawing interface
>>>> which a developer can override and use any text attributes that are
>>>> necessary.
>>>>
>>> We aren't talking about a separate API to provide text attributes,
>>> we are talking about the
>>> ones that are part of the font and the implementation is not
>>> respecting.
>>>
>>> -phil.
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/swing-dev/attachments/20160427/630b4eda/attachment.html>
More information about the swing-dev
mailing list