From d29590b7ab42fa54d30dbb5db985c7b253abbc46 Mon Sep 17 00:00:00 2001
From: Nick Burch <nick@apache.org>
Date: Tue, 15 Jan 2008 16:31:29 +0000
Subject: [PATCH] Some more updates to the records->usermodel support for hssf
 shapes. Still not enough there to be useful to pretty much anyone, but at
 least there's now a framework in place that people can use if they want to
 support more, and some tests

git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@612148 13f79535-47bb-0310-9956-ffa450edef68
---
 .../apache/poi/ddf/EscherContainerRecord.java |  68 +++++-
 .../poi/hssf/model/DrawingManager2.java       |  13 +-
 .../poi/hssf/record/EscherAggregate.java      | 129 +++++++++++
 .../poi/hssf/usermodel/HSSFPatriarch.java     |   6 +
 .../poi/hssf/usermodel/HSSFShapeGroup.java    |   5 +-
 .../apache/poi/hssf/usermodel/HSSFSheet.java  |  16 +-
 .../poi/ddf/TestEscherContainerRecord.java    |  48 +++-
 .../hssf/usermodel/TestEscherGraphics.java    | 216 +++++++++++++++++-
 .../poi/hssf/usermodel/TestHSSFWorkbook.java  |  14 ++
 9 files changed, 483 insertions(+), 32 deletions(-)

diff --git a/src/java/org/apache/poi/ddf/EscherContainerRecord.java b/src/java/org/apache/poi/ddf/EscherContainerRecord.java
index 05ee6b096..fc1442be9 100644
--- a/src/java/org/apache/poi/ddf/EscherContainerRecord.java
+++ b/src/java/org/apache/poi/ddf/EscherContainerRecord.java
@@ -100,11 +100,47 @@ public class EscherContainerRecord extends EscherRecord
         }
         return 8 + childRecordsSize;
     }
+    
+    /**
+     * Do any of our (top level) children have the
+     *  given recordId?
+     */
+    public boolean hasChildOfType(short recordId) {
+        for ( Iterator iterator = getChildRecords().iterator(); iterator.hasNext(); )
+        {
+            EscherRecord r = (EscherRecord) iterator.next();
+            if(r.getRecordId() == recordId) {
+            	return true;
+            }
+        }
+        return false;
+    }
 
+    /**
+     * Returns a list of all the child (escher) records
+     *  of the container.
+     */
     public List getChildRecords()
     {
         return childRecords;
     }
+    
+    /**
+     * Returns all of our children which are also
+     *  EscherContainers (may be 0, 1, or vary rarely
+     *   2 or 3)
+     */
+    public List getChildContainers() {
+    	List containers = new ArrayList();
+        for ( Iterator iterator = getChildRecords().iterator(); iterator.hasNext(); )
+        {
+            EscherRecord r = (EscherRecord) iterator.next();
+            if(r instanceof EscherContainerRecord) {
+            	containers.add(r);
+            }
+        }
+        return containers;
+    }
 
     public void setChildRecords( List childRecords )
     {
@@ -148,6 +184,10 @@ public class EscherContainerRecord extends EscherRecord
     }
 
     public String toString()
+    {
+    	return toString("");
+    }
+    public String toString(String indent)
     {
         String nl = System.getProperty( "line.separator" );
 
@@ -155,20 +195,32 @@ public class EscherContainerRecord extends EscherRecord
         if ( getChildRecords().size() > 0 )
         {
             children.append( "  children: " + nl );
+            
+            int count = 0;
             for ( Iterator iterator = getChildRecords().iterator(); iterator.hasNext(); )
             {
+            	String newIndent = indent + "   ";
+            	
                 EscherRecord record = (EscherRecord) iterator.next();
-                children.append( record.toString() );
-//                children.append( nl );
+                children.append(newIndent + "Child " + count + ":\n");
+                
+                if(record instanceof EscherContainerRecord) {
+                	EscherContainerRecord ecr = (EscherContainerRecord)record;
+                	children.append( ecr.toString(newIndent));
+                } else {
+                	children.append( record.toString() );
+                }
+                count++;
             }
         }
 
-        return getClass().getName() + " (" + getRecordName() + "):" + nl +
-                "  isContainer: " + isContainerRecord() + nl +
-                "  options: 0x" + HexDump.toHex( getOptions() ) + nl +
-                "  recordId: 0x" + HexDump.toHex( getRecordId() ) + nl +
-                "  numchildren: " + getChildRecords().size() + nl +
-                children.toString();
+        return 
+        	indent + getClass().getName() + " (" + getRecordName() + "):" + nl +
+            indent + "  isContainer: " + isContainerRecord() + nl +
+            indent + "  options: 0x" + HexDump.toHex( getOptions() ) + nl +
+            indent + "  recordId: 0x" + HexDump.toHex( getRecordId() ) + nl +
+            indent + "  numchildren: " + getChildRecords().size() + nl +
+            indent + children.toString();
 
     }
 
