<Swing Dev> [9] Review request for 8132119 Provide public API for text related methods in SwingUtilities2

[Sergey Bylokhov]

Just a small clarification. In general implementations of these methods 
can make some "magic", but the purpose of the change is to provide to 
the user this "magic", I mean the user's components will be able to 
apply this "magic" to the custom components(or change behavior of 
standard l&f). So in the latest webrev some of implementations details 
of these methods were hidden.
I am not sure we express this goal successfully or not.

On 31.03.16 22:23, Phil Race wrote:
> Maybe the test to pass in that case is whether the default implementation
> is going to be satisfactory for 90% of uses. If they are frequently
> over-ridden
> it would not be an appropriate use.
>
> Based on that criterion I think it is OK to use here.
>
> Another thought:
> When you add default implementations you should also be on the hook
> for explaining what that does. It is a deeper contract than you would
> otherwise have as an interface and maybe needs to be an @implNote
> or you need to call out the default implementation.
> ie there is what someone must code to satisfy the contract of the
> interface and what is a behaviour in the default method ?
>
>
> Also here is a link to some comments by Brian Goetz on default methods :
>
> http://stackoverflow.com/questions/28681737/java-8-default-methods-as-traits-safe/28684917#28684917
>
>
>    75      * No character is underlined if the index is negative or greater
>    76      * than the string length {@code (index < 0 || index >=
> string.length())}
>    77      * or if the char value specified at the given index
>    78      * is in the low-surrogate range.
>
>
> I suppose if you point at the last char and it is a hi-surrogate nothing
> is underlined in that case either.
>
> But I find the whole writing of this a bit inadequate as if you are
> going to
> this kind of detail you perhaps also need to say what happens in a complex
> script where what happens is two unicode characters end up as a ligature,
> and/or perhaps you aren't even pointing to a base character.
> Maybe it is in fact over-specified. I see that the implementation draws
> the underline itself rather than delegate to TextLayout. This might
> make sense for performance reasons where it is simple text but some day
> this maybe should be re-examined and so I would not over-specify it.
>
> How about :
> "The underline will be positioned at the base glyph which
> represents the valid char indicated by the index.
> If the char index is not valid or is not the index of a
> valid unicode code point then no underline is drawn"
>
> -phil.
>
>
>
> On 03/24/2016 07:22 AM, Sergey Bylokhov wrote:
>> On 24.03.16 16:52, Alexander Scherbatiy wrote:
>>> On 24/03/16 10:36, Semyon Sadetsky wrote:
>>>> Hi Alexander,
>>>>
>>>> Could you answer one question:
>>>> Why did you choose default interface methods to implement
>>>> TextUIDrawing and not implement them in DefaultTextUIDrawing having
>>>> declarations only in the interface?
>>>> AFAIK the common point of view is default methods should be used
>>>> rarely because they may lead to unreadable code.
>>>>
>>>    The only problem which I know about default methods is multiple
>>> inheritance which has its own solution.
>>
>> What kind of problems? The benefit is obvious: it will not be
>> necessary to implement all methods if only one of them should be tweaked.
>>
>>>
>>>    Could you give links to discussion or provide use cases where default
>>> methods leads to the unreadable code and show how does it relate to the
>>> TextUIDrawing  implementation?
>>>
>>>    Thanks,
>>>    Alexandr.
>>>> --Semyon
>>>>
>>>> On 3/18/2016 6:49 PM, Alexander Scherbatiy wrote:
>>>>>
>>>>> Could you review the updated fix:
>>>>>   http://cr.openjdk.java.net/~alexsch/8132119/webrev.08/
>>>>>
>>>>>   - Public TextUIDrawing interface is added to the javax.swing.plaf
>>>>> package
>>>>>   - TextUIDrawing methods description does not mention component
>>>>> properties to be more general
>>>>>   - TextUIDrawing methods are made default
>>>>>   - L&F sets an instance of the TextUIDrawing to look and feel
>>>>> defaults using "uiDrawing.text" property
>>>>>   - ComponentUI class is not changed
>>>>>   - Each ComponentUI reads TextUIDrawing from UI defaults
>>>>>   - There is an interesting issue described in
>>>>> http://mail.openjdk.java.net/pipermail/swing-dev/2016-March/005509.html
>>>>>
>>>>>     which is related to the fact that MetalLabelUI returns a static
>>>>> field from createUI() method.
>>>>>     TitleBorder creates a JLabel but does not put it to any component
>>>>> hierarchy. In this case SwingUtilities.updateComponentTreeUI() method
>>>>> calls MetalLabelUI.uninstallDefaults() on the static metalLabelUI
>>>>> field and sets a new LabelUI for ordinary labels. The TitleBorder
>>>>> label UI is not changed in this case and it still uses the
>>>>> metalLabelUI field which is not initialized.
>>>>>     It seems that other applications can also use components just for
>>>>> drawing and have the same issue.
>>>>>     For this case the textUIDrawing field is not cleared in the
>>>>> uninstallDefaults but just set to a static default value which should
>>>>> not lead to memory leaks.
>>>>>
>>>>>   Thanks,
>>>>>   Alexandr.
>>>>>
>>>>> On 29/01/16 19:51, Alexander Scherbatiy wrote:
>>>>>> On 25/01/16 13:44, Andrej Golovnin wrote:
>>>>>>> Hi Alexandr,
>>>>>>>
>>>>>>>> Could you review the updated fix:
>>>>>>>> http://cr.openjdk.java.net/~alexsch/8132119/webrev.07/
>>>>>>> ....
>>>>>>>> - public TextUIDrawing interface is added to the javax.swing.plaf
>>>>>>>> package
>>>>>>>> - public "TextUIDrawing getTextUIDrawing()" method is added to the
>>>>>>>> ComponentUI class
>>>>>>>> - L&F sets an instance of the TextUIDrawing to look and feel
>>>>>>>> defaults using
>>>>>>>> "uiDrawing.text" property
>>>>>>>> - Look and Feel delegates use the instance of the TextUIDrawing
>>>>>>>> for text
>>>>>>>> drawing and measuring
>>>>>>> Some thoughts on the current design/implementation:
>>>>>>>
>>>>>>> By adding a field to the ComponentUI class the current
>>>>>>> implementation increases
>>>>>>> memory consumption for all Swing applications. And you get the
>>>>>>> feeling that
>>>>>>> there are different implementations of TextUIDrawing per
>>>>>>> ComponentUI instances.
>>>>>>> Personally I can't imagine to have different implementations of
>>>>>>> TextUIDrawing for
>>>>>>> a given LookAndFeel. If I would design/implement it, then I would
>>>>>>> implement it as
>>>>>>> a property of the LookAndFeel class (similar to LayoutStyle) and not
>>>>>>> the ComponentUI.
>>>>>>> Developers can use then the following code to obtain the instance of
>>>>>>> TextUIDrawing:
>>>>>>>
>>>>>>> UIManager.getLookAndFeel().getUIDrawing() // or
>>>>>>> UIManager.getLookAndFeelUIDrawing() // use this static method as a
>>>>>>> short cut for the line above.
>>>>>>   LayoutStyle keeps its instance per App context. The same is for
>>>>>> the LookAndFeel
>>>>>>   when it is got through UIManager.getLookAndFeel() call.
>>>>>>   It means that accessing an instance of a TextUIDrawing will leads
>>>>>> to a time consumption.
>>>>>>
>>>>>>   There are 3 main ways of the SwingUtilities2.drawString(...) usage:
>>>>>>   1. ComponentUI classes
>>>>>>   2. Components created in UI (like BasicInternalFrameTitlePane)
>>>>>>   3. Public utilities methods (like WindowsGraphicsUtils.paintText())
>>>>>>
>>>>>>   For the cases 1 and 2 it is possible to load and store the
>>>>>> UIDrawing instance during installUI()/updateUI() calls to decrease a
>>>>>> time access to it.
>>>>>>
>>>>>>   For the case 3 it is necessary to get LookAndFeel instance each
>>>>>> time (which is taken from an App context)
>>>>>>   or use the passed JComponent object. It requires to have a public
>>>>>> method and the associated variable for each instance of
>>>>>> JComponent/ComponentUI/... class.
>>>>>>> You can use this methods then in JDK too.
>>>>>>>
>>>>>>> And maybe rename the TextUIDrawing class to just UIDrawing and add
>>>>>>> more useful methods,
>>>>>>> e.g. a method to create a composite font, a method to convert DLUs
>>>>>>> to pixels.
>>>>>>   UIDrawing name may look like it should be used for any UI drawing,
>>>>>> not only for text ones. I am afraid that it can be misleading.
>>>>>>
>>>>>>   Thanks,
>>>>>>   Alexandr.
>>>>>>
>>>>>>>
>>>>>>> Best regards,
>>>>>>> Andrej Golovnin
>>>>>>
>>>>>
>>>>
>>>
>>
>>
>


-- 
Best regards, Sergey.