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

[Sergey Bylokhov]

On 21.03.16 17:41, Alexander Scherbatiy wrote:
>
> On 18/03/16 19:49, 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.
>
>    I used JMH for an average time measuring of a test method which calls
> text UI drawing class from a SynthLookAndFeel method:
> ------------
>      @Benchmark
>      @BenchmarkMode(Mode.SampleTime)
>      @OutputTimeUnit(TimeUnit.MICROSECONDS)
>      public void testDrawMethod() throws Exception {
>          int N = 10000;
>          JComponent component = new JLabel(TEXT);
>          SynthGraphicsUtils synthGrapicsUtils = new SynthGraphicsUtils();
>          ...
>          for (int i = 0; i < N; i++) {
>              synthGrapicsUtils.computeStringWidth(synthContext, font,
> fontMetrics, TEXT);
>              synthGrapicsUtils.paintText(synthContext, g, TEXT, 10, 10, 0);
>          }
>      }
>    ------------
>
>    Here is the full code of the tested method:
> http://cr.openjdk.java.net/~alexsch/8132119/jmh/test.00/TextUIDrawingBenchmark.java
>
>
>   I used 3 samples with different JDK:
>   Sample 1: JDK without fixes
>   Sample 2: JDK where instance of TextUIDrawing class is placed in base
> ComponentUI class.
>   Sample 3: JDK where instance of TextUIDrawing is loaded for each
> necessary UI class from UIManager "uiDrawing.text" property.

can you please provide the second patch as well?

>
>   In the Sample 2 SynthGraphicsUtils retrieves a TextUIDrawing instance
> from the provided JComponent.getUI() class.
>   In the Sample 3 SynthGraphicsUtils gets a TextUIDrawing instance from
> UIManager property.
>
>   The calculated average time in microseconds is:
>   Sample 1: 20976.041 ± 60.181
>   Sample 2: 21556.180 ± 89.476
>   Sample 3: 23607.186 ± 62.319
>
>   If just roughly use a formula like:
>     method_time = initialization_time + loop_count *
> (time_for_same_operation + time_for different_operations)
>   it is possible to get a time estimation for different operation like:
> (method_time2 - method_time1) / loop_count
>
>   Such time difference calculation gives:
>   0.08 microseconds for Sample 2
>   0.26 microseconds for Sample 3
>
>   Thanks,
>   Alexandr.
>
>>
>>   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.