diff --git a/src/java/org/apache/poi/hssf/model/DrawingManager2.java b/src/java/org/apache/poi/hssf/model/DrawingManager2.java
index f14d62ff3..dee95fb4f 100644
--- a/src/java/org/apache/poi/hssf/model/DrawingManager2.java
+++ b/src/java/org/apache/poi/hssf/model/DrawingManager2.java
@@ -39,6 +39,13 @@ public class DrawingManager2
     {
         this.dgg = dgg;
     }
+    
+    /**
+     * Clears the cached list of drawing groups
+     */
+    public void clearDrawingGroups() {
+    	drawingGroups.clear(); 
+    }
 
     public EscherDgRecord createDgRecord()
     {
@@ -93,9 +100,13 @@ public class DrawingManager2
     }
 
     ////////////  Non-public methods /////////////
+    
+    /**
+     * Finds the next available (1 based) drawing group id
+     */
     short findNewDrawingGroupId()
     {
-        short dgId = 1;
+        short dgId = 1; 
         while ( drawingGroupExists( dgId ) )
             dgId++;
         return dgId;
diff --git a/src/java/org/apache/poi/hssf/record/EscherAggregate.java b/src/java/org/apache/poi/hssf/record/EscherAggregate.java
index 4f5d18c23..d62b59adb 100644
--- a/src/java/org/apache/poi/hssf/record/EscherAggregate.java
+++ b/src/java/org/apache/poi/hssf/record/EscherAggregate.java
@@ -24,6 +24,8 @@ import org.apache.poi.hssf.model.TextboxShape;
 import org.apache.poi.hssf.model.DrawingManager2;
 import org.apache.poi.hssf.model.ConvertAnchor;
 import org.apache.poi.hssf.model.CommentShape;
+import org.apache.poi.util.POILogFactory;
+import org.apache.poi.util.POILogger;
 
 import java.util.*;
 
@@ -47,6 +49,7 @@ import java.util.*;
 public class EscherAggregate extends AbstractEscherHolderRecord
 {
     public static final short sid = 9876;
+    private static POILogger log = POILogFactory.getLogger(EscherAggregate.class);
 
     public static final short ST_MIN = (short) 0;
     public static final short ST_NOT_PRIMATIVE = ST_MIN;
@@ -533,8 +536,134 @@ public class EscherAggregate extends AbstractEscherHolderRecord
     		throw new IllegalStateException("Must call setPatriarch() first");
     	}
     	
+    	// The top level container ought to have
+    	//  the DgRecord and the container of one container
+    	//  per shape group (patriach overall first)
+    	EscherContainerRecord topContainer =
+    		(EscherContainerRecord)getEscherContainer();
+    	if(topContainer == null) {
+    		return;
+    	}
+    	topContainer = (EscherContainerRecord) 
+    		topContainer.getChildContainers().get(0);
+    	
+    	List tcc = topContainer.getChildContainers();
+    	if(tcc.size() == 0) {
+    		throw new IllegalStateException("No child escher containers at the point that should hold the patriach data, and one container per top level shape!");
+    	}
+    	
+    	// First up, get the patriach position
+    	// This is in the first EscherSpgrRecord, in
+    	//  the first container, with a EscherSRecord too
+    	EscherContainerRecord patriachContainer =
+    		(EscherContainerRecord)tcc.get(0);
+    	EscherSpgrRecord spgr = null;
+    	for(Iterator it = patriachContainer.getChildRecords().iterator(); it.hasNext();) {
+    		EscherRecord r = (EscherRecord)it.next();
+    		if(r instanceof EscherSpgrRecord) {
+    			spgr = (EscherSpgrRecord)r;
+    			break;
+    		}
+    	}
+    	if(spgr != null) {
+    		patriarch.setCoordinates(
+    				spgr.getRectX1(), spgr.getRectY1(),
+    				spgr.getRectX2(), spgr.getRectY2()
+    		);
+    	}
+    	
+    	// Now process the containers for each group
+    	//  and objects
+    	for(int i=1; i<tcc.size(); i++) {
+    		EscherContainerRecord shapeContainer =
+    			(EscherContainerRecord)tcc.get(i);
+    		//System.err.println("\n\n*****\n\n");
+    		//System.err.println(shapeContainer);
+    		
+    		// Could be a group, or a base object
+    		if(shapeContainer.getChildRecords().size() == 1 &&
+    				shapeContainer.getChildContainers().size() == 1) {
+    			// Group
+    			HSSFShapeGroup group =
+    				new HSSFShapeGroup(null, new HSSFClientAnchor());
+    			patriarch.getChildren().add(group);
+    			
+    			EscherContainerRecord groupContainer =
+    				(EscherContainerRecord)shapeContainer.getChild(0);
+    			convertRecordsToUserModel(groupContainer, group);
+    		} else if(shapeContainer.hasChildOfType((short)0xF00D)) {
+    			// TextBox
+    			HSSFTextbox box =
+    				new HSSFTextbox(null, new HSSFClientAnchor());
+    			patriarch.getChildren().add(box);
+    			
+    			convertRecordsToUserModel(shapeContainer, box);
+    		} else if(shapeContainer.hasChildOfType((short)0xF011)) {
+    			// Not yet supporting EscherClientDataRecord stuff
+    		} else {
+    			// Base level
+    			convertRecordsToUserModel(shapeContainer, patriarch);
+    		}
+    	}
+    	
+    	// Now, clear any trace of what records make up
+    	//  the patriarch
+    	// Otherwise, everything will go horribly wrong
+    	//  when we try to write out again....
+//    	clearEscherRecords();
+    	drawingManager.getDgg().setFileIdClusters(new EscherDggRecord.FileIdCluster[0]);
+
     	// TODO: Support converting our records
     	//  back into shapes
+    	log.log(POILogger.WARN, "Not processing objects into Patriarch!");
+    }
+    
+    private void convertRecordsToUserModel(EscherContainerRecord shapeContainer, Object model) {
+    	for(Iterator it = shapeContainer.getChildRecords().iterator(); it.hasNext();) {
+    		EscherRecord r = (EscherRecord)it.next();
+    		if(r instanceof EscherSpgrRecord) {
+    			// This may be overriden by a later EscherClientAnchorRecord
+    			EscherSpgrRecord spgr = (EscherSpgrRecord)r;
+    			
+    			if(model instanceof HSSFShapeGroup) {
+    				HSSFShapeGroup g = (HSSFShapeGroup)model;
+    				g.setCoordinates(
+    						spgr.getRectX1(), spgr.getRectY1(),
+    						spgr.getRectX2(), spgr.getRectY2()
+    				);
+    			} else {
+    				throw new IllegalStateException("Got top level anchor but not processing a group");
+    			}
+    		} 
+    		else if(r instanceof EscherClientAnchorRecord) {
+    			EscherClientAnchorRecord car = (EscherClientAnchorRecord)r;
+    			
+    			if(model instanceof HSSFShape) {
+    				HSSFShape g = (HSSFShape)model;
+    				g.getAnchor().setDx1(car.getDx1());
+    				g.getAnchor().setDx2(car.getDx2());
+    				g.getAnchor().setDy1(car.getDy1());
+    				g.getAnchor().setDy2(car.getDy2());
+    			} else {
+    				throw new IllegalStateException("Got top level anchor but not processing a group or shape");
+    			}
+    		}
+    		else if(r instanceof EscherTextboxRecord) {
+    			EscherTextboxRecord tbr = (EscherTextboxRecord)r;
+    			
+    			// Also need to find the TextObjectRecord too
+    			// TODO
+    		}
+    		else if(r instanceof EscherSpRecord) {
+    			// Use flags if needed
+    		}
+    		else if(r instanceof EscherOptRecord) {
+    			// Use properties if needed
+    		}
+    		else {
+    			//System.err.println(r);
+    		}
+    	}
     }
     
     public void clear()
