filter/source/svg/svgfilter.cxx | 349 +++++++++++++++++++--------------------- 1 file changed, 171 insertions(+), 178 deletions(-)
New commits: commit 6650e2cf483ce8d28e0b77f182127465376e7d86 Author: Mike Kaganski <[email protected]> AuthorDate: Tue Dec 2 12:21:41 2025 +0500 Commit: Mike Kaganski <[email protected]> CommitDate: Tue Dec 2 13:04:56 2025 +0100 Simplify SVGFilter::filterImpressOrDraw by using a ScopeGuard The fake loop was used only to allow early exit, and the flow had to go thwough Window::LeaveWait in the end. Use of the ScopeGuard streamlines the flow. Change-Id: I244035c0f641b8c82c0d5ce2c9cf2fb443bbca6d Reviewed-on: https://gerrit.libreoffice.org/c/core/+/194901 Tested-by: Jenkins Reviewed-by: Mike Kaganski <[email protected]> diff --git a/filter/source/svg/svgfilter.cxx b/filter/source/svg/svgfilter.cxx index ca9b07a80b24..e59e36d2e1ee 100644 --- a/filter/source/svg/svgfilter.cxx +++ b/filter/source/svg/svgfilter.cxx @@ -205,217 +205,215 @@ bool SVGFilter::filterImpressOrDraw( const Sequence< PropertyValue >& rDescripto { SolarMutexGuard aGuard; vcl::Window* pFocusWindow(Application::GetFocusWindow()); - bool bRet(false); - if(pFocusWindow) { pFocusWindow->EnterWait(); } + comphelper::ScopeGuard aLeaveWaitGuard( + [pFocusWindow]() + { + if (pFocusWindow) + pFocusWindow->LeaveWait(); + }); if(mxDstDoc.is()) { - // Import. Use an endless loop to have easy exits for error handling - while(true) - { - // use MediaDescriptor to get needed data out of Sequence< PropertyValue > - utl::MediaDescriptor aMediaDescriptor(rDescriptor); - uno::Reference<io::XInputStream> xInputStream; + // Import. + // use MediaDescriptor to get needed data out of Sequence< PropertyValue > + utl::MediaDescriptor aMediaDescriptor(rDescriptor); + uno::Reference<io::XInputStream> xInputStream; - xInputStream.set(aMediaDescriptor[utl::MediaDescriptor::PROP_INPUTSTREAM], UNO_QUERY); + xInputStream.set(aMediaDescriptor[utl::MediaDescriptor::PROP_INPUTSTREAM], UNO_QUERY); - if(!xInputStream.is()) - { - // we need the InputStream - break; - } + if(!xInputStream.is()) + { + // we need the InputStream + return false; + } - // get the DrawPagesSupplier - uno::Reference< drawing::XDrawPagesSupplier > xDrawPagesSupplier( mxDstDoc, uno::UNO_QUERY ); + // get the DrawPagesSupplier + uno::Reference< drawing::XDrawPagesSupplier > xDrawPagesSupplier( mxDstDoc, uno::UNO_QUERY ); - if(!xDrawPagesSupplier.is()) - { - // we need the DrawPagesSupplier - break; - } + if(!xDrawPagesSupplier.is()) + { + // we need the DrawPagesSupplier + return false; + } - // get the DrawPages - uno::Reference< drawing::XDrawPages > xDrawPages = xDrawPagesSupplier->getDrawPages(); + // get the DrawPages + uno::Reference< drawing::XDrawPages > xDrawPages = xDrawPagesSupplier->getDrawPages(); - if(!xDrawPages.is()) - { - // we need the DrawPages - break; - } + if(!xDrawPages.is()) + { + // we need the DrawPages + return false; + } - // check DrawPageCount (there should be one by default) - sal_Int32 nDrawPageCount(xDrawPages->getCount()); + // check DrawPageCount (there should be one by default) + sal_Int32 nDrawPageCount(xDrawPages->getCount()); - if(0 == nDrawPageCount) - { - // at least one DrawPage should be there - we need that - break; - } + if(0 == nDrawPageCount) + { + // at least one DrawPage should be there - we need that + return false; + } - // get that DrawPage - uno::Reference< drawing::XDrawPage > xDrawPage( xDrawPages->getByIndex(0), uno::UNO_QUERY ); + // get that DrawPage + uno::Reference< drawing::XDrawPage > xDrawPage( xDrawPages->getByIndex(0), uno::UNO_QUERY ); - if(!xDrawPage.is()) - { - // we need that DrawPage - break; - } + if(!xDrawPage.is()) + { + // we need that DrawPage + return false; + } - // get that DrawPage's UNO API implementation - SvxDrawPage* pSvxDrawPage(comphelper::getFromUnoTunnel<SvxDrawPage>(xDrawPage)); + // get that DrawPage's UNO API implementation + SvxDrawPage* pSvxDrawPage(comphelper::getFromUnoTunnel<SvxDrawPage>(xDrawPage)); - if(nullptr == pSvxDrawPage || nullptr == pSvxDrawPage->GetSdrPage()) - { - // we need a SvxDrawPage - break; - } + if(nullptr == pSvxDrawPage || nullptr == pSvxDrawPage->GetSdrPage()) + { + // we need a SvxDrawPage + return false; + } - // get the SvStream to work with - std::unique_ptr< SvStream > aStream(utl::UcbStreamHelper::CreateStream(xInputStream, true)); + // get the SvStream to work with + std::unique_ptr< SvStream > aStream(utl::UcbStreamHelper::CreateStream(xInputStream, true)); - if (!aStream) - { - // we need the SvStream - break; - } + if (!aStream) + { + // we need the SvStream + return false; + } - // create a GraphicFilter and load the SVG (format already known, thus *could* - // be handed over to ImportGraphic - but detection is fast). - // As a bonus, zipped data is already detected and handled there - GraphicFilter aGraphicFilter; - Graphic aGraphic; - const ErrCode nGraphicFilterErrorCode( - aGraphicFilter.ImportGraphic(aGraphic, u"", *aStream)); + // create a GraphicFilter and load the SVG (format already known, thus *could* + // be handed over to ImportGraphic - but detection is fast). + // As a bonus, zipped data is already detected and handled there + GraphicFilter aGraphicFilter; + Graphic aGraphic; + const ErrCode nGraphicFilterErrorCode( + aGraphicFilter.ImportGraphic(aGraphic, u"", *aStream)); - if(ERRCODE_NONE != nGraphicFilterErrorCode) - { - // SVG import error, cannot continue - break; - } + if(ERRCODE_NONE != nGraphicFilterErrorCode) + { + // SVG import error, cannot continue + return false; + } - // get the GraphicPrefSize early to check if we have any content - // (the SVG may contain nothing and/or just <g visibility="hidden"> stuff...) - const Size aGraphicPrefSize(aGraphic.GetPrefSize()); + // get the GraphicPrefSize early to check if we have any content + // (the SVG may contain nothing and/or just <g visibility="hidden"> stuff...) + const Size aGraphicPrefSize(aGraphic.GetPrefSize()); - if(0 == aGraphicPrefSize.Width() || 0 == aGraphicPrefSize.Height()) - { - // SVG has no displayable content, stop import. - // Also possible would be to get the sequence< Primitives > - // from aGraphic and check if it is empty. - // Possibility to set some error message here to tell - // the user what/why loading went wrong, but I do not - // know how this could be done here - break; - } + if(0 == aGraphicPrefSize.Width() || 0 == aGraphicPrefSize.Height()) + { + // SVG has no displayable content, stop import. + // Also possible would be to get the sequence< Primitives > + // from aGraphic and check if it is empty. + // Possibility to set some error message here to tell + // the user what/why loading went wrong, but I do not + // know how this could be done here + return false; + } + + // tdf#118232 Get the sequence of primitives and check if geometry is completely + // hidden. If so, there is no need to add a SdrObject at all + auto const & rVectorGraphicData(aGraphic.getVectorGraphicData()); + bool bContainsNoGeometry(false); - // tdf#118232 Get the sequence of primitives and check if geometry is completely - // hidden. If so, there is no need to add a SdrObject at all - auto const & rVectorGraphicData(aGraphic.getVectorGraphicData()); - bool bContainsNoGeometry(false); + if(bool(rVectorGraphicData) && VectorGraphicDataType::Svg == rVectorGraphicData->getType()) + { + const drawinglayer::primitive2d::Primitive2DContainer aContainer(rVectorGraphicData->getPrimitive2DSequence()); - if(bool(rVectorGraphicData) && VectorGraphicDataType::Svg == rVectorGraphicData->getType()) + if(!aContainer.empty()) { - const drawinglayer::primitive2d::Primitive2DContainer aContainer(rVectorGraphicData->getPrimitive2DSequence()); + bool bAllAreHiddenGeometry(true); - if(!aContainer.empty()) + for(const auto& rCandidate : aContainer) { - bool bAllAreHiddenGeometry(true); - - for(const auto& rCandidate : aContainer) - { - if(rCandidate && PRIMITIVE2D_ID_HIDDENGEOMETRYPRIMITIVE2D != rCandidate->getPrimitive2DID()) - { - bAllAreHiddenGeometry = false; - break; - } - } - - if(bAllAreHiddenGeometry) + if(rCandidate && PRIMITIVE2D_ID_HIDDENGEOMETRYPRIMITIVE2D != rCandidate->getPrimitive2DID()) { - bContainsNoGeometry = true; + bAllAreHiddenGeometry = false; + break; } } - } - - // create a SdrModel-GraphicObject to insert to page - SdrPage* pTargetSdrPage(pSvxDrawPage->GetSdrPage()); - rtl::Reference< SdrGrafObj > aNewSdrGrafObj; - // tdf#118232 only add an SdrGrafObj when we have Geometry - if(!bContainsNoGeometry) - { - aNewSdrGrafObj = - new SdrGrafObj( - pTargetSdrPage->getSdrModelFromSdrPage(), - aGraphic); + if(bAllAreHiddenGeometry) + { + bContainsNoGeometry = true; + } } + } - // Evtl. adapt the GraphicPrefSize to target-MapMode of target-Model - // (should be 100thmm here, but just stay safe by doing the conversion) - const MapMode aGraphicPrefMapMode(aGraphic.GetPrefMapMode()); - const MapUnit eDestUnit(pTargetSdrPage->getSdrModelFromSdrPage().GetItemPool().GetMetric(0)); - const MapUnit eSrcUnit(aGraphicPrefMapMode.GetMapUnit()); - Size aGraphicSize(aGraphicPrefSize); + // create a SdrModel-GraphicObject to insert to page + SdrPage* pTargetSdrPage(pSvxDrawPage->GetSdrPage()); + rtl::Reference< SdrGrafObj > aNewSdrGrafObj; - if (eDestUnit != eSrcUnit) - { - aGraphicSize = Size( - OutputDevice::LogicToLogic(aGraphicSize.Width(), eSrcUnit, eDestUnit), - OutputDevice::LogicToLogic(aGraphicSize.Height(), eSrcUnit, eDestUnit)); - } + // tdf#118232 only add an SdrGrafObj when we have Geometry + if(!bContainsNoGeometry) + { + aNewSdrGrafObj = + new SdrGrafObj( + pTargetSdrPage->getSdrModelFromSdrPage(), + aGraphic); + } - // Based on GraphicSize, set size of Page. Do not forget to adapt PageBorders, - // but interpret them relative to PageSize so that they do not 'explode/shrink' - // in comparison. Use a common scaling factor for hor/ver to not get - // asynchronous border distances, though. All in all this will adapt borders - // nicely and is based on office-defaults for standard-page-border-sizes. - const Size aPageSize(pTargetSdrPage->GetSize()); - const double fBorderRelation(( - static_cast< double >(pTargetSdrPage->GetLeftBorder()) / aPageSize.Width() + - static_cast< double >(pTargetSdrPage->GetRightBorder()) / aPageSize.Width() + - static_cast< double >(pTargetSdrPage->GetUpperBorder()) / aPageSize.Height() + - static_cast< double >(pTargetSdrPage->GetLowerBorder()) / aPageSize.Height()) / 4.0); - const tools::Long nAllBorder(basegfx::fround<tools::Long>((aGraphicSize.Width() + aGraphicSize.Height()) * fBorderRelation * 0.5)); - - // Adapt PageSize and Border stuff. To get all MasterPages and PresObjs - // correctly adapted, do not just use - // pTargetSdrPage->SetBorder(...) and - // pTargetSdrPage->SetSize(...), - // but ::adaptSizeAndBorderForAllPages - // Do use original Size and borders to get as close to original - // as possible for better turn-arounds. - pTargetSdrPage->getSdrModelFromSdrPage().adaptSizeAndBorderForAllPages( - Size( - aGraphicSize.Width(), - aGraphicSize.Height()), - nAllBorder, - nAllBorder, - nAllBorder, - nAllBorder); - - // tdf#118232 set pos/size at SdrGraphicObj - use zero position for - // better turn-around results - if(!bContainsNoGeometry) - { - aNewSdrGrafObj->SetSnapRect( - tools::Rectangle( - Point(0, 0), - aGraphicSize)); + // Evtl. adapt the GraphicPrefSize to target-MapMode of target-Model + // (should be 100thmm here, but just stay safe by doing the conversion) + const MapMode aGraphicPrefMapMode(aGraphic.GetPrefMapMode()); + const MapUnit eDestUnit(pTargetSdrPage->getSdrModelFromSdrPage().GetItemPool().GetMetric(0)); + const MapUnit eSrcUnit(aGraphicPrefMapMode.GetMapUnit()); + Size aGraphicSize(aGraphicPrefSize); - // insert to page (owner change of SdrGrafObj) - pTargetSdrPage->InsertObject(aNewSdrGrafObj.get()); - } + if (eDestUnit != eSrcUnit) + { + aGraphicSize = Size( + OutputDevice::LogicToLogic(aGraphicSize.Width(), eSrcUnit, eDestUnit), + OutputDevice::LogicToLogic(aGraphicSize.Height(), eSrcUnit, eDestUnit)); + } - // done - set positive result now - bRet = true; + // Based on GraphicSize, set size of Page. Do not forget to adapt PageBorders, + // but interpret them relative to PageSize so that they do not 'explode/shrink' + // in comparison. Use a common scaling factor for hor/ver to not get + // asynchronous border distances, though. All in all this will adapt borders + // nicely and is based on office-defaults for standard-page-border-sizes. + const Size aPageSize(pTargetSdrPage->GetSize()); + const double fBorderRelation(( + static_cast< double >(pTargetSdrPage->GetLeftBorder()) / aPageSize.Width() + + static_cast< double >(pTargetSdrPage->GetRightBorder()) / aPageSize.Width() + + static_cast< double >(pTargetSdrPage->GetUpperBorder()) / aPageSize.Height() + + static_cast< double >(pTargetSdrPage->GetLowerBorder()) / aPageSize.Height()) / 4.0); + const tools::Long nAllBorder(basegfx::fround<tools::Long>((aGraphicSize.Width() + aGraphicSize.Height()) * fBorderRelation * 0.5)); + + // Adapt PageSize and Border stuff. To get all MasterPages and PresObjs + // correctly adapted, do not just use + // pTargetSdrPage->SetBorder(...) and + // pTargetSdrPage->SetSize(...), + // but ::adaptSizeAndBorderForAllPages + // Do use original Size and borders to get as close to original + // as possible for better turn-arounds. + pTargetSdrPage->getSdrModelFromSdrPage().adaptSizeAndBorderForAllPages( + Size( + aGraphicSize.Width(), + aGraphicSize.Height()), + nAllBorder, + nAllBorder, + nAllBorder, + nAllBorder); + + // tdf#118232 set pos/size at SdrGraphicObj - use zero position for + // better turn-around results + if(!bContainsNoGeometry) + { + aNewSdrGrafObj->SetSnapRect( + tools::Rectangle( + Point(0, 0), + aGraphicSize)); - // always leave helper endless loop - break; + // insert to page (owner change of SdrGrafObj) + pTargetSdrPage->InsertObject(aNewSdrGrafObj.get()); } + + // done - set positive result now + return true; } else if( mxSrcDoc.is() ) { @@ -506,12 +504,12 @@ bool SVGFilter::filterImpressOrDraw( const Sequence< PropertyValue >& rDescripto } } - if(bSelectionOnly && bGotSelection && 0 == maShapeSelection->getCount()) + if (bGotSelection && 0 == maShapeSelection->getCount()) { // #i124608# export selection, got maShapeSelection but no shape selected -> nothing // to export, we are done (maybe return true, but a hint that nothing was done // may be useful; it may have happened by error) - bRet = false; + return false; } else { @@ -532,16 +530,11 @@ bool SVGFilter::filterImpressOrDraw( const Sequence< PropertyValue >& rDescripto mMasterPageTargets.insert(mMasterPageTargets.end(), aMasterPageTargetSet.begin(), aMasterPageTargetSet.end()); - bRet = implExport( rDescriptor ); + return implExport( rDescriptor ); } } - else - bRet = false; - - if( pFocusWindow ) - pFocusWindow->LeaveWait(); - return bRet; + return false; } bool SVGFilter::filterWriterOrCalc( const Sequence< PropertyValue >& rDescriptor )
