Skip to content

Commit db57af0

Browse files
oleibmanoleibman
authored
Fix Chart Problems and Memory Leak in Xlsx Writer (#2930)
This was supposed to be mopping up some longstanding chart issues. But one of the sample files exposed a memory leak in Xlsx Writer, unrelated to charts. Since that is my best sample file for this problem, I would like to fix both problems at the same time. Xlsx Writer for Worksheets calls getRowDimension for all rows on the sheet. As it happens, the sample file had data in the last rows after a huge gap of rows without any data. It correctly did not write anything for the unused rows. However, the call to getRowDimension actually creates a new RowDimension object if it doesn't already exist, and so it wound up creating over a million totally unneeded objects. This caused it to run out of memory when I tried to make a copy of the 8K input file. The logic is changed to call getRowDimension if and only if (there is data in the row or the RowDimension object already exists). It still has to loop through a million rows, but it no longer allocates the unneeded storage. As for the Chart problems - fix #1797. This is where the file that caused the memory leak originated. Many of its problems were already resolved by the earlier large set of changes to Charts. However, there were a few new properties that needed to be added to Layout to make things complete - numberFormat code and source-linked, and dLblPos (position for labels); and autoTitleDeleted needs to be added to Charts. Also fix #2077, by allowing the format to be specified in the Layout rather than the DataSeriesValues constructor.
1 parent f0059bb commit db57af0

File tree

9 files changed

+266
-46
lines changed

9 files changed

+266
-46
lines changed

‎samples/templates/32readwriteLineChart5.xlsx

8.15 KB
Binary file not shown.

‎src/PhpSpreadsheet/Chart/Chart.php

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,9 @@ class Chart
141141
/** @var bool */
142142
private $oneCellAnchor = false;
143143

144+
/** @var bool */
145+
private $autoTitleDeleted = false;
146+
144147
/**
145148
* Create a new Chart.
146149
* majorGridlines and minorGridlines are deprecated, moved to Axis.
@@ -732,4 +735,16 @@ public function setOneCellAnchor(bool $oneCellAnchor): self
732735

733736
return $this;
734737
}
738+
739+
public function getAutoTitleDeleted(): bool
740+
{
741+
return $this->autoTitleDeleted;
742+
}
743+
744+
public function setAutoTitleDeleted(bool $autoTitleDeleted): self
745+
{
746+
$this->autoTitleDeleted = $autoTitleDeleted;
747+
748+
return $this;
749+
}
735750
}

‎src/PhpSpreadsheet/Chart/Layout.php

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,19 @@ class Layout
5353
*/
5454
private $height;
5555

56+
/**
57+
* Position - t=top.
58+
*
59+
* @var string
60+
*/
61+
private $dLblPos = '';
62+
63+
/** @var string */
64+
private $numFmtCode = '';
65+
66+
/** @var bool */
67+
private $numFmtLinked = false;
68+
5669
/**
5770
* show legend key
5871
* Specifies that legend keys should be shown in data labels.
@@ -143,13 +156,20 @@ public function __construct(array $layout = [])
143156
if (isset($layout['h'])) {
144157
$this->height = (float) $layout['h'];
145158
}
159+
if (isset($layout['dLblPos'])) {
160+
$this->dLblPos = (string) $layout['dLblPos'];
161+
}
162+
if (isset($layout['numFmtCode'])) {
163+
$this->numFmtCode = (string) $layout['numFmtCode'];
164+
}
146165
$this->initBoolean($layout, 'showLegendKey');
147166
$this->initBoolean($layout, 'showVal');
148167
$this->initBoolean($layout, 'showCatName');
149168
$this->initBoolean($layout, 'showSerName');
150169
$this->initBoolean($layout, 'showPercent');
151170
$this->initBoolean($layout, 'showBubbleSize');
152171
$this->initBoolean($layout, 'showLeaderLines');
172+
$this->initBoolean($layout, 'numFmtLinked');
153173
$this->initColor($layout, 'labelFillColor');
154174
$this->initColor($layout, 'labelBorderColor');
155175
$this->initColor($layout, 'labelFontColor');
@@ -484,4 +504,40 @@ public function setLabelFontColor(?ChartColor $chartColor): self
484504

485505
return $this;
486506
}
507+
508+
public function getDLblPos(): string
509+
{
510+
return $this->dLblPos;
511+
}
512+
513+
public function setDLblPos(string $dLblPos): self
514+
{
515+
$this->dLblPos = $dLblPos;
516+
517+
return $this;
518+
}
519+
520+
public function getNumFmtCode(): string
521+
{
522+
return $this->numFmtCode;
523+
}
524+
525+
public function setNumFmtCode(string $numFmtCode): self
526+
{
527+
$this->numFmtCode = $numFmtCode;
528+
529+
return $this;
530+
}
531+
532+
public function getNumFmtLinked(): bool
533+
{
534+
return $this->numFmtLinked;
535+
}
536+
537+
public function setNumFmtLinked(bool $numFmtLinked): self
538+
{
539+
$this->numFmtLinked = $numFmtLinked;
540+
541+
return $this;
542+
}
487543
}

‎src/PhpSpreadsheet/Reader/Xlsx/Chart.php

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,12 +72,18 @@ public function readChart(SimpleXMLElement $chartElements, $chartName)
7272
$rotX = $rotY = $rAngAx = $perspective = null;
7373
$xAxis = new Axis();
7474
$yAxis = new Axis();
75+
$autoTitleDeleted = null;
7576
foreach ($chartElementsC as $chartElementKey => $chartElement) {
7677
switch ($chartElementKey) {
7778
case 'chart':
7879
foreach ($chartElement as $chartDetailsKey => $chartDetails) {
7980
$chartDetailsC = $chartDetails->children($this->cNamespace);
8081
switch ($chartDetailsKey) {
82+
case 'autoTitleDeleted':
83+
/** @var bool */
84+
$autoTitleDeleted = self::getAttribute($chartElementsC->chart->autoTitleDeleted, 'val', 'boolean');
85+
86+
break;
8187
case 'view3D':
8288
$rotX = self::getAttribute($chartDetails->rotX, 'val', 'integer');
8389
$rotY = self::getAttribute($chartDetails->rotY, 'val', 'integer');
@@ -324,6 +330,9 @@ public function readChart(SimpleXMLElement $chartElements, $chartName)
324330
}
325331
}
326332
$chart = new \PhpOffice\PhpSpreadsheet\Chart\Chart($chartName, $title, $legend, $plotArea, $plotVisOnly, (string) $dispBlanksAs, $XaxisLabel, $YaxisLabel, $xAxis, $yAxis);
333+
if (is_bool($autoTitleDeleted)) {
334+
$chart->setAutoTitleDeleted($autoTitleDeleted);
335+
}
327336
if (is_int($rotX)) {
328337
$chart->setRotX($rotX);
329338
}
@@ -967,6 +976,13 @@ private function readChartAttributes($chartDetail): array
967976
{
968977
$plotAttributes = [];
969978
if (isset($chartDetail->dLbls)) {
979+
if (isset($chartDetail->dLbls->dLblPos)) {
980+
$plotAttributes['dLblPos'] = self::getAttribute($chartDetail->dLbls->dLblPos, 'val', 'string');
981+
}
982+
if (isset($chartDetail->dLbls->numFmt)) {
983+
$plotAttributes['numFmtCode'] = self::getAttribute($chartDetail->dLbls->numFmt, 'formatCode', 'string');
984+
$plotAttributes['numFmtLinked'] = self::getAttribute($chartDetail->dLbls->numFmt, 'sourceLinked', 'boolean');
985+
}
970986
if (isset($chartDetail->dLbls->showLegendKey)) {
971987
$plotAttributes['showLegendKey'] = self::getAttribute($chartDetail->dLbls->showLegendKey, 'val', 'string');
972988
}

‎src/PhpSpreadsheet/Worksheet/Worksheet.php

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1413,6 +1413,11 @@ public function getRowDimension(int $row): RowDimension
14131413
return $this->rowDimensions[$row];
14141414
}
14151415

1416+
public function rowDimensionExists(int $row): bool
1417+
{
1418+
return isset($this->rowDimensions[$row]);
1419+
}
1420+
14161421
/**
14171422
* Get column dimension at a specific column.
14181423
*

‎src/PhpSpreadsheet/Writer/Xlsx/Chart.php

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ public function writeChart(\PhpOffice\PhpSpreadsheet\Chart\Chart $chart, $calcul
6868
$this->writeTitle($objWriter, $chart->getTitle());
6969

7070
$objWriter->startElement('c:autoTitleDeleted');
71-
$objWriter->writeAttribute('val', '0');
71+
$objWriter->writeAttribute('val', (string) (int) $chart->getAutoTitleDeleted());
7272
$objWriter->endElement();
7373

7474
$objWriter->startElement('c:view3D');
@@ -420,6 +420,17 @@ private function writeDataLabels(XMLWriter $objWriter, ?Layout $chartLayout = nu
420420
$objWriter->endElement(); // c:txPr
421421
}
422422

423+
if ($chartLayout->getNumFmtCode() !== '') {
424+
$objWriter->startElement('c:numFmt');
425+
$objWriter->writeAttribute('formatCode', $chartLayout->getnumFmtCode());
426+
$objWriter->writeAttribute('sourceLinked', (string) (int) $chartLayout->getnumFmtLinked());
427+
$objWriter->endElement(); // c:numFmt
428+
}
429+
if ($chartLayout->getDLblPos() !== '') {
430+
$objWriter->startElement('c:dLblPos');
431+
$objWriter->writeAttribute('val', $chartLayout->getDLblPos());
432+
$objWriter->endElement(); // c:dLblPos
433+
}
423434
$this->writeDataLabelsBool($objWriter, 'showLegendKey', $chartLayout->getShowLegendKey());
424435
$this->writeDataLabelsBool($objWriter, 'showVal', $chartLayout->getShowVal());
425436
$this->writeDataLabelsBool($objWriter, 'showCatName', $chartLayout->getShowCatName());

‎src/PhpSpreadsheet/Writer/Xlsx/Worksheet.php

Lines changed: 48 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -1155,58 +1155,61 @@ private function writeSheetData(XMLWriter $objWriter, PhpspreadsheetWorksheet $w
11551155

11561156
$currentRow = 0;
11571157
while ($currentRow++ < $highestRow) {
1158-
// Get row dimension
1159-
$rowDimension = $worksheet->getRowDimension($currentRow);
1160-
1161-
// Write current row?
1162-
$writeCurrentRow = isset($cellsByRow[$currentRow]) || $rowDimension->getRowHeight() >= 0 || $rowDimension->getVisible() === false || $rowDimension->getCollapsed() === true || $rowDimension->getOutlineLevel() > 0 || $rowDimension->getXfIndex() !== null;
1163-
1164-
if ($writeCurrentRow) {
1165-
// Start a new row
1166-
$objWriter->startElement('row');
1167-
$objWriter->writeAttribute('r', $currentRow);
1168-
$objWriter->writeAttribute('spans', '1:' . $colCount);
1169-
1170-
// Row dimensions
1171-
if ($rowDimension->getRowHeight() >= 0) {
1172-
$objWriter->writeAttribute('customHeight', '1');
1173-
$objWriter->writeAttribute('ht', StringHelper::formatNumber($rowDimension->getRowHeight()));
1174-
}
1158+
$isRowSet = isset($cellsByRow[$currentRow]);
1159+
if ($isRowSet || $worksheet->rowDimensionExists($currentRow)) {
1160+
// Get row dimension
1161+
$rowDimension = $worksheet->getRowDimension($currentRow);
1162+
1163+
// Write current row?
1164+
$writeCurrentRow = $isRowSet || $rowDimension->getRowHeight() >= 0 || $rowDimension->getVisible() === false || $rowDimension->getCollapsed() === true || $rowDimension->getOutlineLevel() > 0 || $rowDimension->getXfIndex() !== null;
1165+
1166+
if ($writeCurrentRow) {
1167+
// Start a new row
1168+
$objWriter->startElement('row');
1169+
$objWriter->writeAttribute('r', $currentRow);
1170+
$objWriter->writeAttribute('spans', '1:' . $colCount);
1171+
1172+
// Row dimensions
1173+
if ($rowDimension->getRowHeight() >= 0) {
1174+
$objWriter->writeAttribute('customHeight', '1');
1175+
$objWriter->writeAttribute('ht', StringHelper::formatNumber($rowDimension->getRowHeight()));
1176+
}
11751177

1176-
// Row visibility
1177-
if (!$rowDimension->getVisible() === true) {
1178-
$objWriter->writeAttribute('hidden', 'true');
1179-
}
1178+
// Row visibility
1179+
if (!$rowDimension->getVisible() === true) {
1180+
$objWriter->writeAttribute('hidden', 'true');
1181+
}
11801182

1181-
// Collapsed
1182-
if ($rowDimension->getCollapsed() === true) {
1183-
$objWriter->writeAttribute('collapsed', 'true');
1184-
}
1183+
// Collapsed
1184+
if ($rowDimension->getCollapsed() === true) {
1185+
$objWriter->writeAttribute('collapsed', 'true');
1186+
}
11851187

1186-
// Outline level
1187-
if ($rowDimension->getOutlineLevel() > 0) {
1188-
$objWriter->writeAttribute('outlineLevel', $rowDimension->getOutlineLevel());
1189-
}
1188+
// Outline level
1189+
if ($rowDimension->getOutlineLevel() > 0) {
1190+
$objWriter->writeAttribute('outlineLevel', $rowDimension->getOutlineLevel());
1191+
}
11901192

1191-
// Style
1192-
if ($rowDimension->getXfIndex() !== null) {
1193-
$objWriter->writeAttribute('s', $rowDimension->getXfIndex());
1194-
$objWriter->writeAttribute('customFormat', '1');
1195-
}
1193+
// Style
1194+
if ($rowDimension->getXfIndex() !== null) {
1195+
$objWriter->writeAttribute('s', $rowDimension->getXfIndex());
1196+
$objWriter->writeAttribute('customFormat', '1');
1197+
}
11961198

1197-
// Write cells
1198-
if (isset($cellsByRow[$currentRow])) {
1199-
// We have a comma-separated list of column names (with a trailing entry); split to an array
1200-
$columnsInRow = explode(',', $cellsByRow[$currentRow]);
1201-
array_pop($columnsInRow);
1202-
foreach ($columnsInRow as $column) {
1203-
// Write cell
1204-
$this->writeCell($objWriter, $worksheet, "{$column}{$currentRow}", $aFlippedStringTable);
1199+
// Write cells
1200+
if (isset($cellsByRow[$currentRow])) {
1201+
// We have a comma-separated list of column names (with a trailing entry); split to an array
1202+
$columnsInRow = explode(',', $cellsByRow[$currentRow]);
1203+
array_pop($columnsInRow);
1204+
foreach ($columnsInRow as $column) {
1205+
// Write cell
1206+
$this->writeCell($objWriter, $worksheet, "{$column}{$currentRow}", $aFlippedStringTable);
1207+
}
12051208
}
1206-
}
12071209

1208-
// End row
1209-
$objWriter->endElement();
1210+
// End row
1211+
$objWriter->endElement();
1212+
}
12101213
}
12111214
}
12121215

0 commit comments

Comments
 (0)