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

Question About Image Size #17

Closed
oleibman opened this issue Nov 6, 2023 · 8 comments
Closed

Question About Image Size #17

oleibman opened this issue Nov 6, 2023 · 8 comments
Assignees
Labels
nothing to fix The problem is not with package itself

Comments

@oleibman
Copy link

oleibman commented Nov 6, 2023

Question About Image Sizes

This is a question; it is definitely not a bug report. A user sent us a spreadsheet which they tried to convert to Html, but it was missing the charts. There were several problems. The first was user error. The second was a bug in our software which I will fix (only 2 of the 3 charts made it to the Html). The third leads to my question.

According to Excel, the first chart is 3.05" high and 4.71" wide. The second chart is 2.62" high and 4.53" wide. The third is 2.62" high and 4.48" wide. I don't know where these values are stored in the xml. I think they are not part of the Chart object which we pass to the renderer. They may be part of the Drawing object (<xdr:to><xdr:colOff> etc.) which is not passed to the renderer, and which I haven't any idea how to convert to human terms.

When each of these charts is passed to the renderer, it returns a diagram which is 640px wide and 480px high. If one inch is 96 pixels, this is about 6.7" by 5", much larger than the original. This results in the diagrams in the html overlaying each other, so they wind up with pieces cut off. Is the renderer getting those values from something in the chart xml, or is it a default value? If it is a default, and I somehow figure out what the width and height ought to be, can I somehow specify those values to the renderer?
issue.3767m.3.zip

@f1mishutka
Copy link
Contributor

Hi,

Our investigation shows that this values are hard-coded in \PhpOffice\PhpSpreadsheet\Chart\Renderer\JpGraphRendererBase . And bad thing is that they hard-coded as static values. Then they used to create graph objects.

So there is some serious refactoring of JpGraphRendererBase is required: there should be some mechanism in PhpSpreadsheet for reading and determining chart size and passing them to JpGraphRendererBase constructor (and making $width and $height non-static).

I can prepare Pull Request for that. But I can not understand where can I get those chart size (in inches or whatever) if I have \PhpOffice\PhpSpreadsheet\Chart\Chart reference.

Any clues?

@f1mishutka
Copy link
Contributor

It looks like there are width and height in chart's getPlotArea()->getLayout()

So I've created Pull Request #3785 to use them instead of hard-coded values. Could you please try it?

@oleibman
Copy link
Author

oleibman commented Nov 8, 2023

I think you are on the right track. Unfortunately, for the spreadsheet in question, all 3 charts have the following in the xml:

<c:plotArea>
<c:layout/>

So there's no height or width available that way. I'll keep looking to see if I can find those elsewhere, and, of course, continue to welcome suggestions as to where to look.

@oleibman
Copy link
Author

oleibman commented Nov 8, 2023

Hmm ... when the Reader reads the chart and drawing xml, it sets Chart TopLeftPosition and BottomRight Position (both of these are cells e.g. A1), and xoffset and yoffset (integers which may prove to be relevant later but I'm not looking at them yet). It might be possible for me to calculate the width and height using those positions. Let me see what I can come up with.

@oleibman
Copy link
Author

oleibman commented Nov 9, 2023

My tests are proceeding nicely so far. I can add new properties for renderedWidth and renderedHeight to Chart, and Writer/Html can fill them in before calling the renderer. I can then adapt your idea by using those properties instead of the Layout properties when JpGraphRenderBase needs width and height. Here's the result.

issue.3767m.3.revised.zip

@oleibman
Copy link
Author

oleibman commented Nov 9, 2023

The user who reported the issue had some other comments. He wondered why the color palette was different. I imagine that might have something to do with $colourSet in JpGraphRendererBase. I'm pretty sure I don't want to touch that, but, just in case you have a clever suggestion for it ...

User also noted that the pie charts were rotated differently in Excel than in JpGraph. Again, I don't want to touch, but ...

@f1mishutka
Copy link
Contributor

Lets make it work as expected with chart size and will take a look at colors and pie rotation.
It is already complicated enough. It would be better to not mix this in one cocktail I suppose.

@oleibman
Copy link
Author

I have pushed PHPOffice/PhpSpreadsheet#3787. I will do some more testing before merging. I agree it is complicated enough as is.

@f1mishutka f1mishutka closed this as not planned Won't fix, can't repro, duplicate, stale Nov 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
nothing to fix The problem is not with package itself
Projects
None yet
Development

No branches or pull requests

2 participants