[Libreoffice-commits] core.git: Branch 'libreoffice-6-0' - include/vcl vcl/Module_vcl.mk vcl/qa vcl/source

Miklos Vajna vmiklos at collabora.co.uk
Sun May 27 15:50:57 UTC 2018


 include/vcl/pdfextoutdevdata.hxx            |    2 
 vcl/Module_vcl.mk                           |    6 +
 vcl/qa/cppunit/pdfexport/data/tdf106702.odt |binary
 vcl/qa/cppunit/pdfexport/pdfexport.cxx      |   85 ++++++++++++++++++++++------
 vcl/source/gdi/pdfextoutdevdata.cxx         |   25 ++++++--
 vcl/source/gdi/pdfwriter_impl2.cxx          |    2 
 6 files changed, 96 insertions(+), 24 deletions(-)

New commits:
commit 327c7ee43fbe53df63693d020356ddf4df56d7f1
Author: Miklos Vajna <vmiklos at collabora.co.uk>
Date:   Tue May 22 16:20:18 2018 +0200

    tdf#106702 PDF export: fix missing images from Writer headers/footers
    
    Position of an image is determined by the relevant bitmap scale metafile
    action when recompressing images.
    
    The same position was determined by PDFExtOutDevData "meta" info when
    not recompressing images. This second rectangle was never correct for
    images repeated in Writer headers/footers on non-first pages: the
    position was relative to the page, while PDF export sets the map mode
    (origin) of the output device during export, so such positions are
    expected to be absolute ones.
    
    The root of the problem seems to be that header images in Writer are
    both repeated (as the user sees it) and unrepeated (as the doc model
    sees it), and by the time we want to get its position, we only see the
    unrepeated SdrObject.
    
    Fix the problem by using the correct position from the scale action and
    not from PDFExtOutDevData if possible.
    
    (Also give up on running CppunitTest_vcl_pdfexport in the non-pdfium
    case, most of the tests there do require pdfium anyway, and the growing
    ifdef forest in that file just made it hard to read the code.)
    
    (cherry picked from commit 4c2172a3e973bc6351107a3a1b554c77b40b75dd)
    
    Conflicts:
            vcl/Module_vcl.mk
            vcl/qa/cppunit/pdfexport/pdfexport.cxx
    
    Change-Id: I31c14d4bd223b2804859982542ebd6d5f9abd312
    Reviewed-on: https://gerrit.libreoffice.org/54690
    Tested-by: Jenkins <ci at libreoffice.org>
    Reviewed-by: Caolán McNamara <caolanm at redhat.com>
    Tested-by: Caolán McNamara <caolanm at redhat.com>

diff --git a/include/vcl/pdfextoutdevdata.hxx b/include/vcl/pdfextoutdevdata.hxx
index 22079323580c..b8bbe589d491 100644
--- a/include/vcl/pdfextoutdevdata.hxx
+++ b/include/vcl/pdfextoutdevdata.hxx
@@ -97,7 +97,7 @@ public:
     PDFExtOutDevData( const OutputDevice& rOutDev );
     virtual ~PDFExtOutDevData() override;
 
-    bool PlaySyncPageAct( PDFWriter& rWriter, sal_uInt32& rCurGDIMtfAction );
+    bool PlaySyncPageAct( PDFWriter& rWriter, sal_uInt32& rCurGDIMtfAction, const GDIMetaFile& rMtf );
     void ResetSyncData();
 
     void PlayGlobalActions( PDFWriter& rWriter );
diff --git a/vcl/Module_vcl.mk b/vcl/Module_vcl.mk
index e27b4db56ab9..40fee27321b4 100644
--- a/vcl/Module_vcl.mk
+++ b/vcl/Module_vcl.mk
@@ -173,10 +173,14 @@ $(eval $(call gb_Module_add_check_targets,vcl,\
 	CppunitTest_vcl_app_test \
 	CppunitTest_vcl_jpeg_read_write_test \
 	CppunitTest_vcl_svm_test \
-	CppunitTest_vcl_pdfexport \
     CppunitTest_vcl_errorhandler \
 ))
 
