dompdf / php-svg-lib

SVG file parsing / rendering library
GNU Lesser General Public License v3.0
1.39k stars 77 forks source link

Added support for percent-based x,y positions #86

Closed ssddanbrown closed 1 year ago

ssddanbrown commented 2 years ago

This PR primarily aims to add percent x/y attribute positions to the core existing tags that support stable positions. This uses the existing Style::convertSize function which supports a range of CSS sizes so the changes here will essentially add some level of support for all sizes within Style::convertSize, but it has primarily been tested using percentages. This should not affect existing numeric-only positions, since they are returned back right away in Style::convertSize.

Testing Performed

Test PHP Script ```php loadFile($inputSvgFile); $surface = new SurfaceCpdf($doc); $doc->render($surface); $output = $surface->out(); file_put_contents($outputPdfFile, $output); ```

Notes

bsweeney commented 2 years ago

Nice, I was going to start putting the convertSize method to greater use when setting length values. This is a good start on that.

bsweeney commented 2 years ago

Finding specifics on some of the arguments for SVG elements can be ... challenging. However, MDN does seem to indicate that percent values are permitted in SVG as x,y are coordinates, which are the same as lengths, and per this definition of length percent is allowed in SVG documents. Also allowed for width/height since those are lengths.

The W3C spec does generally address x/y. And though the width/height discussion doesn't specifically outline how percentage is calculated, it does indicate that for shapes the default for auto is 100% so I think it's safe to use percent there as well.

bsweeney commented 2 years ago

No need to apologize for anything this is a good PR.

I think rather than falling out on error or using the percent-based reference, to just pass in ... null? The default reference size would be used, which is probably OK as for default behavior. 10em with a reference of 11 is much better than 10em with a reference of the height of the document. Probably the most straightforward way to do that would be to determine of the unit type at the point of calling convertSize, only passing in a value if it's a percentage. Though certainly you could try to tackle that by other means.

As for supporting other units ... if you want to add that to this PR please do. But I do think that with just a change to the reference size handling this PR will be sufficiently functional to support most use cases.

ssddanbrown commented 1 year ago

Hi @bsweeney,

I've now pushed some further changes for unit handling. I have kept to level-3 units, since level-4 adds a lot of additional units & complexity while the docs still seem to be a little rough in areas (working draft).

I think this should cover the vast majority of common use-cases. I've added a convertSize method to the abstract tag to be used for tag-specific sizes, so the reference size can be centrally managed while having the required access to the tag-specific detail which might affect that reference.

Where possible, I've attempted to use the most relevant reference unit, or a spec-defined fallback value.

I performed some refactoring of this logic, to extract length handling into its own class. This refactoring is largely contained within the last commit, so it can be easily reverted if you're not fond of those changes. Where possible I've tried to follow existing conventions & styles.

Here's the test SVG I used for this portion: test-units.svg All the bars should roughly be the same size, albeit within a range of rounding error, when viewed within a 150px x 100px viewport.