diff --git a/src/java/org/apache/poi/hssf/usermodel/HSSFPatriarch.java b/src/java/org/apache/poi/hssf/usermodel/HSSFPatriarch.java
index 3df804da7..583e1b479 100644
--- a/src/java/org/apache/poi/hssf/usermodel/HSSFPatriarch.java
+++ b/src/java/org/apache/poi/hssf/usermodel/HSSFPatriarch.java
@@ -254,4 +254,10 @@ public class HSSFPatriarch
         return y2;
     }
 
+    /**
+     * Returns the aggregate escher record we're bound to 
+     */
+    protected EscherAggregate _getBoundAggregate() {
+    	return boundAggregate;
+    }
 }
diff --git a/src/java/org/apache/poi/hssf/usermodel/HSSFShapeGroup.java b/src/java/org/apache/poi/hssf/usermodel/HSSFShapeGroup.java
index 19e54e2ad..fa4952861 100644
--- a/src/java/org/apache/poi/hssf/usermodel/HSSFShapeGroup.java
+++ b/src/java/org/apache/poi/hssf/usermodel/HSSFShapeGroup.java
@@ -121,7 +121,7 @@ public class HSSFShapeGroup
     }
 
     /**
-     * Sets the coordinate space of this group.  All children are contrained
+     * Sets the coordinate space of this group.  All children are constrained
      * to these coordinates.
      */
     public void setCoordinates( int x1, int y1, int x2, int y2 )