+ifneq (,$(filter PDFIUM,$(BUILD_TYPE)))
+$(eval $(call gb_Module_add_check_targets,vcl,\
+	CppunitTest_vcl_pdfexport \
+))
+endif
 
 ifeq ($(USING_X11),TRUE)
 $(eval $(call gb_Module_add_check_targets,vcl,\
diff --git a/vcl/qa/cppunit/pdfexport/data/tdf106702.odt b/vcl/qa/cppunit/pdfexport/data/tdf106702.odt
new file mode 100644
index 000000000000..da3b7e81456e
Binary files /dev/null and b/vcl/qa/cppunit/pdfexport/data/tdf106702.odt differ
diff --git a/vcl/qa/cppunit/pdfexport/pdfexport.cxx b/vcl/qa/cppunit/pdfexport/pdfexport.cxx
index b2ebb23cf6cd..9e6924f2e66a 100644
--- a/vcl/qa/cppunit/pdfexport/pdfexport.cxx
+++ b/vcl/qa/cppunit/pdfexport/pdfexport.cxx
@@ -24,11 +24,9 @@
 #include <unotools/tempfile.hxx>
 #include <vcl/filter/pdfdocument.hxx>
 #include <tools/zcodec.hxx>
-#if HAVE_FEATURE_PDFIUM
 #include <fpdf_edit.h>
 #include <fpdf_text.h>
 #include <fpdfview.h>
-#endif
 
 using namespace ::com::sun::star;
 
@@ -40,15 +38,12 @@ class PdfExportTest : public test::BootstrapFixture, public unotest::MacrosTest
 {
     uno::Reference<uno::XComponentContext> mxComponentContext;
     uno::Reference<lang::XComponent> mxComponent;
-#if HAVE_FEATURE_PDFIUM
     FPDF_PAGE mpPdfPage = nullptr;
     FPDF_DOCUMENT mpPdfDocument = nullptr;
-#endif
 
 public:
     virtual void setUp() override;
     virtual void tearDown() override;
-#if HAVE_FEATURE_PDFIUM
     void load(const OUString& rFile, vcl::filter::PDFDocument& rDocument);
     /// Tests that a pdf image is roundtripped back to PDF as a vector format.
     void testTdf106059();
@@ -79,12 +74,11 @@ public:
     /// Text extracting RTL text with ligatures.
     void testTdf115117_2a();
 #endif
-    void testTdf105954();
-#endif
     void testTdf109143();
+    void testTdf105954();
+    void testTdf106702();
 
     CPPUNIT_TEST_SUITE(PdfExportTest);
-#if HAVE_FEATURE_PDFIUM
     CPPUNIT_TEST(testTdf106059);
     CPPUNIT_TEST(testTdf105461);
     CPPUNIT_TEST(testTdf107868);
@@ -105,9 +99,9 @@ public:
     CPPUNIT_TEST(testTdf115117_2);
     CPPUNIT_TEST(testTdf115117_2a);
 #endif
-    CPPUNIT_TEST(testTdf105954);
-#endif
     CPPUNIT_TEST(testTdf109143);
+    CPPUNIT_TEST(testTdf105954);
+    CPPUNIT_TEST(testTdf106702);
     CPPUNIT_TEST_SUITE_END();
 };
 
@@ -118,23 +112,19 @@ void PdfExportTest::setUp()
     mxComponentContext.set(comphelper::getComponentContext(getMultiServiceFactory()));
     mxDesktop.set(frame::Desktop::create(mxComponentContext));
 
-#if HAVE_FEATURE_PDFIUM
     FPDF_LIBRARY_CONFIG config;
     config.version = 2;
     config.m_pUserFontPaths = nullptr;
     config.m_pIsolate = nullptr;
     config.m_v8EmbedderSlot = 0;
     FPDF_InitLibraryWithConfig(&config);
-#endif
 }
 
 void PdfExportTest::tearDown()
 {
-#if HAVE_FEATURE_PDFIUM
     FPDF_ClosePage(mpPdfPage);
     FPDF_CloseDocument(mpPdfDocument);
     FPDF_DestroyLibrary();
-#endif
 
     if (mxComponent.is())
         mxComponent->dispose();
@@ -142,8 +132,6 @@ void PdfExportTest::tearDown()
     test::BootstrapFixture::tearDown();
 }
 
-#if HAVE_FEATURE_PDFIUM
-
 char const DATA_DIRECTORY[] = "/vcl/qa/cppunit/pdfexport/data/";
 
 void PdfExportTest::load(const OUString& rFile, vcl::filter::PDFDocument& rDocument)
@@ -1066,7 +1054,70 @@ void PdfExportTest::testTdf105954()
     CPPUNIT_ASSERT_LESS(static_cast<unsigned int>(250), aMeta.width);
 }
 
