Two Problems with Html Chart Rendering - Minor Break

oleibman · oleibman · commit f427d02197ff · 2023-11-09T18:51:06.000-08:00
Several problems are noted in PHPOffice#3783. This PR addresses those problems which make the rendering unsatisfactory (see following paragraphs). It does not address some items where the rendering is IMO satisfactory although it doesn't match Excel. In particular, the use of a different color palette and the rotation of charts are not addressed. I will leave the issue open for now because of those. As for the items which are addressed, in some cases the Html was omitting a chart altogether. This is because it had been extending the column range for charts only when it decided that extending the row range was needed. The code is changed to now extend the column range whenever the chart begins beyond the current column range of the sheet. Also, the rendering always produced a fixed-size image, so saving as Html could result in charts overlaying each other or other parts of the spreadsheet. New properties `renderedWidth` and `renderedHeight` are added to Chart, along with setters and getters. Writer/Html is changed to set these values using the chart's top left and bottom right cells to try to determine the actual size that is needed. Users can also set these properties outside of Writer/Html if they wish. Thanks to @f1mishutka for determining the source of this problem and suggesting an approach to resolving it. Because the size of the rendered image in Html/Pdf is changed, this could be considered a breaking change. To restore the prior behavior, do the following for all charts before saving as Html: ```php $chart->setRenderedWidth(640.0); $chart->setRenderedHeight(480.0); ```
diff --git a/samples/templates/36writeMultiple1.xlsx b/samples/templates/36writeMultiple1.xlsx
diff --git a/src/PhpSpreadsheet/Chart/Chart.php b/src/PhpSpreadsheet/Chart/Chart.php
@@ -132,6 +132,16 @@ class Chart
 
     private ChartColor $fillColor;
 
+    /**
+     * Rendered width in pixels.
+     */
+    private ?float $renderedWidth = null;
+
+    /**
+     * Rendered height in pixels.
+     */
+    private ?float $renderedHeight = null;
+
     /**
      * Create a new Chart.
      * majorGridlines and minorGridlines are deprecated, moved to Axis.
@@ -791,4 +801,28 @@ public function getFillColor(): ChartColor
     {
         return $this->fillColor;
     }
+
+    public function setRenderedWidth(?float $width): self
+    {
+        $this->renderedWidth = $width;
+
+        return $this;
+    }
+
+    public function getRenderedWidth(): ?float
+    {
+        return $this->renderedWidth;
+    }
+
+    public function setRenderedHeight(?float $height): self
+    {
+        $this->renderedHeight = $height;
+
+        return $this;
+    }
+
+    public function getRenderedHeight(): ?float
+    {
+        return $this->renderedHeight;
+    }
 }
diff --git a/src/PhpSpreadsheet/Chart/Renderer/JpGraphRendererBase.php b/src/PhpSpreadsheet/Chart/Renderer/JpGraphRendererBase.php
@@ -26,9 +26,9 @@
  */
 abstract class JpGraphRendererBase implements IRenderer
 {
-    private static $width = 640;
+    private const DEFAULT_WIDTH = 640.0;
 
-    private static $height = 480;
+    private const DEFAULT_HEIGHT = 480.0;
 
     private static $colourSet = [
         'mediumpurple1', 'palegreen3', 'gold1', 'cadetblue1',
@@ -70,6 +70,16 @@ public function __construct(Chart $chart)
         ];
     }
 
+    private function getGraphWidth(): float
+    {
+        return $this->chart->getRenderedWidth() ?? self::DEFAULT_WIDTH;
+    }
+
+    private function getGraphHeight(): float
+    {
+        return $this->chart->getRenderedHeight() ?? self::DEFAULT_HEIGHT;
+    }
+
     /**
      * This method should be overriden in descendants to do real JpGraph library initialization.
      */
