On 01/08/2015 09:45 AM, Stephan Bergmann wrote:
Running CppunitTest_sw_ooxmlsdrexport with -fsanitize=float-divide-by-zero fails with
[...]
ensuing IRC conversation:
<sberg> vmiklos, maybe you have some idea about the "CppunitTest_sw_ooxmlsdrexport: division by zero in chart2/source/controller/chartapiwrapper/LegendWrapper.cxx" mail I just sent to the list? <vmiklos> sberg: i'm not entirely sure what is the proper level to fix it; i know that at various places we do "if height is 0, let it 1", and doing the same at the lowest level (chart2) would solve the problem; but there might be a better fix. <sberg> vmiklos, would that height=width=100 special case be relevant in this scenario, or is it a red herring? <vmiklos> sberg: i think the height=width=100 case is for ole objects embedded as an icon, which is not relevant for this bugdoc. but even if that happens, that should be handled, i guess LegendWrapper::setPosition() can be invoked by a user basic macro as well, so not dividing by 0 for any user input there is necessary. <sberg> vmiklos, in LegendWrapper::setPosition() it's not the (user-controlled) position that caused div by zero, but the model's page size; anyway, will see to hack-fix it at the SvxOle2Shape::createObject level then, maybe not calling xObj->setVisualAreaSize if GetLogicRect returns an empty rect and see how that works out <vmiklos> sberg: ah, i see. yes, i would just work it around in the lower level, then ask moggi if he thinks there is a more proper way. i don't have too much clue about chart2, either. <moggi> vmiklos: sberg: I have no idea, I would have some if it would be odf import but I have no idea how docx import handles chart init (there are quite a few corner cases that you can only hit in the import code) <vmiklos> moggi: it's the same as chart-in-xlsx, OOXML handling for charts is all in oox, so i guess it's not specific to Writer/docx <moggi> vmiklos: at least the second backtrace in sberg's mail is writer specific <vmiklos> moggi: hmm, writerfilter just sends all SAX events of the shape to oox, and then at the end it calls oox::shape::ShapeContextHandler::getShape() to get the shape. so it doesn't know too much if the resulting shape will be a chart or a ellipse or something else. <vmiklos> moggi: of the resulting* <moggi> vmiklos: the ChartModel::setVisualArea that sets the 0 size is from parsing the document.xml file and not the chart file, the containing application is responsible for the size of the shape and not the chart stream <vmiklos> moggi: aha, indeed. writerfilter first imports the chart, and once it has an uno reference to the chart, it sets the size -- is that unexpected? <moggi> vmiklos: let me check what calc is doing <moggi> vmiklos: I checked and at least in calc we call setVisualAreaSize only with non zero values, in some places we have some code to set a default size if we requested a zero sized chart <moggi> vmiklos: e.g. fuins2.cxx:338 <vmiklos> moggi: so you first create the chart uno object, set its size, and only then do the real import? <moggi> vmiklos: yes <vmiklos> moggi: OK, thanks <moggi> vmiklos: I think one problem if you go the other way is that some positions/sizes might not be correct if the chart size is not correct <moggi> e.g. places where we need to convert from relative to absolute positioning <vmiklos> i see. <moggi> and in calc the whole spreadsheet data needs to be imported as otherwise we might switch from calc data provider to internal data provider
worked around for now with <http://cgit.freedesktop.org/libreoffice/core/commit/?id=8eb880a04d17c888c2c986426e935e02ae309e7a> "HACK to avoid empty page size/div by 0 in chart2 LegendWrapper::setPosition"
_______________________________________________ LibreOffice mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/libreoffice
