mitoteam / jpgraph

JpGraph library composer package with PHP 8.4 support
https://packagist.org/packages/mitoteam/jpgraph
Other
36 stars 5 forks source link

Question About Image Size #17

Closed oleibman closed 1 year ago

oleibman commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

I have pushed https://github.com/PHPOffice/PhpSpreadsheet/pull/3787. I will do some more testing before merging. I agree it is complicated enough as is.