Some comments (at last) :
> this is the big FontManager refactoring patch I already mentioned a
> couple of times. It's primary purpose is to make the font implementation
> more portable (or: portable at all), allowing alternative/derived
> implementations to be plugged in. The general architecture breaks down
> as follows:
Plugging in alternate architectures is somewhat interesting but the refactoring
priority I see is that the FontManager class which has grown too large and
needs to be "downsized" This is done, which is OK, and the most important
part of how this is done is to separate out the platform, and that's partly
handled but, not as much as I'd like. For example : SunFontManager.java :
getDefaultPlatformFont() seems like a perfect candidate for pushing
down into the platform code.
The FontConfigManager class should go with it into the X11 side of the code.
It would be nice if each of these new classes had its charter documented
at the top, in a class comment.
SunFontManager
DefaultFontManager
X11FontManager
Win32FontManager
FontConfigManager
FontUtiltities
FontManagerFactory
FontScaler
I guess previously we had an X11/Win32GE instance for API reasons, but when the
font code was moved out to the FontManager subclasses it maybe wasn't
needed to make that also have an instance?
Is DefaultFontManager really necessary?? Seems not to me.
Also a number of changes appear unrelated, eg :
make/java/nio/Makefile
make/java/nio/spp.sh
src/share/classes/com/sun/jmx/mbeanserver/DefaultMXBeanMappingFactory.java
src/share/classes/com/sun/jmx/mbeanserver/MBeanIntrospector.java
make/sun/font/mapfile-vers
The disappearance of these seems strange ..
40 Java_sun_font_FontManager_getFont2D;
41 Java_sun_font_FontManager_setFont2D;
42 Java_sun_font_FontManager_isCreatedFont;
43 Java_sun_font_FontManager_setCreatedFont;
.. ah, I see these are now using reflection In FontUtilities
Hmm. I don't think this change should be made
1) As it stands it'll get a SecurityException
2) I'm very dubious about changing the accessibility of these internal methods
Looks like a potential security hole.
3) I'd want to such a change separately anyway to test its relative performance.
src/share/classes/java/awt/Component.java, and
src/windows/classes/sun/awt/windows/WToolkit.java
This was left in for a licensee, I'd be inclined to leave it alone
unless removing it has definite benefits.
2770 // REMIND: PlatformFont flag should be obsolete soon...
2771 if (sun.font.FontManager.usePlatformFontMetrics()) {
2772 if (peer != null &&
2773 !(peer instanceof LightweightPeer)) {
2774 return peer.getFontMetrics(font);
2775 }
2776 }
2777 return sun
We don't need these and similar changes, since they aren't pre-defined constants
:
isSolaris->IS_SOLARIS, isLinux->IS_LINUX
ie all caps is just for pre-defined constants, not ones determined at run time.
Cmap.java
Please try to keep lines <=80 chars, eg line 411 here :
410 if (FontUtilities.isLogging()) {
411 FontUtilities.getLogger().warning("Cmap subtable overflows
buffer.");
412 }
413 }
BTW code conventions are at
http://java.sun.com/docs/codeconv/html/CodeConvTOC.doc.html
CompositeFont.java
57 private SunFontManager fm = null;
I don't see why every comp-font needs a ptr to this singleton.
PhysicalStrike.java
- 29 import java.awt.GraphicsEnvironment;
this isn't used
====
I'm not sure about the movement of addToPool() to TrueTypeFont.java
Theoretically its independent of the font format, and could apply
to any FileFont subclass.
also the file closer shut down hook isn't needed except for created
fonts from tmp files, so that shouldn't be started unless needed.
FontManager.java has a lot of "TODO" comments
Why is FontManagerForSGE needed?
ie why cannot SunFontManager be the class in which these are defined.
Win32FontManager :
22 // FIXME: Windows build still needs to be abstracted from
23 // SunGraphicEnvironment
24
What does that mean?
25 // please, don't reference sgEnv in any other code in this class
26 // use getGraphicsEnvironment.
27 @Deprecated
This doesn't seem an appropriate use of deprecated.
28 private static SunGraphicsEnvironment sgEnv = null;
29
-phil.
Roman Kennke wrote:
Hi,
this is the big FontManager refactoring patch I already mentioned a
couple of times. It's primary purpose is to make the font implementation
more portable (or: portable at all), allowing alternative/derived
implementations to be plugged in. The general architecture breaks down
as follows:
- FontManager: is now a relatively small interface. This is the part
that the AWT API classes (esp. java.awt.Font) talk to.
- FontManagerForSGE: A subinterface of FontManager.
SunGraphicsEnvironment uses this. If you implement a backend based on
SGE, then you also need to implement this. Otherwise you can go with
plain FontManager.
- SunFontManager: A base implementation of FontManager(ForSGE). This has
all the shared code, a lot of stuff that was previously in FontManager
has been moved there.
- X11FontManager, Win32FontManager: The platform specific stuff went
there.
- FontManagerFactory: Creates FontManager instance according to a
property or default.
- SunGraphicsEnvironment: Almost all font-related code has been moved
out of this class.
- FontUtilities: A new utility class. A couple of things from
FontManager went there, i.e. logging, access tricks (get/setFont2D()),
OS determination and general shared stuff.
For the most part, this was only moving around code, without changing
functionality. However, in some places it was necessary or seemed useful
to change some things:
- in sunFont.c we can now call the real isDisplayLocal() on the graphics
environment.
- in TrueTypeFont, the handling of the channel pool has been improved.
Before, the cleanup-hook was only initialized when somebody called
Font.createFont(), now it gets initialized whenever a channel is added
to the pool. Is slightly cleaner than before (although I guess it
doesn't matter much, since modern OSes cleanup resources quite well
anyway).
- the FontManager.usePlatformFontMetrics() for windows flag has been
removed. I don't know if this is feasible, but the comments seemed to
indicate that this was the plan anyway. Might break some obscure apps
that rely on buggy code.
These are all functional changes I can think of from the top off my
head.
The webrev for this is here:
http://kennke.org/~roman/fontmanager/webrev/
The raw patch can also be downloaded somewhere in the webrev.
I'd be happy to hear your comments soon. Note that I did only very basic
testing on Windows. It is hellish to setup a build on windows,
especially when you don't have the resources to buy the necessary
licenses... Would be nice if somebody could test the stuff on Windows a
bit more. I hope that this patch is feasible to be included in OpenJDK
mainline, or that we can make it so...
Cheers, Roman
