You are viewing a plain text version of this content. The canonical link for it is here.
Posted to fop-commits@xmlgraphics.apache.org by je...@apache.org on 2006/07/21 17:01:18 UTC

svn commit: r424349 - in /xmlgraphics/fop/trunk: src/java/org/apache/fop/fo/flow/ExternalGraphic.java src/java/org/apache/fop/image/ImageFactory.java status.xml

Author: jeremias
Date: Fri Jul 21 08:01:17 2006
New Revision: 424349

URL: http://svn.apache.org/viewvc?rev=424349&view=rev
Log:
Fixed two memory-leaks in image handling (ImageFactory and ExternalGraphic). The image cache is finally working properly. Currently implemented without the cleanup thread as done by Batik.
Added ImageIO provider for handling PNG in addition to the internal codec. ImageIO proved to be faster and less memory-intensive for PNGs. ImageIO takes precedence of the internal codec.

Modified:
    xmlgraphics/fop/trunk/src/java/org/apache/fop/fo/flow/ExternalGraphic.java
    xmlgraphics/fop/trunk/src/java/org/apache/fop/image/ImageFactory.java
    xmlgraphics/fop/trunk/status.xml

Modified: xmlgraphics/fop/trunk/src/java/org/apache/fop/fo/flow/ExternalGraphic.java
URL: http://svn.apache.org/viewvc/xmlgraphics/fop/trunk/src/java/org/apache/fop/fo/flow/ExternalGraphic.java?rev=424349&r1=424348&r2=424349&view=diff
==============================================================================
--- xmlgraphics/fop/trunk/src/java/org/apache/fop/fo/flow/ExternalGraphic.java (original)
+++ xmlgraphics/fop/trunk/src/java/org/apache/fop/fo/flow/ExternalGraphic.java Fri Jul 21 08:01:17 2006
@@ -41,7 +41,8 @@
 
     //Additional values
     private String url;
-    private FopImage fopimage;
+    private int intrinsicWidth;
+    private int intrinsicHeight;
     
     /**
      * Create a new External graphic node.
@@ -63,7 +64,7 @@
         url = ImageFactory.getURL(getSrc());
         FOUserAgent userAgent = getUserAgent();
         ImageFactory fact = userAgent.getFactory().getImageFactory();
-        fopimage = fact.getImage(url, userAgent);
+        FopImage fopimage = fact.getImage(url, userAgent);
         if (fopimage == null) {
             getLogger().error("Image not available: " + getSrc());
         } else {
@@ -71,6 +72,8 @@
             if (!fopimage.load(FopImage.DIMENSIONS)) {
                 getLogger().error("Cannot read image dimensions: " + getSrc());
             }
+            this.intrinsicWidth = fopimage.getIntrinsicWidth();
+            this.intrinsicHeight = fopimage.getIntrinsicHeight();
         }
         //TODO Report to caller so he can decide to throw an exception
     }
@@ -122,22 +125,14 @@
      * @see org.apache.fop.fo.flow.AbstractGraphics#getIntrinsicWidth()
      */
     public int getIntrinsicWidth() {
-        if (fopimage != null) {
-            return fopimage.getIntrinsicWidth();
-        } else {
-            return 0;
-        }
+        return this.intrinsicWidth;
     }
 
     /**
      * @see org.apache.fop.fo.flow.AbstractGraphics#getIntrinsicHeight()
      */
     public int getIntrinsicHeight() {
-        if (fopimage != null) {
-            return fopimage.getIntrinsicHeight();
-        } else {
-            return 0;
-        }
+        return this.intrinsicHeight;
     }
     
 }

Modified: xmlgraphics/fop/trunk/src/java/org/apache/fop/image/ImageFactory.java
URL: http://svn.apache.org/viewvc/xmlgraphics/fop/trunk/src/java/org/apache/fop/image/ImageFactory.java?rev=424349&r1=424348&r2=424349&view=diff
==============================================================================
--- xmlgraphics/fop/trunk/src/java/org/apache/fop/image/ImageFactory.java (original)
+++ xmlgraphics/fop/trunk/src/java/org/apache/fop/image/ImageFactory.java Fri Jul 21 08:01:17 2006
@@ -20,6 +20,9 @@
 
 // Java
 import java.io.InputStream;
+import java.lang.ref.Reference;
+import java.lang.ref.ReferenceQueue;
+import java.lang.ref.SoftReference;
 import java.lang.reflect.Constructor;
 import java.util.ArrayList;
 import java.util.Map;
@@ -28,6 +31,8 @@
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.List;
+import java.util.Map.Entry;
+
 import javax.xml.transform.Source;
 import javax.xml.transform.stream.StreamSource;
 
@@ -39,7 +44,6 @@
 import org.apache.fop.apps.FOUserAgent;
 import org.apache.fop.datatypes.URISpecification;
 