@@ -177,5 +177,4 @@ public class HSSFShapeGroup
         }
         return count;
     }
-
-}
+}
\ No newline at end of file
diff --git a/src/java/org/apache/poi/hssf/usermodel/HSSFSheet.java b/src/java/org/apache/poi/hssf/usermodel/HSSFSheet.java
index 3ad8a9477..7fe6ea9dc 100644
--- a/src/java/org/apache/poi/hssf/usermodel/HSSFSheet.java
+++ b/src/java/org/apache/poi/hssf/usermodel/HSSFSheet.java
@@ -1528,7 +1528,14 @@ public class HSSFSheet
     /**
      * Returns the top-level drawing patriach, if there is
      *  one.
-     * This will hold any graphics or charts for the sheet
+     * This will hold any graphics or charts for the sheet.
+     * WARNING - calling this will trigger a parsing of the
+     *  associated escher records. Any that aren't supported
+     *  (such as charts and complex drawing types) will almost
+     *  certainly be lost or corrupted when written out. Only
+     *  use this with simple drawings, otherwise call
+     *  {@link HSSFSheet#createDrawingPatriarch()} and
+     *  start from scratch!
      */
     public HSSFPatriarch getDrawingPatriarch() {
     	book.findDrawingGroup();
@@ -1547,10 +1554,17 @@ public class HSSFSheet
     		return null;
     	}
     	
+    	// Grab our aggregate record, and wire it up
         EscherAggregate agg = (EscherAggregate) sheet.findFirstRecordBySid(EscherAggregate.sid);
         HSSFPatriarch patriarch = new HSSFPatriarch(this, agg);
         agg.setPatriarch(patriarch);
+        
+        // Have it process the records into high level objects
+        //  as best it can do (this step may eat anything
+        //  that isn't supported, you were warned...)
         agg.convertRecordsToUserModel();
+        
+        // Return what we could cope with
         return patriarch;
     }
 
diff --git a/src/testcases/org/apache/poi/ddf/TestEscherContainerRecord.java b/src/testcases/org/apache/poi/ddf/TestEscherContainerRecord.java
index 04bbe47bb..7c139827b 100644
--- a/src/testcases/org/apache/poi/ddf/TestEscherContainerRecord.java
+++ b/src/testcases/org/apache/poi/ddf/TestEscherContainerRecord.java
@@ -82,21 +82,45 @@ public class TestEscherContainerRecord extends TestCase
         r2.setOptions( (short) 0x9876 );
         r2.setRecordId( EscherOptRecord.RECORD_ID );
 
