Hi Phil, Thanks for taking a quick look. Seems the webrev is in a rather chaotic state, I'll try to clean it up a bit to make review easier.
> I have a very few superficial comments from my quick look at a few files > that are *changed*. > X11SurfaceData and the glyph caching changes need a slightly longer > look. I do note we'd need to make some closed source changes to > match some of it. Yes, the glyph caching changes and the modifications to X11SurfaceData are basically the modifications I think would be best included first. Most of the other stuff hasn't even been reviewed by myself. > 1) src/share/classes/sun/font/FontManager.java > shows that this isn't synced with Roman's refactoring. > Shouldn't be a big deal though as you only change > one line and I don't think even that line should be changed > where you changed an array size from 20 to 50 : >>private static CompositeFont [] compFonts = new CompositeFont[50]; > > I can't guess how x render defines new logical fonts. Argh, I thought I did a sync with the 2d-repo. I really don't like Mercurial ;) I remember I did that change, because without it I gout ArrayIndexOutOfBoundsExceptions on my system, independent from my changes. I just tried to resize the Array without looking a lot why or what, and it worked. Sorry, I forgot about that change, its not ment to go upstream. > 2) src/share/classes/sun/java2d/pipe/AAShapePipe.java > imports sun.java2d.pisces.* > but doesn't (and shouldn't) do this directly, and > also has new CairoRenderingEngine(); //.getInstance(); > > I'm not sure what's intended here. Obviously it > shouldn't directly instantiate that here, but also > I don't even see this class in the webrev. Sorry, this is from my experiments replacing Pisces with Cairo: http://linuxhippy.blogspot.com/2009/09/ductus-vs-cairo-vs-pisces.html Because it requires a slightly modified version of cairo, which I guess would cause build-system troubles, I don't think its ready for upstream too. > 3) src/share/classes/sun/java2d/pipe/RenderQueue.java > the size change from 32000 to 12000 probably should be explained : > 75 private static final int BUFFER_SIZE = 12000; Accidential. > 4) src/share/classes/sun/java2d/pisces/PiscesRenderingEngine.java > This is just adding white space .. Sorry. > 6) src/solaris/native/sun/awt/awt_GraphicsEnv.c > > What was wrong with using the #define here ? > > 969 if (xerr->minor_code == X_ShmAttach) { > -> > 969 if (xerr->minor_code == 1) { Gave me a compile-time problem on Rawhide, the IcedTea guys said hard to fix, so I worked arround it. - Clemens
