Skip to content

Commit 8513c64

Browse files
authored
Memory Leak in Sample35 (#3062)
* Memory Leak in Sample35 All but 6 chart samples can be rendered by Sample35. Of those 6, 3 of the problems are because the script runs out of memory processing them. Adopting a suggestion from @MAKS-dev in issue #2092, adding a call to gc_collect_cycles after the charts from each spreadsheet is rendered appears to make it possible to include those 3 spreadsheets in Sample35 after all. Also take advantage of this opportunity to correct a number (hopefully all) of Scrutinizer problems with JpgraphRendererBases. * Minor Fix Problem running 8.1 unit tests. * Resolve Problems with Pie 3D Charts Minor fix, leaving only one spreadsheet unusable in Sample35. The reasons for its unusability are now documented in the code. * Mitoteam Made Changes Discussing this problem with them, they decided they should make a change for Pie3D rather than forcing us to use the workaround pushed earlier. Change to require mitoteam 10.2.3, revert workaround.
1 parent 6c1651e commit 8513c64

File tree

4 files changed

+38
-54
lines changed

4 files changed

+38
-54
lines changed

composer.json

+1-1
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@
8181
"dealerdirect/phpcodesniffer-composer-installer": "dev-master",
8282
"dompdf/dompdf": "^1.0 || ^2.0",
8383
"friendsofphp/php-cs-fixer": "^3.2",
84-
"mitoteam/jpgraph": "10.2.2",
84+
"mitoteam/jpgraph": "10.2.3",
8585
"mpdf/mpdf": "8.1.1",
8686
"phpcompatibility/php-compatibility": "^9.3",
8787
"phpstan/phpstan": "^1.1",

composer.lock

+8-8
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

samples/Chart/35_Chart_render.php

+7-6
Original file line numberDiff line numberDiff line change
@@ -25,12 +25,10 @@
2525
$unresolvedErrors = [];
2626
} else {
2727
$unresolvedErrors = [
28+
// The following spreadsheet was created by 3rd party software,
29+
// and doesn't include the data that usually accompanies a chart.
30+
// That is good enough for Excel, but not for JpGraph.
2831
'32readwriteBubbleChart2.xlsx',
29-
'32readwritePieChart3.xlsx',
30-
'32readwritePieChart4.xlsx',
31-
'32readwritePieChart3D1.xlsx',
32-
'32readwritePieChartExploded1.xlsx',
33-
'32readwritePieChartExploded3D1.xlsx',
3432
];
3533
}
3634
foreach ($inputFileNames as $inputFileName) {
@@ -42,7 +40,9 @@
4240
continue;
4341
}
4442
if (in_array($inputFileNameShort, $unresolvedErrors, true)) {
45-
$helper->log('File ' . $inputFileNameShort . ' does not yet work with this script');
43+
$helper->log('*****');
44+
$helper->log('***** File ' . $inputFileNameShort . ' does not yet work with this script');
45+
$helper->log('*****');
4646

4747
continue;
4848
}
@@ -92,6 +92,7 @@
9292

9393
$spreadsheet->disconnectWorksheets();
9494
unset($spreadsheet);
95+
gc_collect_cycles();
9596
}
9697

9798
$helper->log('Done rendering charts as images');

src/PhpSpreadsheet/Chart/Renderer/JpGraphRendererBase.php

+22-39
Original file line numberDiff line numberDiff line change
@@ -102,13 +102,11 @@ private function formatPointMarker($seriesPlot, $markerID)
102102
return $seriesPlot;
103103
}
104104

