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

[Alexander Scherbatiy]

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.

  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
>>
>