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

[Sergey Bylokhov]

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.