basegfx/source/polygon/b2dpolygontools.cxx |  136 ++++++++++++++---------------
 filter/source/msfilter/escherex.cxx        |  122 ++++----------------------
 2 files changed, 90 insertions(+), 168 deletions(-)

New commits:
commit d7efa0382ef236293991e4c7c4f48ce6bb2d64c3
Author:     Armin Le Grand <armin.le.gr...@cib.de>
AuthorDate: Wed Jul 4 10:13:16 2018 +0200
Commit:     Thorsten Behrens <thorsten.behr...@cib.de>
CommitDate: Sat Sep 12 18:07:40 2020 +0200

    Only access css::drawing::PointSequence when not empty
    
    Had to adapt EscherPropertyContainer::GetPolyPolygon due
    to it using conversions from UNO API drawing::PointSequence
    to old tools::polygon, containing unsafe memory accesses.
    It is not useful to fix that, use new tooling instead.
    
    Before correctly testing nCount for zero in b2dpolygontools.cxx
    when you look closely always a point was added - a random
    one due to accessing random memory.
    This is corrected, so in test case for "tdf104115.docx"
    a PolyPolygon with two polys is used, the first being
    empty (had one point due to the error mentioned above).
    When having no points, CreatePolygonProperties in escherex.cxx
    does badly calculate and alloc the pSegmentBuf array used
    to write to doc formats (in the test case - 20 bytes alloced,
    22 written). This did not happen before due to having
    always a point due to the error before - argh!
    Corrected that and hopefully this will work now.
    
    To be on the safe side and to not need to redefine that whole
    CreatePolygonProperties I will turn back that little change
    and better sort-out empty polygons inside GetPolyPolygon
    alrteady. That should bring us back to the original state,
    at the same time avoiding that CreatePolygonProperties has
    to handle empty Polygons at all.
    That stuff urgently needs cleanup - I took a look and thought
    about using std::vector<sal_uInt8> so no wrong alloc or write
    too much could happen, but that nTotalBezPoints needs to be
    pre-calculated because it gets itself written to that
    buffers...
    
    Change-Id: Iefc885928f5bb29bceaf36c2a1555346bb21fd26
    Reviewed-on: https://gerrit.libreoffice.org/56927
    Tested-by: Jenkins
    Reviewed-by: Armin Le Grand <armin.le.gr...@cib.de>
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/102533
    Tested-by: Thorsten Behrens <thorsten.behr...@cib.de>
    Reviewed-by: Thorsten Behrens <thorsten.behr...@cib.de>

diff --git a/basegfx/source/polygon/b2dpolygontools.cxx 
b/basegfx/source/polygon/b2dpolygontools.cxx
index db3365eee313..94590d7483a8 100644
--- a/basegfx/source/polygon/b2dpolygontools.cxx
+++ b/basegfx/source/polygon/b2dpolygontools.cxx
@@ -3285,90 +3285,94 @@ namespace basegfx
 
             // prepare new polygon
             B2DPolygon aRetval;
-            const css::awt::Point* pPointSequence = 
rPointSequenceSource.getConstArray();
-            const css::drawing::PolygonFlags* pFlagSequence = 
rFlagSequenceSource.getConstArray();
 
