Cloned worksheet updates reflected on original (Drawing) #92

Tomino2112 · 2017-02-13T17:05:15Z

Hi,

This has been an issue in PHPExcel and is present in this library also.

Steps to reproduce:

Create MemoryDrawing and insert image to a sheet
Clone current sheet to new one
On the new sheet add couple of rows and columns
Export to Excel2007

What you will see is that if you add the drawing to say coordinates E2 and then add rows and columns to clone and image will shift to F10 when you look at the original sheet, image also moved to F10 over there

The text was updated successfully, but these errors were encountered:

PowerKiKi · 2017-02-14T04:00:23Z

Can you demonstrate the bug by code only (without the need to write and open an xlsx file) ?

Tomino2112 · 2017-02-14T10:59:28Z

@PowerKiKi Here you go:

public function actionExcel(){
        $image = "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAACAAAAAgCAYAAABzenr0AAAACXBIWXMAAC4jAAAuIwF4pT92AAAAGXRFWHRTb2Z0d2FyZQBBZG9iZSBJbWFnZVJlYWR5ccllPAAACdhJREFUeNqll2lsXNUVx+/M2OMsDk4cr4kd70lEaVCLWkRpQotEEaAiUIDSZsGesePEjoPtjLeZee++zVsc7ASv4w0TokBIIBGraMsSSksghIAIlE9IFVFb6t2O7Zn33r23576xjeOWyLQfju4b2e+e//mdc895F0mShL6XYfztCoYljGSv4MSCaK9uVFDvr8t+e2ZLUatfxjZBlZBSIyzHIrbh2f9fvN//5JxvNmeSX4zwaxJfnQO/Ki3+c7TrbxdsedNnflTc2VjiTfPWy/A3wQ4i0JwIvCCIJTvGs8/zL0NUkigiv4ZR4z7vlpc37jv1CdrFPkJ57EOUZ3yCHmN/inVd7tp+8EERKIkyvCeI8A6+Zs/vFb3lnOMUsF2ADQVZRN13V+57a+XeK+CUvW/LC30AzsEIPAcvolx20fZY6OxNRf2axx/nBVIipApo2PFSCISRzSKXLMwOjlKAqOuKhE0nNnrOvIn207fte9k7jj3BvyAX+RDl0gsoj/L1vM0V+hC5jEtA49zq/C8DD3m2K5IfiYoINHAEvq4AvCh6iJxjxKIYeeR+v+uFVZWDr9nL2WvLHtf/4Cw2QQR9D+WT8xA9dw6OSVhIHjlvc+ufop2Mp6j9Ntzuq1LTJMWPsCDZl0yAq9b24y2BH4pnTzl97IWV1ezMDR791WWl9PfOEvKmo4i8a9tD5iiAAMpT8QH8/gTtpG87C422dY2Gsr6HKTe2X5Z2NjwqKsL1CYRzLtlFyLVaIt3YnaJ+/swKiR1f6w+ejKkxX4yupC8vL6evRx2gf4woJu/YCxlQoEABzMUuot3kI7Sbno6pIlpGBxU39hJlY2BGTetmanpAV35+5KXvFiDOroLkwApGtS7lzu64usn+BEU/tlY0TsT46KkbqsnZlRX0leWl9I3IEvKWo4ies+8x3re59I/RTvNcRCFtT26kQk4flXO6aW1mF1UzA1TNCRhqdoCoG3rYkghgFSOtQNnamdww1Bev0YE4yTy+RiAnY7wUKJCXVoQpvBG1X38XFbL3UT47FVOl12Z2mhicg2NSmwWOs7upFjaiZQVMLafbmO9o13S5hUUohgmo+codbUmHRrsT6ml/vGIei8XkxGqgsAooRHuMl5eXmW+hEvZ6YtGF1luU0+KGp5ic2cO07M6QmtVFLMdZ3UQFIbU5PZYQWNl8g5lvNvNiFgkoULY9ue7wSGdiA+tN0MyBtTI9HiuYz62u0c9EVbJXHGXs2E8qjmoVQpxXUZzKow25anbnN2oqFF1OwFSyu3QtO8C0bylYhmQsOhY7v0YIF6ByAeodR1KaR9qSm2ggsc7oS1ShFiTz+Sgfezap5rMndvju480JSyKSBcE6NbhMyVJ+duR5LRUiTwchG7uCQIBAKsJC4Bl5JRWcQIvE4rUDYxEBZY+6rTmlZbR13WHWsb4h2Bdby56OkYMdtwntSoUYx5sT9PwIkTcqLPMz7uAt2BL0SEOBuqnzKzXFoqFrWV2mZtUECOivcm2TRAH54SVYrf6OF047XoQQmVyobWtKOzoEFEjHmibWkVF3uXEXvldQeYsWEIaB48MKavJ7Eg/7DiYLEhch2kRRsoka7FmqbFZubzmpQh9QM+AYZncZFoHJ3OTRS0W3vtjiK83kNDAXgaFf41khInZg6AMgYGtjSttUc+JRcugXdS2SR0oQIWrZ73fyqL2yik54dj1wJX/Tp09Vuu/yghgJwxTkp8mPI0TF6qaR0m8acpWczq8hLQyK00DDrnRzKjeZ/cud8fXpg4/kguNIn6QgRfRbvZoTsHp3ft1ddZtbP1V31T4gqpyOiBS/4KyRNHRIqEh4r/SXLVN5SWQyL2Wqp6pwK99DFmF28JoCqlZDE6CdqxaNTKiNF9XU7iAacaWRIVe6Me5KZcHcRPZx8W0nW7ylm/nGmNcGYOQpkDxyMvbIieFBIvDNEI/yuHfXvX8/kHU5VLiWjRakkmF32lBP9Z5tXAAnKeK5graCgWko2ThRLkre2fAQGnGnk1F3Gh1xp5mD7gx9OjeJ/cOd9dUL5Q8XCPAyN1kUrDTwXMuCP6IGa6hRrIg7X3N7+0x5bHD6QBwbKkkNjheuZ6P5qaM9VWEB/IThhd8Qc2KgLrhhoAEC0kBAOh11pTNOY9CdGZxwpbCruckEauP5Zm9pFt+M14QgyMgvKugZvOO+b4T1n5HqFWzCE2+OliXr4yVJxvi+dXRsT8pIb1XBHT6opzkBC4+3VROcBrbafASynC80EAJ1oQ+7MsxpqzbSv+G1IfhlZ6MEUas/PWrKUSwkRrMJ31p9vDLOnDwYT8dKk8zx4mQ2tnf9SC+kwLuAwH8c7QWjHo3lZzCeAlgpEKBzREby0shwfkZoPHcDm9qRzC5V3XL6SlPyBdaA2LS6yrwqxRhXhTVksiaWTlTEkfHyRHPiQBIFCqO91UAAhwWI39XicdjQSH6mAXVgjlq1YBGYFbKBDO/OoMPuDeakFKOzo4jRZjubaozWZ2pX0mllFZ3CMeSqbw2drFpLJjwJ5mRpIh0rTh7qqSnYajU46zj/96/heQJTeesYCKBD7gxjxLWBjuSD5abT4V0ZdLw8gYbaHNToQeZMW6Q+0xJlhA5H0Zn6FWRai6bT8irKKVytWUMnKuONsfIkffJA/GS/130nPyFW8Vq9RPxuAV/s/fG5sbyU4GTeejboypwZ3p1JRtypdFpbRcgAouYAIkYXYgYI0Z+MoKHmKBY8tIzM1K+kU2o0p0AnfbHmeHV80PREs3+WpX5+1Fdyo0+Ul0ZAwNj+rGfHI1fyN16eeSyBjZUlGqH2SIM8gyjpRcQEAwLU6LJTvdVBgkecJNgURaYbVhCeiglptR4UbmDEu4x9Xn3z2Tb//i08ejx7acHXcR6+mMDx8skKaqiuSjvvv7WdPY0YO4aY2WvTST8iYJSL0AOI6h12EnoyggSbnXS6aYU5XR+tUzWSTeLVg2eF+138g9WaKRIfbnhBwV2HAFfI264PXhQVCZ1sfHj7UGD1l2wARPQhw+izhbgIoMBTQfU2B5lpWRbUn3AycshG/6psPtMqFm/izYn3Chn7HeLc19RSCISvWJKVL5iG9hpBQ/WSJ+5S6039ZMAWYk9B/vvsQRDDKRjBjogQa0dsonnllVe1e/bxXFtRw+CxRroUdrzUC8/sw9zVi1erYBNgU27PNW5/cLAn9jJPCwjQaR+kpwexL1pyTrXK+7Z4Rc1KIcx8G57dZynYF9XAotuuZVyI314l1KMnlMdTL7beHOAkpnqjBl9tursIoo3wiipS8OzEnL8pS98r+mtux/PfhtK31atIvkhZ8jsq/XXopaZ7fteluX9QJdSBY59z7oj9P865/RvItF+hzPUB+AAAAABJRU5ErkJggg==";

        $excelOb = new Spreadsheet();

        $excelOb->getProperties()->setCreator("Tomas")
            ->setLastModifiedBy("Tomas")
            ->setTitle("Test for #92");

        // Select sheet
        $excelOb->setActiveSheetIndex(0);

        // St title
        $excelOb->getActiveSheet()->setTitle("Sheet_1");

        // Resample image
        $orig = imagecreatefrompng($image);

        $imgWidth = imagesx($orig);
        $imgHeight = imagesy($orig);

        $target = imagecreatetruecolor($imgWidth, $imgHeight);

        imagealphablending($target, false);
        imagesavealpha($target, true);

        imagecopyresampled($target, $orig, 0, 0, 0, 0, $imgWidth, $imgHeight, $imgWidth, $imgHeight);

        // Add image to sheet
        $objDrawing = new MemoryDrawing();
        $objDrawing->setName('Sample image');
        $objDrawing->setDescription('Sample image');
        $objDrawing->setImageResource($target);
        $objDrawing->setRenderingFunction(MemoryDrawing::RENDERING_PNG);
        $objDrawing->setMimeType(MemoryDrawing::MIMETYPE_DEFAULT);
        $objDrawing->setCoordinates("E2");
        $objDrawing->setWorksheet($excelOb->getActiveSheet());

        // Clone sheet
        // @note: Doing copy() vs clone does not make difference
        $newSheet = $excelOb->getActiveSheet()->copy();
        $newSheet->setTitle("Sheet_2");
        $excelOb->addSheet($newSheet);

        $newSheet->insertNewRowBefore(1, 1)->insertNewColumnBeforeByIndex(0, 1);

        for($i = 0; $i < 7; $i++){
            $newSheet->insertNewRowBefore(1, 1);
        }

        for($i = 0; $i < $excelOb->getSheetCount(); $i++){
            echo "Drawing location on sheet \"" . $excelOb->getSheet($i)->getTitle() . "\": " . $excelOb->getSheet($i)->getDrawingCollection()[0]->getCoordinates(). "<br />";
        }
    }