@@ -221,7 +231,7 @@ private function renderLegend(): void
 
     private function renderCartesianPlotArea(string $type = 'textlin'): void
     {
-        $this->graph = new Graph(self::$width, self::$height);
+        $this->graph = new Graph($this->getGraphWidth(), $this->getGraphHeight());
         $this->graph->SetScale($type);
 
         $this->renderTitle();
@@ -258,14 +268,14 @@ private function renderCartesianPlotArea(string $type = 'textlin'): void
 
     private function renderPiePlotArea(): void
     {
-        $this->graph = new PieGraph(self::$width, self::$height);
+        $this->graph = new PieGraph($this->getGraphWidth(), $this->getGraphHeight());
 
         $this->renderTitle();
     }
 
     private function renderRadarPlotArea(): void
     {
-        $this->graph = new RadarGraph(self::$width, self::$height);
+        $this->graph = new RadarGraph($this->getGraphWidth(), $this->getGraphHeight());
         $this->graph->SetScale('lin');
 
         $this->renderTitle();
@@ -460,15 +470,14 @@ private function renderPlotScatter(int $groupID, bool $bubble): void
                     $dataValuesY[$k] = $k;
                 }
             }
-            //var_dump($dataValuesY, $dataValuesX, $bubbleSize);
 
             $seriesPlot = new ScatterPlot($dataValuesX, $dataValuesY);
             if ($scatterStyle == 'lineMarker') {
                 $seriesPlot->SetLinkPoints();
                 $seriesPlot->link->SetColor(self::$colourSet[self::$plotColour]);
             } elseif ($scatterStyle == 'smoothMarker') {
                 $spline = new Spline($dataValuesY, $dataValuesX);
-                [$splineDataY, $splineDataX] = $spline->Get(count($dataValuesX) * self::$width / 20);
+                [$splineDataY, $splineDataX] = $spline->Get(count($dataValuesX) * $this->getGraphWidth() / 20);
                 $lplot = new LinePlot($splineDataX, $splineDataY);
                 $lplot->SetColor(self::$colourSet[self::$plotColour]);
 
diff --git a/src/PhpSpreadsheet/Shared/Drawing.php b/src/PhpSpreadsheet/Shared/Drawing.php
@@ -110,7 +110,7 @@ public static function pixelsToPoints($pixelValue): float
     /**
      * Convert points to pixels.
      *
-     * @param int $pointValue Value in points
+     * @param float|int $pointValue Value in points
      *
      * @return int Value in pixels
      */
diff --git a/src/PhpSpreadsheet/Writer/Html.php b/src/PhpSpreadsheet/Writer/Html.php
@@ -31,6 +31,10 @@
 
 class Html extends BaseWriter
 {
+    private const DEFAULT_CELL_WIDTH_POINTS = 42;
+
+    private const DEFAULT_CELL_WIDTH_PIXELS = 56;
+
     /**
      * Spreadsheet object.
      */
@@ -580,9 +584,9 @@ private function extendRowsForCharts(Worksheet $worksheet, int $row): array
                     $chartCol = Coordinate::columnIndexFromString($chartTL[0]);
                     if ($chartTL[1] > $rowMax) {
                         $rowMax = $chartTL[1];
-                        if ($chartCol > Coordinate::columnIndexFromString($colMax)) {
-                            $colMax = $chartTL[0];
-                        }
+                    }
+                    if ($chartCol > Coordinate::columnIndexFromString($colMax)) {
+                        $colMax = $chartTL[0];
                     }
                 }
             }
@@ -601,9 +605,9 @@ private function extendRowsForChartsAndImages(Worksheet $worksheet, int $row): s
             $imageCol = Coordinate::columnIndexFromString($imageTL[0]);
             if ($imageTL[1] > $rowMax) {
                 $rowMax = $imageTL[1];
-                if ($imageCol > Coordinate::columnIndexFromString($colMax)) {
-                    $colMax = $imageTL[0];
-                }
+            }
+            if ($imageCol > Coordinate::columnIndexFromString($colMax)) {
+                $colMax = $imageTL[0];
             }
         }
 
