Bug 156842 - Export to PDF that includes PDF images are incorrect in size (macOS)
Summary: Export to PDF that includes PDF images are incorrect in size (macOS)
Status: VERIFIED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Impress (show other bugs)
Version:
(earliest affected)
7.5.5.2 release
Hardware: All macOS (All)
: medium normal
Assignee: Patrick Luby
URL:
Whiteboard: target:24.2.0 target:7.6.2 target:7.5.7
Keywords:
Depends on:
Blocks: PDF-Export
  Show dependency treegraph
 
Reported: 2023-08-21 13:59 UTC by cytan299
Modified: 2023-10-11 23:18 UTC (History)
5 users (show)

See Also:
Crash report or crash signature:


Attachments
Zip file that contains the pdf_export_bug.odp and pdf_export_bug.pdf (624.36 KB, application/zip)
2023-08-21 13:59 UTC, cytan299
Details
Pdf sample to verify (93.39 KB, application/pdf)
2023-08-21 16:14 UTC, m_a_riosv
Details
Zip file that contains screenshot to show problem remains in safe mode and bad pdf file (2.11 MB, application/zip)
2023-08-21 17:04 UTC, cytan299
Details
Another sample file that reproduces the bug (2.43 MB, application/vnd.oasis.opendocument.presentation)
2023-09-19 20:34 UTC, Patrick Luby
Details

Note You need to log in before you can comment on or make changes to this bug.
Description cytan299 2023-08-21 13:59:45 UTC
Created attachment 189073 [details]
Zip file that contains the pdf_export_bug.odp and pdf_export_bug.pdf

The "pdf_export_bug.odp" file contains a pdf formatted image.

After exporting "pdf_export_bug.odp" to a pdf file: "pdf_export_bug.pdf", the image is now very small and to the bottom of the slide.

Attached zip file contains: 
    (1) pdf_export_bug.odp. PDF formatted image displays correctly.
    (2) pdf_export_bug.pdf: The PDF file created from pdf_export_bug.odp. The PDF image has the wrong size.
Comment 1 m_a_riosv 2023-08-21 16:14:55 UTC
Created attachment 189076 [details]
Pdf sample to verify

Works fine for me.

Please test in safe mode, Menu/Help/Restart in Safe Mode
Comment 2 cytan299 2023-08-21 17:04:15 UTC
Created attachment 189077 [details]
Zip file that contains screenshot to show problem remains in safe mode and bad pdf file

I went into safe mode and exported the file "pdf_export_bug_safemode.pdf" and the problem remains.

See attached zip file problem_remains.zip. Contents:

pdf_export_bug_safemode.pdf: bad pdf file that was exported.
Screen Shot.png: Screenshot to show LibreOffice in safemode and the pdf file displayed with bug.
Comment 3 cytan299 2023-08-22 16:47:29 UTC
Bug reproduced in LibreOffice 7.6.0.3
Comment 4 Telesto 2023-08-22 17:02:58 UTC
No issue with
Version: 24.2.0.0.alpha0+ (X86_64) / LibreOffice Community
Build ID: 518fa99dd7693d64a53e404a065090aedc0002b1
CPU threads: 4; OS: Windows 6.3 Build 9600; UI render: Skia/Raster; VCL: win
Locale: nl-NL (nl_NL); UI: en-US
Calc: CL threaded
Comment 5 cytan299 2023-08-22 17:39:47 UTC
I reproduced this bug on 

MacOS Ventura 13.5
MacOS Monterey 12.6.8

running LibreOffice 7.6.0.3
Comment 6 Telesto 2023-08-22 23:24:58 UTC
Confirm with macOS
Version: 24.2.0.0.alpha0+ (X86_64) / LibreOffice Community
Build ID: e60ef8651cfb30335471d1622e58c13eebc7d58b
CPU threads: 8; OS: Mac OS X 13.4.1; UI render: Skia/Metal; VCL: osx
Locale: nl-NL (nl_NL.UTF-8); UI: en-US
Calc: threaded
Comment 7 Patrick Luby 2023-08-27 22:59:06 UTC
I see this bug in my local master builds with all Skia/Metal, Skia/Raster, and Skia disabled. I will see if I can isolate where the scaling is occurring in the code.