I have glued this from pieces of actual code so let's not get hold up on the code quality

Outcome:

Drawing location on sheet "Sheet_1": F10
Drawing location on sheet "Sheet_2": F10

ankitm123 · 2017-02-24T20:31:39Z

I can take a look at this issue, and see if I can help. Sounds like a good weekend project :)

PowerKiKi · 2017-02-25T07:28:16Z

@ankitm123 your help would be, once again, appreciated

ankitm123 · 2017-03-02T03:54:48Z

After taking a look at the code, there seems to be an issue here:
https://github.com/PHPOffice/PhpSpreadsheet/blob/develop/src/PhpSpreadsheet/Worksheet.php#L2977
Is there a reason why we have a special case for the drawingCollection? If we remove that case, then the unserialize(serialize ..) trick does the proper deep cloning, and the issue seems to be fixed.

I do understand that unserialize(serialize) has side effects as well as performance issues, but it does fix the issue of circular references.

Here are the passing tests. Let me know if this works for you, I will go ahead and submit a PR.

Tomino2112 · 2017-03-02T07:46:31Z

@ankitm123 I did play around with that clone function, not sure if I ever actually tried to remove that condition all together. Will give it a try and let you know

PowerKiKi · 2017-03-02T11:41:53Z

Is there a reason why we have a special case for the drawingCollection?

