Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Chart styling is lost on simple load / save process #1797

Closed
lawless-m opened this issue Jan 15, 2021 · 3 comments · Fixed by #2930
Closed

Chart styling is lost on simple load / save process #1797

lawless-m opened this issue Jan 15, 2021 · 3 comments · Fixed by #2930

Comments

@lawless-m
Copy link

This is:

- [ x] a bug report
- [ ] a feature request
- [ ] **not** a usage question (ask them on https://stackoverflow.com/questions/tagged/phpspreadsheet or https://gitter.im/PHPOffice/PhpSpreadsheet)

What is the expected behavior?

Output styling should be the same format as the input

What is the current behavior?

Styling is not preserved

What are the steps to reproduce?

ChartInput.xlsx

ChartOutput.xlsx

<?php

require __DIR__ . '/vendor/autoload.php';

$reader = IOFactory::createReader('Xlsx');
$reader->setIncludeCharts(true);
$spreadsheet = $reader->load('ChartInput.xlsx');

$writer = IOFactory::createWriter($spreadsheet, 'Xlsx');    

$writer->setIncludeCharts(true);
$writer->save('ChartOutput.xlsx');

?>

### Which versions of PhpSpreadsheet and PHP are affected?

## 1.16.0 - 2020-12-31

 php --version
PHP 7.4.3 (cli) (built: Oct  6 2020 15:47:56) ( NTS )
@pierre-H
Copy link

pierre-H commented Dec 7, 2021

Same problem 😢

oleibman added a commit to oleibman/PhpSpreadsheet that referenced this issue Jul 11, 2022
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 PHPOffice#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 PHPOffice#2077, by allowing the format to be specified in the Layout rather than the DataSeriesValues constructor.
oleibman added a commit that referenced this issue Jul 14, 2022
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.
MarkBaker added a commit that referenced this issue Jul 18, 2022
### Added

- Add Chart Axis Option textRotation [Issue #2705](#2705) [PR #2940](#2940)

### Changed

- Nothing

### Deprecated

- Nothing

### Removed

- Nothing

### Fixed

- Fix Encoding issue with Html reader (PHP 8.2 deprecation for mb_convert_encoding) [Issue #2942](#2942) [PR #2943](#2943)
- Additional Chart fixes
  - Pie chart with part separated unwantedly [Issue #2506](#2506) [PR #2928](#2928)
  - Chart styling is lost on simple load / save process [Issue #1797](#1797) [Issue #2077](#2077) [PR #2930](#2930)
  - Can't create contour chart (surface 2d) [Issue #2931](#2931) [PR #2933](#2933)
- VLOOKUP Breaks When Array Contains Null Cells [Issue #2934](#2934) [PR #2939](#2939)
@kozhevnikovpe
Copy link

not fixed
try to run code from 1st message
saved file looks different

@oleibman
Copy link
Collaborator

@kozhevnikovpe It makes things a lot easier if you describe the differences you see rather than just "saved file looks different". It also helps if you open new issues (possibly cross-referencing the closed issue in your description) rather than keep adding to an issue that was closed 8 months ago.

The sample file 32readwriteLineChart5 was added for the PR which addressed this issue. They are very close, but I agree that there are still some discrepancies.

  • The input chart has a border.
  • In the input chart, there is no vertical line next to the vertical axis.
  • The font sizes on the axis labels and the data points differ.

Can you please open separate issues for each of these, and any other discrepancies that you see. I think I am pretty close to a solution for the first two. The third may require a lot more effort.

oleibman added a commit to oleibman/PhpSpreadsheet that referenced this issue Mar 17, 2023
All chart linestyles will be supported for Chart Border. Also add fill color for Chart. Also 'nofill' for Axis (allows suppressing of vertical axis line). These are demonstrated in sample 32_readwriteChartLine5, and a new unit test member is added. This addresses some vague problems added to issue PHPOffice#1797 over 8 months after it was closed; there is still at least one problem, much more complicated than the 2 being addressed in this PR.
@oleibman oleibman mentioned this issue Mar 17, 2023
11 tasks
oleibman added a commit that referenced this issue Mar 20, 2023
* Support Border for Charts

All chart linestyles will be supported for Chart Border. Also add fill color for Chart. Also 'nofill' for Axis (allows suppressing of vertical axis line). These are demonstrated in sample 32_readwriteChartLine5, and a new unit test member is added. This addresses some vague problems added to issue #1797 over 8 months after it was closed; there is still at least one problem, much more complicated than the 2 being addressed in this PR.

* Handle Legend Borders in Same Way as Chart Borders

Consistency is good, and the implementation of Chart Borders offers more possibilities than the implementation of Legend Borders had. Since Legend Borders hasn't made it to a release yet, there is no need for deprecations.
oleibman added a commit to oleibman/PhpSpreadsheet that referenced this issue Mar 22, 2023
Addresses some remaining issues with 32readwriteLineChart5 (see issue PHPOffice#1797). Font size is covered. So are effects, although the results are a bit odd. For the new spreadsheet 32readwriteLineChart6, the Axis labels have a yellow-ish glow, but reading and writing the spreadsheet in PhpSpreadsheet gives them a purple-ish glow. Nevertheless, the new test shows that the output file uses schemeClr accent4, as does the input file. So the effect is handled correctly, but it seems there is likely to be a difference between theme colors (Writer/Xlsx/Theme appears to write hard-coded color schemes, and, in any case, Reader/Xlsx does not appear to handle schemeClr). Fixing that will be a great deal more difficult, with a large chance of regression, and will need to happen in a separate PR (one that I am not currently investigating, but I will open a new issue). Effects using srgbClr (and probably sysclr) should be okay.
oleibman added a commit that referenced this issue Mar 26, 2023
* Font and Effects Support for Chart Data Labels and Axis

Addresses some remaining issues with 32readwriteLineChart5 (see issue #1797). Font size is covered. So are effects, although the results are a bit odd. For the new spreadsheet 32readwriteLineChart6, the Axis labels have a yellow-ish glow, but reading and writing the spreadsheet in PhpSpreadsheet gives them a purple-ish glow. Nevertheless, the new test shows that the output file uses schemeClr accent4, as does the input file. So the effect is handled correctly, but it seems there is likely to be a difference between theme colors (Writer/Xlsx/Theme appears to write hard-coded color schemes, and, in any case, Reader/Xlsx does not appear to handle schemeClr). Fixing that will be a great deal more difficult, with a large chance of regression, and will need to happen in a separate PR (one that I am not currently investigating, but I will open a new issue). Effects using srgbClr (and probably sysclr) should be okay.

* Better Theme Support

When reading Xlsx, the theme colors will now also be used for writing. This means that a file can be loaded and saved and its chart colors will now be preserved. If the spreadsheet is created new, Excel 2007-2010 colors are used. The writer is currently hard-coded to use them, so this avoids making this a breaking change. The theme colors can be explicitly changed if desired, and Excel 2013+ colors can be introduced very easily.
```php
$spreadsheet->getTheme()
    ->setThemeColorName(Theme::COLOR_SCHEME_2013_PLUS_NAME);
```
Likewise, if the old behavior of changing to the 2007-2010 scheme rather than using the input values is desired, that is easy to achieve after the load has taken place.
```php
$spreadsheet->getTheme()
    ->setThemeColorName(Theme::COLOR_SCHEME_2007_2010_NAME);
```
The new Theme class introduced by this change can easily be extended to include Fonts and Effects. Unlike Colors, I am unsure what the practical effects of changing those to, say, the 2013+ defaults would be.

* Scrutinizer

Use an alias in a use statement.

* Update Change Log

Due to potential behavior change.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

4 participants