+        String expected;
         r.addChildRecord( r2 );
-        String expected = "org.apache.poi.ddf.EscherContainerRecord (SpContainer):" + nl +
-                        "  isContainer: true" + nl +
-                        "  options: 0x000F" + nl +
-                        "  recordId: 0xF004" + nl +
-                        "  numchildren: 1" + nl +
-                        "  children: " + nl +
-                        "org.apache.poi.ddf.EscherOptRecord:" + nl +
-                        "  isContainer: false" + nl +
-                        "  options: 0x0003" + nl +
-                        "  recordId: 0xF00B" + nl +
-                        "  numchildren: 0" + nl +
-                        "  properties:" + nl;
+        expected = "org.apache.poi.ddf.EscherContainerRecord (SpContainer):" + nl +
+                   "  isContainer: true" + nl +
+                   "  options: 0x000F" + nl +
+                   "  recordId: 0xF004" + nl +
+                   "  numchildren: 1" + nl +
+                   "  children: " + nl +
+                   "   Child 0:" + nl +
+                   "org.apache.poi.ddf.EscherOptRecord:" + nl +
+                   "  isContainer: false" + nl +
+                   "  options: 0x0003" + nl +
+                   "  recordId: 0xF00B" + nl +
+                   "  numchildren: 0" + nl +
+                   "  properties:" + nl;
         assertEquals( expected, r.toString() );
 
+        r.addChildRecord( r2 );
+        expected = "org.apache.poi.ddf.EscherContainerRecord (SpContainer):" + nl +
+                   "  isContainer: true" + nl +
+                   "  options: 0x000F" + nl +
+                   "  recordId: 0xF004" + nl +
+                   "  numchildren: 2" + nl +
+                   "  children: " + nl +
+                   "   Child 0:" + nl +
+                   "org.apache.poi.ddf.EscherOptRecord:" + nl +
+                   "  isContainer: false" + nl +
+                   "  options: 0x0003" + nl +
+                   "  recordId: 0xF00B" + nl +
+                   "  numchildren: 0" + nl +
+                   "  properties:" + nl +
+                   "   Child 1:" + nl +
+                   "org.apache.poi.ddf.EscherOptRecord:" + nl +
+                   "  isContainer: false" + nl +
+                   "  options: 0x0003" + nl +
+                   "  recordId: 0xF00B" + nl +
+                   "  numchildren: 0" + nl +
+                   "  properties:" + nl;
+        assertEquals( expected, r.toString() );
     }
 
     public void testGetRecordSize() throws Exception
