Skip to content

Fix SVG curve parameter extraction once more#864

Open
ferdiusa wants to merge 1 commit intotecnickcom:mainfrom
ferdiusa:patch-1
Open

Fix SVG curve parameter extraction once more#864
ferdiusa wants to merge 1 commit intotecnickcom:mainfrom
ferdiusa:patch-1

Conversation

@ferdiusa
Copy link
Copy Markdown

The switch from preg_match_all to preg_split in PR #855 broke parsing of "optimized" svg paths. In these, the "-" can be used as delimiter between numbers as well.

See Issue #767 for report by @tmhf.

The regular expression suggested in this PR reverts back to token based behaviour while keeping the fix for "e" exponents from the original issue.

The switch from `preg_match_all` to `preg_split` in PR tecnickcom#855 broke parsing of "optimized" svg paths. In these, the "-" can be used as delimiter between numbers as well.
@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Apr 14, 2026

CLA assistant check
All committers have signed the CLA.

Copy link
Copy Markdown
Contributor

@williamdes williamdes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you

@nicolaasuni
Copy link
Copy Markdown
Member

Thank you very much, @ferdiusa, for the SVG curve parameter fix - the negative-number-as-delimiter edge case in preg_split on SVG path data is a subtle and tricky bug, and it is genuinely appreciated that you tracked it down once more.

TCPDF 6 is now deprecated and no further changes will be merged into this repository. Please see the pinned issue Thank You - and the Future of TCPDF for the full explanation and the path forward.

SVG rendering has been redesigned in tc-lib-pdf. If you are able to reproduce the path parsing issue there, feel free to open a PR or issue. If you need any help adapting your fix to the new library, please open a discussion and I will do my best to assist.

I am leaving this PR open in case you or anyone else wishes to continue the discussion, but it is unlikely that this PR will ever be merged.

Thank you again for your patience and understanding.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants