diff --git a/src/java/org/apache/poi/sl/usermodel/Shape.java b/src/java/org/apache/poi/sl/usermodel/Shape.java index a8ed1cfaaa..4929f48bb8 100644 --- a/src/java/org/apache/poi/sl/usermodel/Shape.java +++ b/src/java/org/apache/poi/sl/usermodel/Shape.java @@ -54,4 +54,18 @@ public interface Shape< * if null, the bounds of the shape are used. */ void draw(Graphics2D graphics, Rectangle2D bounds); + + + /** + * Returns a unique identifier for this shape within the current slide. + * This ID may be used to assist in uniquely identifying this object so that it can + * be referred to by other parts of the document. + *

+ * If multiple objects within the same slide share the same id attribute value, + * then the document shall be considered non-conformant. + *

+ * + * @return unique id of this shape + */ + int getShapeId(); } diff --git a/src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFAutoShape.java b/src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFAutoShape.java index 9dfa62fa04..708dc9b745 100644 --- a/src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFAutoShape.java +++ b/src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFAutoShape.java @@ -67,7 +67,7 @@ public class XSLFAutoShape extends XSLFTextShape CTShapeNonVisual nvSpPr = ct.addNewNvSpPr(); CTNonVisualDrawingProps cnv = nvSpPr.addNewCNvPr(); cnv.setName("AutoShape " + shapeId); - cnv.setId(shapeId + 1); + cnv.setId(shapeId); nvSpPr.addNewCNvSpPr(); nvSpPr.addNewNvPr(); CTShapeProperties spPr = ct.addNewSpPr(); diff --git a/src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFConnectorShape.java b/src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFConnectorShape.java index c7bf948a91..346131c79d 100644 --- a/src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFConnectorShape.java +++ b/src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFConnectorShape.java @@ -51,7 +51,7 @@ public class XSLFConnectorShape extends XSLFSimpleShape CTConnectorNonVisual nvSpPr = ct.addNewNvCxnSpPr(); CTNonVisualDrawingProps cnv = nvSpPr.addNewCNvPr(); cnv.setName("Connector " + shapeId); - cnv.setId(shapeId + 1); + cnv.setId(shapeId); nvSpPr.addNewCNvCxnSpPr(); nvSpPr.addNewNvPr(); CTShapeProperties spPr = ct.addNewSpPr(); diff --git a/src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFDrawing.java b/src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFDrawing.java index 42e375ae09..f4cd0d8e1c 100644 --- a/src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFDrawing.java +++ b/src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFDrawing.java @@ -18,26 +18,13 @@ package org.apache.poi.xslf.usermodel; import java.awt.Color; import java.awt.geom.Rectangle2D; -import java.io.ByteArrayOutputStream; -import java.io.IOException; -import java.io.OutputStream; -import org.apache.poi.POIXMLDocumentPart.RelationPart; -import org.apache.poi.hpsf.ClassID; -import org.apache.poi.openxml4j.exceptions.InvalidFormatException; -import org.apache.poi.openxml4j.opc.OPCPackage; -import org.apache.poi.openxml4j.opc.PackagePart; -import org.apache.poi.openxml4j.opc.PackagePartName; -import org.apache.poi.openxml4j.opc.PackageRelationship; -import org.apache.poi.openxml4j.opc.PackagingURIHelper; -import org.apache.poi.poifs.filesystem.POIFSFileSystem; import org.apache.poi.util.Beta; import org.apache.xmlbeans.XmlObject; import org.openxmlformats.schemas.drawingml.x2006.main.CTNonVisualDrawingProps; import org.openxmlformats.schemas.presentationml.x2006.main.CTConnector; import org.openxmlformats.schemas.presentationml.x2006.main.CTGraphicalObjectFrame; import org.openxmlformats.schemas.presentationml.x2006.main.CTGroupShape; -import org.openxmlformats.schemas.presentationml.x2006.main.CTOleObject; import org.openxmlformats.schemas.presentationml.x2006.main.CTPicture; import org.openxmlformats.schemas.presentationml.x2006.main.CTShape; @@ -45,7 +32,6 @@ import org.openxmlformats.schemas.presentationml.x2006.main.CTShape; @Beta public class XSLFDrawing { private XSLFSheet _sheet; - private int _shapeId = 1; private CTGroupShape _spTree; /*package*/ XSLFDrawing(XSLFSheet sheet, CTGroupShape spTree){ @@ -58,14 +44,14 @@ public class XSLFDrawing { // ignore them for now if (o instanceof CTNonVisualDrawingProps) { CTNonVisualDrawingProps p = (CTNonVisualDrawingProps)o; - _shapeId = (int)Math.max(_shapeId, p.getId()); + sheet.registerShapeId((int)p.getId()); } } } public XSLFAutoShape createAutoShape(){ CTShape sp = _spTree.addNewSp(); - sp.set(XSLFAutoShape.prototype(_shapeId++)); + sp.set(XSLFAutoShape.prototype(_sheet.allocateShapeId())); XSLFAutoShape shape = new XSLFAutoShape(sp, _sheet); shape.setAnchor(new Rectangle2D.Double()); return shape; @@ -73,7 +59,7 @@ public class XSLFDrawing { public XSLFFreeformShape createFreeform(){ CTShape sp = _spTree.addNewSp(); - sp.set(XSLFFreeformShape.prototype(_shapeId++)); + sp.set(XSLFFreeformShape.prototype(_sheet.allocateShapeId())); XSLFFreeformShape shape = new XSLFFreeformShape(sp, _sheet); shape.setAnchor(new Rectangle2D.Double()); return shape; @@ -81,7 +67,7 @@ public class XSLFDrawing { public XSLFTextBox createTextBox(){ CTShape sp = _spTree.addNewSp(); - sp.set(XSLFTextBox.prototype(_shapeId++)); + sp.set(XSLFTextBox.prototype(_sheet.allocateShapeId())); XSLFTextBox shape = new XSLFTextBox(sp, _sheet); shape.setAnchor(new Rectangle2D.Double()); return shape; @@ -89,7 +75,7 @@ public class XSLFDrawing { public XSLFConnectorShape createConnector(){ CTConnector sp = _spTree.addNewCxnSp(); - sp.set(XSLFConnectorShape.prototype(_shapeId++)); + sp.set(XSLFConnectorShape.prototype(_sheet.allocateShapeId())); XSLFConnectorShape shape = new XSLFConnectorShape(sp, _sheet); shape.setAnchor(new Rectangle2D.Double()); shape.setLineColor(Color.black); @@ -98,33 +84,33 @@ public class XSLFDrawing { } public XSLFGroupShape createGroup(){ - CTGroupShape obj = _spTree.addNewGrpSp(); - obj.set(XSLFGroupShape.prototype(_shapeId++)); - XSLFGroupShape shape = new XSLFGroupShape(obj, _sheet); + CTGroupShape sp = _spTree.addNewGrpSp(); + sp.set(XSLFGroupShape.prototype(_sheet.allocateShapeId())); + XSLFGroupShape shape = new XSLFGroupShape(sp, _sheet); shape.setAnchor(new Rectangle2D.Double()); return shape; } public XSLFPictureShape createPicture(String rel){ - CTPicture obj = _spTree.addNewPic(); - obj.set(XSLFPictureShape.prototype(_shapeId++, rel)); - XSLFPictureShape shape = new XSLFPictureShape(obj, _sheet); + CTPicture sp = _spTree.addNewPic(); + sp.set(XSLFPictureShape.prototype(_sheet.allocateShapeId(), rel)); + XSLFPictureShape shape = new XSLFPictureShape(sp, _sheet); shape.setAnchor(new Rectangle2D.Double()); return shape; } public XSLFTable createTable(){ - CTGraphicalObjectFrame obj = _spTree.addNewGraphicFrame(); - obj.set(XSLFTable.prototype(_shapeId++)); - XSLFTable shape = new XSLFTable(obj, _sheet); + CTGraphicalObjectFrame sp = _spTree.addNewGraphicFrame(); + sp.set(XSLFTable.prototype(_sheet.allocateShapeId())); + XSLFTable shape = new XSLFTable(sp, _sheet); shape.setAnchor(new Rectangle2D.Double()); return shape; } public XSLFObjectShape createOleShape(String pictureRel) { - CTGraphicalObjectFrame obj = _spTree.addNewGraphicFrame(); - obj.set(XSLFObjectShape.prototype(_shapeId++, pictureRel)); - XSLFObjectShape shape = new XSLFObjectShape(obj, _sheet); + CTGraphicalObjectFrame sp = _spTree.addNewGraphicFrame(); + sp.set(XSLFObjectShape.prototype(_sheet.allocateShapeId(), pictureRel)); + XSLFObjectShape shape = new XSLFObjectShape(sp, _sheet); shape.setAnchor(new Rectangle2D.Double()); return shape; } diff --git a/src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFFreeformShape.java b/src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFFreeformShape.java index 92ec316a69..54acd2b7ba 100644 --- a/src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFFreeformShape.java +++ b/src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFFreeformShape.java @@ -232,7 +232,7 @@ public class XSLFFreeformShape extends XSLFAutoShape CTShapeNonVisual nvSpPr = ct.addNewNvSpPr(); CTNonVisualDrawingProps cnv = nvSpPr.addNewCNvPr(); cnv.setName("Freeform " + shapeId); - cnv.setId(shapeId + 1); + cnv.setId(shapeId); nvSpPr.addNewCNvSpPr(); nvSpPr.addNewNvPr(); CTShapeProperties spPr = ct.addNewSpPr(); diff --git a/src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFGroupShape.java b/src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFGroupShape.java index adcdad2147..bcab1dabae 100644 --- a/src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFGroupShape.java +++ b/src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFGroupShape.java @@ -174,9 +174,12 @@ implements XSLFShapeContainer, GroupShape { public boolean removeShape(XSLFShape xShape) { XmlObject obj = xShape.getXmlObject(); CTGroupShape grpSp = (CTGroupShape)getXmlObject(); + getSheet().deregisterShapeId(xShape.getShapeId()); if(obj instanceof CTShape){ grpSp.getSpList().remove(obj); } else if (obj instanceof CTGroupShape){ + XSLFGroupShape gs = (XSLFGroupShape)xShape; + new ArrayList<>(gs.getShapes()).forEach(gs::removeShape); grpSp.getGrpSpList().remove(obj); } else if (obj instanceof CTConnector){ grpSp.getCxnSpList().remove(obj); @@ -203,7 +206,7 @@ implements XSLFShapeContainer, GroupShape { CTGroupShapeNonVisual nvSpPr = ct.addNewNvGrpSpPr(); CTNonVisualDrawingProps cnv = nvSpPr.addNewCNvPr(); cnv.setName("Group " + shapeId); - cnv.setId(shapeId + 1); + cnv.setId(shapeId); nvSpPr.addNewCNvGrpSpPr(); nvSpPr.addNewNvPr(); diff --git a/src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFObjectShape.java b/src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFObjectShape.java index ffa6b9eb19..48ae87b122 100644 --- a/src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFObjectShape.java +++ b/src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFObjectShape.java @@ -268,8 +268,8 @@ public class XSLFObjectShape extends XSLFGraphicFrame implements ObjectShape { return getCNvPr().getName(); } - /** - * Returns a unique identifier for this shape within the current document. - * This ID may be used to assist in uniquely identifying this object so that it can - * be referred to by other parts of the document. - *

- * If multiple objects within the same document share the same id attribute value, - * then the document shall be considered non-conformant. - *

- * - * @return unique id of this shape - */ + @Override public int getShapeId() { return (int)getCNvPr().getId(); } diff --git a/src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFSheet.java b/src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFSheet.java index 2d45cb2d13..a6320d6c2e 100644 --- a/src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFSheet.java +++ b/src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFSheet.java @@ -25,6 +25,7 @@ import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; import java.util.ArrayList; +import java.util.BitSet; import java.util.HashMap; import java.util.Iterator; import java.util.List; @@ -79,6 +80,8 @@ implements XSLFShapeContainer, Sheet { private Map _placeholderByIdMap; private Map _placeholderByTypeMap; + private final BitSet shapeIds = new BitSet(); + public XSLFSheet() { super(); } @@ -105,6 +108,26 @@ implements XSLFShapeContainer, Sheet { throw new IllegalStateException("SlideShow was not found"); } + protected int allocateShapeId() { + final int nextId = shapeIds.nextClearBit(1); + shapeIds.set(nextId); + return nextId; + } + + protected void registerShapeId(final int shapeId) { + if (shapeIds.get(shapeId)) { + LOG.log(POILogger.WARN, "shape id "+shapeId+" has been already used."); + } + shapeIds.set(shapeId); + } + + protected void deregisterShapeId(final int shapeId) { + if (!shapeIds.get(shapeId)) { + LOG.log(POILogger.WARN, "shape id "+shapeId+" hasn't been registered."); + } + shapeIds.clear(shapeId); + } + protected static List buildShapes(CTGroupShape spTree, XSLFShapeContainer parent){ final XSLFSheet sheet = (parent instanceof XSLFSheet) ? (XSLFSheet)parent : ((XSLFShape)parent).getSheet(); @@ -328,9 +351,12 @@ implements XSLFShapeContainer, Sheet { public boolean removeShape(XSLFShape xShape) { XmlObject obj = xShape.getXmlObject(); CTGroupShape spTree = getSpTree(); + deregisterShapeId(xShape.getShapeId()); if(obj instanceof CTShape){ spTree.getSpList().remove(obj); } else if (obj instanceof CTGroupShape) { + XSLFGroupShape gs = (XSLFGroupShape)xShape; + new ArrayList<>(gs.getShapes()).forEach(gs::removeShape); spTree.getGrpSpList().remove(obj); } else if (obj instanceof CTConnector) { spTree.getCxnSpList().remove(obj); diff --git a/src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFTable.java b/src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFTable.java index ba9fe227d8..50cc38fb77 100644 --- a/src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFTable.java +++ b/src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFTable.java @@ -166,7 +166,7 @@ public class XSLFTable extends XSLFGraphicFrame implements Iterable, int[]> ids = (shapes) -> + shapes.stream().mapToInt(Shape::getShapeId).toArray(); + + try (final XMLSlideShow ppt = new XMLSlideShow()) { + final XSLFSlide slide = ppt.createSlide(); + final List shapes = new ArrayList<>(); + shapes.add(slide.createAutoShape()); + final XSLFGroupShape g1 = slide.createGroup(); + shapes.add(g1); + final XSLFGroupShape g2 = g1.createGroup(); + shapes.add(g2); + shapes.add(g2.createAutoShape()); + shapes.add(slide.createAutoShape()); + shapes.add(g2.createAutoShape()); + shapes.add(g1.createAutoShape()); + assertArrayEquals(new int[]{ 2,3,4,5,6,7,8 }, ids.apply(shapes)); + g1.removeShape(g2); + shapes.remove(5); + shapes.remove(3); + shapes.remove(2); + shapes.add(g1.createAutoShape()); + assertArrayEquals(new int[]{ 2,3,6,8,4 }, ids.apply(shapes)); + + } + } } diff --git a/src/ooxml/testcases/org/apache/poi/xslf/usermodel/TestXSLFTable.java b/src/ooxml/testcases/org/apache/poi/xslf/usermodel/TestXSLFTable.java index 736d276d92..1df72597d5 100644 --- a/src/ooxml/testcases/org/apache/poi/xslf/usermodel/TestXSLFTable.java +++ b/src/ooxml/testcases/org/apache/poi/xslf/usermodel/TestXSLFTable.java @@ -94,7 +94,7 @@ public class TestXSLFTable { assertNotNull(tbl.getCTTable().getTblGrid()); assertNotNull(tbl.getCTTable().getTblPr()); assertTrue(tbl.getXmlObject() instanceof CTGraphicalObjectFrame); - assertEquals("Table 1", tbl.getShapeName()); + assertEquals("Table 2", tbl.getShapeName()); assertEquals(2, tbl.getShapeId()); assertEquals(0, tbl.getRows().size()); assertEquals(0, tbl.getCTTable().sizeOfTrArray()); diff --git a/src/scratchpad/src/org/apache/poi/hslf/usermodel/HSLFShape.java b/src/scratchpad/src/org/apache/poi/hslf/usermodel/HSLFShape.java index 4edcb53603..085d617eb8 100644 --- a/src/scratchpad/src/org/apache/poi/hslf/usermodel/HSLFShape.java +++ b/src/scratchpad/src/org/apache/poi/hslf/usermodel/HSLFShape.java @@ -538,9 +538,7 @@ public abstract class HSLFShape implements Shape { return new Color(r, g, b); } - /** - * @return id for the shape. - */ + @Override public int getShapeId(){ EscherSpRecord spRecord = getEscherChild(EscherSpRecord.RECORD_ID); return spRecord == null ? 0 : spRecord.getShapeId();