Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reusing an image binary data when multiple copies exists #1153

Closed
MAKS-dev opened this issue Sep 5, 2019 · 17 comments · Fixed by #2416
Closed

Reusing an image binary data when multiple copies exists #1153

MAKS-dev opened this issue Sep 5, 2019 · 17 comments · Fixed by #2416

Comments

@MAKS-dev
Copy link

MAKS-dev commented Sep 5, 2019

This is:

- [x] a bug report
- [x] a feature request
- [x] **not** a usage question

This is a hint for solving #543

What is the expected behavior?

When the same image binary is reused in a workbook, it should only be added once in the actual file. Just like Excel does.

What is the current behavior?

Every copy in the workbook will add its own binary data in the file. The creates very large files.

What are the steps to reproduce?

Use PhpSpreadsheet to open and save the attached file: test-in.xlsx

Please provide a Minimal, Complete, and Verifiable example of code that exhibits the issue:
(sorry; it does rely on an external Excel file)

<?php

require __DIR__ . '/vendor/autoload.php';

// Create new Spreadsheet object
$spreadsheet = IOFactory::load('test-in.xls');

// add code that show the issue here...
$writer = IOFactory::createWriter($spreadsheet, 'Xlsx');
$writer->save('test-out.xlsx');

You'll notice that the new file has increased roughly 2x the image size.
A check with WinRAR shows 3 copies of the file.

Which versions of PhpSpreadsheet and PHP are affected?

Version 1.8.2 on PHP 7.1.25

Hints for reusing the images

When only loading an Excel file, modifying it without touching the images, and saving it, the code below can be a quick-and-dirty fix:

Add the following in class PhpOffice\PhpSpreadsheet\Worksheet\BaseDrawing

    /**
     * Global images index table.
     *
     * @var array
     */
    private static $imageIndexTable = array();

    /**
     * Change the current image index value based on the supplied file path and name.
     */
    public function setMergeDrawings($pValue)
    {
        $i = array_search($pValue, self::$imageIndexTable);
        if ($i === false) {
            self::$imageIndexTable[$this->imageIndex] = $pValue;
        } else {
            $this->imageIndex = $i;
        }
    }

Add the following in class PhpOffice\PhpSpreadsheet\Worksheet\Drawing
in function setPath just before the return

        $this->setMergeDrawings($pValue);

When opening a file, this will reuse the same image index for images with the same name and thus end up with one image binary in the saved file.
Note that this is just a hint for a possible solution. Someone who is more at home in this code should make a better solution.
But since the old issue was reported 14 months ago and closed (even although someone else reported that he could reproduce it), I hope that this time it will stay open untill a full solution is presented.

(as a side note ... I have added a workaround for another issue which is loosly related, see last comment on: #544 - in case anyone tries to save a file with the same name as it was opened)

@MAKS-dev
Copy link
Author

Bump

@MAKS-dev
Copy link
Author

I would really appriciate it when someone wants to look at this and can propose a complete solution. Am I the only one who wants to add a company logo on every sheet?

@Collie-IT
Copy link
Contributor

Why you aren't use the HeaderFooter functions?

@MAKS-dev
Copy link
Author

Thanks for the respons.
I have tested various options in Excel first. As the header is set per sheet, it would still cause duplicate image binaries in the file when saving it with PhpSpreadSheet.
But I cannot use the header at all since Excel does not support a background color there nor the layout needed. Also I need the logo at the top of evey sheet, not on every page. And visible without printing as most times the file is only viewed and not printed. Although that is my current spec, the header could help others with the same problem ;-)

It still would be a workaround instead of a fix for this bug in de code. My solution above seems to work in my case but might cause some side effects since I'm not familiair with the whole code and details of image handling in xlsx files.

@machinchose
Copy link

Yes, I experience the same issue a fix would be greatly appreciated

@stale
Copy link

stale bot commented Jan 21, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
If this is still an issue for you, please try to help by debugging it further and sharing your results.
Thank you for your contributions.

@stale stale bot added the stale label Jan 21, 2020
@MAKS-dev
Copy link
Author

This is far from stale ... still waiting for someone to check and improve my proposal ...

@stale stale bot removed the stale label Jan 22, 2020
@stale
Copy link

stale bot commented Mar 22, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
If this is still an issue for you, please try to help by debugging it further and sharing your results.
Thank you for your contributions.

@stale stale bot added the stale label Mar 22, 2020
@MAKS-dev
Copy link
Author

This is far from stale ... still waiting for someone to check and improve my proposal ... please?

@stale stale bot removed the stale label Mar 24, 2020
@stale
Copy link

stale bot commented May 23, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
If this is still an issue for you, please try to help by debugging it further and sharing your results.
Thank you for your contributions.

@stale stale bot added the stale label May 23, 2020
@MAKS-dev
Copy link
Author

There was activity 18 days ago :-| ... it's not stale unstill it is solved.

@stale stale bot removed the stale label May 24, 2020
@stale
Copy link

stale bot commented Jul 25, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
If this is still an issue for you, please try to help by debugging it further and sharing your results.
Thank you for your contributions.

@stale stale bot added the stale label Jul 25, 2020
@stale stale bot closed this as completed Aug 1, 2020
@MAKS-dev
Copy link
Author

MAKS-dev commented Aug 3, 2020

Darn it ... cannot leave this thread alone for a few days.
Can this be opened again? And please, please someone to have a look to fix this issue ... I'm waiting 2 years now (see #543) :-(

@MAKS-dev
Copy link
Author

Can this be opened again? It's not solved and still an issue.
I don't like to start a new issue and loosing the history ...
... please ??

@robertpustulka
Copy link

That helped me, thanks!

@MAKS-dev maybe post a PR with your solution?

@MAKS-dev
Copy link
Author

@robertpustulka glad it helped and thanks for your reply.

Sorry, I'm not experienced with git and github and don't know how this works. Also I don't think this is a 100% perfect solution, but don't know the whole image handling code.

@oleibman
Copy link
Collaborator

Fixed by PR 2416.

@oleibman oleibman removed the stale label Aug 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

5 participants