-
 /**
  * Create FopImage objects (with a configuration file - not yet implemented).
  * @author Eric SCHAEFFER
@@ -99,6 +103,8 @@
 
         imt = new ImageMimeType("image/png");
         imageMimeTypes.put(imt.getMimeType(), imt);
+        //Image I/O is faster and more memory-efficient than own codec for PNG
+        imt.addProvider(imageIoImage);
         imt.addProvider(pngImage);
 
         imt = new ImageMimeType("image/tga");
@@ -109,8 +115,9 @@
 
         imt = new ImageMimeType("image/tiff");
         imageMimeTypes.put(imt.getMimeType(), imt);
-        imt.addProvider(tiffImage);
-        imt.addProvider(jaiImage);
+        imt.addProvider(tiffImage); //Slower but supports CCITT embedding
+        imt.addProvider(imageIoImage); //Fast but doesn't support CCITT embedding
+        imt.addProvider(jaiImage); //Fast but doesn't support CCITT embedding
 
         imt = new ImageMimeType("image/svg+xml");
         imageMimeTypes.put(imt.getMimeType(), imt);
@@ -357,12 +364,13 @@
     private boolean collective;
     private Map contextStore = Collections.synchronizedMap(new java.util.HashMap());
     private Set invalid = null;
-    private Map weakStore = null;
+    private Map refStore = null;
+    private ReferenceQueue refQueue = new ReferenceQueue();
 
     public ContextImageCache(boolean col) {
         collective = col;
         if (collective) {
-            weakStore = Collections.synchronizedMap(new java.util.WeakHashMap());
+            refStore = Collections.synchronizedMap(new java.util.HashMap());
             invalid = Collections.synchronizedSet(new java.util.HashSet());
         }
     }
@@ -399,7 +407,14 @@
                     }
                 }
                 if (im == null) {
-                    im = (ImageLoader) weakStore.get(url);
+                    Reference ref = (Reference)refStore.get(url);
+                    if (ref != null) {
+                        im = (ImageLoader) ref.get();
+                        if (im == null) {
+                            //Remove key if its value has been garbage collected
+                            refStore.remove(url);
+                        }
+                    }
                 }
             }
 
@@ -423,7 +438,7 @@
         if (con != null) {
             if (collective) {
                 ImageLoader im = con.getImage(url);
-                weakStore.put(url, im);
+                refStore.put(url, wrapInReference(im, url));
             }
             con.releaseImage(url);
         }
@@ -443,15 +458,47 @@
         }
     }
 
+    private Reference wrapInReference(Object obj, Object key) {
+        return new SoftReferenceWithKey(obj, key, refQueue);
+    }
+    
+    private static class SoftReferenceWithKey extends SoftReference {
+        
+        private Object key;
+        
+        public SoftReferenceWithKey(Object referent, Object key, ReferenceQueue q) {
+            super(referent, q);
+            this.key = key;
+        }
+    }
+    
     public void removeContext(FOUserAgent context) {
         Context con = (Context) contextStore.get(context);
         if (con != null) {
             if (collective) {
                 Map images = con.getImages();
-                weakStore.putAll(images);
+                Iterator iter = images.entrySet().iterator();
+                while (iter.hasNext()) {
+                    Entry entry = (Entry)iter.next();
+                    refStore.put(entry.getKey(), 
+                            wrapInReference(entry.getValue(), entry.getKey()));
+                }
             }
             contextStore.remove(context);
         }
+        //House-keeping (remove cleared references)
+        checkReferenceQueue();
+    }
+    
+    /**
+     * Checks the reference queue if any references have been cleared and removes them from the
+     * cache.
+     */
+    private void checkReferenceQueue() {
+        SoftReferenceWithKey ref;
+        while ((ref = (SoftReferenceWithKey)refQueue.poll()) != null) {
+            refStore.remove(ref.key);
+        }
     }
 
     class Context {
@@ -503,7 +550,7 @@
 
     /** @see org.apache.fop.image.ImageCache#clearAll() */
     public void clearAll() {
-        this.weakStore.clear();
+        this.refStore.clear();
         this.invalid.clear();
         //The context-sensitive caches are not cleared so there are no negative side-effects
         //in a multi-threaded environment. Not that it's a good idea to use this method at

Modified: xmlgraphics/fop/trunk/status.xml
URL: http://svn.apache.org/viewvc/xmlgraphics/fop/trunk/status.xml?rev=424349&r1=424348&r2=424349&view=diff
==============================================================================
--- xmlgraphics/fop/trunk/status.xml (original)
+++ xmlgraphics/fop/trunk/status.xml Fri Jul 21 08:01:17 2006
@@ -28,6 +28,15 @@
   <changes>
     <release version="FOP Trunk">
       <action context="Code" dev="JM" type="fix">
+        Fixed two memory-leaks in image handling. The image cache is finally working
+        properly.
+      </action>
+      <action context="Code" dev="JM" type="fix" fixes-bug="39608">
+        Let numeric property values without a unit be treated as pixels like in HTML.
+        This fixes certain NullPointerException when no units are specified.
+        (Note: the use of pixels in XSL-FO is discouraged!)
+      </action>
+      <action context="Code" dev="JM" type="fix">
         Bugfix: Potential multi-threading issue (ConcurrentModificationException) 
         eliminated for ElementMapping classes.
       </action>



---------------------------------------------------------------------
To unsubscribe, e-mail: fop-commits-unsubscribe@xmlgraphics.apache.org
For additional commands, e-mail: fop-commits-help@xmlgraphics.apache.org