Skip to content

Commit

Permalink
Replace mPDF ImageProcessor with DW2PDF one
Browse files Browse the repository at this point in the history
  • Loading branch information
@eduardomozart
eduardomozart committed Nov 7, 2023
1 parent f6b6ef8 commit bd07ae4
Show file tree
Hide file tree
Showing 4 changed files with 6 additions and 5 deletions.
2 changes: 1 addition & 1 deletion vendor/mpdf/mpdf/src/ServiceFactory.php
View file
Expand Up @@ -7,7 +7,7 @@
use Mpdf\Color\ColorSpaceRestrictor;
use Mpdf\Fonts\FontCache;
use Mpdf\Fonts\FontFileFinder;
use Mpdf\Image\ImageProcessor;
use \dokuwiki\plugin\dw2pdf\DokuImageProcessorDecorator as ImageProcessor;
use Mpdf\Pdf\Protection;
use Mpdf\Pdf\Protection\UniqidGenerator;
use Mpdf\Writer\BaseWriter;
Expand Down
4 changes: 2 additions & 2 deletions vendor/mpdf/mpdf/src/Tag.php
View file
Expand Up @@ -4,7 +4,7 @@

use Mpdf\Strict;
use Mpdf\Color\ColorConverter;
use Mpdf\Image\ImageProcessor;
use \dokuwiki\plugin\dw2pdf\DokuImageProcessorDecorator as ImageProcessor;
use Mpdf\Language\LanguageToFontInterface;

class Tag
Expand Down Expand Up @@ -53,7 +53,7 @@ class Tag
private $colorConverter;

/**
* @var \Mpdf\Image\ImageProcessor
* @var ImageProcessor
*/
private $imageProcessor;

Expand Down
1 change: 1 addition & 0 deletions vendor/mpdf/mpdf/src/Tag/Img.php
View file
@@ -1,6 +1,7 @@
<?php

namespace Mpdf\Tag;
use \dokuwiki\plugin\dw2pdf\DokuImageProcessorDecorator as ImageProcessor;

use Mpdf\Mpdf;

Expand Down
4 changes: 2 additions & 2 deletions vendor/mpdf/mpdf/src/Tag/Tag.php
View file
Expand Up @@ -8,7 +8,7 @@
use Mpdf\Color\ColorConverter;
use Mpdf\CssManager;
use Mpdf\Form;
use Mpdf\Image\ImageProcessor;
use \dokuwiki\plugin\dw2pdf\DokuImageProcessorDecorator as ImageProcessor;
use Mpdf\Language\LanguageToFontInterface;
use Mpdf\Mpdf;
use Mpdf\Otl;
Expand Down Expand Up @@ -61,7 +61,7 @@ abstract class Tag
protected $colorConverter;

/**
* @var \Mpdf\Image\ImageProcessor
* @var ImageProcessor
*/
protected $imageProcessor;

Expand Down

6 comments on commit bd07ae4

@splitbrain
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think that's the proper way. The mpdf source code should not be modified.

@eduardomozart
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello, I know, but that's actually how it's currently implemented, as discussed in #476 and as you can see on DW2PDF source code: https://github.com/splitbrain/dokuwiki-plugin-dw2pdf/blob/master/vendor/mpdf/mpdf/src/ServiceFactory.php#L12

@Klap-in
Copy link
Collaborator

@Klap-in Klap-in commented on bd07ae4 Nov 7, 2023 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So far I know there is not an alternative at the moment.
If I remember it right it has also an issue at the mpdf library (but not completely sure about that).

edit: mpdf/mpdf#460 the not accepted approach.

@splitbrain
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Current mpdf master accepts a Dependency Injection container: https://github.com/mpdf/mpdf/blob/73f0511e15f6b76c5c2f947128ae676754cbd7ee/src/Mpdf.php#L1049 I don't know from which version onward this is available.

With this container it should be possible to set the HTTPClient and a LocalContentLoader:

https://github.com/mpdf/mpdf/blob/73f0511e15f6b76c5c2f947128ae676754cbd7ee/src/ServiceFactory.php#L72-L82

It seems not possible to override the ImageProcessor:

https://github.com/mpdf/mpdf/blob/73f0511e15f6b76c5c2f947128ae676754cbd7ee/src/ServiceFactory.php#L102-L114

However, I think we could completely avoid the need for the ImageProcessor, by registering our own PHP stream wrapper for the dw2pdf:// protocol.

@Klap-in
Copy link
Collaborator

@Klap-in Klap-in commented on bd07ae4 Nov 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That became available for mpdf version 8.1, so further this fine to merge?

@eduardomozart
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At the time the adoption to 8.1 wasn't accepted because it would break compatibility with PHP 7.4 which DokuWiki relies for, as discussed in #476. So I believe it's fine to merge this PR as it fixes known mPDF issues like mpdf/mpdf@5b19fb6 and DW2PDF is currently using a mPDF version that is unsupported by mPDF team by now. When I opened an issue there they requested me to update it first, so I don't see any reason for DW2PDF to be stuck in a version that it have no support for.

Please sign in to comment.