That was introduced in 5e27d03 and is related to that old issue. Maybe that can help you dig further...

ankitm123 · 2017-03-02T15:00:02Z

@PowerKiKi Yes, I see the issue now. It shows a symbol for the image in the excel file when I comment out the case for drawingCollection. I will dig a bit deeper into this issue. Thanks for the link.

@Tomino2112, I think you will also see the same thing, a symbol and not the image itself in the cell.

ankitm123 · 2017-03-03T14:50:14Z

@Tomino2112 and @PowerKiKi, did you get time to check my commit? I did try running @Tomino2112's code, and this time, the drawing location on the original sheet did not change. I also wrote it to an xslx file, and it shows the image correctly. Let me know if that works for you.

ankitm123 · 2017-03-06T03:23:52Z

This is the code sample I used (copy paste from @Tomino2112):

    require '../vendor/autoload.php';
    use PhpOffice\PhpSpreadsheet\{Spreadsheet,Worksheet\MemoryDrawing,IOFactory};

    $image = 'some image!';

    $excelOb = new Spreadsheet();

    $excelOb->getProperties()->setCreator("Tomas")
        ->setLastModifiedBy("Tomas")
        ->setTitle("Test for #92");

    // Select sheet
    $excelOb->setActiveSheetIndex(0);

    // St title
    $excelOb->getActiveSheet()->setTitle("Sheet_1");

    // Resample image
    $orig = imagecreatefrompng($image);

    $imgWidth = imagesx($orig);
    $imgHeight = imagesy($orig);

    $target = imagecreatetruecolor($imgWidth, $imgHeight);

    imagealphablending($target, false);
    imagesavealpha($target, true);

    imagecopyresampled($target, $orig, 0, 0, 0, 0, $imgWidth, $imgHeight, $imgWidth, $imgHeight);

    // Add image to sheet
    $objDrawing1 = new MemoryDrawing();
    $objDrawing1->setName('Sample image');
    $objDrawing1->setDescription('Sample image');
    $objDrawing1->setImageResource($target);
    $objDrawing1->setRenderingFunction(MemoryDrawing::RENDERING_PNG);
    $objDrawing1->setMimeType(MemoryDrawing::MIMETYPE_DEFAULT);
    $objDrawing1->setCoordinates("E2");

    $objDrawing2 = new MemoryDrawing();
    $objDrawing2->setName('Sample image');
    $objDrawing2->setDescription('Sample image');
    $objDrawing2->setImageResource($target);
    $objDrawing2->setRenderingFunction(MemoryDrawing::RENDERING_PNG);
    $objDrawing2->setMimeType(MemoryDrawing::MIMETYPE_DEFAULT);
    $objDrawing2->setCoordinates("G10");

    $objDrawing1->setWorksheet($excelOb->getActiveSheet());
    $objDrawing2->setWorksheet($excelOb->getActiveSheet());

    // Clone sheet
    // @note: Doing copy() vs clone does not make difference
    $newSheet = clone $excelOb->getActiveSheet();
    $newSheet->setTitle("Sheet_2");
    $excelOb->addSheet($newSheet);

    $newSheet->insertNewRowBefore(1, 1)->insertNewColumnBeforeByIndex(0, 1);

    for($i = 0; $i < 7; $i++){
        $newSheet->insertNewRowBefore(1, 1);
    }

    for($i = 0; $i < $excelOb->getSheetCount(); $i++){
        echo "Drawing location on sheet \"" . $excelOb->getSheet($i)->getTitle() . "\": " . $excelOb->getSheet($i)->getDrawingCollection()[0]->getCoordinates(). "<br />";
        echo "Drawing location on sheet \"" . $excelOb->getSheet($i)->getTitle() . "\": " . $excelOb->getSheet($i)->getDrawingCollection()[1]->getCoordinates(). "<br />";
    }

    $writer = IOFactory::createWriter($excelOb, "Xlsx");
    $writer->save("fix92.xlsx");

