Re: [PATCH] enable Graphics2D at configure and runtime

[Tom Tromey <tromey at redhat dot com>]

>>>>> "graydon" == graydon hoare <graydon@redhat.com> writes:

graydon> I'm sure there'll be a few suggestions about how "best" to do this, so
graydon> I just picked what seemed like an unintrusive method and implemented
graydon> it, to make sure it's even possible. any suggestions on how to do it
graydon> better?

I think adding a switch like this is fine.  Would it make sense to do
the check unconditionally and enable this code if the required
libraries are found?  Or is there a situation where you would want the
Gtk peers but not the Cairo code?

Overall I think this patch is fine.  I've got a new minor comments.

graydon> +if GTK_CAIRO
graydon> +lib_gnu_java_awt_peer_gtk_la_LIBADD = $(GTK_LIBS) $(GLIB_LIBS) $(LIBART_LIBS) -lcairo -lpixman -lpangoft2-1.0

Is there a pkg-config invocation or something to determine the
library names?  I'm thinking that ideally this would be run in
configure and then we would just have another possibly-empty variable
substitution in the definition of lib_gnu_java_awt_peer_gtk_la_LIBADD.

graydon> +    useGraphics2d = System.getProperty("gnu.java.awt.peer.gtk.Graphics", 
graydon> +                                       "Graphics").equals("Graphics2D");

I think we've been documenting all our properties somewhere.  I think
it is just in the javadoc for the class.  So, please add a javadoc
comment explaining this extension.

graydon> +  /* deprecated, part of the older AWT Toolkit API. */
graydon>    protected FontPeer getFontPeer (String name, int style) 

If this is really deprecated, mark it in javadoc:

    /** @deprecated reason */

Then it will show up in compilations.

Tom