svtools/CppunitTest_svtools_graphic.mk | 40 +++ svtools/qa/unit/GraphicObjectTest.cxx | 108 +++++++++- svtools/qa/unit/data/document_with_two_images.odt |binary svx/source/svdraw/svdograf.cxx | 2 sw/CppunitTest_sw_odfexport.mk | 4 sw/qa/extras/odfexport/data/document_with_two_images_with_special_IDs.odt |binary sw/qa/extras/odfexport/odfexport.cxx | 50 ++++ sw/source/core/graphic/ndgrf.cxx | 49 ---- 8 files changed, 207 insertions(+), 46 deletions(-)
New commits: commit b0c97aecb22cfa8d845cdc7d2764a4320b53baf6 Author: Zolnai Tamás <[email protected]> Date: Sun Nov 16 15:23:54 2014 +0100 Set back these lines, later it can be useful Removed in: 9dc3b49c891fb9fe45c24de4b7e1e88fe400afe0 Change-Id: Id8cee4e17d214ca0eaa5cd11dc25849e5f68851e diff --git a/sw/source/core/graphic/ndgrf.cxx b/sw/source/core/graphic/ndgrf.cxx index 682546e..ff601f5 100644 --- a/sw/source/core/graphic/ndgrf.cxx +++ b/sw/source/core/graphic/ndgrf.cxx @@ -663,6 +663,10 @@ bool SwGrfNode::SavePersistentData() return true; } + // swap in first if in storage + if( HasEmbeddedStreamName() && !SwapIn() ) + return false; + // #i44367# // Do not delete graphic file in storage, because the graphic file could // be referenced by other graphic nodes. commit a6e2e54c7e9ec5b852db3b3be578d79fda666601 Author: Zolnai Tamás <[email protected]> Date: Sun Nov 16 15:17:43 2014 +0100 Avoid DelStreamName because it can lead to image loss. See also: f811e628411bda29a76ebb1f72eb107ce67d27f0 The problem is that more images can have the same stream name so we can't decide here when to remove one stream name. Better to leak in the storage as to loose images (actually we already leak here, so) Change-Id: I2c2afe87e024c2521fe22d62126b567931604101 diff --git a/sw/source/core/graphic/ndgrf.cxx b/sw/source/core/graphic/ndgrf.cxx index c6013af..682546e 100644 --- a/sw/source/core/graphic/ndgrf.cxx +++ b/sw/source/core/graphic/ndgrf.cxx @@ -223,20 +223,12 @@ bool SwGrfNode::ReRead( } else if( pGraphic && rGrfName.isEmpty() ) { - // Old stream must be deleted before the new one is set. - if( HasEmbeddedStreamName() ) - DelStreamName(); - maGrfObj.SetGraphic( *pGraphic ); onGraphicChanged(); bReadGrf = true; } else if( pGrfObj && rGrfName.isEmpty() ) { - // Old stream must be deleted before the new one is set. - if( HasEmbeddedStreamName() ) - DelStreamName(); - maGrfObj = *pGrfObj; onGraphicChanged(); bReadGrf = true; @@ -246,9 +238,6 @@ bool SwGrfNode::ReRead( return true; else { - if( HasEmbeddedStreamName() ) - DelStreamName(); - // create new link for the graphic object InsertLink( rGrfName, rFltName ); @@ -859,36 +848,6 @@ void SwGrfNode::ScaleImageMap() } } -void SwGrfNode::DelStreamName() -{ - if( HasEmbeddedStreamName() ) - { - // then remove graphic from storage - uno::Reference < embed::XStorage > xDocStg = GetDoc()->GetDocStorage(); - if( xDocStg.is() ) - { - try - { - const StreamAndStorageNames aNames = lcl_GetStreamStorageNames( maGrfObj.GetUserData() ); - uno::Reference < embed::XStorage > refPics = xDocStg; - if ( !aNames.sStorage.isEmpty() ) - refPics = xDocStg->openStorageElement( aNames.sStorage, embed::ElementModes::READWRITE ); - refPics->removeElement( aNames.sStream ); - uno::Reference < embed::XTransactedObject > xTrans( refPics, uno::UNO_QUERY ); - if ( xTrans.is() ) - xTrans->commit(); - } - catch (const uno::Exception&) - { - // #i48434# - OSL_FAIL( "<SwGrfNode::DelStreamName()> - unhandled exception!" ); - } - } - - maGrfObj.SetUserData(); - } -} - /** helper method to get a substorage of the document storage for readonly access. OD, MAV 2005-08-17 #i53025# commit 286e2f5c6ec829bc0987b1be7016699f7ef03e5e Author: Zolnai Tamás <[email protected]> Date: Sun Nov 16 15:12:54 2014 +0100 Related fdo#82953: Forget package URL of image after it is loaded It causes problems if we handle those imported images differently which are identified by a package URL, so after the first load remove this URL and handle images on the same way as inserted images. Some related bugs: * #i44367# * #i124946# * #i114361# * fdo#73270 The image in the test document has a special ID which is different from that one which is generated by LO internally so after ODF export the new generated image URL is different from the imported one. Change-Id: I4e7d3490674c5f86bec5c7c6e1c975dcafd7c265 diff --git a/svx/source/svdraw/svdograf.cxx b/svx/source/svdraw/svdograf.cxx index c2bbeb1..40d652b 100644 --- a/svx/source/svdraw/svdograf.cxx +++ b/svx/source/svdraw/svdograf.cxx @@ -1351,7 +1351,7 @@ IMPL_LINK( SdrGrafObj, ImpSwapHdl, GraphicObject*, pO ) const OUString aNewUserData( pGraphic->GetUserData() ); pGraphic->SetGraphic( aGraphic ); - pGraphic->SetUserData( aNewUserData ); + pGraphic->SetUserData(); // Graphic successfully swapped in. pRet = GRFMGR_AUTOSWAPSTREAM_LOADED; diff --git a/sw/CppunitTest_sw_odfexport.mk b/sw/CppunitTest_sw_odfexport.mk index 080a840..97398af 100644 --- a/sw/CppunitTest_sw_odfexport.mk +++ b/sw/CppunitTest_sw_odfexport.mk @@ -80,6 +80,10 @@ $(eval $(call gb_CppunitTest_use_components,sw_odfexport,\ xmloff/util/xo \ )) +$(eval $(call gb_CppunitTest_use_custom_headers,sw_odfexport,\ + officecfg/registry \ +)) + $(eval $(call gb_CppunitTest_use_configuration,sw_odfexport)) $(eval $(call gb_CppunitTest_use_unittest_configuration,sw_odfexport)) diff --git a/sw/qa/extras/odfexport/data/document_with_two_images_with_special_IDs.odt b/sw/qa/extras/odfexport/data/document_with_two_images_with_special_IDs.odt new file mode 100644 index 0000000..867c075 Binary files /dev/null and b/sw/qa/extras/odfexport/data/document_with_two_images_with_special_IDs.odt differ diff --git a/sw/qa/extras/odfexport/odfexport.cxx b/sw/qa/extras/odfexport/odfexport.cxx index c140540..d7360ba 100644 --- a/sw/qa/extras/odfexport/odfexport.cxx +++ b/sw/qa/extras/odfexport/odfexport.cxx @@ -17,6 +17,9 @@ #include <com/sun/star/text/RelOrientation.hpp> #include <com/sun/star/text/XDocumentIndex.hpp> #include <com/sun/star/drawing/TextVerticalAdjust.hpp> +#include <com/sun/star/awt/XBitmap.hpp> +#include <com/sun/star/graphic/XGraphic.hpp> +#include <officecfg/Office/Common.hxx> class Test : public SwModelTestBase { @@ -401,6 +404,53 @@ DECLARE_ODFEXPORT_TEST(testTextboxRoundedCorners, "textbox-rounded-corners.odt") CPPUNIT_ASSERT_EQUAL(OUString("a"), xCell->getString()); } +DECLARE_ODFEXPORT_TEST(testImageWithSpecialID, "document_with_two_images_with_special_IDs.odt") +{ + // Here the problem was that LO did not handle well those image URLs in the ODT file which are + // not match with that one which is generated by LO internaly (based on the image's size, type and so on) + + // Trigger swap out mechanism to test swapped state factor too. + boost::shared_ptr< comphelper::ConfigurationChanges > batch(comphelper::ConfigurationChanges::create()); + officecfg::Office::Common::Cache::GraphicManager::TotalCacheSize::set(sal_Int32(1), batch); + batch->commit(); + + uno::Reference<drawing::XShape> xImage = getShape(1); + uno::Reference< beans::XPropertySet > XPropSet( xImage, uno::UNO_QUERY_THROW ); + // Check URL + { + OUString sURL; + XPropSet->getPropertyValue("GraphicURL") >>= sURL; + CPPUNIT_ASSERT(sURL != OUString("vnd.sun.star.GraphicObject:00000000000000000000000000000000")); + } + // Check size + { + uno::Reference<graphic::XGraphic> xGraphic; + XPropSet->getPropertyValue("Graphic") >>= xGraphic; + uno::Reference<awt::XBitmap> xBitmap(xGraphic, uno::UNO_QUERY); + CPPUNIT_ASSERT(xBitmap.is()); + CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int32>(610), xBitmap->getSize().Width ); + CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int32>(381), xBitmap->getSize().Height ); + } + // Second Image + xImage = getShape(2); + XPropSet.set( xImage, uno::UNO_QUERY_THROW ); + // Check URL + { + OUString sURL; + XPropSet->getPropertyValue("GraphicURL") >>= sURL; + CPPUNIT_ASSERT(sURL != OUString("vnd.sun.star.GraphicObject:00000000000000000000000000000000")); + } + // Check size + { + uno::Reference<graphic::XGraphic> xGraphic; + XPropSet->getPropertyValue("Graphic") >>= xGraphic; + uno::Reference<awt::XBitmap> xBitmap(xGraphic, uno::UNO_QUERY); + CPPUNIT_ASSERT(xBitmap.is()); + CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int32>(900), xBitmap->getSize().Width ); + CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int32>(600), xBitmap->getSize().Height ); + } +} + #endif CPPUNIT_PLUGIN_IMPLEMENT(); diff --git a/sw/source/core/graphic/ndgrf.cxx b/sw/source/core/graphic/ndgrf.cxx index f9cbb9c..c6013af 100644 --- a/sw/source/core/graphic/ndgrf.cxx +++ b/sw/source/core/graphic/ndgrf.cxx @@ -581,6 +581,10 @@ bool SwGrfNode::SwapIn( bool bWaitForData ) { bRet = ImportGraphic( *pStrm ); delete pStrm; + if( bRet ) + { + maGrfObj.SetUserData(); + } } } catch (const uno::Exception&) commit c454a0cd0cc7b315b1353b151a2e95654df72c69 Author: Zolnai Tamás <[email protected]> Date: Fri Nov 7 16:44:54 2014 +0100 Test for size based auto swap out mechanism Change-Id: Iff0942b9b545f27dd74b73bee3f8ac785539867d diff --git a/svtools/CppunitTest_svtools_graphic.mk b/svtools/CppunitTest_svtools_graphic.mk index ef9c7ab..6036afb 100644 --- a/svtools/CppunitTest_svtools_graphic.mk +++ b/svtools/CppunitTest_svtools_graphic.mk @@ -11,6 +11,7 @@ $(eval $(call gb_CppunitTest_CppunitTest,svtools_graphic)) $(eval $(call gb_CppunitTest_use_externals,svtools_graphic,\ boost_headers \ + libxml2 \ )) $(eval $(call gb_CppunitTest_use_api,svtools_graphic, \ @@ -29,18 +30,53 @@ $(eval $(call gb_CppunitTest_use_libraries,svtools_graphic, \ tl \ sal \ svt \ + sw \ test \ unotest \ vcl \ $(gb_UWINAPI) \ )) +$(eval $(call gb_CppunitTest_set_include,svtools_graphic,\ + -I$(SRCDIR)/sw/inc \ + $$(INCLUDE) \ +)) + +$(eval $(call gb_CppunitTest_use_custom_headers,svtools_graphic,\ + officecfg/registry \ +)) + $(eval $(call gb_CppunitTest_use_configuration,svtools_graphic)) $(eval $(call gb_CppunitTest_use_components,svtools_graphic,\ - configmgr/source/configmgr \ - ucb/source/core/ucb1 \ + basic/util/sb \ + comphelper/util/comphelp \ + configmgr/source/configmgr \ + embeddedobj/util/embobj \ + filter/source/config/cache/filterconfig1 \ + filter/source/storagefilterdetect/storagefd \ + framework/util/fwk \ + i18npool/util/i18npool \ + linguistic/source/lng \ + oox/util/oox \ + package/source/xstor/xstor \ + package/util/package2 \ + sax/source/expatwrap/expwrap \ + sfx2/util/sfx \ + starmath/util/sm \ + svl/source/fsstor/fsstorage \ + svtools/util/svt \ + sw/util/sw \ + sw/util/swd \ + toolkit/util/tk \ + ucb/source/core/ucb1 \ ucb/source/ucp/file/ucpfile1 \ + unotools/util/utl \ + unoxml/source/service/unoxml \ + uui/util/uui \ + writerfilter/util/writerfilter \ + $(if $(filter DESKTOP,$(BUILD_TYPE)),xmlhelp/util/ucpchelp1) \ + xmloff/util/xo \ )) $(eval $(call gb_CppunitTest_add_exception_objects,svtools_graphic, \ diff --git a/svtools/qa/unit/GraphicObjectTest.cxx b/svtools/qa/unit/GraphicObjectTest.cxx index 29259c4..34e26fd 100644 --- a/svtools/qa/unit/GraphicObjectTest.cxx +++ b/svtools/qa/unit/GraphicObjectTest.cxx @@ -19,21 +19,42 @@ #include <vcl/image.hxx> +#include <com/sun/star/frame/Desktop.hpp> +#include <officecfg/Office/Common.hxx> +#include <unotest/macros_test.hxx> +#include <comphelper/processfactory.hxx> +#include <unotxdoc.hxx> +#include <docsh.hxx> +#include <doc.hxx> +#include <ndgrf.hxx> +#include <boost/shared_ptr.hpp> + +using namespace css; + namespace { -class GraphicObjectTest: public test::BootstrapFixture +class GraphicObjectTest: public test::BootstrapFixture, public unotest::MacrosTest { public: void testSwap(); + void testSizeBasedAutoSwap(); + + + virtual void setUp() SAL_OVERRIDE + { + test::BootstrapFixture::setUp(); + + mxDesktop.set(css::frame::Desktop::create(comphelper::getComponentContext(getMultiServiceFactory()))); + } private: DECL_LINK(getLinkStream, GraphicObject*); private: CPPUNIT_TEST_SUITE(GraphicObjectTest); - CPPUNIT_TEST(testSwap); + CPPUNIT_TEST(testSizeBasedAutoSwap); CPPUNIT_TEST_SUITE_END(); }; @@ -120,6 +141,89 @@ void GraphicObjectTest::testSwap() } } +void GraphicObjectTest::testSizeBasedAutoSwap() +{ + // Set cache size to a very small value to check what happens + { + boost::shared_ptr< comphelper::ConfigurationChanges > aBatch(comphelper::ConfigurationChanges::create()); + officecfg::Office::Common::Cache::GraphicManager::TotalCacheSize::set(sal_Int32(1), aBatch); + aBatch->commit(); + } + + uno::Reference< lang::XComponent > xComponent = + loadFromDesktop(getURLFromSrc("svtools/qa/unit/data/document_with_two_images.odt"), "com.sun.star.text.TextDocument"); + + SwXTextDocument* pTxtDoc = dynamic_cast<SwXTextDocument *>(xComponent.get()); + CPPUNIT_ASSERT(pTxtDoc); + SwDoc* pDoc = pTxtDoc->GetDocShell()->GetDoc(); + CPPUNIT_ASSERT(pDoc); + SwNodes& aNodes = pDoc->GetNodes(); + + // Find images + const GraphicObject* pGrafObj1 = 0; + const GraphicObject* pGrafObj2 = 0; + for( sal_uLong nIndex = 0; nIndex < aNodes.Count(); ++nIndex) + { + if( aNodes[nIndex]->IsGrfNode() ) + { + SwGrfNode* pGrfNode = aNodes[nIndex]->GetGrfNode(); + if( !pGrafObj1 ) + { + pGrafObj1 = &pGrfNode->GetGrfObj(); + } + else + { + pGrafObj2 = &pGrfNode->GetGrfObj(); + } + } + } + CPPUNIT_ASSERT_MESSAGE("Missing image", pGrafObj1 != 0 && pGrafObj2 != 0); + + { + // First image should be swapped out + CPPUNIT_ASSERT(pGrafObj1->IsSwappedOut()); + CPPUNIT_ASSERT_EQUAL(sal_uLong(697230), pGrafObj1->GetSizeBytes()); + + // Still swapped out: size is cached + CPPUNIT_ASSERT(pGrafObj1->IsSwappedOut()); + } + + { + // Second image should be in the memory + // Size based swap out is triggered by swap in, so the last swapped in image should be + // in the memory despite of size limit is reached. + CPPUNIT_ASSERT(!pGrafObj2->IsSwappedOut()); + CPPUNIT_ASSERT_EQUAL(sal_uLong(1620000), pGrafObj2->GetSizeBytes()); + } + + // Swap in first image -> second image will be swapped out + { + pGrafObj1->GetGraphic(); // GetGraphic calls swap in on a const object + CPPUNIT_ASSERT(!pGrafObj1->IsSwappedOut()); + CPPUNIT_ASSERT(pGrafObj2->IsSwappedOut()); + } + + // Swap in second image -> first image will be swapped out + { + pGrafObj2->GetGraphic(); // GetGraphic calls swap in on a const object + CPPUNIT_ASSERT(!pGrafObj2->IsSwappedOut()); + CPPUNIT_ASSERT(pGrafObj1->IsSwappedOut()); + } + + // Use bigger cache + { + GraphicManager& rGrfMgr = pGrafObj1->GetGraphicManager(); + rGrfMgr.SetMaxCacheSize(pGrafObj1->GetSizeBytes()+pGrafObj2->GetSizeBytes()); + } + // Swap in both images -> both should be swapped in + { + pGrafObj1->GetGraphic(); + pGrafObj2->GetGraphic(); + CPPUNIT_ASSERT(!pGrafObj1->IsSwappedOut()); + CPPUNIT_ASSERT(!pGrafObj2->IsSwappedOut()); + } +} + CPPUNIT_TEST_SUITE_REGISTRATION(GraphicObjectTest); } diff --git a/svtools/qa/unit/data/document_with_two_images.odt b/svtools/qa/unit/data/document_with_two_images.odt new file mode 100644 index 0000000..54d3d66 Binary files /dev/null and b/svtools/qa/unit/data/document_with_two_images.odt differ
_______________________________________________ Libreoffice-commits mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/libreoffice-commits
