Bug 53361 - feature: enhancements in EscherAggregate
Summary: feature: enhancements in EscherAggregate
Alias: None
Product: POI
Classification: Unclassified
Component: HSSF (show other bugs)
Version: unspecified
Hardware: All All
: P2 normal (vote)
Target Milestone: ---
Assignee: POI Developers List
Depends on:
Blocks: 53010
  Show dependency tree
Reported: 2012-06-05 08:42 UTC by Joachim Herrmann
Modified: 2012-08-15 20:23 UTC (History)
0 users

suggested enhancements (42.99 KB, application/octet-stream)
2012-06-05 08:42 UTC, Joachim Herrmann

Note You need to log in before you can comment on or make changes to this bug.
Description Joachim Herrmann 2012-06-05 08:42:24 UTC
Created attachment 28887 [details]
suggested enhancements


I complied a few enhancements covering shape creation in EscherAggregate:

* support for HSSFChildAnchor
* improvement for HSSFClientAnchor (col and row values are read from record)
* creation of HSSFPolygon (points are read from record)
* improvement for HSSFPicture (now reads original file name, if provided)

Besides, I suggest a new Anchor interface to make the various HSSFAnchor and XSSFAnchor classes more coherent.

The provided test classes illustrate the new features.

I also found that in EscherContainerRecord.createSpContainer(HSSFPolygon, int)
an additional point is added to the polygon point array - probably the intention is to always have a closed polygon path. In my view, this is questionable, as Excel supports open polygon paths (see also note in test class TestHSSFPolyogn).

I would be pleased if you find some of the suggestions suitable to be commited to POI.

Comment 1 Yegor Kozlov 2012-06-06 13:18:32 UTC
Thanks for the patch.

There is an ongoing effort to improve drawing support in HSSF: This year Apache POI participates in the Google Summer of Code program and Evgeniy Berlog, our student, is working on this task. You can track progress in the gsoc2012  branch: http://svn.apache.org/repos/asf/poi/branches/gsoc2012/

We would like to merge your patch with the gsoc code first and then move it to trunk in the end of the gsoc program, in late August 2012. 

So it will take a while to apply your patch. 

Comment 2 Evgeniy Berlog 2012-06-07 11:37:42 UTC
Thanks for your patch.

Hi, I am the student who works at GSoC project.
I've noticed that you modified EscherAggregate.convertRecordsToUserModel(), but the main idea in new drawing layer is to remove methods convertUserModelToRecords() and convertRecordsToUserModel(). The tree of shapes will be created after EcherAggregate.createAggregate(). Each shape will have a constructor like HSSFShape(EcherContainerRecord spRecord, ObjRecord objRecord) and each  property will be taken directly from EscherRecord. 

You are welcome to send your ideas. 

With best regards, Evgeniy Berlog
Comment 3 Joachim Herrmann 2012-06-07 13:38:05 UTC
Hi Yegor and Evgeniy

I was not aware that you are currently conducting more extensive changes to this code area, so my contribution may happen to stumble upon your roadworks. Sorry for that.
You are certainly welcome to adapt (or discard) the modifications I added, if they conflict with the broader design changes that are in progress. Feel free to fit it into your design.

I also noticed that extracting all properties from the records, and storing them into member variables of the Shape object may be a waste and unnecessarily degrade performance, because most of the data may never be requested by the user. 
Therefore I also rate it a better method to give each shape a reference to the original EscherContainerRecord, and extract properties only on request, saving memory and processing time.

It also seems that this design makes the HSSF Shapes more similar to shapes in other areas of POI (but I just peeked into them superficially):

HSLF: org.apache.poi.hslf.model.Shape contains the following constructor:
  protected Shape(EscherContainerRecord escherRecord, Shape parent)

HWPF: org.apache.poi.hwpf.usermodel.Picture contains the following constructor:
  public Picture(EscherBlipRecord blipRecord)

So there are already Shape classes in other POI components that go this way, and it makes sense to me.

From a wider perspective, it would also seem that a unified Drawing and Shape package would be very valuable.
While you can currently use shapes in hssf/xssf, hslf, hwpf (and maybe some more components I didn't discover), the only way to pass shapes across component borders (say a shape in a hssf sheet to a hslf slide) is to create a corresponding shape in the target document and use getter/setter methods to copy properties. This of course only works, if
- the source and target document support the same shape type
- the source and target document each provide getter and setter for all properties
- you create code to handle each different shape type explicitly, because you must use different getters/setters for each shape type

it would give users great power to do instead something like:

  HSSFSheet excelSheet = ...
  HWPFDocument wordDoc = ...
  Drawing excelDrawing = excelSheet.getDrawing();
  for (Shape s : excelDrawing.getShapes()) {
     wordDoc.addDrawing().addShape(s);  // implicit cloning of shape assumed


  Shape s = userMethodToConstructAVeryComplexShape();
  // note that at shape creation time, it is unknown if the shape is 
  // going to be added to an excel sheet or a word document
  HSSFSheet excelSheet = ...
  HWPFDocument wordDoc = ...

Comment 4 Evgeniy Berlog 2012-08-15 20:23:50 UTC
All of the features you proposed are included in trunk now 

Please try with a nightly build - see download links on http://poi.apache.org/
or build yourself from SVN trunk, see http://poi.apache.org/subversion.html