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.
