Two Problems with Html Chart Rendering - Minor Break (PHPOffice#3787)

* Two Problems with Html Chart Rendering - Minor Break 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); ``` * Update CHANGELOG.md
oleibman · Nov 13, 2023 · f9eb35d · f9eb35d
1 parent 5b27082
commit f9eb35d
Show file tree

Hide file tree

Showing 6 changed files with 108 additions and 18 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -30,6 +30,8 @@ and this project adheres to [Semantic Versioning](https://semver.org).
 - Drop support for PHP 7.4, according to https://phpspreadsheet.readthedocs.io/en/latest/#php-version-support [PR #3713](https://github.com/PHPOffice/PhpSpreadsheet/pull/3713)
 - RLM Added to NumberFormatter Currency. This happens depending on release of ICU which Php is using (it does not yet happen with any official release). PhpSpreadsheet will continue to use the value returned by Php, but a method is added to keep the result unchanged from release to release. [Issue #3571](https://github.com/PHPOffice/PhpSpreadsheet/issues/3571) [PR #3640](https://github.com/PHPOffice/PhpSpreadsheet/pull/3640)
 - `toFormattedString` will now always return a string. This was introduced with 1.28.0, but was not properly documented at the time. This can affect the results of `toArray`, `namedRangeToArray`, and `rangeToArray`. [PR #3304](https://github.com/PHPOffice/PhpSpreadsheet/pull/3304)
+- Value of constants FORMAT_CURRENCY_EUR and FORMAT_CURRENCY_USD was changed in 1.28.0, but was not properly documented at the time. [Issue #3577](https://github.com/PHPOffice/PhpSpreadsheet/issues/3577)
+- Html Writer will attempt to use Chart coordinates to determine image size. [Issue #3783](https://github.com/PHPOffice/PhpSpreadsheet/issues/3783) [PR #3787](https://github.com/PHPOffice/PhpSpreadsheet/pull/3787)
 
 ### Deprecated
 
@@ -69,6 +71,7 @@ and this project adheres to [Semantic Versioning](https://semver.org).
 - Load Tables even with READ_DATA_ONLY. [PR #3726](https://github.com/PHPOffice/PhpSpreadsheet/pull/3726)
 - Theme File Missing but Referenced in Spreadsheet. [Issue #3770](https://github.com/PHPOffice/PhpSpreadsheet/issues/3770) [PR #3772](https://github.com/PHPOffice/PhpSpreadsheet/pull/3772)
 - Slk Shared Formulas. [Issue #2267](https://github.com/PHPOffice/PhpSpreadsheet/issues/2267) [PR #3776](https://github.com/PHPOffice/PhpSpreadsheet/pull/3776)
+- Html omitting some charts. [Issue #3767](https://github.com/PHPOffice/PhpSpreadsheet/issues/3767) [PR #3771](https://github.com/PHPOffice/PhpSpreadsheet/pull/3771)
 
 ## 1.29.0 - 2023-06-15
 
@@ -148,10 +151,11 @@ and this project adheres to [Semantic Versioning](https://semver.org).
 - Improved support for locale settings and currency codes when matching formatted strings to numerics in the Calculation Engine [PR #3373](https://github.com/PHPOffice/PhpSpreadsheet/pull/3373) and [PR #3374](https://github.com/PHPOffice/PhpSpreadsheet/pull/3374) 
 - Improved support for locale settings and matching in the Advanced Value Binder [PR #3376](https://github.com/PHPOffice/PhpSpreadsheet/pull/3376)
 - `toFormattedString` will now always return a string. This can affect the results of `toArray`, `namedRangeToArray`, and `rangeToArray`. [PR #3304](https://github.com/PHPOffice/PhpSpreadsheet/pull/3304)
+- Value of constants FORMAT_CURRENCY_EUR and FORMAT_CURRENCY_USD is changed. [Issue #3577](https://github.com/PHPOffice/PhpSpreadsheet/issues/3577) [PR #3377](https://github.com/PHPOffice/PhpSpreadsheet/pull/3377)
 
 ### Deprecated
 
-- Rationalisation of Pre-defined Currency Format Masks
+- Rationalisation of Pre-defined Currency Format Masks [PR #3377](https://github.com/PHPOffice/PhpSpreadsheet/pull/3377)
 
 ### Removed
 

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