105-
private function formatDataSetLabels($groupID, $datasetLabels, $labelCount, $rotation = '')
105+
private function formatDataSetLabels($groupID, $datasetLabels, $rotation = '')
106106
{
107-
$datasetLabelFormatCode = $this->chart->getPlotArea()->getPlotGroupByIndex($groupID)->getPlotCategoryByIndex(0)->getFormatCode();
108-
if ($datasetLabelFormatCode !== null) {
109-
// Retrieve any label formatting code
110-
$datasetLabelFormatCode = stripslashes($datasetLabelFormatCode);
111-
}
107+
$datasetLabelFormatCode = $this->chart->getPlotArea()->getPlotGroupByIndex($groupID)->getPlotCategoryByIndex(0)->getFormatCode() ?? '';
108+
// Retrieve any label formatting code
109+
$datasetLabelFormatCode = stripslashes($datasetLabelFormatCode);
112110

113111
$testCurrentIndex = 0;
114112
foreach ($datasetLabels as $i => $datasetLabel) {
@@ -273,15 +271,15 @@ private function renderRadarPlotArea(): void
273271
$this->renderTitle();
274272
}
275273

276-
private function renderPlotLine($groupID, $filled = false, $combination = false, $dimensions = '2d'): void
274+
private function renderPlotLine($groupID, $filled = false, $combination = false): void
277275
{
278276
$grouping = $this->chart->getPlotArea()->getPlotGroupByIndex($groupID)->getPlotGrouping();
279277

280278
$index = array_keys($this->chart->getPlotArea()->getPlotGroupByIndex($groupID)->getPlotOrder())[0];
281279
$labelCount = $this->chart->getPlotArea()->getPlotGroupByIndex($groupID)->getPlotValuesByIndex($index)->getPointCount();
282280
if ($labelCount > 0) {
283281
$datasetLabels = $this->chart->getPlotArea()->getPlotGroupByIndex($groupID)->getPlotCategoryByIndex(0)->getDataValues();
284-
$datasetLabels = $this->formatDataSetLabels($groupID, $datasetLabels, $labelCount);
282+
$datasetLabels = $this->formatDataSetLabels($groupID, $datasetLabels);
285283
$this->graph->xaxis->SetTickLabels($datasetLabels);
286284
}
287285

@@ -353,7 +351,7 @@ private function renderPlotBar($groupID, $dimensions = '2d'): void
353351
$labelCount = $this->chart->getPlotArea()->getPlotGroupByIndex($groupID)->getPlotValuesByIndex($index)->getPointCount();
354352
if ($labelCount > 0) {
355353
$datasetLabels = $this->chart->getPlotArea()->getPlotGroupByIndex($groupID)->getPlotCategoryByIndex(0)->getDataValues();
356-
$datasetLabels = $this->formatDataSetLabels($groupID, $datasetLabels, $labelCount, $rotation);
354+
$datasetLabels = $this->formatDataSetLabels($groupID, $datasetLabels, $rotation);
357355
// Rotate for bar rather than column chart
358356
if ($rotation == 'bar') {
359357
$datasetLabels = array_reverse($datasetLabels);
@@ -430,11 +428,9 @@ private function renderPlotBar($groupID, $dimensions = '2d'): void
430428

431429
private function renderPlotScatter($groupID, $bubble): void
432430
{
433-
$grouping = $this->chart->getPlotArea()->getPlotGroupByIndex($groupID)->getPlotGrouping();
434431
$scatterStyle = $bubbleSize = $this->chart->getPlotArea()->getPlotGroupByIndex($groupID)->getPlotStyle();
435432

436433
$seriesCount = $this->chart->getPlotArea()->getPlotGroupByIndex($groupID)->getPlotSeriesCount();
437-
$seriesPlots = [];
438434

439435
// Loop through each data series in turn
440436
for ($i = 0; $i < $seriesCount; ++$i) {
@@ -478,7 +474,6 @@ private function renderPlotRadar($groupID): void
478474
$radarStyle = $this->chart->getPlotArea()->getPlotGroupByIndex($groupID)->getPlotStyle();
479475

480476
$seriesCount = $this->chart->getPlotArea()->getPlotGroupByIndex($groupID)->getPlotSeriesCount();
481-
$seriesPlots = [];
482477

483478
// Loop through each data series in turn
484479
for ($i = 0; $i < $seriesCount; ++$i) {
@@ -513,15 +508,11 @@ private function renderPlotRadar($groupID): void
513508

514509
private function renderPlotContour($groupID): void
515510
{
516-
$contourStyle = $this->chart->getPlotArea()->getPlotGroupByIndex($groupID)->getPlotStyle();
517-
518511
$seriesCount = $this->chart->getPlotArea()->getPlotGroupByIndex($groupID)->getPlotSeriesCount();
519-
$seriesPlots = [];
520512

521513
$dataValues = [];
522514
// Loop through each data series in turn
523515
for ($i = 0; $i < $seriesCount; ++$i) {
524-
$dataValuesY = $this->chart->getPlotArea()->getPlotGroupByIndex($groupID)->getPlotCategoryByIndex($i)->getDataValues();
525516
$dataValuesX = $this->chart->getPlotArea()->getPlotGroupByIndex($groupID)->getPlotValuesByIndex($i)->getDataValues();
526517

527518
$dataValues[$i] = $dataValuesX;
@@ -565,7 +556,7 @@ private function renderPlotStock($groupID): void
565556
$labelCount = $this->chart->getPlotArea()->getPlotGroupByIndex($groupID)->getPlotValuesByIndex(0)->getPointCount();
566557
if ($labelCount > 0) {
567558
$datasetLabels = $this->chart->getPlotArea()->getPlotGroupByIndex($groupID)->getPlotCategoryByIndex(0)->getDataValues();
568-
$datasetLabels = $this->formatDataSetLabels($groupID, $datasetLabels, $labelCount);
559+
$datasetLabels = $this->formatDataSetLabels($groupID, $datasetLabels);
569560
$this->graph->xaxis->SetTickLabels($datasetLabels);
570561
}
571562

@@ -575,21 +566,21 @@ private function renderPlotStock($groupID): void
575566
$this->graph->Add($seriesPlot);
576567
}
577568

578-
private function renderAreaChart($groupCount, $dimensions = '2d'): void
569+
private function renderAreaChart($groupCount): void
579570
{
580571
$this->renderCartesianPlotArea();
581572

582573
for ($i = 0; $i < $groupCount; ++$i) {
583-
$this->renderPlotLine($i, true, false, $dimensions);
574+
$this->renderPlotLine($i, true, false);
584575
}
585576
}
586577

587-
private function renderLineChart($groupCount, $dimensions = '2d'): void
578+
private function renderLineChart($groupCount): void
588579
{
589580
$this->renderCartesianPlotArea();
590581

591582
for ($i = 0; $i < $groupCount; ++$i) {
592-
$this->renderPlotLine($i, false, false, $dimensions);
583+
$this->renderPlotLine($i, false, false);
593584
}
594585
}
595586

@@ -626,19 +617,17 @@ private function renderPieChart($groupCount, $dimensions = '2d', $doughnut = fal
626617

627618
$iLimit = ($multiplePlots) ? $groupCount : 1;
628619
for ($groupID = 0; $groupID < $iLimit; ++$groupID) {
629-
$grouping = $this->chart->getPlotArea()->getPlotGroupByIndex($groupID)->getPlotGrouping();
630620
$exploded = $this->chart->getPlotArea()->getPlotGroupByIndex($groupID)->getPlotStyle();
631621
$datasetLabels = [];
632622
if ($groupID == 0) {
633623
$labelCount = $this->chart->getPlotArea()->getPlotGroupByIndex($groupID)->getPlotValuesByIndex(0)->getPointCount();
634624
if ($labelCount > 0) {
635625
$datasetLabels = $this->chart->getPlotArea()->getPlotGroupByIndex($groupID)->getPlotCategoryByIndex(0)->getDataValues();
636-
$datasetLabels = $this->formatDataSetLabels($groupID, $datasetLabels, $labelCount);
626+
$datasetLabels = $this->formatDataSetLabels($groupID, $datasetLabels);
637627
}
638628
}
639629

640630
$seriesCount = $this->chart->getPlotArea()->getPlotGroupByIndex($groupID)->getPlotSeriesCount();
641-
$seriesPlots = [];
642631
// For pie charts, we only display the first series: doughnut charts generally display all series
643632
$jLimit = ($multiplePlots) ? $seriesCount : 1;
644633
// Loop through each data series in turn
@@ -669,7 +658,7 @@ private function renderPieChart($groupCount, $dimensions = '2d', $doughnut = fal
669658
$seriesPlot->SetSize(($jLimit - $j) / ($jLimit * 4));
670659
}
671660

672-
if ($doughnut) {
661+
if ($doughnut && method_exists($seriesPlot, 'SetMidColor')) {
673662
$seriesPlot->SetMidColor('white');
674663
}
675664

@@ -710,7 +699,7 @@ private function renderStockChart($groupCount): void
710699
}
711700
}
712701

713-
private function renderContourChart($groupCount, $dimensions): void
702+
private function renderContourChart($groupCount): void
714703
{
715704
$this->renderCartesianPlotArea('intint');
716705

@@ -719,7 +708,7 @@ private function renderContourChart($groupCount, $dimensions): void
719708
}
720709
}
721710

722-
private function renderCombinationChart($groupCount, $dimensions, $outputDestination)
711+
private function renderCombinationChart($groupCount, $outputDestination)
723712
{
724713
$this->renderCartesianPlotArea();
725714

@@ -728,10 +717,8 @@ private function renderCombinationChart($groupCount, $dimensions, $outputDestina
728717
$chartType = $this->chart->getPlotArea()->getPlotGroupByIndex($i)->getPlotType();
729718
switch ($chartType) {
730719
case 'area3DChart':
731-
$dimensions = '3d';
732-
// no break
733720
case 'areaChart':
734-
$this->renderPlotLine($i, true, true, $dimensions);
721+
$this->renderPlotLine($i, true, true);
735722

736723
break;
737724
case 'bar3DChart':
@@ -742,10 +729,8 @@ private function renderCombinationChart($groupCount, $dimensions, $outputDestina
742729

743730
break;
744731
case 'line3DChart':
745-
$dimensions = '3d';
746-
// no break
747732
case 'lineChart':
748-
$this->renderPlotLine($i, false, true, $dimensions);
733+
$this->renderPlotLine($i, false, true);
749734

750735
break;
751736
case 'scatterChart':
@@ -792,7 +777,7 @@ public function render($outputDestination)
792777

793778
return false;
794779
} else {
795-
return $this->renderCombinationChart($groupCount, $dimensions, $outputDestination);
780+
return $this->renderCombinationChart($groupCount, $outputDestination);
796781
}
797782
}
798783

@@ -801,7 +786,7 @@ public function render($outputDestination)
801786
$dimensions = '3d';
802787
// no break
803788
case 'areaChart':
804-
$this->renderAreaChart($groupCount, $dimensions);
789+
$this->renderAreaChart($groupCount);
805790

806791
break;
807792
case 'bar3DChart':
@@ -815,7 +800,7 @@ public function render($outputDestination)
815800
$dimensions = '3d';
816801
// no break
817802
case 'lineChart':
818-
$this->renderLineChart($groupCount, $dimensions);
803+
$this->renderLineChart($groupCount);
819804

820805
break;
821806
case 'pie3DChart':
@@ -845,10 +830,8 @@ public function render($outputDestination)
845830

846831
break;
847832
case 'surface3DChart':
848-
$dimensions = '3d';
849-
// no break
850833
case 'surfaceChart':
851-
$this->renderContourChart($groupCount, $dimensions);
834+
$this->renderContourChart($groupCount);
852835

853836
break;
854837
case 'stockChart':

0 commit comments

Comments
 (0)