-#endif
+void PdfExportTest::testTdf106702()
+{
+    // Import the bugdoc and export as PDF.
+    OUString aURL = m_directories.getURLFromSrc(DATA_DIRECTORY) + "tdf106702.odt";
+    mxComponent = loadFromDesktop(aURL);
+    CPPUNIT_ASSERT(mxComponent.is());
+
+    uno::Reference<frame::XStorable> xStorable(mxComponent, uno::UNO_QUERY);
+    utl::TempFile aTempFile;
+    aTempFile.EnableKillingFile();
+    utl::MediaDescriptor aMediaDescriptor;
+    aMediaDescriptor["FilterName"] <<= OUString("writer_pdf_Export");
+    xStorable->storeToURL(aTempFile.GetURL(), aMediaDescriptor.getAsConstPropertyValueList());
+
+    // Parse the export result with pdfium.
+    SvFileStream aFile(aTempFile.GetURL(), StreamMode::READ);
+    SvMemoryStream aMemory;
+    aMemory.WriteStream(aFile);
+    mpPdfDocument
+        = FPDF_LoadMemDocument(aMemory.GetData(), aMemory.GetSize(), /*password=*/nullptr);
+    CPPUNIT_ASSERT(mpPdfDocument);
+
+    // The document has two pages.
+    CPPUNIT_ASSERT_EQUAL(2, FPDF_GetPageCount(mpPdfDocument));
+
+    // First page already has the correct image position.
+    mpPdfPage = FPDF_LoadPage(mpPdfDocument, /*page_index=*/0);
+    CPPUNIT_ASSERT(mpPdfPage);
+    int nExpected = 0;
+    int nPageObjectCount = FPDFPage_CountObjects(mpPdfPage);
+    for (int i = 0; i < nPageObjectCount; ++i)
+    {
+        FPDF_PAGEOBJECT pPageObject = FPDFPage_GetObject(mpPdfPage, i);
+        if (FPDFPageObj_GetType(pPageObject) != FPDF_PAGEOBJ_IMAGE)
+            continue;
+
+        float fLeft = 0, fBottom = 0, fRight = 0, fTop = 0;
+        FPDFPageObj_GetBounds(pPageObject, &fLeft, &fBottom, &fRight, &fTop);
+        nExpected = fTop;
+        break;
+    }
+
+    // Second page had an incorrect image position.
+    FPDF_ClosePage(mpPdfPage);
+    mpPdfPage = FPDF_LoadPage(mpPdfDocument, /*page_index=*/1);
+    CPPUNIT_ASSERT(mpPdfPage);
+    int nActual = 0;
+    nPageObjectCount = FPDFPage_CountObjects(mpPdfPage);
+    for (int i = 0; i < nPageObjectCount; ++i)
+    {
+        FPDF_PAGEOBJECT pPageObject = FPDFPage_GetObject(mpPdfPage, i);
+        if (FPDFPageObj_GetType(pPageObject) != FPDF_PAGEOBJ_IMAGE)
+            continue;
+
+        float fLeft = 0, fBottom = 0, fRight = 0, fTop = 0;
+        FPDFPageObj_GetBounds(pPageObject, &fLeft, &fBottom, &fRight, &fTop);
+        nActual = fTop;
+        break;
+    }
+
+    // This failed, vertical pos is 818 points, was 1674 (outside visible page
+    // bounds).
+    CPPUNIT_ASSERT_EQUAL(nExpected, nActual);
+}
 
 CPPUNIT_TEST_SUITE_REGISTRATION(PdfExportTest);
 
diff --git a/vcl/source/gdi/pdfextoutdevdata.cxx b/vcl/source/gdi/pdfextoutdevdata.cxx
index 3c689c3ccce0..a31a7d568201 100644
--- a/vcl/source/gdi/pdfextoutdevdata.cxx
+++ b/vcl/source/gdi/pdfextoutdevdata.cxx
@@ -22,6 +22,7 @@
 #include <vcl/outdev.hxx>
 #include <vcl/gfxlink.hxx>
 #include <vcl/dllapi.h>
+#include <vcl/metaact.hxx>
 #include <basegfx/polygon/b2dpolygon.hxx>
 #include <basegfx/polygon/b2dpolygontools.hxx>
 
@@ -301,7 +302,7 @@ struct PageSyncData
     { mpGlobalData = pGlobal; }
 
     void PushAction( const OutputDevice& rOutDev, const PDFExtOutDevDataSync::Action eAct );