I will post an update when I have any news.
Comment 8 Patrick Luby 2023-08-29 13:38:12 UTC
So I have uploaded the following patch:

https://gerrit.libreoffice.org/c/core/+/156252

It fixes the bug, but it seems that there is some underlying data corruption in the document's extended PDF data so my fix is just a simple hack that abandons creating extended PDF data for the current meta action when any data corruption is encountered.

I have no clue how the extended PDF is getting messed up (i.e. why is no PDFWriter::Document extended PDF element ever added) so I would like someone more familiar with this code to review my patch.
Comment 9 Patrick Luby 2023-08-29 14:25:51 UTC
(In reply to Patrick Luby from comment #8)
> I have no clue how the extended PDF is getting messed up (i.e. why is no
> PDFWriter::Document extended PDF element ever added) so I would like someone
> more familiar with this code to review my patch.

Well apparently this messed extended PDF data also exists in a unit test and my change triggers that broken test PDF to now hit an assertion.

Unassigning myself as I don't really have any idea how the extended PDF data gets so messed up so often. Apparently calling OSL_FAIL is "normal" and the code must create garbage data (which causes this bug) for the CppunitTest_vcl_pdfexport unit test to succeed.

I still have no idea why in this bug and the CppunitTest_vcl_pdfexport unit test case there is no PDFWriter::Document attached to the root PDF extended data. Which code is supposed to set this?
Comment 10 Patrick Luby 2023-08-29 14:39:35 UTC
For future debugging, disabling extended PDF data completely (see patch below) stops this bug. But, it will cause the CppunitTest_vcl_pdfexport to go into an infinite loop.

TL;DR this bug is caused by i_pOutDevData->PlaySyncPageAct( m_rOuterFace, i, aMtf ) returning true for several EnsureStruct/InitStruct/StartStruct/EndStruct cycles despite InitStruct hitting an OSL_FAIL each time.


diff --git a/vcl/source/gdi/pdfwriter_impl2.cxx b/vcl/source/gdi/pdfwriter_impl2.cxx
index 37583d0b01d1..88ee7f1fd60f 100644
--- a/vcl/source/gdi/pdfwriter_impl2.cxx
+++ b/vcl/source/gdi/pdfwriter_impl2.cxx
@@ -282,7 +282,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, aMtf ) )
+        // if ( !i_pOutDevData || !i_pOutDevData->PlaySyncPageAct( m_rOuterFace, i, aMtf ) )
         {
             const MetaAction*    pAction = aMtf.GetAction( i );
             const MetaActionType nType = pAction->GetType();
Comment 11 Patrick Luby 2023-08-30 20:34:30 UTC
I took a fresh look at this bug and I no longer think the OSL_FAIL is the actual cause this bug. AFAICT, PDFWriter::Document is only added in Writer documents and it is apparently normal to have one or more top level struct elements other than PDFWriter::Document or PDFWriter::NonStructElement.

So, I decided to relook at the bug to see if I can find what is causing the scaling. I set the following environment variable and ran LibreOffice from the command line:

  export VCL_DEBUG_DISABLE_PDFCOMPRESSION=1

This exports to PDF without any compression and I found the following in the exported PDF file:

  << /Type /XObject /Subtype /Form /Resources << /XObject<< /Im12 12 0 R>> >> /Matrix [ 0.0002195871 0 0 0.0002254283 0 0 ] /BBox [ 0 0 4554 4436 ]

If I edit the exported PDF manually and multiply the non-zero /Matrix elements by 8 and divide the non-zero /BBox elements by 8 like in the following line, the graphic renders at the right size:

  << /Type /XObject /Subtype /Form /Resources << /XObject<< /Im12 12 0 R>> >> /Matrix [ 0.0017566968 0 0 0.0018034264 0 0 ] /BBox [ 0 0 569  556  ]

So, I suspect scaling should be applied to /BBox instead of /Matrix. Next step is to find where the above numbers are set in the code.
Comment 12 Patrick Luby 2023-08-30 21:35:16 UTC
(In reply to Patrick Luby from comment #11)
> So, I suspect scaling should be applied to /BBox instead of /Matrix. Next
> step is to find where the above numbers are set in the code.

So I found the code where the scaling occurs. If you apply the following debug patch, the bug no longer occurs. My conclusion is that the the image is 1024 DPI but the code currently assumes the default PDF DPI of 127. Next step is to see if I can somehow resolve these two different DPI values:


diff --git a/vcl/source/gdi/pdfwriter_impl.cxx b/vcl/source/gdi/pdfwriter_impl.cxx
index 55ed6d514b9d..88e2afcdde06 100644
--- a/vcl/source/gdi/pdfwriter_impl.cxx
+++ b/vcl/source/gdi/pdfwriter_impl.cxx
@@ -9068,7 +9068,8 @@ void PDFWriterImpl::writeReferenceXObject(const ReferenceXObjectEmit& rEmit)
     // Count /Matrix and /BBox.
     // vcl::ImportPDF() uses getDefaultPdfResolutionDpi to set the desired
     // rendering DPI so we have to take into account that here too.
-    static const double fResolutionDPI = vcl::pdf::getDefaultPdfResolutionDpi();
+    // static const double fResolutionDPI = vcl::pdf::getDefaultPdfResolutionDpi();
+    static const double fResolutionDPI = 1024;
 
     sal_Int32 nOldDPIX = GetDPIX();
     sal_Int32 nOldDPIY = GetDPIY();
Comment 13 cytan299 2023-09-15 12:13:19 UTC
Bug remains in LibreOffice 7.6.1
Comment 14 Patrick Luby 2023-09-19 20:34:21 UTC
Created attachment 189705 [details]
Another sample file that reproduces the bug
Comment 15 Patrick Luby 2023-09-19 21:01:06 UTC
(In reply to Patrick Luby from comment #14)
> Created attachment 189705 [details]
> Another sample file that reproduces the bug

I have submitted a fix in the following patch:

https://gerrit.libreoffice.org/c/core/+/157090

It still needs to be reviewed, but I think the above patch fixes this bug without causing non-external PDF XObjects to be incorrectly resized that occurred in my debug patch in https://bugs.documentfoundation.org/show_bug.cgi?id=156842#c12.
Comment 16 Patrick Luby 2023-09-19 21:57:44 UTC
> I have submitted a fix in the following patch:
> 
> https://gerrit.libreoffice.org/c/core/+/157090
> 
> It still needs to be reviewed, but I think the above patch fixes this bug
> without causing non-external PDF XObjects to be incorrectly resized that
> occurred in my debug patch in
> https://bugs.documentfoundation.org/show_bug.cgi?id=156842#c12.

Unfortunately, the above patch fails an export to PDF unit test on Linux.

Can anyone reproduce this bug in Windows or Linux? From looking the CppunitTest_vcl_pdfexport test, weird values in the exported PDF were being seen on macOS but not other platforms.

I don't have access to any Windows or Linux machines, but if someone could confirm whether this bug occurs or not on those platforms with the following attachment, I can then adjust my patch:

https://bugs.documentfoundation.org/attachment.cgi?id=189705
Comment 17 Commit Notification 2023-09-20 06:15:16 UTC
Patrick Luby committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/c2d7c8e5c616980d5fa77ac48babbf27342b2f0a

tdf#156842 increase scale for external PDF data

It will be available in 24.2.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 18 Patrick Luby 2023-09-20 12:35:54 UTC
My patch passed all unit tests after only enabling my fix for macOS. My fix should be in tomorrow's (21 September 2023) nightly master build.

Based on the unit tests, I am assuming that this bug only occurred on macOS. But if anyone sees this bug on Windows or Linux, please reopen the bug and I'll add my fix for that platform.
Comment 19 Commit Notification 2023-09-20 17:11:14 UTC
Patrick Luby committed a patch related to this issue.
It has been pushed to "libreoffice-7-6":

https://git.libreoffice.org/core/commit/36cd5e909b7454b1b79c69d2f0db7a76ee2bb47d

tdf#156842 increase scale for external PDF data

It will be available in 7.6.2.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 20 BogdanB 2023-09-21 03:40:05 UTC
(In reply to Patrick Luby from comment #18)
> 
> Based on the unit tests, I am assuming that this bug only occurred on macOS.
> But if anyone sees this bug on Windows or Linux, please reopen the bug and
> I'll add my fix for that platform.

I don't repro this bug on Linux. I tried on 
Version: 7.5.6.2 (X86_64) / LibreOffice Community
Build ID: f654817fb68d6d4600d7d2f6b647e47729f55f15
CPU threads: 4; OS: Linux 5.15; UI render: default; VCL: gtk3
Locale: ro-RO (ro_RO.UTF-8); UI: en-US
Calc: threaded

Also with
Version: 7.6.1.2 (X86_64) / LibreOffice Community
Build ID: f5defcebd022c5bc36bbb79be232cb6926d8f674
CPU threads: 4; OS: Linux 5.15; UI render: default; VCL: gtk3
Locale: ro-RO (ro_RO.UTF-8); UI: en-US
Calc: threaded

So, it seems you are right, it's just macOS.
Comment 21 Commit Notification 2023-09-21 06:29:34 UTC
Patrick Luby committed a patch related to this issue.
It has been pushed to "libreoffice-7-5":

https://git.libreoffice.org/core/commit/58b35faee869383974da0acfdf24577603634b7b

tdf#156842 increase scale for external PDF data

It will be available in 7.5.7.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 22 Patrick Luby 2023-09-21 12:51:12 UTC
(In reply to BogdanB from comment #20)
> So, it seems you are right, it's just macOS.

This is good to know. Thank you for testing on Linux.

Does anyone still have a non-Retina Mac? I have had no luck getting Quartz Debug to show non-HiDPI resolutions in my display list. If anyone still has one, does my fix work in the latest nightly master build? I ask because the mysterious "multiply scale by 8.0" fix for macOS seems suspiciously like Retina window scale (i.e. 2.0) cubed. So, I am curious if my fix works on older non-Retina machines or does my fix cause the the image in the exported PDF to be huge.
Comment 23 cytan299 2023-09-21 12:57:00 UTC
(In reply to Patrick Luby from comment #22)
> (In reply to BogdanB from comment #20)
> > So, it seems you are right, it's just macOS.
> 
> This is good to know. Thank you for testing on Linux.
> 
> Does anyone still have a non-Retina Mac? I have had no luck getting Quartz
> Debug to show non-HiDPI resolutions in my display list. If anyone still has
> one, does my fix work in the latest nightly master build? I ask because the
> mysterious "multiply scale by 8.0" fix for macOS seems suspiciously like
> Retina window scale (i.e. 2.0) cubed. So, I am curious if my fix works on
> older non-Retina machines or does my fix cause the the image in the exported
> PDF to be huge.

Perhaps a way to test this is to connect the Mac to a lower resolution display to check.
Comment 24 Patrick Luby 2023-09-21 13:50:37 UTC
(In reply to cytan299 from comment #23)
> Perhaps a way to test this is to connect the Mac to a lower resolution
> display to check.

The integrated display on my MacBook Pro can't be turned off so macOS will keep returning 2.0 for the backing scale factor for at least one of the displays.

Unfortunately, I have been unable to use "clamshell mode" to turn off the integrated display. I don't have an external keyboard so the laptop goes to sleep within a few seconds after entering clamshell mode.
Comment 25 cytan299 2023-09-28 13:45:40 UTC
Confirm that bug is fixed in LibreOffice 7.6.2.1 running on MacOS.
Comment 26 BogdanB 2023-09-28 14:35:22 UTC
Verified, based on comment 25.