Request for review : FreeType embedded in OSX packages

[Scott Kovatch]

On Oct 16, 2012, at 9:02 AM, Henri Gomez <henri.gomez at gmail.com> wrote:

>> 2012/10/15 Henri Gomez <henri.gomez at gmail.com>:
>> Thanks Mike.
>> 
>> Hope it will be applied to OpenJDK 7, 8 and more so Mountain Lion
>> won't require X11 or XQuartz.
> 
> What about this patch ?
> 

Does the JDK still build properly if you installed X11 and do not have your own copy of libfreetype?  I think that having the option of building with a non-system-provided libfreetype.dylib is a good idea, but the main nightly builds will continue to use X11 or XQuartz' library for the time being.

To that end, a few comments. In:

+++ jdk/make/common/Defs.gmk	2012-10-16 09:36:11.000000000 +0200
@@ -180,7 +180,7 @@
   ifdef ALT_FREETYPE_LIB_PATH
     FREETYPE_LIB_PATH = $(ALT_FREETYPE_LIB_PATH)
     ifeq ($(PLATFORM), macosx)
-      USING_SYSTEM_FT_LIB=true
+      USING_SYSTEM_FT_LIB=false
     endif
--------

you could just get rid of the entire ifeq block, since USING_SYSTEM_FT_LIB was initialized to false.

In jdk/make/sun/font/Makefile:
--------
-    FREETYPE_LIB = $(LIB_LOCATION)/$(LIB_PREFIX)freetype.$(LIBRARY_SUFFIX).6
+    ifeq ($(PLATFORM), macosx)
+      FREETYPE_LIB = $(LIB_LOCATION)/$(LIB_PREFIX)freetype.$(LIBRARY_SUFFIX)
+      OTHER_LDLIBS += -lfreetype
+    else
+      FREETYPE_LIB = $(LIB_LOCATION)/$(LIB_PREFIX)freetype.$(LIBRARY_SUFFIX).6
+      OTHER_LDLIBS += -L$(FREETYPE_LIB_PATH) -lfreetype
+    endif
   endif
-  OTHER_LDLIBS += -L$(FREETYPE_LIB_PATH) -lfreetype
---------

I don't think you want to remove '-L$(FREETYPE_LIB_PATH)' in the macosx case. If it was overridden by ALT_FREETYPE_LIB_PATH the build won't be able to find the pre-build libfreetype.dylib. And if you still want to use the system-provided libfreetype you have to give it the path to the X11 libraries.

> Will it be included ?
> 
> For informations I also worked on patchs for :
> 
> * jdk8 : http://openjdk-osx-build.googlecode.com/svn/trunk/patches-jdk8-osx/freetype-osx.patch
> * lambda : http://openjdk-osx-build.googlecode.com/svn/trunk/patches-lambda-osx/freetype-osx.patch
> * jigsaw : http://openjdk-osx-build.googlecode.com/svn/trunk/patches-jigsaw-osx/freetype-osx.patch
> 
> FYI, openjdk-osx-build project produce right now all of its packages
> with FreeType embedded.

You need to create a bug at <http://bugreport.sun.com/bugreport/> describing what you want to change, and then attach a new patch. Mike's review plus one other will be enough to get it into JDK 8. JDK 7u will need two reviewers, since I think the Java2D team owns this part of the code. (I cc'd Phil Race).

-- Scott K.

----------------------------------------
Scott Kovatch
scott.kovatch at oracle.com
Santa Clara/Pleasanton, CA