diff --git a/src/testcases/org/apache/poi/hssf/usermodel/TestEscherGraphics.java b/src/testcases/org/apache/poi/hssf/usermodel/TestEscherGraphics.java
index 04ae9fdf1..266200b2d 100644
--- a/src/testcases/org/apache/poi/hssf/usermodel/TestEscherGraphics.java
+++ b/src/testcases/org/apache/poi/hssf/usermodel/TestEscherGraphics.java
@@ -20,24 +20,36 @@ package org.apache.poi.hssf.usermodel;
 import junit.framework.TestCase;
 
 import java.awt.*;
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
 
 /**
  * Tests the capabilities of the EscherGraphics class.
+ * 
+ * All tests have two escher groups available to them,
+ *  one anchored at 0,0,1022,255 and another anchored
+ *  at 20,30,500,200 
  *
  * @author Glen Stampoultzis (glens at apache.org)
  */
 public class TestEscherGraphics extends TestCase
 {
-    private HSSFShapeGroup escherGroup;
+	private HSSFWorkbook workbook;
+	private HSSFPatriarch patriarch;
+    private HSSFShapeGroup escherGroupA;
+    private HSSFShapeGroup escherGroupB;
     private EscherGraphics graphics;
 
     protected void setUp() throws Exception
     {
-        HSSFWorkbook workbook = new HSSFWorkbook();
+        workbook = new HSSFWorkbook();
+        
         HSSFSheet sheet = workbook.createSheet("test");
-        escherGroup = sheet.createDrawingPatriarch().createGroup(new HSSFClientAnchor(0,0,1023,255,(short)0,0,(short) 0,0));
-        escherGroup = new HSSFShapeGroup(null, new HSSFChildAnchor());
-        graphics = new EscherGraphics(this.escherGroup, workbook, Color.black, 1.0f);
+        patriarch = sheet.createDrawingPatriarch();
+        escherGroupA = patriarch.createGroup(new HSSFClientAnchor(0,0,1022,255,(short)0,0,(short) 0,0));
+        escherGroupB = patriarch.createGroup(new HSSFClientAnchor(20,30,500,200,(short)0,0,(short) 0,0));
+//        escherGroup = new HSSFShapeGroup(null, new HSSFChildAnchor());
+        graphics = new EscherGraphics(this.escherGroupA, workbook, Color.black, 1.0f);
         super.setUp();
     }
 
@@ -74,7 +86,7 @@ public class TestEscherGraphics extends TestCase
     public void testFillRect() throws Exception
     {
         graphics.fillRect( 10, 10, 20, 20 );
-        HSSFSimpleShape s = (HSSFSimpleShape) escherGroup.getChildren().get(0);
+        HSSFSimpleShape s = (HSSFSimpleShape) escherGroupA.getChildren().get(0);
         assertEquals(HSSFSimpleShape.OBJECT_TYPE_RECTANGLE, s.getShapeType());
         assertEquals(10, s.getAnchor().getDx1());
         assertEquals(10, s.getAnchor().getDy1());
@@ -85,8 +97,198 @@ public class TestEscherGraphics extends TestCase
     public void testDrawString() throws Exception
     {
         graphics.drawString("This is a test", 10, 10);
-        HSSFTextbox t = (HSSFTextbox) escherGroup.getChildren().get(0);
+        HSSFTextbox t = (HSSFTextbox) escherGroupA.getChildren().get(0);
         assertEquals("This is a test", t.getString().getString().toString());
     }
 
+    public void testGetDataBackAgain() throws Exception {
+    	HSSFSheet s;
+    	HSSFShapeGroup s1;
+    	HSSFShapeGroup s2;
+    	
+    	patriarch.setCoordinates(10, 20, 30, 40);
+    	
+    	ByteArrayOutputStream baos = new ByteArrayOutputStream();
+    	workbook.write(baos);
+    	workbook = new HSSFWorkbook(new ByteArrayInputStream(baos.toByteArray()));
+    	s = workbook.getSheetAt(0);
+    	
+    	patriarch = s.getDrawingPatriarch();
+    	
+    	assertNotNull(patriarch);
+    	assertEquals(10, patriarch.getX1());
+    	assertEquals(20, patriarch.getY1());
+    	assertEquals(30, patriarch.getX2());
+    	assertEquals(40, patriarch.getY2());
+    	
+    	// Check the two groups too
+    	assertEquals(2, patriarch.countOfAllChildren());
+    	assertTrue(patriarch.getChildren().get(0) instanceof HSSFShapeGroup);
+    	assertTrue(patriarch.getChildren().get(1) instanceof HSSFShapeGroup);
+    	
+    	s1 = (HSSFShapeGroup)patriarch.getChildren().get(0);
+    	s2 = (HSSFShapeGroup)patriarch.getChildren().get(1);
+    	
+    	assertEquals(0, s1.getX1());
+    	assertEquals(0, s1.getY1());
+    	assertEquals(1023, s1.getX2());
+    	assertEquals(255, s1.getY2());
+    	assertEquals(0, s2.getX1());
+    	assertEquals(0, s2.getY1());
+    	assertEquals(1023, s2.getX2());
+    	assertEquals(255, s2.getY2());
+    	
+    	assertEquals(0, s1.getAnchor().getDx1());
+    	assertEquals(0, s1.getAnchor().getDy1());
+    	assertEquals(1022, s1.getAnchor().getDx2());
+    	assertEquals(255, s1.getAnchor().getDy2());
+    	assertEquals(20, s2.getAnchor().getDx1());
+    	assertEquals(30, s2.getAnchor().getDy1());
+    	assertEquals(500, s2.getAnchor().getDx2());
+    	assertEquals(200, s2.getAnchor().getDy2());
+    	
+    	
+    	// Write and re-load once more, to check that's ok
+    	baos = new ByteArrayOutputStream();
+    	workbook.write(baos);
+    	workbook = new HSSFWorkbook(new ByteArrayInputStream(baos.toByteArray()));
+    	s = workbook.getSheetAt(0);
+    	patriarch = s.getDrawingPatriarch();
+    	
+    	assertNotNull(patriarch);
+    	assertEquals(10, patriarch.getX1());
+    	assertEquals(20, patriarch.getY1());
+    	assertEquals(30, patriarch.getX2());
+    	assertEquals(40, patriarch.getY2());
+    	
+    	// Check the two groups too
+    	assertEquals(2, patriarch.countOfAllChildren());
+    	assertTrue(patriarch.getChildren().get(0) instanceof HSSFShapeGroup);
+    	assertTrue(patriarch.getChildren().get(1) instanceof HSSFShapeGroup);
+    	
+    	s1 = (HSSFShapeGroup)patriarch.getChildren().get(0);
+    	s2 = (HSSFShapeGroup)patriarch.getChildren().get(1);
+    	
+    	assertEquals(0, s1.getX1());
+    	assertEquals(0, s1.getY1());
+    	assertEquals(1023, s1.getX2());
+    	assertEquals(255, s1.getY2());
+    	assertEquals(0, s2.getX1());
+    	assertEquals(0, s2.getY1());
+    	assertEquals(1023, s2.getX2());
+    	assertEquals(255, s2.getY2());
+    	
+    	assertEquals(0, s1.getAnchor().getDx1());
+    	assertEquals(0, s1.getAnchor().getDy1());
+    	assertEquals(1022, s1.getAnchor().getDx2());
+    	assertEquals(255, s1.getAnchor().getDy2());
+    	assertEquals(20, s2.getAnchor().getDx1());
+    	assertEquals(30, s2.getAnchor().getDy1());
+    	assertEquals(500, s2.getAnchor().getDx2());
+    	assertEquals(200, s2.getAnchor().getDy2());
+    	
+    	// Change the positions of the first groups,
+    	//  but not of their anchors
+    	s1.setCoordinates(2, 3, 1021, 242);
+    	
+    	baos = new ByteArrayOutputStream();
+    	workbook.write(baos);
+    	workbook = new HSSFWorkbook(new ByteArrayInputStream(baos.toByteArray()));
+    	s = workbook.getSheetAt(0);
+    	patriarch = s.getDrawingPatriarch();
+    	
+    	assertNotNull(patriarch);
+    	assertEquals(10, patriarch.getX1());
+    	assertEquals(20, patriarch.getY1());
+    	assertEquals(30, patriarch.getX2());
+    	assertEquals(40, patriarch.getY2());
+    	
+    	// Check the two groups too
+    	assertEquals(2, patriarch.countOfAllChildren());
+    	assertEquals(2, patriarch.getChildren().size());
+    	assertTrue(patriarch.getChildren().get(0) instanceof HSSFShapeGroup);
+    	assertTrue(patriarch.getChildren().get(1) instanceof HSSFShapeGroup);
+    	
+    	s1 = (HSSFShapeGroup)patriarch.getChildren().get(0);
+    	s2 = (HSSFShapeGroup)patriarch.getChildren().get(1);
+    	
+    	assertEquals(2, s1.getX1());
+    	assertEquals(3, s1.getY1());
+    	assertEquals(1021, s1.getX2());
+    	assertEquals(242, s1.getY2());
+    	assertEquals(0, s2.getX1());
+    	assertEquals(0, s2.getY1());
+    	assertEquals(1023, s2.getX2());
+    	assertEquals(255, s2.getY2());
+    	
+    	assertEquals(0, s1.getAnchor().getDx1());
+    	assertEquals(0, s1.getAnchor().getDy1());
+    	assertEquals(1022, s1.getAnchor().getDx2());
+    	assertEquals(255, s1.getAnchor().getDy2());
+    	assertEquals(20, s2.getAnchor().getDx1());
+    	assertEquals(30, s2.getAnchor().getDy1());
+    	assertEquals(500, s2.getAnchor().getDx2());
+    	assertEquals(200, s2.getAnchor().getDy2());
+    	
+    	
+    	// Now add some text to one group, and some more
+    	//  to the base, and check we can get it back again
+    	HSSFTextbox tbox1 =
+    		patriarch.createTextbox(new HSSFClientAnchor(1,2,3,4, (short)0,0,(short)0,0));
+    	tbox1.setString(new HSSFRichTextString("I am text box 1"));
+    	HSSFTextbox tbox2 =
+    		s2.createTextbox(new HSSFChildAnchor(41,42,43,44));
+    	tbox2.setString(new HSSFRichTextString("This is text box 2"));
+    	
+    	assertEquals(3, patriarch.getChildren().size());
+    	
+    	
+    	baos = new ByteArrayOutputStream();
+    	workbook.write(baos);
+    	workbook = new HSSFWorkbook(new ByteArrayInputStream(baos.toByteArray()));
+    	s = workbook.getSheetAt(0);
+    	
+    	patriarch = s.getDrawingPatriarch();
+    	
+    	assertNotNull(patriarch);
+    	assertEquals(10, patriarch.getX1());
+    	assertEquals(20, patriarch.getY1());
+    	assertEquals(30, patriarch.getX2());
+    	assertEquals(40, patriarch.getY2());
+    	
+    	// Check the two groups and the text
+    	assertEquals(3, patriarch.countOfAllChildren());
+    	assertEquals(2, patriarch.getChildren().size());
+    	
+    	// Should be two groups and a text
+    	assertTrue(patriarch.getChildren().get(0) instanceof HSSFShapeGroup);
+    	assertTrue(patriarch.getChildren().get(1) instanceof HSSFTextbox);
+//    	assertTrue(patriarch.getChildren().get(2) instanceof HSSFShapeGroup);
+    	
+    	s1 = (HSSFShapeGroup)patriarch.getChildren().get(0);
+    	tbox1 = (HSSFTextbox)patriarch.getChildren().get(1);
+    	
+//    	s2 = (HSSFShapeGroup)patriarch.getChildren().get(1);
+    	
+    	assertEquals(2, s1.getX1());
+    	assertEquals(3, s1.getY1());
+    	assertEquals(1021, s1.getX2());
+    	assertEquals(242, s1.getY2());
+    	assertEquals(0, s2.getX1());
+    	assertEquals(0, s2.getY1());
+    	assertEquals(1023, s2.getX2());
+    	assertEquals(255, s2.getY2());
+    	
+    	assertEquals(0, s1.getAnchor().getDx1());
+    	assertEquals(0, s1.getAnchor().getDy1());
+    	assertEquals(1022, s1.getAnchor().getDx2());
+    	assertEquals(255, s1.getAnchor().getDy2());
+    	assertEquals(20, s2.getAnchor().getDx1());
+    	assertEquals(30, s2.getAnchor().getDy1());
+    	assertEquals(500, s2.getAnchor().getDx2());
+    	assertEquals(200, s2.getAnchor().getDy2());
+    	
+    	// Not working just yet
+    	//assertEquals("I am text box 1", tbox1.getString().getString());
+    }
 }
diff --git a/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFWorkbook.java b/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFWorkbook.java
index 40a577240..1be3a9085 100644
--- a/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFWorkbook.java
+++ b/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFWorkbook.java
@@ -151,6 +151,13 @@ public class TestHSSFWorkbook extends TestCase
         assertNull(b.getSheetAt(1).getDrawingPatriarch());
         assertFalse(b.getSheetAt(0).getDrawingPatriarch().containsChart());
         
+        // We've now called getDrawingPatriarch() so 
+        //  everything will be all screwy
+        // So, start again
+        b = new HSSFWorkbook(
+        		new FileInputStream(new File(filename,"44010-SingleChart.xls"))
+        );
+        
         b = writeRead(b);
         assertEquals(2, b.getNumberOfSheets());
         s = b.getSheetAt(1);
@@ -178,6 +185,13 @@ public class TestHSSFWorkbook extends TestCase
         assertNull(b.getSheetAt(2).getDrawingPatriarch());
         assertFalse(b.getSheetAt(0).getDrawingPatriarch().containsChart());
         
+        // We've now called getDrawingPatriarch() so 
+        //  everything will be all screwy
+        // So, start again
+        b = new HSSFWorkbook(
+        		new FileInputStream(new File(filename,"44010-TwoCharts.xls"))
+        );
+        
         b = writeRead(b);
         assertEquals(3, b.getNumberOfSheets());