[bug-68094] try to improve cell width derivation by not rounding

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

3 files changed, 64 insertions, 8 deletions

diff --git a/poi-ooxml/src/main/java/org/apache/poi/xssf/streaming/AutoSizeColumnTracker.java b/poi-ooxml/src/main/java/org/apache/poi/xssf/streaming/AutoSizeColumnTracker.java
index 9df140d414..0891b300a7 100644
--- a/poi-ooxml/src/main/java/org/apache/poi/xssf/streaming/AutoSizeColumnTracker.java
+++ b/poi-ooxml/src/main/java/org/apache/poi/xssf/streaming/AutoSizeColumnTracker.java
@@ -47,7 +47,7 @@ import org.apache.poi.util.Internal;
 */
 @Internal
 /*package*/ class AutoSizeColumnTracker {
-    private final int defaultCharWidth;
+    private final float defaultCharWidth;
     private final DataFormatter dataFormatter = new DataFormatter();
 
     // map of tracked columns, with values containing the best-fit width for the column
@@ -114,7 +114,7 @@ import org.apache.poi.util.Internal;
      */
     public AutoSizeColumnTracker(final Sheet sheet) {
         // If sheet needs to be saved, use a java.lang.ref.WeakReference to avoid garbage collector gridlock.
-        defaultCharWidth = SheetUtil.getDefaultCharWidth(sheet.getWorkbook());
+        defaultCharWidth = SheetUtil.getDefaultCharWidthAsFloat(sheet.getWorkbook());
     }
     
     /**
diff --git a/poi/src/main/java/org/apache/poi/ss/util/SheetUtil.java b/poi/src/main/java/org/apache/poi/ss/util/SheetUtil.java
index 2ced5742b7..57ee821c5d 100644
--- a/poi/src/main/java/org/apache/poi/ss/util/SheetUtil.java
+++ b/poi/src/main/java/org/apache/poi/ss/util/SheetUtil.java
@@ -117,8 +117,24 @@ public class SheetUtil {
      * @param formatter formatter used to prepare the text to be measured
      * @param useMergedCells    whether to use merged cells
      * @return  the width in pixels or -1 if cell is empty
+     * @deprecated since POI 5.2.5, it is better to pass defaultCharWidth as a float
      */
     public static double getCellWidth(Cell cell, int defaultCharWidth, DataFormatter formatter, boolean useMergedCells) {
+        return getCellWidth(cell, (float) defaultCharWidth, formatter, useMergedCells);
+    }
+
+
+    /**
+     * Compute width of a single cell
+     *
+     * @param cell the cell whose width is to be calculated
+     * @param defaultCharWidth the width of a single character
+     * @param formatter formatter used to prepare the text to be measured
+     * @param useMergedCells    whether to use merged cells
+     * @return  the width in pixels or -1 if cell is empty
+     * @since POI 5.2.5
+     */
+    public static double getCellWidth(Cell cell, float defaultCharWidth, DataFormatter formatter, boolean useMergedCells) {
         List<CellRangeAddress> mergedRegions = cell.getSheet().getMergedRegions();
         return getCellWidth(cell, defaultCharWidth, formatter, useMergedCells, mergedRegions);
     }
@@ -136,9 +152,30 @@ public class SheetUtil {
      * @param useMergedCells    whether to use merged cells
      * @param mergedRegions The list of merged regions as received via cell.getSheet().getMergedRegions()
      * @return  the width in pixels or -1 if cell is empty
+     * @deprecated since POI 5.2.5, it is better to pass defaultCharWidth as a float
      */
     public static double getCellWidth(Cell cell, int defaultCharWidth, DataFormatter formatter, boolean useMergedCells,
                                       List<CellRangeAddress> mergedRegions) {
+        return getCellWidth(cell, (float) defaultCharWidth, formatter, useMergedCells, mergedRegions);
+    }
+
+    /**
+     * Compute width of a single cell
+     *
+     * This method receives the list of merged regions as querying it from the cell/sheet
+     * is time-consuming and thus caching the list across cells speeds up certain operations
+     * considerably.
+     *
+     * @param cell the cell whose width is to be calculated
+     * @param defaultCharWidth the width of a single character
+     * @param formatter formatter used to prepare the text to be measured
+     * @param useMergedCells    whether to use merged cells
+     * @param mergedRegions The list of merged regions as received via cell.getSheet().getMergedRegions()
+     * @return  the width in pixels or -1 if cell is empty
+     * @since POI 5.2.5
+     */
+    public static double getCellWidth(Cell cell, float defaultCharWidth, DataFormatter formatter, boolean useMergedCells,
+                                      List<CellRangeAddress> mergedRegions) {
         Sheet sheet = cell.getSheet();
         Workbook wb = sheet.getWorkbook();
         Row row = cell.getRow();
@@ -219,11 +256,11 @@ public class SheetUtil {
      * @param str the text contained in the cell
      * @return the best fit cell width
      */
-    private static double getCellWidth(int defaultCharWidth, int colspan,
+    private static double getCellWidth(float defaultCharWidth, int colspan,
             CellStyle style, double minWidth, AttributedString str) {
         TextLayout layout = new TextLayout(str.getIterator(), fontRenderContext);
         final Rectangle2D bounds;
-        if(style.getRotation() != 0){
+        if (style.getRotation() != 0) {
             /*
              * Transform the text using a scale so that its height is increased by a multiple of the leading,
              * and then rotate the text before computing the bounds. The scale results in some whitespace around
@@ -270,7 +307,7 @@ public class SheetUtil {
      */
     public static double getColumnWidth(Sheet sheet, int column, boolean useMergedCells, int firstRow, int lastRow){
         DataFormatter formatter = new DataFormatter();
-        int defaultCharWidth = getDefaultCharWidth(sheet.getWorkbook());
+        float defaultCharWidth = getDefaultCharWidthAsFloat(sheet.getWorkbook());
 
         List<CellRangeAddress> mergedRegions = sheet.getMergedRegions();
         double width = -1;
@@ -290,16 +327,30 @@ public class SheetUtil {
      *
      * @param wb the workbook to get the default character width from
      * @return default character width in pixels
+     * @deprecated since POI 5.2.5, it is recommended to switch to {@link #getDefaultCharWidthAsFloat(Workbook)}.
      */
     @Internal
     public static int getDefaultCharWidth(final Workbook wb) {
+        return Math.round(getDefaultCharWidthAsFloat(wb));
+    }
+
+    /**
+     * Get default character width using the Workbook's default font. Note that this can
+     * fail if your OS does not have the right fonts installed.
+     *
+     * @param wb the workbook to get the default character width from
+     * @return default character width in pixels (as a float)
+     * @since POI 5.2.5
+     */
+    @Internal
+    public static float getDefaultCharWidthAsFloat(final Workbook wb) {
         Font defaultFont = wb.getFontAt( 0);
 
         AttributedString str = new AttributedString(String.valueOf(defaultChar));
         copyAttributes(defaultFont, str, 0, 1);
         try {
             TextLayout layout = new TextLayout(str.getIterator(), fontRenderContext);
-            return Math.round(layout.getAdvance());
+            return layout.getAdvance();
         } catch (UnsatisfiedLinkError | NoClassDefFoundError | InternalError e) {
             if (ignoreMissingFontSystem) {
                 return DEFAULT_CHAR_WIDTH;
@@ -321,7 +372,7 @@ public class SheetUtil {
      * @return  the width in pixels or -1 if cell is empty
      */
     private static double getColumnWidthForRow(
-            Row row, int column, int defaultCharWidth, DataFormatter formatter, boolean useMergedCells,
+            Row row, int column, float defaultCharWidth, DataFormatter formatter, boolean useMergedCells,
             List<CellRangeAddress> mergedRegions) {
         if( row == null ) {
             return -1;
diff --git a/poi/src/test/java/org/apache/poi/ss/util/TestSheetUtil.java b/poi/src/test/java/org/apache/poi/ss/util/TestSheetUtil.java
index 288ded538d..b63460596c 100644
--- a/poi/src/test/java/org/apache/poi/ss/util/TestSheetUtil.java
+++ b/poi/src/test/java/org/apache/poi/ss/util/TestSheetUtil.java
@@ -108,6 +108,9 @@ final class TestSheetUtil {
 
             // no contents: cell.setCellValue("sometext");
             assertEquals(-1.0, SheetUtil.getCellWidth(cell, 1, null, true), 0.01);
+            assertEquals(-1.0, SheetUtil.getCellWidth(cell, 1.0f, null, true), 0.01);
+
+            assertEquals(-1.0, SheetUtil.getCellWidth(cell, 1.5f, null, true), 0.01);
         }
     }
 
@@ -120,7 +123,9 @@ final class TestSheetUtil {
 
             cell.setCellValue("sometext");
 
-            assertTrue(SheetUtil.getCellWidth(cell, 1, null, true) > 0);
+            final double width = SheetUtil.getCellWidth(cell, 1.0f, null, true);
+            assertTrue(width > 0);
+            assertEquals(width, SheetUtil.getCellWidth(cell, 1, null, true));
         }
     }