-            // get first point and flag
-            B2DPoint aNewCoordinatePair(pPointSequence->X, pPointSequence->Y); 
pPointSequence++;
-            css::drawing::PolygonFlags ePolygonFlag(*pFlagSequence); 
pFlagSequence++;
-            B2DPoint aControlA;
-            B2DPoint aControlB;
+            if(0 != nCount)
+            {
+                const css::awt::Point* pPointSequence = 
rPointSequenceSource.getConstArray();
+                const css::drawing::PolygonFlags* pFlagSequence = 
rFlagSequenceSource.getConstArray();
 
-            // first point is not allowed to be a control point
-            OSL_ENSURE(ePolygonFlag != css::drawing::PolygonFlags_CONTROL,
-                "UnoPolygonBezierCoordsToB2DPolygon: Start point is a control 
point, illegal input polygon (!)");
+                // get first point and flag
+                B2DPoint aNewCoordinatePair(pPointSequence->X, 
pPointSequence->Y); pPointSequence++;
+                css::drawing::PolygonFlags ePolygonFlag(*pFlagSequence); 
pFlagSequence++;
+                B2DPoint aControlA;
+                B2DPoint aControlB;
 
-            // add first point as start point
-            aRetval.append(aNewCoordinatePair);
+                // first point is not allowed to be a control point
+                OSL_ENSURE(ePolygonFlag != css::drawing::PolygonFlags_CONTROL,
+                    "UnoPolygonBezierCoordsToB2DPolygon: Start point is a 
control point, illegal input polygon (!)");
 
-            for(sal_uInt32 b(1); b < nCount;)
-            {
-                // prepare loop
-                bool bControlA(false);
-                bool bControlB(false);
+                // add first point as start point
+                aRetval.append(aNewCoordinatePair);
 
-                // get next point and flag
-                aNewCoordinatePair = B2DPoint(pPointSequence->X, 
pPointSequence->Y);
-                ePolygonFlag = *pFlagSequence;
-                pPointSequence++; pFlagSequence++; b++;
-
-                if(b < nCount && ePolygonFlag == 
css::drawing::PolygonFlags_CONTROL)
+                for(sal_uInt32 b(1); b < nCount;)
                 {
-                    aControlA = aNewCoordinatePair;
-                    bControlA = true;
+                    // prepare loop
+                    bool bControlA(false);
+                    bool bControlB(false);
 
                     // get next point and flag
                     aNewCoordinatePair = B2DPoint(pPointSequence->X, 
pPointSequence->Y);
                     ePolygonFlag = *pFlagSequence;
                     pPointSequence++; pFlagSequence++; b++;
-                }
 
-                if(b < nCount && ePolygonFlag == 
css::drawing::PolygonFlags_CONTROL)
-                {
-                    aControlB = aNewCoordinatePair;
-                    bControlB = true;
+                    if(b < nCount && ePolygonFlag == 
css::drawing::PolygonFlags_CONTROL)
+                    {
+                        aControlA = aNewCoordinatePair;
+                        bControlA = true;
 
-                    // get next point and flag
-                    aNewCoordinatePair = B2DPoint(pPointSequence->X, 
pPointSequence->Y);
-                    ePolygonFlag = *pFlagSequence;
-                    pPointSequence++; pFlagSequence++; b++;
-                }
+                        // get next point and flag
+                        aNewCoordinatePair = B2DPoint(pPointSequence->X, 
pPointSequence->Y);
+                        ePolygonFlag = *pFlagSequence;
+                        pPointSequence++; pFlagSequence++; b++;
+                    }
 
-                // two or no control points are consumed, another one would be 
an error.
-                // It's also an error if only one control point was read
-                SAL_WARN_IF(ePolygonFlag == css::drawing::PolygonFlags_CONTROL 
|| bControlA != bControlB,
-                    "basegfx", "UnoPolygonBezierCoordsToB2DPolygon: Illegal 
source polygon (!)");
+                    if(b < nCount && ePolygonFlag == 
css::drawing::PolygonFlags_CONTROL)
+                    {
+                        aControlB = aNewCoordinatePair;
+                        bControlB = true;
 
-                // the previous writes used the B2DPolyPoygon -> 
utils::PolyPolygon converter
-                // which did not create minimal PolyPolygons, but created all 
control points
-                // as null vectors (identical points). Because of the former 
P(CA)(CB)-norm of
-                // B2DPolygon and it's unused sign of being the zero-vector 
and CA and CB being
-                // relative to P, an empty edge was exported as P == CA == CB. 
Luckily, the new
-                // export format can be read without errors by the old 
OOo-versions, so we need only
-                // to correct here at read and do not need to export a wrong 
but compatible version
-                // for the future.
-                if(bControlA
-                    && aControlA.equal(aControlB)
-                    && aControlA.equal(aRetval.getB2DPoint(aRetval.count() - 
1)))
-                {
-                    bControlA = false;
-                }
+                        // get next point and flag
+                        aNewCoordinatePair = B2DPoint(pPointSequence->X, 
pPointSequence->Y);
+                        ePolygonFlag = *pFlagSequence;
+                        pPointSequence++; pFlagSequence++; b++;
+                    }
 
-                if(bControlA)
-                {
-                    // add bezier edge
-                    aRetval.appendBezierSegment(aControlA, aControlB, 
aNewCoordinatePair);
-                }
-                else
-                {
-                    // add edge
-                    aRetval.append(aNewCoordinatePair);
+                    // two or no control points are consumed, another one 
would be an error.
+                    // It's also an error if only one control point was read
+                    SAL_WARN_IF(ePolygonFlag == 
css::drawing::PolygonFlags_CONTROL || bControlA != bControlB,
+                        "basegfx", "UnoPolygonBezierCoordsToB2DPolygon: 
Illegal source polygon (!)");
+
+                    // the previous writes used the B2DPolyPoygon -> 
utils::PolyPolygon converter
+                    // which did not create minimal PolyPolygons, but created 
all control points
+                    // as null vectors (identical points). Because of the 
former P(CA)(CB)-norm of
+                    // B2DPolygon and it's unused sign of being the 
zero-vector and CA and CB being
+                    // relative to P, an empty edge was exported as P == CA == 
CB. Luckily, the new
+                    // export format can be read without errors by the old 
OOo-versions, so we need only
+                    // to correct here at read and do not need to export a 
wrong but compatible version
+                    // for the future.
+                    if(bControlA
+                        && aControlA.equal(aControlB)
+                        && aControlA.equal(aRetval.getB2DPoint(aRetval.count() 
- 1)))
+                    {
+                        bControlA = false;
+                    }
+
+                    if(bControlA)
+                    {
+                        // add bezier edge
+                        aRetval.appendBezierSegment(aControlA, aControlB, 
aNewCoordinatePair);
+                    }
+                    else
+                    {
+                        // add edge
+                        aRetval.append(aNewCoordinatePair);
+                    }
                 }
-            }
 
-            // #i72807# API import uses old line start/end-equal definition 
for closed,
-            // so we need to correct this to closed state here
-            checkClosed(aRetval);
+                // #i72807# API import uses old line start/end-equal 
definition for closed,
+                // so we need to correct this to closed state here
+                checkClosed(aRetval);
+            }
 
             return aRetval;
         }
diff --git a/filter/source/msfilter/escherex.cxx 
b/filter/source/msfilter/escherex.cxx
index c0da5ae84b81..50eb50394054 100644
--- a/filter/source/msfilter/escherex.cxx
+++ b/filter/source/msfilter/escherex.cxx
@@ -93,6 +93,8 @@
 #include <vcl/virdev.hxx>
 #include <rtl/crc.h>
 #include <rtl/strbuf.hxx>
+#include <basegfx/polygon/b2dpolypolygontools.hxx>
+#include <basegfx/polygon/b2dpolygontools.hxx>
 #include <memory>
 
 using namespace css;
@@ -1735,120 +1737,36 @@ tools::PolyPolygon 
EscherPropertyContainer::GetPolyPolygon( const uno::Reference
     return aRetPolyPoly;
 }
 
+// adapting to basegfx::B2DPolyPolygon now, has no sense to do corrections in 
the
+// old tools::PolyPolygon creation code. Convert to that at return time
 tools::PolyPolygon EscherPropertyContainer::GetPolyPolygon( const uno::Any& 
rAny )
 {
-    bool bNoError = true;
+    basegfx::B2DPolyPolygon aRetval;
 
-    tools::Polygon aPolygon;
-    tools::PolyPolygon aPolyPolygon;
-
-    if ( rAny.getValueType() == 
cppu::UnoType<drawing::PolyPolygonBezierCoords>::get())
+    if(auto pBCC = o3tl::tryAccess<drawing::PolyPolygonBezierCoords>(rAny))
     {
-        auto pSourcePolyPolygon = 
o3tl::doAccess<drawing::PolyPolygonBezierCoords>(rAny);
-        sal_uInt16 nOuterSequenceCount = 
static_cast<sal_uInt16>(pSourcePolyPolygon->Coordinates.getLength());
-
-        // get pointer of inner sequences
-        drawing::PointSequence const * pOuterSequence = 
pSourcePolyPolygon->Coordinates.getConstArray();
-        drawing::FlagSequence const *  pOuterFlags = 
pSourcePolyPolygon->Flags.getConstArray();
-
-        bNoError = pOuterSequence && pOuterFlags;
-        if ( bNoError )
-        {
-            sal_uInt16  a, b, nInnerSequenceCount;
-            awt::Point const * pArray;
-
-            // this will be a polygon set
-            for ( a = 0; a < nOuterSequenceCount; a++ )
-            {
-                drawing::PointSequence const * pInnerSequence = 
pOuterSequence++;
-                drawing::FlagSequence const *  pInnerFlags = pOuterFlags++;
-
-                bNoError = pInnerSequence && pInnerFlags;
-                if  ( bNoError )
-                {
-                    // get pointer to arrays
-                    pArray = pInnerSequence->getConstArray();
-                    drawing::PolygonFlags const * pFlags = 
pInnerFlags->getConstArray();
-
-                    if ( pArray && pFlags )
-                    {
-                        nInnerSequenceCount = 
static_cast<sal_uInt16>(pInnerSequence->getLength());
-                        aPolygon = tools::Polygon( nInnerSequenceCount );
-                        for( b = 0; b < nInnerSequenceCount; b++)
-                        {
-                            drawing::PolygonFlags ePolyFlags = *pFlags++;
-                            awt::Point aPoint( *(pArray++) );
-                            aPolygon[ b ] = Point( aPoint.X, aPoint.Y );
-                            aPolygon.SetFlags( b, 
static_cast<PolyFlags>(ePolyFlags) );
-
-                            if ( ePolyFlags == drawing::PolygonFlags_CONTROL )
-                                continue;
-                        }
-                        aPolyPolygon.Insert( aPolygon );
-                    }
-                }
-            }
-        }
+        aRetval = 
basegfx::utils::UnoPolyPolygonBezierCoordsToB2DPolyPolygon(*pBCC);
+    }
+    else if(auto pCC = o3tl::tryAccess<drawing::PointSequenceSequence>(rAny))
+    {
+        aRetval = 
basegfx::utils::UnoPointSequenceSequenceToB2DPolyPolygon(*pCC);
     }
-    else if ( auto pSourcePolyPolygon = 
o3tl::tryAccess<drawing::PointSequenceSequence>(rAny) )
+    else if(auto pC = o3tl::tryAccess<drawing::PointSequence>(rAny))
     {
-        sal_uInt16 nOuterSequenceCount = 
static_cast<sal_uInt16>(pSourcePolyPolygon->getLength());
+        aRetval.append(basegfx::utils::UnoPointSequenceToB2DPolygon(*pC));
+    }
 
-        // get pointer to inner sequences
-        drawing::PointSequence const * pOuterSequence = 
pSourcePolyPolygon->getConstArray();
-        bNoError = pOuterSequence != nullptr;
-        if ( bNoError )
-        {
-            sal_uInt16 a, b, nInnerSequenceCount;
+    basegfx::B2DPolyPolygon aRetval2;
 
-            // this will be a polygon set
-            for( a = 0; a < nOuterSequenceCount; a++ )
-            {
-                drawing::PointSequence const * pInnerSequence = 
pOuterSequence++;
-                bNoError = pInnerSequence != nullptr;
-                if ( bNoError )
-                {
-                    // get pointer to arrays
-                    awt::Point const * pArray =
-                          pInnerSequence->getConstArray();
-                    if ( pArray != nullptr )
-                    {
-                        nInnerSequenceCount = 
static_cast<sal_uInt16>(pInnerSequence->getLength());
-                        aPolygon = tools::Polygon( nInnerSequenceCount );
-                        for( b = 0; b < nInnerSequenceCount; b++)
-                        {
-                            aPolygon[ b ] = Point( pArray->X, pArray->Y );
-                            pArray++;
-                        }
-                        aPolyPolygon.Insert( aPolygon );
-                    }
-                }
-            }
-        }
-    }
-    else if ( auto pInnerSequence = 
o3tl::tryAccess<drawing::PointSequence>(rAny) )
+    for(sal_uInt32 a(0); a < aRetval.count(); a++)
     {
-        bNoError = pInnerSequence != nullptr;
-        if ( bNoError )
+        if(0 != aRetval.getB2DPolygon(a).count())
         {
-            sal_uInt16 a, nInnerSequenceCount;
-
-            // get pointer to arrays
-            awt::Point const * pArray = pInnerSequence->getConstArray();
-            if ( pArray != nullptr )
-            {
-                nInnerSequenceCount = 
static_cast<sal_uInt16>(pInnerSequence->getLength());
-                aPolygon = tools::Polygon( nInnerSequenceCount );
-                for( a = 0; a < nInnerSequenceCount; a++)
-                {
-                    aPolygon[ a ] = Point( pArray->X, pArray->Y );
-                    pArray++;
-                }
-                aPolyPolygon.Insert( aPolygon );
-            }
+            aRetval2.append(aRetval.getB2DPolygon(a));
         }
     }
-    return aPolyPolygon;
+
+    return tools::PolyPolygon(aRetval2);
 }
 
 bool EscherPropertyContainer::CreatePolygonProperties(
_______________________________________________
Libreoffice-commits mailing list
libreoffice-comm...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits

Reply via email to