-    bool PlaySyncPageAct( PDFWriter& rWriter, sal_uInt32& rCurGDIMtfAction, const PDFExtOutDevData& rOutDevData );
+    bool PlaySyncPageAct( PDFWriter& rWriter, sal_uInt32& rCurGDIMtfAction, const GDIMetaFile& rMtf, const PDFExtOutDevData& rOutDevData );
 };
 
 void PageSyncData::PushAction( const OutputDevice& rOutDev, const PDFExtOutDevDataSync::Action eAct )
@@ -317,7 +318,7 @@ void PageSyncData::PushAction( const OutputDevice& rOutDev, const PDFExtOutDevDa
         aSync.nIdx = 0x7fffffff;    // sync not possible
     mActions.push_back( aSync );
 }
-bool PageSyncData::PlaySyncPageAct( PDFWriter& rWriter, sal_uInt32& rCurGDIMtfAction, const PDFExtOutDevData& rOutDevData )
+bool PageSyncData::PlaySyncPageAct( PDFWriter& rWriter, sal_uInt32& rCurGDIMtfAction, const GDIMetaFile& rMtf, const PDFExtOutDevData& rOutDevData )
 {
     bool bRet = false;
     if ( mActions.size() && ( mActions.front().nIdx == rCurGDIMtfAction ) )
@@ -463,6 +464,22 @@ bool PageSyncData::PlaySyncPageAct( PDFWriter& rWriter, sal_uInt32& rCurGDIMtfAc
                         if( pData && nBytes )
                         {
                             aTmp.WriteBytes( pData, nBytes );
+
+                            // Look up the output rectangle from the previous
+                            // bitmap scale action if possible. This has the
+                            // correct position for images repeated in
+                            // Writer headers/footers for non-first pages.
+                            if (rCurGDIMtfAction > 0)
+                            {
+                                const MetaAction* pAction = rMtf.GetAction(rCurGDIMtfAction - 1);
+                                if (pAction && pAction->GetType() == MetaActionType::BMPSCALE)
+                                {
+                                    const MetaBmpScaleAction* pA
+                                        = static_cast<const MetaBmpScaleAction*>(pAction);
+                                    aOutputRect.SetPos(pA->GetPoint());
+                                }
+                            }
+
                             rWriter.DrawJPGBitmap( aTmp, aGraphic.GetBitmap().GetBitCount() > 8, aGraphic.GetSizePixel(), aOutputRect, aMask, aGraphic );
                         }
 
@@ -584,9 +601,9 @@ void PDFExtOutDevData::ResetSyncData()
 {
     *mpPageSyncData = PageSyncData( mpGlobalSyncData );
 }
-bool PDFExtOutDevData::PlaySyncPageAct( PDFWriter& rWriter, sal_uInt32& rIdx )
+bool PDFExtOutDevData::PlaySyncPageAct( PDFWriter& rWriter, sal_uInt32& rIdx, const GDIMetaFile& rMtf )
 {
-    return mpPageSyncData->PlaySyncPageAct( rWriter, rIdx, *this );
+    return mpPageSyncData->PlaySyncPageAct( rWriter, rIdx, rMtf, *this );
 }
 void PDFExtOutDevData::PlayGlobalActions( PDFWriter& rWriter )
 {
diff --git a/vcl/source/gdi/pdfwriter_impl2.cxx b/vcl/source/gdi/pdfwriter_impl2.cxx
index 9bc58989a46c..e4f567d6bfd9 100644
--- a/vcl/source/gdi/pdfwriter_impl2.cxx
+++ b/vcl/source/gdi/pdfwriter_impl2.cxx
@@ -267,7 +267,7 @@ void PDFWriterImpl::playMetafile( const GDIMetaFile& i_rMtf, vcl::PDFExtOutDevDa
 
     for( sal_uInt32 i = 0, nCount = aMtf.GetActionSize(); i < nCount; )
     {
-        if ( !i_pOutDevData || !i_pOutDevData->PlaySyncPageAct( m_rOuterFace, i ) )
+        if ( !i_pOutDevData || !i_pOutDevData->PlaySyncPageAct( m_rOuterFace, i, aMtf ) )
         {
             const MetaAction*    pAction = aMtf.GetAction( i );
             const MetaActionType nType = pAction->GetType();


More information about the Libreoffice-commits mailing list