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.
