checked all examples, added several features

git-svn-id: https://svn.apache.org/repos/asf/poi/branches/gsoc2012@1360132 13f79535-47bb-0310-9956-ffa450edef68

src/java/org/apache/poi/hssf/record/EscherAggregate.java

@@ -303,6 +303,12 @@ public final class EscherAggregate extends AbstractEscherHolderRecord {
 
     protected HSSFPatriarch patriarch;
 
+    /**
+     * if we want to get the same byte array if we open existing file and serialize it we should save 
+     * note records in right order. This list contains ids of NoteRecords in such order as we read from existing file
+     */
+    private List<Integer> _tailIds = new ArrayList<Integer>();
+
     /**
      * Maps shape container objects to their {@link TextObjectRecord} or {@link ObjRecord}
      */
@@ -313,7 +319,7 @@ public final class EscherAggregate extends AbstractEscherHolderRecord {
     /**
      * list of "tail" records that need to be serialized after all drawing group records
      */
-    private List<Record> tailRec = new ArrayList<Record>();
+     private Map<Integer, NoteRecord> tailRec = new HashMap<Integer, NoteRecord>();
 
     public EscherAggregate() {
         buildBaseTree();
@@ -440,7 +446,8 @@ public final class EscherAggregate extends AbstractEscherHolderRecord {
         while (loc < records.size()) {
             if (sid(records, loc) == NoteRecord.sid) {
                 NoteRecord r = (NoteRecord) records.get(loc);
-                agg.tailRec.add(r);
+                agg.tailRec.put(r.getShapeId(), r);
+                agg._tailIds.add(agg._tailIds.size(), r.getShapeId());
             } else {
                 break;
             }
@@ -531,8 +538,18 @@ public final class EscherAggregate extends AbstractEscherHolderRecord {
         }
 
         // write records that need to be serialized after all drawing group records
-        for (i = 0; i < tailRec.size(); i++) {
+        Map<Integer, NoteRecord> tailCopy = new HashMap<Integer, NoteRecord>(tailRec);
-            Record rec = tailRec.get(i);
+        // at first we should save records in correct order which were already in the file during EscherAggregate.createAggregate()
+        for (Integer id : _tailIds){
+            NoteRecord note = tailCopy.get(id);
+            if (null != note){
+                pos += note.serialize(pos, data);
+                tailCopy.remove(id);
+            }
+        }
+        // Add all other notes which were created after createAggregate()
+        for (i = 0; i < tailCopy.size(); i++) {
+            Record rec = (Record) tailCopy.values().toArray()[i];
             pos += rec.serialize(pos, data);
         }
         int bytesWritten = pos - offset;
@@ -622,7 +639,7 @@ public final class EscherAggregate extends AbstractEscherHolderRecord {
             objRecordSize += r.getRecordSize();
         }
         int tailRecordSize = 0;
-        for (Iterator iterator = tailRec.iterator(); iterator.hasNext(); ) {
+        for (Iterator iterator = tailRec.values().iterator(); iterator.hasNext(); ) {
             Record r = (Record) iterator.next();
             tailRecordSize += r.getRecordSize();
         }
@@ -636,6 +653,10 @@ public final class EscherAggregate extends AbstractEscherHolderRecord {
         return shapeToObj.put(r, objRecord);
     }
 
+    public void removeShapeToObjRecord(EscherRecord rec){
+        shapeToObj.remove(rec);
+    }
+
     public HSSFPatriarch getPatriarch() {
         return patriarch;
     }
@@ -938,7 +959,7 @@ public final class EscherAggregate extends AbstractEscherHolderRecord {
 
                     if (shapeModel instanceof CommentShape) {
                         CommentShape comment = (CommentShape) shapeModel;
-                        tailRec.add(comment.getNoteRecord());
+                        tailRec.put(comment.getNoteRecord().getShapeId(), comment.getNoteRecord());
                     }
 
                 }
@@ -1058,7 +1079,9 @@ public final class EscherAggregate extends AbstractEscherHolderRecord {
         spgr.setRectX2(1023);
         spgr.setRectY2(255);
         sp1.setRecordId(EscherSpRecord.RECORD_ID);
+
         sp1.setOptions((short) 0x0002);
+        sp1.setVersion((short) 0x2);
         sp1.setShapeId(-1);
         sp1.setFlags(EscherSpRecord.FLAG_GROUP | EscherSpRecord.FLAG_PATRIARCH);
         dgContainer.addChildRecord(dg);
@@ -1161,22 +1184,20 @@ public final class EscherAggregate extends AbstractEscherHolderRecord {
      * @return tails records. We need to access them when building shapes.
      *         Every HSSFComment shape has a link to a NoteRecord from the tailRec collection.
      */
-    public List<Record> getTailRecords() {
+    public Map<Integer, NoteRecord> getTailRecords() {
-        return Collections.unmodifiableList(tailRec);
+        return tailRec;
     }
 
     public NoteRecord getNoteRecordByObj(ObjRecord obj) {
-        for (Record rec : tailRec) {
-            NoteRecord note = (NoteRecord) rec;
         CommonObjectDataSubRecord cod = (CommonObjectDataSubRecord) obj.getSubRecords().get(0);
-            if (note.getShapeId() == cod.getObjectId()) {
+        return tailRec.get(cod.getObjectId());
-                return note;
-            }
-        }
-        return null;
     }
 
     public void addTailRecord(NoteRecord note){
-        tailRec.add(note);
+        tailRec.put(note.getShapeId(), note);
+    }
+
+    public void removeTailRecord(NoteRecord note){
+        tailRec.remove(note.getShapeId());
     }
 }

src/java/org/apache/poi/hssf/usermodel/EscherGraphics.java

@@ -349,6 +349,7 @@ public class EscherGraphics
         shape.setLineStyle(HSSFShape.LINESTYLE_NONE);
         shape.setFillColor(foreground.getRed(), foreground.getGreen(), foreground.getBlue());
         shape.setLineStyleColor(foreground.getRed(), foreground.getGreen(), foreground.getBlue());
+        shape.setNoFill(false);
     }
 
     /**

src/java/org/apache/poi/hssf/usermodel/HSSFAnchor.java

@@ -31,6 +31,9 @@ import org.apache.poi.ddf.EscherRecord;
  */
 public abstract class HSSFAnchor {
 
+    protected boolean _isHorizontallyFlipped = false;
+    protected boolean _isVerticallyFlipped = false;
+
     public HSSFAnchor() {
         createEscherAnchor();
     }
@@ -51,7 +54,6 @@ public abstract class HSSFAnchor {
                 return new HSSFClientAnchor((EscherClientAnchorRecord) container.getChildById(EscherClientAnchorRecord.RECORD_ID));
             }
             return null;
-//            throw new IllegalArgumentException("continer must have anchor record");
         }
     }
 

src/java/org/apache/poi/hssf/usermodel/HSSFCell.java

@@ -1021,7 +1021,7 @@ public class HSSFCell implements Cell {
      */
      public HSSFComment getCellComment(){
         if (_comment == null) {
-            _comment = findCellComment(_sheet.getSheet(), _record.getRow(), _record.getColumn());
+            _comment = _sheet.findCellComment(_record.getRow(), _record.getColumn());
         }
         return _comment;
     }
@@ -1033,41 +1033,12 @@ public class HSSFCell implements Cell {
      *  all comments after performing this action!
      */
     public void removeCellComment() {
-        HSSFComment comment = findCellComment(_sheet.getSheet(), _record.getRow(), _record.getColumn());
+        HSSFComment comment = _sheet.findCellComment(_record.getRow(), _record.getColumn());
         _comment = null;
-
+        if (null == comment){
-        if(comment == null) {
-            // Nothing to do
             return;
         }
-
+        _sheet.getDrawingPatriarch().removeShape(comment);
-        // Zap the underlying NoteRecord
-        List<RecordBase> sheetRecords = _sheet.getSheet().getRecords();
-        sheetRecords.remove(comment.getNoteRecord());
-
-        // If we have a TextObjectRecord, is should
-        //  be proceeed by:
-        // MSODRAWING with container
-        // OBJ
-        // MSODRAWING with EscherTextboxRecord
-        if(comment.getTextObjectRecord() != null) {
-            TextObjectRecord txo = comment.getTextObjectRecord();
-            int txoAt = sheetRecords.indexOf(txo);
-
-            if(sheetRecords.get(txoAt-3) instanceof DrawingRecord &&
-                sheetRecords.get(txoAt-2) instanceof ObjRecord &&
-                sheetRecords.get(txoAt-1) instanceof DrawingRecord) {
-                // Zap these, in reverse order
-                sheetRecords.remove(txoAt-1);
-                sheetRecords.remove(txoAt-2);
-                sheetRecords.remove(txoAt-3);
-            } else {
-                throw new IllegalStateException("Found the wrong records before the TextObjectRecord, can't remove comment");
-            }
-
-            // Now remove the text record
-            sheetRecords.remove(txo);
-        }
     }
 
     /**

src/java/org/apache/poi/hssf/usermodel/HSSFChildAnchor.java

@@ -34,7 +34,13 @@ public final class HSSFChildAnchor extends HSSFAnchor {
     }
 
     public HSSFChildAnchor(int dx1, int dy1, int dx2, int dy2) {
-        super(dx1, dy1, dx2, dy2);
+        super(Math.min(dx1, dx2), Math.min(dy1, dy2), Math.max(dx1, dx2), Math.max(dy1, dy2));
+        if (dx1 > dx2){
+            _isHorizontallyFlipped = true;
+        }
+        if (dy1 > dy2){
+            _isVerticallyFlipped = true;
+        }
     }
 
     @Override
@@ -78,18 +84,18 @@ public final class HSSFChildAnchor extends HSSFAnchor {
     }
 
     public void setAnchor(int dx1, int dy1, int dx2, int dy2) {
-        setDx1(dx1);
+        setDx1(Math.min(dx1, dx2));
-        setDy1(dy1);
+        setDy1(Math.min(dy1, dy2));
-        setDx2(dx2);
+        setDx2(Math.max(dx1, dx2));
-        setDy2(dy2);
+        setDy2(Math.max(dy1, dy2));
     }
 
     public boolean isHorizontallyFlipped() {
-        return getDx1() > getDx2();
+        return _isHorizontallyFlipped;
     }
 
     public boolean isVerticallyFlipped() {
-        return getDy1() > getDy2();
+        return _isVerticallyFlipped;
     }
 
     @Override

src/java/org/apache/poi/hssf/usermodel/HSSFClientAnchor.java

@@ -66,10 +66,17 @@ public final class HSSFClientAnchor extends HSSFAnchor implements ClientAnchor {
         checkRange(row1, 0, 255 * 256, "row1");
         checkRange(row2, 0, 255 * 256, "row2");
 
-        setCol1(col1);
+        setCol1((short) Math.min(col1, col2));
-        setCol2(col2);
+        setCol2((short) Math.max(col1, col2));
-        setRow1(row1);
+        setRow1((short) Math.min(row1, row2));
-        setRow2(row2);
+        setRow2((short) Math.max(row1, row2));
+
+        if (col1 > col2){
+            _isHorizontallyFlipped = true;
+        }
+        if (row1 > row2){
+            _isVerticallyFlipped = true;
+        }
     }
 
     /**
@@ -187,20 +194,14 @@ public final class HSSFClientAnchor extends HSSFAnchor implements ClientAnchor {
      * @return true if the anchor goes from right to left.
      */
     public boolean isHorizontallyFlipped() {
-        if (getCol1() == getCol2()) {
+        return _isHorizontallyFlipped;
-            return getDx1() > getDx2();
-        }
-        return getCol1() > getCol2();
     }
 
     /**
      * @return true if the anchor goes from bottom to top.
      */
     public boolean isVerticallyFlipped() {
-        if (getRow1() == getRow2()) {
+        return _isVerticallyFlipped;
-            return getDy1() > getDy2();
-        }
-        return getRow1() > getRow2();
     }
 
     @Override

src/java/org/apache/poi/hssf/usermodel/HSSFComment.java

@@ -16,7 +16,7 @@
 ==================================================================== */
 package org.apache.poi.hssf.usermodel;
 
-import org.apache.poi.ddf.EscherContainerRecord;
+import org.apache.poi.ddf.*;
 import org.apache.poi.hssf.record.*;
 import org.apache.poi.ss.usermodel.Comment;
 import org.apache.poi.ss.usermodel.RichTextString;
@@ -64,7 +64,6 @@ public class HSSFComment extends HSSFTextbox implements Comment {
 
     protected HSSFComment(NoteRecord note, TextObjectRecord txo) {
         this(null, new HSSFClientAnchor());
-        _textObjectRecord = txo;
         _note = note;
     }
 
@@ -74,6 +73,17 @@ public class HSSFComment extends HSSFTextbox implements Comment {
         _patriarch._getBoundAggregate().addTailRecord(getNoteRecord());
     }
 
+    @Override
+    protected EscherContainerRecord createSpContainer() {
+        EscherContainerRecord spContainer = super.createSpContainer();
+        EscherOptRecord opt = spContainer.getChildById(EscherOptRecord.RECORD_ID);
+        removeEscherProperty(opt, EscherProperties.TEXT__TEXTLEFT);
+        removeEscherProperty(opt, EscherProperties.TEXT__TEXTRIGHT);
+        removeEscherProperty(opt, EscherProperties.TEXT__TEXTTOP);
+        removeEscherProperty(opt, EscherProperties.TEXT__TEXTBOTTOM);
+        return spContainer;
+    }
+
     @Override
     protected ObjRecord createObjRecord() {
         ObjRecord obj = new ObjRecord();
@@ -94,7 +104,7 @@ public class HSSFComment extends HSSFTextbox implements Comment {
 
     private NoteRecord createNoteRecord(){
         NoteRecord note = new NoteRecord();
-        note.setFlags(NoteRecord.NOTE_VISIBLE);
+        note.setFlags(NoteRecord.NOTE_HIDDEN);
         note.setAuthor("");
         return note;
     }
@@ -102,7 +112,7 @@ public class HSSFComment extends HSSFTextbox implements Comment {
     @Override
     void setShapeId(int shapeId) {
         super.setShapeId(shapeId);
-        CommonObjectDataSubRecord cod = (CommonObjectDataSubRecord) _objRecord.getSubRecords().get(0);
+        CommonObjectDataSubRecord cod = (CommonObjectDataSubRecord) getObjRecord().getSubRecords().get(0);
         cod.setObjectId((short) (shapeId));
         _note.setShapeId(shapeId);
     }
@@ -187,20 +197,6 @@ public class HSSFComment extends HSSFTextbox implements Comment {
         if (_note != null) _note.setAuthor(author);
     }
 
-    /**
-     * Sets the rich text string used by this comment.
-     *
-     * @param string Sets the rich text string used by this object.
-     */
-    public void setString(RichTextString string) {
-        HSSFRichTextString hstring = (HSSFRichTextString) string;
-        //if font is not set we must set the default one
-        if (hstring.numFormattingRuns() == 0) hstring.applyFont((short) 0);
-
-        _textObjectRecord.setStr(hstring);
-        super.setString(string);
-    }
-
     /**
      * Returns the underlying Note record
      */
@@ -208,10 +204,14 @@ public class HSSFComment extends HSSFTextbox implements Comment {
         return _note;
     }
 
-    /**
+    @Override
-     * Returns the underlying Text record
+    public void setShapeType(int shapeType) {
-     */
+        throw new IllegalStateException("Shape type can not be changed in "+this.getClass().getSimpleName());
-    public TextObjectRecord getTextObjectRecord() {
+    }
-        return _textObjectRecord;
+
+    public void afterRemove(HSSFPatriarch patriarch){
+        patriarch._getBoundAggregate().removeShapeToObjRecord(getEscherContainer().getChildById(EscherClientDataRecord.RECORD_ID));
+        patriarch._getBoundAggregate().removeShapeToObjRecord(getEscherContainer().getChildById(EscherTextboxRecord.RECORD_ID));
+        patriarch._getBoundAggregate().removeTailRecord(getNoteRecord());
     }
 }

src/java/org/apache/poi/hssf/usermodel/HSSFPatriarch.java

@@ -44,6 +44,7 @@ public final class HSSFPatriarch implements HSSFShapeContainer, Drawing {
 //    private int _y2 = 255;
 
     private final EscherSpgrRecord _spgrRecord;
+    private final EscherContainerRecord _mainSpgrContainer;
 
     /**
      * The EscherAggregate we have been bound to.
@@ -61,12 +62,23 @@ public final class HSSFPatriarch implements HSSFShapeContainer, Drawing {
     HSSFPatriarch(HSSFSheet sheet, EscherAggregate boundAggregate){
         _sheet = sheet;
 		_boundAggregate = boundAggregate;
+        _mainSpgrContainer = _boundAggregate.getEscherContainer().getChildContainers().get(0);
         EscherContainerRecord spContainer = (EscherContainerRecord) _boundAggregate.getEscherContainer()
                 .getChildContainers().get(0).getChild(0);
         _spgrRecord = spContainer.getChildById(EscherSpgrRecord.RECORD_ID);
         buildShapeTree();
     }
 
+    /**
+     * remove first level shapes
+     * @param shape to be removed
+     */
+    public void removeShape(HSSFShape shape){
+        _mainSpgrContainer.removeChildRecord(shape.getEscherContainer());
+        shape.afterRemove(this);
+        _shapes.remove(shape);
+    }
+
     public void afterCreate(){
         DrawingManager2 drawingManager = _sheet.getWorkbook().getWorkbook().getDrawingManager();
         short dgId = drawingManager.findNewDrawingGroupId();
@@ -106,6 +118,13 @@ public final class HSSFPatriarch implements HSSFShapeContainer, Drawing {
         addShape(shape);
         //open existing file
         onCreate(shape);
+        EscherSpRecord sp = shape.getEscherContainer().getChildById(EscherSpRecord.RECORD_ID);
+        if (shape.anchor.isHorizontallyFlipped()){
+            sp.setFlags(sp.getFlags() | EscherSpRecord.FLAG_FLIPHORIZ);
+        }
+        if (shape.anchor.isVerticallyFlipped()){
+            sp.setFlags(sp.getFlags() | EscherSpRecord.FLAG_FLIPVERT);
+        }
         return shape;
     }
 
@@ -125,8 +144,13 @@ public final class HSSFPatriarch implements HSSFShapeContainer, Drawing {
         //open existing file
         onCreate(shape);
 
-        EscherBSERecord bse = _sheet.getWorkbook().getWorkbook().getBSERecord(pictureIndex);
+        EscherSpRecord sp = shape.getEscherContainer().getChildById(EscherSpRecord.RECORD_ID);
-        bse.setRef(bse.getRef() + 1);
+        if (shape.anchor.isHorizontallyFlipped()){
+            sp.setFlags(sp.getFlags() | EscherSpRecord.FLAG_FLIPHORIZ);
+        }
+        if (shape.anchor.isVerticallyFlipped()){
+            sp.setFlags(sp.getFlags() | EscherSpRecord.FLAG_FLIPVERT);
+        }
         return shape;
     }
 
@@ -368,11 +392,7 @@ public final class HSSFPatriarch implements HSSFShapeContainer, Drawing {
         for(int i = 0; i < spgrChildren.size(); i++){
             EscherContainerRecord spContainer = spgrChildren.get(i);
             if (i == 0){
-                EscherSpgrRecord spgr = (EscherSpgrRecord)spContainer.getChildById(EscherSpgrRecord.RECORD_ID);
+                continue;
-                setCoordinates(
-                        spgr.getRectX1(), spgr.getRectY1(),
-                        spgr.getRectX2(), spgr.getRectY2()
-                );
             } else {
                 HSSFShapeFactory.createShapeTree(spContainer, _boundAggregate, this);
             }

src/java/org/apache/poi/hssf/usermodel/HSSFPicture.java

@@ -19,6 +19,7 @@ package org.apache.poi.hssf.usermodel;
 
 import java.awt.Dimension;
 import java.io.ByteArrayInputStream;
+import java.io.UnsupportedEncodingException;
 
 import org.apache.poi.ddf.*;
 import org.apache.poi.hssf.record.ObjRecord;
@@ -69,7 +70,7 @@ public final class HSSFPicture extends HSSFSimpleShape implements Picture {
 
     public int getPictureIndex()
     {
-        EscherSimpleProperty property = _optRecord.lookup(EscherProperties.BLIP__BLIPTODISPLAY);
+        EscherSimpleProperty property = getOptRecord().lookup(EscherProperties.BLIP__BLIPTODISPLAY);
         if (null == property){
             return -1;
         }
@@ -81,6 +82,15 @@ public final class HSSFPicture extends HSSFSimpleShape implements Picture {
         setPropertyValue(new EscherSimpleProperty( EscherProperties.BLIP__BLIPTODISPLAY, false, true, pictureIndex));
     }
 
+    @Override
+    protected EscherContainerRecord createSpContainer() {
+        EscherContainerRecord spContainer = super.createSpContainer();
+        EscherOptRecord opt = spContainer.getChildById(EscherOptRecord.RECORD_ID);
+        removeEscherProperty(opt, EscherProperties.LINESTYLE__LINEDASHING);
+        removeEscherProperty(opt, EscherProperties.LINESTYLE__NOLINEDRAWDASH);
+        return spContainer;
+    }
+
     /**
      * Resize the image
      * <p>
@@ -237,4 +247,37 @@ public final class HSSFPicture extends HSSFSimpleShape implements Picture {
     	EscherBlipRecord blipRecord = iwb.getBSERecord(getPictureIndex()).getBlipRecord();
     	return new HSSFPictureData(blipRecord);
     }
+
+    @Override
+    void afterInsert(HSSFPatriarch patriarch) {
+        super.afterInsert(patriarch);
+        EscherBSERecord bse =
+                patriarch._sheet.getWorkbook().getWorkbook().getBSERecord(getPictureIndex());
+        bse.setRef(bse.getRef() + 1);
+    }
+
+    /**
+     * The color applied to the lines of this shape.
+     */
+    public String getAdditionalData() {
+        EscherComplexProperty propFile = (EscherComplexProperty) getOptRecord().lookup(
+                      EscherProperties.BLIP__BLIPFILENAME);
+        try {
+            if (null == propFile){
+                return "";
+            }
+            return new String(propFile.getComplexData(), "UTF-16LE").trim();
+        } catch (UnsupportedEncodingException e) {
+            return "";
+        }
+    }
+    
+    public void setAdditionalData(String data){
+        try {
+            EscherComplexProperty prop = new EscherComplexProperty(EscherProperties.BLIP__BLIPFILENAME, true, data.getBytes("UTF-16LE"));
+            setPropertyValue(prop);
+        } catch (UnsupportedEncodingException e) {
+            System.out.println("Unsupported encoding: UTF-16LE");
+        }
+    }
 }

src/java/org/apache/poi/hssf/usermodel/HSSFPolygon.java

@@ -106,8 +106,12 @@ public class HSSFPolygon  extends HSSFShape {
         return obj;
     }
 
+    @Override
+    protected void afterRemove(HSSFPatriarch patriarch) {
+    }
+
     public int[] getXPoints() {
-        EscherArrayProperty verticesProp = _optRecord.lookup(EscherProperties.GEOMETRY__VERTICES);
+        EscherArrayProperty verticesProp = getOptRecord().lookup(EscherProperties.GEOMETRY__VERTICES);
         if (null == verticesProp){
             return new int[]{};
         }
@@ -121,7 +125,7 @@ public class HSSFPolygon  extends HSSFShape {
     }
 
     public int[] getYPoints() {
-        EscherArrayProperty verticesProp = _optRecord.lookup(EscherProperties.GEOMETRY__VERTICES);
+        EscherArrayProperty verticesProp = getOptRecord().lookup(EscherProperties.GEOMETRY__VERTICES);
         if (null == verticesProp){
             return new int[]{};
         }
@@ -188,18 +192,18 @@ public class HSSFPolygon  extends HSSFShape {
     }
 
     public int getDrawAreaWidth() {
-        EscherSimpleProperty property = _optRecord.lookup(EscherProperties.GEOMETRY__RIGHT);
+        EscherSimpleProperty property = getOptRecord().lookup(EscherProperties.GEOMETRY__RIGHT);
         return property == null ? 100: property.getPropertyValue();
     }
 
     public int getDrawAreaHeight() {
-        EscherSimpleProperty property = _optRecord.lookup(EscherProperties.GEOMETRY__BOTTOM);
+        EscherSimpleProperty property = getOptRecord().lookup(EscherProperties.GEOMETRY__BOTTOM);
         return property == null ? 100: property.getPropertyValue();
     }
 
     @Override
     void afterInsert(HSSFPatriarch patriarch) {
         EscherAggregate agg = patriarch._getBoundAggregate();
-        agg.associateShapeToObjRecord(_escherContainer.getChildById(EscherClientDataRecord.RECORD_ID), getObjRecord());
+        agg.associateShapeToObjRecord(getEscherContainer().getChildById(EscherClientDataRecord.RECORD_ID), getObjRecord());
     }
 }

src/java/org/apache/poi/hssf/usermodel/HSSFShape.java

@@ -21,6 +21,8 @@ import org.apache.poi.ddf.*;
 import org.apache.poi.hssf.record.CommonObjectDataSubRecord;
 import org.apache.poi.hssf.record.ObjRecord;
 
+import java.util.Iterator;
+
 /**
  * An abstract shape.
  *
@@ -53,9 +55,12 @@ public abstract class HSSFShape {
     HSSFAnchor anchor;
     HSSFPatriarch _patriarch;
 
-    protected final EscherContainerRecord _escherContainer;
+    private final EscherContainerRecord _escherContainer;
-    protected final ObjRecord _objRecord;
+    private final ObjRecord _objRecord;
-    protected final EscherOptRecord _optRecord;
+    private final EscherOptRecord _optRecord;
+    
+    public final static int NO_FILLHITTEST_TRUE = 0x00110000;
+    public final static int NO_FILLHITTEST_FALSE = 0x00010000;
 
     public HSSFShape(EscherContainerRecord spContainer, ObjRecord objRecord) {
         this._escherContainer = spContainer;
@@ -80,6 +85,17 @@ public abstract class HSSFShape {
 
     protected abstract ObjRecord createObjRecord();
 
+    protected abstract void afterRemove(HSSFPatriarch patriarch);
+
+    protected void removeEscherProperty(EscherOptRecord opt, int num){
+        for ( Iterator<EscherProperty> iterator = opt.getEscherProperties().iterator(); iterator.hasNext(); ) {
+            EscherProperty prop = iterator.next();
+            if (prop.getPropertyNumber() == num){
+                iterator.remove();
+            }
+        }
+    }
+
     void setShapeId(int shapeId){
         EscherSpRecord spRecord = _escherContainer.getChildById(EscherSpRecord.RECORD_ID);
         spRecord.setShapeId(shapeId);
@@ -91,8 +107,7 @@ public abstract class HSSFShape {
         return ((EscherSpRecord)_escherContainer.getChildById(EscherSpRecord.RECORD_ID)).getShapeId();
     }
 
-    void afterInsert(HSSFPatriarch patriarch){
+    abstract void afterInsert(HSSFPatriarch patriarch);
-    }
 
     public EscherContainerRecord getEscherContainer() {
         return _escherContainer;
@@ -102,6 +117,10 @@ public abstract class HSSFShape {
         return _objRecord;
     }
 
+    public EscherOptRecord getOptRecord() {
+        return _optRecord;
+    }
+
     /**
      * Gets the parent shape.
      */
@@ -248,26 +267,29 @@ public abstract class HSSFShape {
      */
     public void setLineStyle(int lineStyle) {
         setPropertyValue(new EscherSimpleProperty(EscherProperties.LINESTYLE__LINEDASHING, lineStyle));
+        if (getLineStyle() != HSSFShape.LINESTYLE_SOLID) {
+            setPropertyValue(new EscherSimpleProperty(EscherProperties.LINESTYLE__LINEENDCAPSTYLE, 0));
             if (getLineStyle() == HSSFShape.LINESTYLE_NONE){
                 setPropertyValue(new EscherBoolProperty( EscherProperties.LINESTYLE__NOLINEDRAWDASH, 0x00080000));
             } else {
                 setPropertyValue( new EscherBoolProperty( EscherProperties.LINESTYLE__NOLINEDRAWDASH, 0x00080008));
             }
         }
+    }
 
     /**
      * @return <code>true</code> if this shape is not filled with a color.
      */
     public boolean isNoFill() {
         EscherBoolProperty property = _optRecord.lookup(EscherProperties.FILL__NOFILLHITTEST);
-        return property == null ? NO_FILL_DEFAULT : property.isTrue();
+        return property == null ? NO_FILL_DEFAULT : property.getPropertyValue() == NO_FILLHITTEST_TRUE;
     }
 
     /**
      * Sets whether this shape is filled or transparent.
      */
     public void setNoFill(boolean noFill) {
-        setPropertyValue(new EscherBoolProperty(EscherProperties.FILL__NOFILLHITTEST, noFill ? 1 : 0));
+        setPropertyValue(new EscherBoolProperty(EscherProperties.FILL__NOFILLHITTEST, noFill ? NO_FILLHITTEST_TRUE : NO_FILLHITTEST_FALSE));
     }
 
     protected void setPropertyValue(EscherProperty property){

src/java/org/apache/poi/hssf/usermodel/HSSFShapeFactory.java

@@ -117,7 +117,7 @@ public class HSSFShapeFactory {
                     shape = new HSSFPicture(container, objRecord);
                     break;
                 case CommonObjectDataSubRecord.OBJECT_TYPE_RECTANGLE:
-                    shape = new HSSFSimpleShape(container, objRecord);
+                    shape = new HSSFSimpleShape(container, objRecord, txtRecord);
                     break;
                 case CommonObjectDataSubRecord.OBJECT_TYPE_LINE:
                     shape = new HSSFSimpleShape(container, objRecord);

src/java/org/apache/poi/hssf/usermodel/HSSFShapeGroup.java

@@ -57,7 +57,7 @@ public class HSSFShapeGroup extends HSSFShape implements HSSFShapeContainer {
 
     public HSSFShapeGroup(HSSFShape parent, HSSFAnchor anchor) {
         super(parent, anchor);
-        _spgrRecord = ((EscherContainerRecord)_escherContainer.getChild(0)).getChildById(EscherSpgrRecord.RECORD_ID);
+        _spgrRecord = ((EscherContainerRecord)getEscherContainer().getChild(0)).getChildById(EscherSpgrRecord.RECORD_ID);
     }
 
     @Override
@@ -122,12 +122,16 @@ public class HSSFShapeGroup extends HSSFShape implements HSSFShapeContainer {
         return obj;
     }
 
+    @Override
+    protected void afterRemove(HSSFPatriarch patriarch) {
+    }
+
     private void onCreate(HSSFShape shape){
         if(_patriarch != null && _patriarch._getBoundAggregate().getPatriarch() == null){
             EscherContainerRecord spContainer = shape.getEscherContainer();
             int shapeId = _patriarch.newShapeId();
             shape.setShapeId(shapeId);
-            _escherContainer.addChildRecord(spContainer);
+            getEscherContainer().addChildRecord(spContainer);
             shape.afterInsert(_patriarch);
             EscherSpRecord sp = shape.getEscherContainer().getChildById(EscherSpRecord.RECORD_ID);
             sp.setFlags(sp.getFlags() | EscherSpRecord.FLAG_CHILD);
@@ -172,6 +176,13 @@ public class HSSFShapeGroup extends HSSFShape implements HSSFShapeContainer {
         shape.anchor = anchor;
         shapes.add(shape);
         onCreate(shape);
+        EscherSpRecord sp = shape.getEscherContainer().getChildById(EscherSpRecord.RECORD_ID);
+        if (shape.anchor.isHorizontallyFlipped()){
+            sp.setFlags(sp.getFlags() | EscherSpRecord.FLAG_FLIPHORIZ);
+        }
+        if (shape.anchor.isVerticallyFlipped()){
+            sp.setFlags(sp.getFlags() | EscherSpRecord.FLAG_FLIPVERT);
+        }
         return shape;
     }
 
@@ -220,6 +231,13 @@ public class HSSFShapeGroup extends HSSFShape implements HSSFShapeContainer {
         shape.setPictureIndex(pictureIndex);
         shapes.add(shape);
         onCreate(shape);
+        EscherSpRecord sp = shape.getEscherContainer().getChildById(EscherSpRecord.RECORD_ID);
+        if (shape.anchor.isHorizontallyFlipped()){
+            sp.setFlags(sp.getFlags() | EscherSpRecord.FLAG_FLIPHORIZ);
+        }
+        if (shape.anchor.isVerticallyFlipped()){
+            sp.setFlags(sp.getFlags() | EscherSpRecord.FLAG_FLIPVERT);
+        }
         return shape;
     }
 
@@ -284,22 +302,22 @@ public class HSSFShapeGroup extends HSSFShape implements HSSFShapeContainer {
     @Override
     void afterInsert(HSSFPatriarch patriarch){
         EscherAggregate agg = patriarch._getBoundAggregate();
-        EscherContainerRecord containerRecord = _escherContainer.getChildById(EscherContainerRecord.SP_CONTAINER);
+        EscherContainerRecord containerRecord = getEscherContainer().getChildById(EscherContainerRecord.SP_CONTAINER);
         agg.associateShapeToObjRecord(containerRecord.getChildById(EscherClientDataRecord.RECORD_ID), getObjRecord());
     }
 
     @Override
     void setShapeId(int shapeId){
-        EscherContainerRecord containerRecord = _escherContainer.getChildById(EscherContainerRecord.SP_CONTAINER);
+        EscherContainerRecord containerRecord = getEscherContainer().getChildById(EscherContainerRecord.SP_CONTAINER);
         EscherSpRecord spRecord = containerRecord.getChildById(EscherSpRecord.RECORD_ID);
         spRecord.setShapeId(shapeId);
-        CommonObjectDataSubRecord cod = (CommonObjectDataSubRecord) _objRecord.getSubRecords().get(0);
+        CommonObjectDataSubRecord cod = (CommonObjectDataSubRecord) getObjRecord().getSubRecords().get(0);
         cod.setObjectId((short) (shapeId));
     }
 
     @Override
     int getShapeId(){
-        EscherContainerRecord containerRecord = _escherContainer.getChildById(EscherContainerRecord.SP_CONTAINER);
+        EscherContainerRecord containerRecord = getEscherContainer().getChildById(EscherContainerRecord.SP_CONTAINER);
         return ((EscherSpRecord)containerRecord.getChildById(EscherSpRecord.RECORD_ID)).getShapeId();
     }
 }

src/java/org/apache/poi/hssf/usermodel/HSSFSheet.java

@@ -1358,15 +1358,17 @@ public final class HSSFSheet implements org.apache.poi.ss.usermodel.Sheet {
             //  exist for cells which are null
             if(moveComments) {
                 // This code would get simpler if NoteRecords could be organised by HSSFRow.
-                for(int i=noteRecs.length-1; i>=0; i--) {
+                HSSFPatriarch patriarch = createDrawingPatriarch();
-                    NoteRecord nr = noteRecs[i];
+                for(int i=patriarch.getChildren().size()-1; i>=0; i--){
-                    if (nr.getRow() != rowNum) {
+                    HSSFShape shape = patriarch.getChildren().get(i);
+                    if (!(shape instanceof HSSFComment)){
                         continue;
                     }
-                    HSSFComment comment = getCellComment(rowNum, nr.getColumn());
+                    HSSFComment comment = (HSSFComment) shape;
-                    if (comment != null) {
+                    if (comment.getRow() != rowNum){
-                       comment.setRow(rowNum + n);
+                        continue;
                     }
+                    comment.setRow(rowNum + n);
                 }
             }
         }
@@ -1875,20 +1877,7 @@ public final class HSSFSheet implements org.apache.poi.ss.usermodel.Sheet {
      * @return cell comment or <code>null</code> if not found
      */
      public HSSFComment getCellComment(int row, int column) {
-        // Don't call findCellComment directly, otherwise
+        return findCellComment(row, column);
-        //  two calls to this method will result in two
-        //  new HSSFComment instances, which is bad
-        HSSFRow r = getRow(row);
-        if(r != null) {
-            HSSFCell c = r.getCell(column);
-            if(c != null) {
-                return c.getCellComment();
-            }
-            // No cell, so you will get new
-            //  objects every time, sorry...
-            return HSSFCell.findCellComment(_sheet, row, column);
-        }
-        return null;
     }
 
     public HSSFSheetConditionalFormatting getSheetConditionalFormatting() {
@@ -2009,7 +1998,11 @@ public final class HSSFSheet implements org.apache.poi.ss.usermodel.Sheet {
     }
 
     protected HSSFComment findCellComment(int row, int column) {
-        return lookForComment(getDrawingPatriarch(), row, column);
+        HSSFPatriarch patriarch = getDrawingPatriarch();
+        if (null == patriarch){
+            patriarch = createDrawingPatriarch();
+        }
+        return lookForComment(patriarch, row, column);
     }
 
     private HSSFComment lookForComment(HSSFShapeContainer container, int row, int column){

src/java/org/apache/poi/hssf/usermodel/HSSFSimpleShape.java

@@ -18,11 +18,9 @@
 package org.apache.poi.hssf.usermodel;
 
 import org.apache.poi.ddf.*;
-import org.apache.poi.hssf.record.CommonObjectDataSubRecord;
+import org.apache.poi.hssf.record.*;
-import org.apache.poi.hssf.record.EndSubRecord;
-import org.apache.poi.hssf.record.EscherAggregate;
-import org.apache.poi.hssf.record.ObjRecord;
 import org.apache.poi.hssf.usermodel.drawing.HSSFShapeType;
+import org.apache.poi.ss.usermodel.RichTextString;
 
 import java.util.HashMap;
 import java.util.Map;
@@ -61,6 +59,8 @@ public class HSSFSimpleShape extends HSSFShape
 
     private static final Map <Short, Short> objTypeToShapeType = new HashMap<Short, Short>();
 
+    private TextObjectRecord _textObjectRecord;
+
     static {
         objTypeToShapeType.put(OBJECT_TYPE_RECTANGLE, HSSFShapeType.RECTANGLE.getType());
         objTypeToShapeType.put(OBJECT_TYPE_PICTURE, HSSFShapeType.PICTURE.getType());
@@ -68,6 +68,11 @@ public class HSSFSimpleShape extends HSSFShape
         objTypeToShapeType.put(OBJECT_TYPE_OVAL, HSSFShapeType.OVAL.getType());
     }
 
+    public HSSFSimpleShape(EscherContainerRecord spContainer, ObjRecord objRecord, TextObjectRecord _textObjectRecord) {
+        super(spContainer, objRecord);
+        this._textObjectRecord = _textObjectRecord;
+    }
+
     public HSSFSimpleShape(EscherContainerRecord spContainer, ObjRecord objRecord) {
         super(spContainer, objRecord);
     }
@@ -75,7 +80,18 @@ public class HSSFSimpleShape extends HSSFShape
     public HSSFSimpleShape( HSSFShape parent, HSSFAnchor anchor)
     {
         super( parent, anchor );
-        setShapeType(OBJECT_TYPE_LINE);
+        _textObjectRecord = createTextObjRecord();
+    }
+
+    public TextObjectRecord getTextObjectRecord() {
+        return _textObjectRecord;
+    }
+
+    protected TextObjectRecord createTextObjRecord(){
+        TextObjectRecord obj = new TextObjectRecord();
+        obj.setTextLocked(true);
+        obj.setTextOrientation(TextObjectRecord.TEXT_ORIENTATION_NONE);
+        return obj;
     }
 
     @Override
@@ -87,10 +103,11 @@ public class HSSFSimpleShape extends HSSFShape
         EscherSpRecord sp = new EscherSpRecord();
         sp.setRecordId( EscherSpRecord.RECORD_ID );
         sp.setFlags( EscherSpRecord.FLAG_HAVEANCHOR | EscherSpRecord.FLAG_HASSHAPETYPE );
+        sp.setVersion((short) 0x2);
 
         EscherClientDataRecord clientData = new EscherClientDataRecord();
         clientData.setRecordId( EscherClientDataRecord.RECORD_ID );
-        clientData.setOptions( (short) 0x0000 );
+        clientData.setOptions( (short) (0x0000) );
 
         EscherOptRecord optRecord = new EscherOptRecord();
         optRecord.setEscherProperty(new EscherSimpleProperty(EscherProperties.LINESTYLE__LINEDASHING, LINESTYLE_SOLID));
@@ -98,7 +115,7 @@ public class HSSFSimpleShape extends HSSFShape
 //        optRecord.setEscherProperty(new EscherSimpleProperty(EscherProperties.LINESTYLE__LINEWIDTH, LINEWIDTH_DEFAULT));
         optRecord.setEscherProperty(new EscherRGBProperty(EscherProperties.FILL__FILLCOLOR, FILL__FILLCOLOR_DEFAULT));
         optRecord.setEscherProperty(new EscherRGBProperty(EscherProperties.LINESTYLE__COLOR, LINESTYLE__COLOR_DEFAULT));
-        optRecord.setEscherProperty(new EscherBoolProperty(EscherProperties.FILL__NOFILLHITTEST, 0));
+        optRecord.setEscherProperty(new EscherBoolProperty(EscherProperties.FILL__NOFILLHITTEST, NO_FILLHITTEST_FALSE));
         optRecord.setEscherProperty( new EscherBoolProperty( EscherProperties.LINESTYLE__NOLINEDRAWDASH, 0x00080008));
 
         optRecord.setEscherProperty( new EscherShapePathProperty( EscherProperties.GEOMETRY__SHAPEPATH, EscherShapePathProperty.COMPLEX ) );
@@ -127,10 +144,46 @@ public class HSSFSimpleShape extends HSSFShape
         return obj;
     }
 
+    @Override
+    protected void afterRemove(HSSFPatriarch patriarch) {
+    }
+
+    /**
+     * @return the rich text string for this textbox.
+     */
+    public HSSFRichTextString getString() {
+        return _textObjectRecord.getStr();
+    }
+
+    /**
+     * @param string Sets the rich text string used by this object.
+     */
+    public void setString(RichTextString string) {
+        HSSFRichTextString rtr = (HSSFRichTextString) string;
+        // If font is not set we must set the default one
+        if (rtr.numFormattingRuns() == 0) rtr.applyFont((short) 0);
+        EscherTextboxRecord textbox = getEscherContainer().getChildById(EscherTextboxRecord.RECORD_ID);
+        if (string.getString()!= null && !string.getString().equals("")){
+            if (null == textbox){
+                EscherTextboxRecord escherTextbox = new EscherTextboxRecord();
+                escherTextbox.setRecordId(EscherTextboxRecord.RECORD_ID);
+                escherTextbox.setOptions((short) 0x0000);
+                getEscherContainer().addChildRecord(escherTextbox);
+                _patriarch._getBoundAggregate().associateShapeToObjRecord(getEscherContainer().getChildById(EscherTextboxRecord.RECORD_ID), getTextObjectRecord());
+            }
+        } else {
+            if (null != textbox){
+                getEscherContainer().removeChildRecord(textbox);
+                _patriarch._getBoundAggregate().removeShapeToObjRecord(textbox);
+            }
+        }
+        _textObjectRecord.setStr(rtr);
+    }
+
     @Override
     void afterInsert(HSSFPatriarch patriarch){
         EscherAggregate agg = patriarch._getBoundAggregate();
-        agg.associateShapeToObjRecord(_escherContainer.getChildById(EscherClientDataRecord.RECORD_ID), getObjRecord());
+        agg.associateShapeToObjRecord(getEscherContainer().getChildById(EscherClientDataRecord.RECORD_ID), getObjRecord());
     }
 
 
@@ -145,7 +198,7 @@ public class HSSFSimpleShape extends HSSFShape
      * @see #OBJECT_TYPE_COMMENT
      */
     public int getShapeType() {
-        CommonObjectDataSubRecord cod = (CommonObjectDataSubRecord) _objRecord.getSubRecords().get(0);
+        CommonObjectDataSubRecord cod = (CommonObjectDataSubRecord) getObjRecord().getSubRecords().get(0);
         return cod.getObjectType();
     }
 
@@ -161,9 +214,9 @@ public class HSSFSimpleShape extends HSSFShape
      * @see #OBJECT_TYPE_COMMENT
      */
     public void setShapeType( int shapeType ){
-        CommonObjectDataSubRecord cod = (CommonObjectDataSubRecord) _objRecord.getSubRecords().get(0);
+        CommonObjectDataSubRecord cod = (CommonObjectDataSubRecord) getObjRecord().getSubRecords().get(0);
         cod.setObjectType((short) shapeType);
-        EscherSpRecord spRecord = _escherContainer.getChildById(EscherSpRecord.RECORD_ID);
+        EscherSpRecord spRecord = getEscherContainer().getChildById(EscherSpRecord.RECORD_ID);
         if (null == objTypeToShapeType.get((short)shapeType)){
             System.out.println("Unknown shape type: "+shapeType);
             return;

src/java/org/apache/poi/hssf/usermodel/HSSFTextbox.java

@@ -47,11 +47,8 @@ public class HSSFTextbox extends HSSFSimpleShape {
     public final static short VERTICAL_ALIGNMENT_JUSTIFY = 4;
     public final static short VERTICAL_ALIGNMENT_DISTRIBUTED = 7;
 
-    protected TextObjectRecord _textObjectRecord;
-
     public HSSFTextbox(EscherContainerRecord spContainer, ObjRecord objRecord, TextObjectRecord textObjectRecord) {
-        super(spContainer, objRecord);
+        super(spContainer, objRecord, textObjectRecord);
-        this._textObjectRecord = textObjectRecord;
     }
 
     HSSFRichTextString string = new HSSFRichTextString("");
@@ -64,25 +61,16 @@ public class HSSFTextbox extends HSSFSimpleShape {
      */
     public HSSFTextbox(HSSFShape parent, HSSFAnchor anchor) {
         super(parent, anchor);
-        _textObjectRecord = createTextObjRecord();
         setHorizontalAlignment(HORIZONTAL_ALIGNMENT_LEFT);
         setVerticalAlignment(VERTICAL_ALIGNMENT_TOP);
         setString(new HSSFRichTextString(""));
-
-    }
-
-    protected TextObjectRecord createTextObjRecord(){
-        TextObjectRecord obj = new TextObjectRecord();
-        obj.setTextLocked(true);
-        obj.setTextOrientation(TextObjectRecord.TEXT_ORIENTATION_NONE);
-        return obj;
     }
 
     @Override
     protected ObjRecord createObjRecord() {
         ObjRecord obj = new ObjRecord();
         CommonObjectDataSubRecord c = new CommonObjectDataSubRecord();
-        c.setObjectType(OBJECT_TYPE_TEXT);
+        c.setObjectType(HSSFTextbox.OBJECT_TYPE_TEXT);
         c.setLocked( true );
         c.setPrintable( true );
         c.setAutofill( true );
@@ -90,8 +78,7 @@ public class HSSFTextbox extends HSSFSimpleShape {
         EndSubRecord e = new EndSubRecord();
         obj.addSubRecord( c );
         obj.addSubRecord( e );
-        return obj;
+        return obj;    }
-    }
 
     @Override
     protected EscherContainerRecord createSpContainer() {
@@ -123,7 +110,7 @@ public class HSSFTextbox extends HSSFSimpleShape {
         opt.setEscherProperty(new EscherSimpleProperty(EscherProperties.LINESTYLE__LINEWIDTH, LINEWIDTH_DEFAULT));
         opt.setEscherProperty(new EscherRGBProperty(EscherProperties.FILL__FILLCOLOR, FILL__FILLCOLOR_DEFAULT));
         opt.setEscherProperty(new EscherRGBProperty(EscherProperties.LINESTYLE__COLOR, LINESTYLE__COLOR_DEFAULT));
-        opt.setEscherProperty(new EscherBoolProperty(EscherProperties.FILL__NOFILLHITTEST, 1));
+        opt.setEscherProperty(new EscherBoolProperty(EscherProperties.FILL__NOFILLHITTEST, NO_FILLHITTEST_FALSE));
         opt.setEscherProperty(new EscherBoolProperty( EscherProperties.GROUPSHAPE__PRINT, 0x080000));
 
         EscherRecord anchor = getAnchor().getEscherAnchor();
@@ -142,34 +129,25 @@ public class HSSFTextbox extends HSSFSimpleShape {
     }
 
     @Override
-    void afterInsert(HSSFPatriarch patriarch){
-        EscherAggregate agg = patriarch._getBoundAggregate();
-        agg.associateShapeToObjRecord(_escherContainer.getChildById(EscherClientDataRecord.RECORD_ID), getObjRecord());
-        agg.associateShapeToObjRecord(_escherContainer.getChildById(EscherTextboxRecord.RECORD_ID), getTextObjectRecord());
-    }
-
-    /**
-     * @return the rich text string for this textbox.
-     */
-    public HSSFRichTextString getString() {
-        return _textObjectRecord.getStr();
-    }
-
-    /**
-     * @param string Sets the rich text string used by this object.
-     */
     public void setString(RichTextString string) {
         HSSFRichTextString rtr = (HSSFRichTextString) string;
         // If font is not set we must set the default one
         if (rtr.numFormattingRuns() == 0) rtr.applyFont((short) 0);
-        _textObjectRecord.setStr(rtr);
+        getTextObjectRecord().setStr(rtr);
+    }
+
+    @Override
+    void afterInsert(HSSFPatriarch patriarch){
+        EscherAggregate agg = patriarch._getBoundAggregate();
+        agg.associateShapeToObjRecord(getEscherContainer().getChildById(EscherClientDataRecord.RECORD_ID), getObjRecord());
+        agg.associateShapeToObjRecord(getEscherContainer().getChildById(EscherTextboxRecord.RECORD_ID), getTextObjectRecord());
     }
 
     /**
      * @return Returns the left margin within the textbox.
      */
     public int getMarginLeft() {
-        EscherSimpleProperty property = _optRecord.lookup(EscherProperties.TEXT__TEXTLEFT);
+        EscherSimpleProperty property = getOptRecord().lookup(EscherProperties.TEXT__TEXTLEFT);
         return property == null ? 0: property.getPropertyValue();
     }
 
@@ -184,7 +162,7 @@ public class HSSFTextbox extends HSSFSimpleShape {
      * @return returns the right margin within the textbox.
      */
     public int getMarginRight() {
-        EscherSimpleProperty property = _optRecord.lookup(EscherProperties.TEXT__TEXTRIGHT);
+        EscherSimpleProperty property = getOptRecord().lookup(EscherProperties.TEXT__TEXTRIGHT);
         return property == null ? 0: property.getPropertyValue();
     }
 
@@ -199,7 +177,7 @@ public class HSSFTextbox extends HSSFSimpleShape {
      * @return returns the top margin within the textbox.
      */
     public int getMarginTop() {
-        EscherSimpleProperty property = _optRecord.lookup(EscherProperties.TEXT__TEXTTOP);
+        EscherSimpleProperty property = getOptRecord().lookup(EscherProperties.TEXT__TEXTTOP);
         return property == null ? 0: property.getPropertyValue();
     }
 
@@ -214,7 +192,7 @@ public class HSSFTextbox extends HSSFSimpleShape {
      * Gets the bottom margin within the textbox.
      */
     public int getMarginBottom() {
-        EscherSimpleProperty property = _optRecord.lookup(EscherProperties.TEXT__TEXTBOTTOM);
+        EscherSimpleProperty property = getOptRecord().lookup(EscherProperties.TEXT__TEXTBOTTOM);
         return property == null ? 0: property.getPropertyValue();
     }
 
@@ -229,34 +207,32 @@ public class HSSFTextbox extends HSSFSimpleShape {
      * Gets the horizontal alignment.
      */
     public short getHorizontalAlignment() {
-        return (short) _textObjectRecord.getHorizontalTextAlignment();
+        return (short) getTextObjectRecord().getHorizontalTextAlignment();
     }
 
     /**
      * Sets the horizontal alignment.
      */
     public void setHorizontalAlignment(short align) {
-        _textObjectRecord.setHorizontalTextAlignment(align);
+        getTextObjectRecord().setHorizontalTextAlignment(align);
     }
 
     /**
      * Gets the vertical alignment.
      */
     public short getVerticalAlignment() {
-        return (short) _textObjectRecord.getVerticalTextAlignment();
+        return (short) getTextObjectRecord().getVerticalTextAlignment();
     }
 
     /**
      * Sets the vertical alignment.
      */
     public void setVerticalAlignment(short align) {
-        _textObjectRecord.setVerticalTextAlignment(align);
+        getTextObjectRecord().setVerticalTextAlignment(align);
-    }
-
-    public TextObjectRecord getTextObjectRecord() {
-        return _textObjectRecord;
     }
 
     @Override
-    public void setShapeType(int shapeType) {/**DO NOTHING**/}
+    public void setShapeType(int shapeType) {
+        throw new IllegalStateException("Shape type can not be changed in "+this.getClass().getSimpleName());
+    }
 }

src/java/org/apache/poi/hssf/usermodel/HSSFUnknownShape.java

@@ -33,11 +33,19 @@ public class HSSFUnknownShape extends HSSFShape {
 
     @Override
     protected EscherContainerRecord createSpContainer() {
-        return null;  //To change body of implemented methods use File | Settings | File Templates.
+        return null;
     }
 
     @Override
     protected ObjRecord createObjRecord() {
-        return null;  //To change body of implemented methods use File | Settings | File Templates.
+        return null;
+    }
+
+    @Override
+    protected void afterRemove(HSSFPatriarch patriarch) {
+    }
+
+    @Override
+    void afterInsert(HSSFPatriarch patriarch) {
     }
 }

src/java/org/apache/poi/hssf/usermodel/HSSFWorkbook.java

@@ -1614,7 +1614,7 @@ public final class HSSFWorkbook extends POIDocument implements org.apache.poi.ss
         r.setUid( uid );
         r.setTag( (short) 0xFF );
         r.setSize( pictureData.length + 25 );
-        r.setRef( 1 );
+        r.setRef( 0 );
         r.setOffset( 0 );
         r.setBlipRecord( blipRecord );
 

src/testcases/org/apache/poi/hssf/model/TestDrawingShapes.java

@@ -47,7 +47,8 @@ public class TestDrawingShapes extends TestCase {
     }
 
     public void testHSSFShapeCompatibility() {
-        HSSFShape shape = new HSSFSimpleShape(null, new HSSFClientAnchor());
+        HSSFSimpleShape shape = new HSSFSimpleShape(null, new HSSFClientAnchor());
+        shape.setShapeType(HSSFSimpleShape.OBJECT_TYPE_LINE);
         assertEquals(0x08000040, shape.getLineStyleColor());
         assertEquals(0x08000009, shape.getFillColor());
         assertEquals(HSSFShape.LINEWIDTH_DEFAULT, shape.getLineWidth());
@@ -80,7 +81,7 @@ public class TestDrawingShapes extends TestCase {
         HSSFPicture picture = new HSSFPicture(null, new HSSFClientAnchor());
         assertEquals(picture.getLineWidth(), HSSFShape.LINEWIDTH_DEFAULT);
         assertEquals(picture.getFillColor(), HSSFShape.FILL__FILLCOLOR_DEFAULT);
-        assertEquals(picture.getLineStyle(), HSSFShape.LINESTYLE_SOLID);
+        assertEquals(picture.getLineStyle(), HSSFShape.LINESTYLE_NONE);
         assertEquals(picture.getLineStyleColor(), HSSFShape.LINESTYLE__COLOR_DEFAULT);
         assertEquals(picture.isNoFill(), false);
         assertEquals(picture.getPictureIndex(), -1);//not set yet
@@ -131,8 +132,10 @@ public class TestDrawingShapes extends TestCase {
         assertEquals(10, rectangle.getLineStyle());
         rectangle.setLineStyleColor(1111);
         rectangle.setNoFill(true);
+        rectangle.setString(new HSSFRichTextString("teeeest"));
         assertEquals(rectangle.getLineStyleColor(), 1111);
         assertEquals(rectangle.isNoFill(), true);
+        assertEquals(rectangle.getString().getString(), "teeeest");
 
         wb = HSSFTestDataSamples.writeOutAndReadBack(wb);
         sheet = wb.getSheetAt(0);
@@ -149,6 +152,7 @@ public class TestDrawingShapes extends TestCase {
         assertEquals(rectangle2.getLineStyleColor(), 1111);
         assertEquals(rectangle2.getFillColor(), 777);
         assertEquals(rectangle2.isNoFill(), true);
+        assertEquals(rectangle2.getString().getString(), "teeeest");
 
         rectangle2.setFillColor(3333);
         rectangle2.setLineStyle(9);
@@ -159,6 +163,7 @@ public class TestDrawingShapes extends TestCase {
         rectangle2.getAnchor().setDx2(3);
         rectangle2.getAnchor().setDy1(4);
         rectangle2.getAnchor().setDy2(5);
+        rectangle2.setString(new HSSFRichTextString("test22"));
 
         wb = HSSFTestDataSamples.writeOutAndReadBack(wb);
         sheet = wb.getSheetAt(0);
@@ -175,6 +180,7 @@ public class TestDrawingShapes extends TestCase {
         assertEquals(rectangle2.getAnchor().getDy1(), 4);
         assertEquals(rectangle2.getAnchor().getDy2(), 5);
         assertEquals(rectangle2.isNoFill(), false);
+        assertEquals(rectangle2.getString().getString(), "test22");
 
         HSSFSimpleShape rect3 = drawing.createSimpleShape(new HSSFClientAnchor());
         rect3.setShapeType(HSSFSimpleShape.OBJECT_TYPE_RECTANGLE);
@@ -196,7 +202,7 @@ public class TestDrawingShapes extends TestCase {
         assertEquals(picture.getFillColor(), 0x5DC943);
         assertEquals(picture.getLineWidth(), HSSFShape.LINEWIDTH_DEFAULT);
         assertEquals(picture.getLineStyle(), HSSFShape.LINESTYLE_DEFAULT);
-        assertEquals(picture.isNoFill(), true);
+        assertEquals(picture.isNoFill(), false);
 
         picture.setPictureIndex(2);
         assertEquals(picture.getPictureIndex(), 2);
@@ -211,11 +217,12 @@ public class TestDrawingShapes extends TestCase {
         assertEquals(1, drawing.getChildren().size());
 
         HSSFSimpleShape shape = (HSSFSimpleShape) drawing.getChildren().get(0);
-        assertEquals(shape.isNoFill(), true);
+        assertEquals(shape.isNoFill(), false);
         assertEquals(shape.getLineStyle(), HSSFShape.LINESTYLE_DASHDOTGEL);
         assertEquals(shape.getLineStyleColor(), 0x616161);
         assertEquals(HexDump.toHex(shape.getFillColor()), shape.getFillColor(), 0x2CE03D);
         assertEquals(shape.getLineWidth(), HSSFShape.LINEWIDTH_ONE_PT * 2);
+        assertEquals(shape.getString().getString(), "POItest");
     }
 
     public void testShapeIds() {

src/testcases/org/apache/poi/hssf/model/TestHSSFAnchor.java

@@ -375,4 +375,38 @@ public class TestHSSFAnchor extends TestCase {
         childAnchor2.setDy2(3);
         assertEquals(childAnchor1, childAnchor2);
     }
+
+    public void testFlipped(){
+        HSSFChildAnchor child = new HSSFChildAnchor(2,2,1,1);
+        assertEquals(child.isHorizontallyFlipped(), true);
+        assertEquals(child.isVerticallyFlipped(), true);
+        assertEquals(child.getDx1(), 1);
+        assertEquals(child.getDx2(), 2);
+        assertEquals(child.getDy1(), 1);
+        assertEquals(child.getDy2(), 2);
+
+        child = new HSSFChildAnchor(3,3,4,4);
+        assertEquals(child.isHorizontallyFlipped(), false);
+        assertEquals(child.isVerticallyFlipped(), false);
+        assertEquals(child.getDx1(), 3);
+        assertEquals(child.getDx2(), 4);
+        assertEquals(child.getDy1(), 3);
+        assertEquals(child.getDy2(), 4);
+
+        HSSFClientAnchor client = new HSSFClientAnchor(1,1,1,1, (short)4,4,(short)3,3);
+        assertEquals(client.isVerticallyFlipped(), true);
+        assertEquals(client.isHorizontallyFlipped(), true);
+        assertEquals(client.getCol1(), 3);
+        assertEquals(client.getCol2(), 4);
+        assertEquals(client.getRow1(), 3);
+        assertEquals(client.getRow2(), 4);
+
+        client = new HSSFClientAnchor(1,1,1,1, (short)5,5,(short)6,6);
+        assertEquals(client.isVerticallyFlipped(), false);
+        assertEquals(client.isHorizontallyFlipped(), false);
+        assertEquals(client.getCol1(), 5);
+        assertEquals(client.getCol2(), 6);
+        assertEquals(client.getRow1(), 5);
+        assertEquals(client.getRow2(), 6);
+    }
 }

src/testcases/org/apache/poi/hssf/usermodel/HSSFTestHelper.java

@@ -83,7 +83,7 @@ public class HSSFTestHelper {
     }
 
     public static EscherOptRecord getOptRecord(HSSFShape shape){
-        return shape._optRecord;
+        return shape.getOptRecord();
     }
 
     public static void convertHSSFGroup(HSSFShapeGroup shape, EscherContainerRecord escherParent, Map shapeToObj){

src/testcases/org/apache/poi/hssf/usermodel/TestComment.java

@@ -212,8 +212,7 @@ public class TestComment extends TestCase {
 
         HSSFSimpleShape shape = patriarch.createSimpleShape(new HSSFClientAnchor());
 
-        //6 properties of HSSFShape + 8 of HSSFTextbox
+        assertEquals(comment.getOptRecord().getEscherProperties().size(), 10);
-        assertEquals(comment._optRecord.getEscherProperties().size(), 14);
     }
 
     public void testShapeId(){

src/testcases/org/apache/poi/hssf/usermodel/TestHSSFPicture.java

@@ -17,8 +17,10 @@
 
 package org.apache.poi.hssf.usermodel;
 
+import org.apache.poi.ddf.EscherBSERecord;
 import org.apache.poi.hssf.HSSFTestDataSamples;
 import org.apache.poi.hssf.HSSFITestDataProvider;
+import org.apache.poi.hssf.model.InternalSheet;
 import org.apache.poi.ss.usermodel.BaseTestPicture;
 import org.apache.poi.ss.usermodel.PictureData;
 import org.apache.poi.ss.usermodel.Workbook;
@@ -149,4 +151,63 @@ public final class TestHSSFPicture extends BaseTestPicture {
         assertTrue(Arrays.equals(data4, ((HSSFPicture)dr.getChildren().get(3)).getPictureData().getData()));
     }
 
+    public void testBSEPictureRef(){
+        HSSFWorkbook wb = new HSSFWorkbook();
+
+        HSSFSheet sh = wb.createSheet("Pictures");
+        HSSFPatriarch dr = sh.createDrawingPatriarch();
+        HSSFClientAnchor anchor = new HSSFClientAnchor();
+
+        InternalSheet ish = HSSFTestHelper.getSheetForTest(sh);
+
+        //register a picture
+        byte[] data1 = new byte[]{1, 2, 3};
+        int idx1 = wb.addPicture(data1, Workbook.PICTURE_TYPE_JPEG);
+        assertEquals(1, idx1);
+        HSSFPicture p1 = dr.createPicture(anchor, idx1);
+
+        EscherBSERecord bse = wb.getWorkbook().getBSERecord(idx1);
+
+        assertEquals(bse.getRef(), 1);
+        dr.createPicture(new HSSFClientAnchor(), idx1);
+        assertEquals(bse.getRef(), 2);
+
+        HSSFShapeGroup gr = dr.createGroup(new HSSFClientAnchor());
+        gr.createPicture(new HSSFChildAnchor(), idx1);
+        assertEquals(bse.getRef(), 3);
+    }
+
+    public void testReadExistingImage(){
+        HSSFWorkbook wb = HSSFTestDataSamples.openSampleWorkbook("drawings.xls");
+        HSSFSheet sheet = wb.getSheet("picture");
+        HSSFPatriarch drawing = sheet.getDrawingPatriarch();
+        assertEquals(1, drawing.getChildren().size());
+
+        HSSFPicture picture = (HSSFPicture) drawing.getChildren().get(0);
+        assertEquals(picture.getAdditionalData(), "test");
+    }
+
+    public void testSetGetProperties(){
+        HSSFWorkbook wb = new HSSFWorkbook();
+
+        HSSFSheet sh = wb.createSheet("Pictures");
+        HSSFPatriarch dr = sh.createDrawingPatriarch();
+        HSSFClientAnchor anchor = new HSSFClientAnchor();
+
+        //register a picture
+        byte[] data1 = new byte[]{1, 2, 3};
+        int idx1 = wb.addPicture(data1, Workbook.PICTURE_TYPE_JPEG);
+        HSSFPicture p1 = dr.createPicture(anchor, idx1);
+
+        assertEquals(p1.getAdditionalData(), "");
+        p1.setAdditionalData("aaa");
+        assertEquals(p1.getAdditionalData(), "aaa");
+
+        wb = HSSFTestDataSamples.writeOutAndReadBack(wb);
+        sh = wb.getSheet("Pictures");
+        dr = sh.getDrawingPatriarch();
+
+        p1 = (HSSFPicture) dr.getChildren().get(0);
+        assertEquals(p1.getAdditionalData(), "aaa");
+    }
 }

src/testcases/org/apache/poi/hssf/usermodel/TestPolygon.java

@@ -73,7 +73,7 @@ public class TestPolygon extends TestCase{
 
         PolygonShape polygonShape = HSSFTestModelHelper.createPolygonShape(0, polygon);
 
-        EscherArrayProperty verticesProp1 = polygon._optRecord.lookup(EscherProperties.GEOMETRY__VERTICES);
+        EscherArrayProperty verticesProp1 = polygon.getOptRecord().lookup(EscherProperties.GEOMETRY__VERTICES);
         EscherArrayProperty verticesProp2 = ((EscherOptRecord)polygonShape.getSpContainer().getChildById(EscherOptRecord.RECORD_ID))
                 .lookup(EscherProperties.GEOMETRY__VERTICES);
 
@@ -85,7 +85,7 @@ public class TestPolygon extends TestCase{
         assertTrue(Arrays.equals(polygon.getYPoints(), new int[]{4, 5, 6}));
 
         polygonShape = HSSFTestModelHelper.createPolygonShape(0, polygon);
-        verticesProp1 = polygon._optRecord.lookup(EscherProperties.GEOMETRY__VERTICES);
+        verticesProp1 = polygon.getOptRecord().lookup(EscherProperties.GEOMETRY__VERTICES);
         verticesProp2 = ((EscherOptRecord)polygonShape.getSpContainer().getChildById(EscherOptRecord.RECORD_ID))
                 .lookup(EscherProperties.GEOMETRY__VERTICES);