@@ -745,7 +749,15 @@ private function writeChartInCell(Worksheet $worksheet, string $coordinates): st
                 $chartCoordinates = $chart->getTopLeftPosition();
                 if ($chartCoordinates['cell'] == $coordinates) {
                     $chartFileName = File::sysGetTempDir() . '/' . uniqid('', true) . '.png';
-                    if (!$chart->render($chartFileName)) {
+                    $renderedWidth = $chart->getRenderedWidth();
+                    $renderedHeight = $chart->getRenderedHeight();
+                    if ($renderedWidth === null || $renderedHeight === null) {
+                        $this->adjustRendererPositions($chart, $worksheet);
+                    }
+                    $renderSuccessful = $chart->render($chartFileName);
+                    $chart->setRenderedWidth($renderedWidth);
+                    $chart->setRenderedHeight($renderedHeight);
+                    if (!$renderSuccessful) {
                         return '';
                     }
 
@@ -770,6 +782,37 @@ private function writeChartInCell(Worksheet $worksheet, string $coordinates): st
         return $html;
     }
 
+    private function adjustRendererPositions(Chart $chart, Worksheet $sheet): void
+    {
+        $topLeft = $chart->getTopLeftPosition();
+        $bottomRight = $chart->getBottomRightPosition();
+        $tlCell = $topLeft['cell'];
+        $brCell = $bottomRight['cell'];
+        if ($tlCell !== '' && $brCell !== '') {
+            $tlCoordinate = Coordinate::indexesFromString($tlCell);
+            $brCoordinate = Coordinate::indexesFromString($brCell);
+            $totalHeight = 0.0;
+            $totalWidth = 0.0;
+            $defaultRowHeight = $sheet->getDefaultRowDimension()->getRowHeight();
+            $defaultRowHeight = SharedDrawing::pointsToPixels(($defaultRowHeight >= 0) ? $defaultRowHeight : SharedFont::getDefaultRowHeightByFont($this->defaultFont));
+            if ($tlCoordinate[1] <= $brCoordinate[1] && $tlCoordinate[0] <= $brCoordinate[0]) {
+                for ($row = $tlCoordinate[1]; $row <= $brCoordinate[1]; ++$row) {
+                    $height = $sheet->getRowDimension($row)->getRowHeight('pt');
+                    $totalHeight += ($height >= 0) ? $height : $defaultRowHeight;
+                }
+                $rightEdge = $brCoordinate[2];
+                ++$rightEdge;
+                for ($column = $tlCoordinate[2]; $column !== $rightEdge; ++$column) {
+                    $width = $sheet->getColumnDimension($column)->getWidth();
+                    $width = ($width < 0) ? self::DEFAULT_CELL_WIDTH_PIXELS : SharedDrawing::cellDimensionToPixels($sheet->getColumnDimension($column)->getWidth(), $this->defaultFont);
+                    $totalWidth += $width;
+                }
+                $chart->setRenderedWidth($totalWidth);
+                $chart->setRenderedHeight($totalHeight);
+            }
+        }
+    }
+
     /**
      * Generate CSS styles.
      *
@@ -844,8 +887,8 @@ private function buildCssPerSheet(Worksheet $sheet, array &$css): void
         $highestColumnIndex = Coordinate::columnIndexFromString($sheet->getHighestColumn()) - 1;
         $column = -1;
         while ($column++ < $highestColumnIndex) {
-            $this->columnWidths[$sheetIndex][$column] = 42; // approximation
-            $css['table.sheet' . $sheetIndex . ' col.col' . $column]['width'] = '42pt';
+            $this->columnWidths[$sheetIndex][$column] = self::DEFAULT_CELL_WIDTH_POINTS; // approximation
+            $css['table.sheet' . $sheetIndex . ' col.col' . $column]['width'] = self::DEFAULT_CELL_WIDTH_POINTS . 'pt';
         }
 
         // col elements, loop through columnDimensions and set width

Original file line number	Diff line number	Diff line change
`@@ -110,7 +110,7 @@ public static function pixelsToPoints($pixelValue): float`
`110`	`110`	`/**`
`111`	`111`	`* Convert points to pixels.`
`112`	`112`	`*`
`113`		`- * @param int $pointValue Value in points`
	`113`	`+ * @param float\|int $pointValue Value in points`
`114`	`114`	`*`
`115`	`115`	`* @return int Value in pixels`
`116`	`116`	`*/`