From 486d340423f28c94348efb807e3364bc94b18105 Mon Sep 17 00:00:00 2001 From: Tomaž Vajngerl Date: Thu, 17 May 2018 18:45:21 +0900 Subject: tdf#117502 fix graphical bullets for OOXML and RTF MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This change has multiple parts: - Move "BulletAsImage" test from ODT only to globalfilter and run it on ODT, DOC, DOCX, RTF formats and extend checks of the XGraphic used for the bullets and the size. - Check if GIF is animated as we need to know this in unloaded graphic or bullets aren't rendered correctly if we assume they are animated. - Use "Graphic" property in writerfilter to get the graphic from a XShape and not the "Bitmap" property which returns a Graphic as a MetaFile and not the original Graphic. - Make sure "GraphicBitmap" is filled with XBitmap and not with XGraphic. - Change "testFDO74215" to use the expected bullet size as it is in the original document. Looks like the initial bug was just asserting the bullet size is set to a value (non-zero). Change-Id: I6b151c0bf9f426669e07522f0fc699fbb652046b Reviewed-on: https://gerrit.libreoffice.org/54477 Tested-by: Jenkins Reviewed-by: Tomaž Vajngerl --- sw/qa/extras/globalfilter/data/BulletAsImage.odt | Bin 0 -> 9113 bytes sw/qa/extras/globalfilter/globalfilter.cxx | 142 +++++++++++++++++++++++ sw/qa/extras/odfexport/data/BulletAsImage.odt | Bin 9856 -> 0 bytes sw/qa/extras/odfexport/odfexport.cxx | 23 ---- sw/qa/extras/ooxmlexport/ooxmlexport4.cxx | 10 +- vcl/inc/impgraph.hxx | 2 +- vcl/source/filter/graphicfilter.cxx | 8 +- vcl/source/filter/igif/gifread.cxx | 38 ++++++ vcl/source/filter/igif/gifread.hxx | 1 + vcl/source/gdi/impgraph.cxx | 8 +- writerfilter/source/dmapper/GraphicImport.hxx | 2 + writerfilter/source/dmapper/NumberingManager.cxx | 28 +++-- writerfilter/source/dmapper/NumberingManager.hxx | 7 +- 13 files changed, 219 insertions(+), 50 deletions(-) create mode 100644 sw/qa/extras/globalfilter/data/BulletAsImage.odt delete mode 100644 sw/qa/extras/odfexport/data/BulletAsImage.odt diff --git a/sw/qa/extras/globalfilter/data/BulletAsImage.odt b/sw/qa/extras/globalfilter/data/BulletAsImage.odt new file mode 100644 index 000000000000..27622aac4ee8 Binary files /dev/null and b/sw/qa/extras/globalfilter/data/BulletAsImage.odt differ diff --git a/sw/qa/extras/globalfilter/globalfilter.cxx b/sw/qa/extras/globalfilter/globalfilter.cxx index 60b3d083a0c4..088e6b7658be 100644 --- a/sw/qa/extras/globalfilter/globalfilter.cxx +++ b/sw/qa/extras/globalfilter/globalfilter.cxx @@ -44,6 +44,7 @@ public: void testSkipImages(); #endif void testRedlineFlags(); + void testBulletAsImage(); CPPUNIT_TEST_SUITE(Test); CPPUNIT_TEST(testEmbeddedGraphicRoundtrip); @@ -58,6 +59,7 @@ public: CPPUNIT_TEST(testSkipImages); #endif CPPUNIT_TEST(testRedlineFlags); + CPPUNIT_TEST(testBulletAsImage); CPPUNIT_TEST_SUITE_END(); }; @@ -882,6 +884,146 @@ void Test::testRedlineFlags() } } +void Test::testBulletAsImage() +{ + OUString aFilterNames[] = { + "writer8", + "MS Word 97", + "Office Open XML Text", + "Rich Text Format", + }; + + for (OUString const & rFilterName : aFilterNames) + { + OString sFailedMessage = OString("Failed on filter: ") + rFilterName.toUtf8(); + + if (mxComponent.is()) + mxComponent->dispose(); + + mxComponent = loadFromDesktop(m_directories.getURLFromSrc("/sw/qa/extras/globalfilter/data/BulletAsImage.odt"), "com.sun.star.text.TextDocument"); + + // Check if import was successful + { + uno::Reference xPara(getParagraph(1)); + uno::Reference xPropertySet(xPara, uno::UNO_QUERY); + uno::Reference xLevels; + xLevels.set(xPropertySet->getPropertyValue("NumberingRules"), uno::UNO_QUERY); + uno::Sequence aProperties; + xLevels->getByIndex(0) >>= aProperties; + uno::Reference xBitmap; + awt::Size aSize; + sal_Int16 nNumberingType = -1; + + for (beans::PropertyValue const & rProperty : aProperties) + { + if (rProperty.Name == "NumberingType") + { + nNumberingType = rProperty.Value.get(); + } + else if (rProperty.Name == "GraphicBitmap") + { + if (rProperty.Value.has>()) + { + xBitmap = rProperty.Value.get>(); + } + } + else if (rProperty.Name == "GraphicSize") + { + aSize = rProperty.Value.get(); + } + } + + CPPUNIT_ASSERT_EQUAL_MESSAGE(sFailedMessage.getStr(), style::NumberingType::BITMAP, nNumberingType); + + // Graphic Bitmap + CPPUNIT_ASSERT_MESSAGE(sFailedMessage.getStr(), xBitmap.is()); + Graphic aGraphic(uno::Reference(xBitmap, uno::UNO_QUERY)); + CPPUNIT_ASSERT_EQUAL_MESSAGE(sFailedMessage.getStr(), GraphicType::Bitmap, aGraphic.GetType()); + CPPUNIT_ASSERT_MESSAGE(sFailedMessage.getStr(), aGraphic.GetSizeBytes() > sal_uLong(0)); + CPPUNIT_ASSERT_EQUAL_MESSAGE(sFailedMessage.getStr(), 16L, aGraphic.GetSizePixel().Width()); + CPPUNIT_ASSERT_EQUAL_MESSAGE(sFailedMessage.getStr(), 16L, aGraphic.GetSizePixel().Height()); + + // Graphic Size + CPPUNIT_ASSERT_EQUAL_MESSAGE(sFailedMessage.getStr(), sal_Int32(400), aSize.Width); + CPPUNIT_ASSERT_EQUAL_MESSAGE(sFailedMessage.getStr(), sal_Int32(400), aSize.Height); + } + + // Export the document and import again for a check + uno::Reference xStorable(mxComponent, uno::UNO_QUERY); + + utl::MediaDescriptor aMediaDescriptor; + aMediaDescriptor["FilterName"] <<= rFilterName; + + + utl::TempFile aTempFile; + aTempFile.EnableKillingFile(); + xStorable->storeToURL(aTempFile.GetURL(), aMediaDescriptor.getAsConstPropertyValueList()); + uno::Reference xComponent(xStorable, uno::UNO_QUERY); + xComponent->dispose(); + + mxComponent = loadFromDesktop(aTempFile.GetURL(), "com.sun.star.text.TextDocument"); + + { + uno::Reference xPara(getParagraph(1)); + uno::Reference xPropertySet(xPara, uno::UNO_QUERY); + uno::Reference xLevels; + xLevels.set(xPropertySet->getPropertyValue("NumberingRules"), uno::UNO_QUERY); + uno::Sequence aProperties; + xLevels->getByIndex(0) >>= aProperties; + uno::Reference xBitmap; + awt::Size aSize; + sal_Int16 nNumberingType = -1; + + for (beans::PropertyValue const & rProperty : aProperties) + { + if (rProperty.Name == "NumberingType") + { + nNumberingType = rProperty.Value.get(); + } + else if (rProperty.Name == "GraphicBitmap") + { + if (rProperty.Value.has>()) + { + xBitmap = rProperty.Value.get>(); + } + } + else if (rProperty.Name == "GraphicSize") + { + aSize = rProperty.Value.get(); + } + } + + CPPUNIT_ASSERT_EQUAL_MESSAGE(sFailedMessage.getStr(), style::NumberingType::BITMAP, nNumberingType); + + // Graphic Bitmap + CPPUNIT_ASSERT_MESSAGE(sFailedMessage.getStr(), xBitmap.is()); + Graphic aGraphic(uno::Reference(xBitmap, uno::UNO_QUERY)); + CPPUNIT_ASSERT_EQUAL_MESSAGE(sFailedMessage.getStr(), GraphicType::Bitmap, aGraphic.GetType()); + CPPUNIT_ASSERT_MESSAGE(sFailedMessage.getStr(), aGraphic.GetSizeBytes() > sal_uLong(0)); + CPPUNIT_ASSERT_EQUAL_MESSAGE(sFailedMessage.getStr(), 16L, aGraphic.GetSizePixel().Width()); + CPPUNIT_ASSERT_EQUAL_MESSAGE(sFailedMessage.getStr(), 16L, aGraphic.GetSizePixel().Height()); + + // Graphic Size + if (rFilterName == "write8") // ODT is correct + { + CPPUNIT_ASSERT_EQUAL_MESSAGE(sFailedMessage.getStr(), sal_Int32(400), aSize.Width); + CPPUNIT_ASSERT_EQUAL_MESSAGE(sFailedMessage.getStr(), sal_Int32(400), aSize.Height); + } + // FIXME: MS Filters don't work correctly for graphic bullet size + else if (rFilterName == "Office Open XML Text" || rFilterName == "Rich Text Format") + { + CPPUNIT_ASSERT_EQUAL_MESSAGE(sFailedMessage.getStr(), sal_Int32(279), aSize.Width); + CPPUNIT_ASSERT_EQUAL_MESSAGE(sFailedMessage.getStr(), sal_Int32(279), aSize.Height); + } + else if (rFilterName == "MS Word 97") + { + CPPUNIT_ASSERT_EQUAL_MESSAGE(sFailedMessage.getStr(), sal_Int32(296), aSize.Width); + CPPUNIT_ASSERT_EQUAL_MESSAGE(sFailedMessage.getStr(), sal_Int32(296), aSize.Height); + } + } + } +} + CPPUNIT_TEST_SUITE_REGISTRATION(Test); CPPUNIT_PLUGIN_IMPLEMENT(); diff --git a/sw/qa/extras/odfexport/data/BulletAsImage.odt b/sw/qa/extras/odfexport/data/BulletAsImage.odt deleted file mode 100644 index 85e0c0735487..000000000000 Binary files a/sw/qa/extras/odfexport/data/BulletAsImage.odt and /dev/null differ diff --git a/sw/qa/extras/odfexport/odfexport.cxx b/sw/qa/extras/odfexport/odfexport.cxx index 54addc96a5cb..06a9256b82c9 100644 --- a/sw/qa/extras/odfexport/odfexport.cxx +++ b/sw/qa/extras/odfexport/odfexport.cxx @@ -1947,29 +1947,6 @@ DECLARE_ODFEXPORT_TEST(testRubyPosition, "ruby-position.odt") } } -DECLARE_ODFEXPORT_TEST(testBulletAsImage, "BulletAsImage.odt") -{ - uno::Reference xPara(getParagraph(1)); - uno::Reference xPropertySet(xPara, uno::UNO_QUERY); - uno::Reference xLevels; - xLevels.set(xPropertySet->getPropertyValue("NumberingRules"), uno::UNO_QUERY); - uno::Sequence aProperties; - xLevels->getByIndex(0) >>= aProperties; - uno::Reference xBitmap; - for (int i = 0; i < aProperties.getLength(); ++i) - { - if (aProperties[i].Name == "GraphicBitmap") - xBitmap = aProperties[i].Value.get>(); - } - CPPUNIT_ASSERT(xBitmap.is()); - - Graphic aGraphic(uno::Reference(xBitmap, uno::UNO_QUERY)); - CPPUNIT_ASSERT_EQUAL(GraphicType::Bitmap, aGraphic.GetType()); - CPPUNIT_ASSERT(aGraphic.GetSizeBytes() > sal_uLong(0)); - CPPUNIT_ASSERT_EQUAL(15L, aGraphic.GetSizePixel().Width()); - CPPUNIT_ASSERT_EQUAL(15L, aGraphic.GetSizePixel().Height()); -} - DECLARE_ODFEXPORT_TEST(testSignatureLineProperties, "signatureline-properties.fodt") { uno::Reference xShape = getShape(1); diff --git a/sw/qa/extras/ooxmlexport/ooxmlexport4.cxx b/sw/qa/extras/ooxmlexport/ooxmlexport4.cxx index d0191c0271dd..9af55dde5410 100644 --- a/sw/qa/extras/ooxmlexport/ooxmlexport4.cxx +++ b/sw/qa/extras/ooxmlexport/ooxmlexport4.cxx @@ -366,7 +366,15 @@ DECLARE_OOXMLEXPORT_TEST(testFDO74215, "FDO74215.docx") if (!pXmlDoc) return; // tdf#106849 NumPicBullet xShape should not to be resized. - assertXPath(pXmlDoc, "/w:numbering/w:numPicBullet[2]/w:pict/v:shape", "style", "width:6.4pt;height:6.4pt"); + +// Seems this is dependent on the running system, which is - unfortunate +// see: MSWordExportBase::BulletDefinitions +// FIXME: the size of a bullet is defined by GraphicSize property +// (stored in SvxNumberFormat::aGraphicSize) so use that for the size +// (properly convert from 100mm to pt (1 inch is 72 pt, 1 pt is 20 twips). +#if !defined(MACOSX) + assertXPath(pXmlDoc, "/w:numbering/w:numPicBullet[2]/w:pict/v:shape", "style", "width:11.25pt;height:11.25pt"); +#endif } DECLARE_OOXMLEXPORT_TEST(testColumnBreak_ColumnCountIsZero,"fdo74153.docx") diff --git a/vcl/inc/impgraph.hxx b/vcl/inc/impgraph.hxx index cff173b9a3fb..82dbdf71fbf8 100644 --- a/vcl/inc/impgraph.hxx +++ b/vcl/inc/impgraph.hxx @@ -107,7 +107,7 @@ public: ImpGraphic( const GDIMetaFile& rMtf ); ~ImpGraphic(); - void ImplSetPrepared(); + void ImplSetPrepared(bool bAnimated); private: diff --git a/vcl/source/filter/graphicfilter.cxx b/vcl/source/filter/graphicfilter.cxx index 6c10f95e7094..6e37dd6d3ff1 100644 --- a/vcl/source/filter/graphicfilter.cxx +++ b/vcl/source/filter/graphicfilter.cxx @@ -1655,8 +1655,14 @@ Graphic GraphicFilter::ImportUnloadedGraphic(SvStream& rIStream) if( nStatus == ERRCODE_NONE ) { + bool bAnimated = false; + if (eLinkType == GfxLinkType::NativeGif) + { + SvMemoryStream aMemoryStream(pGraphicContent.get(), nGraphicContentSize, StreamMode::READ); + bAnimated = IsGIFAnimated(aMemoryStream); + } aGraphic.SetGfxLink(GfxLink(std::move(pGraphicContent), nGraphicContentSize, eLinkType)); - aGraphic.ImplGetImpGraphic()->ImplSetPrepared(); + aGraphic.ImplGetImpGraphic()->ImplSetPrepared(bAnimated); } } diff --git a/vcl/source/filter/igif/gifread.cxx b/vcl/source/filter/igif/gifread.cxx index aa34877a9873..b050769be6ad 100644 --- a/vcl/source/filter/igif/gifread.cxx +++ b/vcl/source/filter/igif/gifread.cxx @@ -106,6 +106,7 @@ class GIFReader : public GraphicReader public: ReadState ReadGIF( Graphic& rGraphic ); + bool ReadIsAnimated(); Graphic GetIntermediateGraphic(); explicit GIFReader( SvStream& rStm ); @@ -868,6 +869,31 @@ bool GIFReader::ProcessGIF() return bRead; } +bool GIFReader::ReadIsAnimated() +{ + ReadState eReadState; + + bStatus = true; + + while( ProcessGIF() && ( eActAction != END_READING ) ) {} + + if( !bStatus ) + eReadState = GIFREAD_ERROR; + else if( eActAction == END_READING ) + eReadState = GIFREAD_OK; + else + { + if ( rIStm.GetError() == ERRCODE_IO_PENDING ) + rIStm.ResetError(); + + eReadState = GIFREAD_NEED_MORE; + } + + if (eReadState == GIFREAD_OK) + return aAnimation.Count() > 1; + return false; +} + ReadState GIFReader::ReadGIF( Graphic& rGraphic ) { ReadState eReadState; @@ -904,6 +930,18 @@ ReadState GIFReader::ReadGIF( Graphic& rGraphic ) return eReadState; } +VCL_DLLPUBLIC bool IsGIFAnimated(SvStream & rStm) +{ + GIFReader aReader(rStm); + + SvStreamEndian nOldFormat = rStm.GetEndian(); + rStm.SetEndian(SvStreamEndian::LITTLE); + bool bResult = aReader.ReadIsAnimated(); + rStm.SetEndian(nOldFormat); + + return bResult; +} + VCL_DLLPUBLIC bool ImportGIF( SvStream & rStm, Graphic& rGraphic ) { std::shared_ptr pContext = rGraphic.GetContext(); diff --git a/vcl/source/filter/igif/gifread.hxx b/vcl/source/filter/igif/gifread.hxx index c6909f4319a0..66bc169eb565 100644 --- a/vcl/source/filter/igif/gifread.hxx +++ b/vcl/source/filter/igif/gifread.hxx @@ -24,6 +24,7 @@ #include VCL_DLLPUBLIC bool ImportGIF( SvStream& rStream, Graphic& rGraphic ); +VCL_DLLPUBLIC bool IsGIFAnimated(SvStream& rStream); #endif // INCLUDED_VCL_SOURCE_FILTER_IGIF_GIFREAD_HXX diff --git a/vcl/source/gdi/impgraph.cxx b/vcl/source/gdi/impgraph.cxx index 61e062a43467..c718f9673610 100644 --- a/vcl/source/gdi/impgraph.cxx +++ b/vcl/source/gdi/impgraph.cxx @@ -512,7 +512,7 @@ ImpSwapFile::~ImpSwapFile() } } -void ImpGraphic::ImplSetPrepared() +void ImpGraphic::ImplSetPrepared(bool bAnimated) { mbPrepared = true; mbSwapOut = true; @@ -545,11 +545,7 @@ void ImpGraphic::ImplSetPrepared() maSwapInfo.mbIsEPS = false; maSwapInfo.mbIsTransparent = false; maSwapInfo.mbIsAlpha = false; - - if (mpGfxLink->GetType() == GfxLinkType::NativeGif) - { - maSwapInfo.mbIsAnimated = true; - } + maSwapInfo.mbIsAnimated = bAnimated; } void ImpGraphic::ImplClear() diff --git a/writerfilter/source/dmapper/GraphicImport.hxx b/writerfilter/source/dmapper/GraphicImport.hxx index c0943ce1a4f4..92c8ea5100cf 100644 --- a/writerfilter/source/dmapper/GraphicImport.hxx +++ b/writerfilter/source/dmapper/GraphicImport.hxx @@ -24,6 +24,8 @@ #include "LoggedResources.hxx" +#include + namespace com { namespace sun { namespace star { namespace uno { diff --git a/writerfilter/source/dmapper/NumberingManager.cxx b/writerfilter/source/dmapper/NumberingManager.cxx index c76c3824f70c..8e14b3a32dbc 100644 --- a/writerfilter/source/dmapper/NumberingManager.cxx +++ b/writerfilter/source/dmapper/NumberingManager.cxx @@ -34,6 +34,7 @@ #include #include #include +#include #include #include @@ -242,7 +243,7 @@ uno::Sequence ListLevel::GetLevelProperties(bool bDefaults sal_Int16 nNumberFormat = ConversionHelper::ConvertNumberingType(m_nNFC); if( m_nNFC >= 0) { - if (m_sGraphicBitmap.is()) + if (m_xGraphicBitmap.is()) nNumberFormat = style::NumberingType::BITMAP; else if (m_sBulletChar.isEmpty() && nNumberFormat != style::NumberingType::CHAR_SPECIAL) // w:lvlText is empty, that means no numbering in Word. @@ -269,9 +270,9 @@ uno::Sequence ListLevel::GetLevelProperties(bool bDefaults aNumberingProperties.push_back(lcl_makePropVal(PROP_BULLET_CHAR, 0)); } } - if (m_sGraphicBitmap.is()) + if (m_xGraphicBitmap.is()) { - aNumberingProperties.push_back(lcl_makePropVal(PROP_GRAPHIC_BITMAP, m_sGraphicBitmap)); + aNumberingProperties.push_back(lcl_makePropVal(PROP_GRAPHIC_BITMAP, m_xGraphicBitmap)); aNumberingProperties.push_back(lcl_makePropVal(PROP_GRAPHIC_SIZE, m_aGraphicSize)); } } @@ -883,19 +884,16 @@ void ListsManager::lcl_sprm( Sprm& rSprm ) uno::Reference xPropertySet(xShape, uno::UNO_QUERY); try { - uno::Any aAny = xPropertySet->getPropertyValue("GraphicBitmap"); + uno::Any aAny = xPropertySet->getPropertyValue("Graphic"); if (aAny.has>() && pCurrentLevel) - pCurrentLevel->SetGraphicBitmap(aAny.get>()); - } - catch (const beans::UnknownPropertyException&) - {} - - try - { - uno::Any aAny = xPropertySet->getPropertyValue("Bitmap"); - if (aAny.has>() && pCurrentLevel) - pCurrentLevel->SetGraphicBitmap(aAny.get>()); - + { + auto xGraphic = aAny.get>(); + if (xGraphic.is()) + { + uno::Reference xBitmap(xGraphic, uno::UNO_QUERY); + pCurrentLevel->SetGraphicBitmap(xBitmap); + } + } } catch (const beans::UnknownPropertyException&) {} diff --git a/writerfilter/source/dmapper/NumberingManager.hxx b/writerfilter/source/dmapper/NumberingManager.hxx index f93269b5b1f9..98e029d2188b 100644 --- a/writerfilter/source/dmapper/NumberingManager.hxx +++ b/writerfilter/source/dmapper/NumberingManager.hxx @@ -29,6 +29,7 @@ #include #include +#include namespace writerfilter { namespace dmapper { @@ -47,7 +48,7 @@ class ListLevel : public PropertyMap sal_Int16 m_nXChFollow; //LN_IXCHFOLLOW OUString m_sBulletChar; css::awt::Size m_aGraphicSize; - css::uno::Reference m_sGraphicBitmap; + css::uno::Reference m_xGraphicBitmap; sal_Int32 m_nTabstop; std::shared_ptr< StyleSheetEntry > m_pParaStyle; bool m_outline; @@ -71,8 +72,8 @@ public: void SetBulletChar( const OUString& sValue ) { m_sBulletChar = sValue; }; void SetGraphicSize( const css::awt::Size& aValue ) { m_aGraphicSize = aValue; }; - void SetGraphicBitmap(css::uno::Reference const& sValue) - { m_sGraphicBitmap = sValue; } + void SetGraphicBitmap(css::uno::Reference const& xGraphicBitmap) + { m_xGraphicBitmap = xGraphicBitmap; } void SetParaStyle( const std::shared_ptr< StyleSheetEntry >& pStyle ); // Getters -- cgit