Bug 55075 - [PATCH] SXSSF: Pictures skewed on rows which have non-default height
Summary: [PATCH] SXSSF: Pictures skewed on rows which have non-default height
Alias: None
Product: POI
Classification: Unclassified
Component: SXSSF (show other bugs)
Version: 3.9-FINAL
Hardware: PC All
: P2 normal (vote)
Target Milestone: ---
Assignee: POI Developers List
Keywords: PatchAvailable
: 51233 (view as bug list)
Depends on: 51233
  Show dependency tree
Reported: 2013-06-07 15:13 UTC by Frank Plößel
Modified: 2016-10-09 10:14 UTC (History)
1 user (show)

patch fixing the problem (see description and comments in the code in the files) (13.72 KB, application/zip)
2013-06-07 15:13 UTC, Frank Plößel
SXSSF pictures and drawings (12.90 KB, patch)
2016-10-09 07:16 UTC, Javen O'Neal
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Frank Plößel 2013-06-07 15:13:05 UTC
Created attachment 30405 [details]
patch fixing the problem (see description and comments in the code in the files)

picture.resize() skews pictures in SXSSF if the picture is in a row having non-default height. As we set the row height so that the picture fits into one row in our code, this results in pictures scaled sometimes by a factor 2 or 3 in height, but having their correct width, looking very ugly.

The reason is that the XSSF code to which the SXSSF code delegates the size calculation checks the rows in the XSSF sheet for their height, and - as all rows are in the SXSSF sheet - does not find any rows and assumes the default row height.

I have attached a fix for the problem that works fine with POI 3.9. This contains two new SXSSF classes which in the current 3.9 code do not exist, as their role is taken by the XSSF classes, and a one line change of SXSSFSheet:
method createDrawingPatriarch() in line 1195 should return the new SXSSFDrawing instead of an XSSFDrawing.

I would assume that in addition to Picture, Comment and Chart may have similar issues, but as we do not need them, I did not check that.

Of course, as always with SXSSF, if the row is already flushed out, there will be problems.
Comment 1 Nick Burch 2013-06-07 15:20:35 UTC
Are you able to write a junit test that shows the problem, and confirms it's fixed with the patch? Perhaps something that uses SXSSF to write a file, then loads it via XSSF and checks to see if the picture is the right size or not?
Comment 2 Frank Plößel 2013-06-07 15:50:13 UTC
Would the test cases in issue 51233 be ok as test case, or would you need a more detailed one? I just discovered this seems to be a duplicate.
Comment 3 Dominik Stadler 2015-03-23 21:16:15 UTC
I tried to review and apply this patch, but unfortunately it is provided as full changed files instead of a diff which makes it hard to apply after other chnages were done to the source. Also there are some new methods in class Picture which break the changes. 

Can you redo your changes on top of a more recent version, at least 3.12-beta1 or even better trunk and provide a real patch that can be applied?
Comment 4 Dominik Stadler 2015-03-23 21:17:17 UTC
And yes, I also think this is the same as 51233 and thus the unit test from there should suffice for now.
Comment 5 Javen O'Neal 2016-10-09 07:16:06 UTC
Created attachment 34345 [details]
SXSSF pictures and drawings

attachment 30405 [details] was based on r1384784, with only 1 line changed:

--- src/ooxml/java/org/apache/poi/xssf/streaming/SXSSFSheet.java	(revision 1763945)
+++ src/ooxml/java/org/apache/poi/xssf/streaming/SXSSFSheet.java	(working copy)
@@ -1704,7 +1704,7 @@
     public Drawing createDrawingPatriarch()
-        return _sh.createDrawingPatriarch();
+        return new SXSSFDrawing((SXSSFWorkbook)getWorkbook(), _sh.createDrawingPatriarch());

I have rebased the patch to the trunk.
Comment 6 Javen O'Neal 2016-10-09 08:52:59 UTC
*** Bug 51233 has been marked as a duplicate of this bug. ***
Comment 7 Javen O'Neal 2016-10-09 10:14:20 UTC
Thanks to Frank Plößel for writing the SXSSFPicture and SXSSFDrawing classes that wrap the XSSF version. These changes, along with unit tests, have been committed in 1763950 and r1763951.