I think the commit fixes the bug. If not, please let me know. I will take a look again.

PowerKiKi · 2017-03-06T05:56:46Z

@Tomino2112 will you be able to test @ankitm123's solution is the coming days ?

Tomino2112 · 2017-03-06T05:59:40Z

Hi, sorry was away from pc, will test in couple of hours.

…

On Mar 6, 2017 05:56, "Adrien Crivelli" ***@***.***> wrote: @Tomino2112 <https://github.com/Tomino2112> will you be able to test @ankitm123 <https://github.com/ankitm123> is the coming days ? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#92 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABFl-VGCTA-xJll_m0vACly20zvXs3VYks5ri6AfgaJpZM4L_dqC> .

Tomino2112 · 2017-03-06T09:46:51Z

I can confirm that this works in PHPExcel library, now will try PHPSpreadsheet

Tomino2112 · 2017-03-06T09:57:37Z

Works for PHPSpreadsheet as well, Thank you @ankitm123 great job!

PowerKiKi · 2017-03-06T13:58:50Z

Thanks all for your collaboration.

ankitm123 mentioned this issue Mar 2, 2017

MemoryDrawing bug fix for Issue #92 #106

Merged

PowerKiKi closed this as completed in f99eb8d Mar 6, 2017

t-iri mentioned this issue Mar 27, 2018

