poi icon indicating copy to clipboard operation
poi copied to clipboard

Bug: Auto sizing not working properly due to rounding error when calculating defaultCharWidth

Open nme-xx opened this issue 2 years ago • 11 comments

The XSSFSheet#autoSizeColumn method doesn't work properly for some specific cell values and font metrics.

The following code snippet is an example for this behavior:

String sheetName = "Sheet 1";
String cellText = "Auto sizing incorrect! :(";

try (XSSFWorkbook workbook = new XSSFWorkbook()) {

    final XSSFSheet sheet = workbook.createSheet(sheetName);

    final XSSFCell cell = sheet.createRow(0).createCell(0);

    cell.setCellValue(cellText);

    final XSSFFont font = workbook.createFont();
    font.setFontName("Arial");
    font.setFontHeight(15);
    font.setBold(true);

    final XSSFCellStyle style = workbook.createCellStyle();
    style.setFont(font);
    cell.setCellStyle(style);

    sheet.autoSizeColumn(0, false);

    try (FileOutputStream fOS = new FileOutputStream("./test-excel.xlsx")) {
        workbook.write(fOS);
    }
}

Generated excel file: image

The bug is caused by a rounding error in SheetUtil#getDefaultCharWidth: https://github.com/apache/poi/blob/5533d35cefd15e98fd988a458ac41c5e8aad06f7/poi/src/main/java/org/apache/poi/ss/util/SheetUtil.java#L301-L302

Rounding is not necessary at all, removing the operation and converting the return type of getDefaultCharWidth() to double solves the issue.

The column width calculation that causes the error (and uses a value returned by SheetUtil#getDefaultCharWidth) occurs in SheetUtil#getCellWidth: https://github.com/apache/poi/blob/5533d35cefd15e98fd988a458ac41c5e8aad06f7/poi/src/main/java/org/apache/poi/ss/util/SheetUtil.java#L244

nme-xx avatar Nov 02 '23 12:11 nme-xx

I added some changes in https://github.com/apache/poi/commit/8819952b2f7210d4201a1b1d75d420abc67d8c2c

I didn't want to break backward compatibility so I've added new methods and deprecated the ones that they replace.

It's hard to automate tests because the calculated widths depend on the font setup that testers have installed.

If you can spot a mistake or could try out my change that would be great. I will double check the code change later and it is possible it might break a few tests.

pjfanning avatar Nov 08 '23 13:11 pjfanning

I can't see any additional failing tests (except the ones that fail due to my locale but aren't related to the issue). My example seems to work too: image Do you know when the next version will be released yet?

nme-xx avatar Nov 09 '23 09:11 nme-xx

It will be a few weeks before POI 5.2.5 is released. We are concentrating on releasing XMLBeans 5.2.0 first and once that is released, we should be in a position to start a POI release vote.

pjfanning avatar Nov 09 '23 10:11 pjfanning

Alright, thank you for addressing the issue so quickly!

nme-xx avatar Nov 09 '23 10:11 nme-xx

@NiklasMelznerExxcellent Some users are complaining that this change is still creating cells that are not wide enough. Would you have any interest in having another look?

I'm wondering whether it might be worth just arbitrarily increasing the width of each cell by some hardcoded width. Maybe we could make this configurable but default to some non-zero value.

pjfanning avatar Dec 10 '23 12:12 pjfanning

I guess you're referring to this bug? I'll have a look at it next week.

niklasmelzner avatar Dec 10 '23 12:12 niklasmelzner

Was this fixed in Apache POI 5.3.0 which was released two days ago?

ThoSap avatar Jul 04 '24 12:07 ThoSap

Changes from this PR were applied as part of Apache POI 5.3.0, see r1913676, however some manual tests showed that the actual result is dependent on the application used to display the workbook, so columns may still be either too small, see https://bz.apache.org/bugzilla/show_bug.cgi?id=68094 for some details.

centic9 avatar Jul 07 '24 16:07 centic9

With Apache POI 5.2.3 the auto sizing works properly with Excel, I'm using that version currently as newer ones (dis not try 5.3.0 yet) are broken. I don't know about LibreOffice thoe.

ThoSap avatar Jul 07 '24 16:07 ThoSap

https://svn.apache.org/viewvc?view=rev&rev=1913676 was in POI 5.2.5. So far the only suggested solution is to have a configurable extra width that is added to all cell widths. There are too many variables to get a works in all cases solution (depending on what fonts are installed on the machine used to run the POI code and which spreadsheet tool is used to view the generated spreadsheet).

pjfanning avatar Jul 07 '24 16:07 pjfanning

I created #657 as an outline

pjfanning avatar Jul 07 '24 17:07 pjfanning