Can't escape from Worksheet's cloning loop and exhausts memory #437

Closed

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Cloned worksheet updates reflected on original (Drawing) #92

Cloned worksheet updates reflected on original (Drawing) #92

Tomino2112 commented Feb 13, 2017

PowerKiKi commented Feb 14, 2017

Tomino2112 commented Feb 14, 2017 •

edited

Loading

ankitm123 commented Feb 24, 2017

PowerKiKi commented Feb 25, 2017

ankitm123 commented Mar 2, 2017 •

edited

Loading

Tomino2112 commented Mar 2, 2017

PowerKiKi commented Mar 2, 2017

ankitm123 commented Mar 2, 2017

ankitm123 commented Mar 3, 2017

ankitm123 commented Mar 6, 2017 •

edited by PowerKiKi

Loading

PowerKiKi commented Mar 6, 2017 •

edited

Loading

Tomino2112 commented Mar 6, 2017 via email

Tomino2112 commented Mar 6, 2017

Tomino2112 commented Mar 6, 2017

PowerKiKi commented Mar 6, 2017

Cloned worksheet updates reflected on original (Drawing) #92

Cloned worksheet updates reflected on original (Drawing) #92

Comments

Tomino2112 commented Feb 13, 2017

PowerKiKi commented Feb 14, 2017

Tomino2112 commented Feb 14, 2017 • edited Loading

ankitm123 commented Feb 24, 2017

PowerKiKi commented Feb 25, 2017

ankitm123 commented Mar 2, 2017 • edited Loading

Tomino2112 commented Mar 2, 2017

PowerKiKi commented Mar 2, 2017

ankitm123 commented Mar 2, 2017

ankitm123 commented Mar 3, 2017

ankitm123 commented Mar 6, 2017 • edited by PowerKiKi Loading

PowerKiKi commented Mar 6, 2017 • edited Loading

Tomino2112 commented Mar 6, 2017 via email

Tomino2112 commented Mar 6, 2017

Tomino2112 commented Mar 6, 2017

PowerKiKi commented Mar 6, 2017

Tomino2112 commented Feb 14, 2017 •

edited

Loading

ankitm123 commented Mar 2, 2017 •

edited

Loading

ankitm123 commented Mar 6, 2017 •

edited by PowerKiKi

Loading

PowerKiKi commented Mar 6, 2017 •

edited

Loading