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

SVG text elements do not support textLength and lengthAdjust attributes #1921

Closed
oshmoun opened this issue Jul 24, 2023 · 9 comments
Closed
Labels
feature New feature that should be supported
Milestone

Comments

@oshmoun
Copy link
Contributor

oshmoun commented Jul 24, 2023

According to the specs, textLength should be a valid attribute for SVG text elements. However, it seems Weasyprint is ignoring it. The same applies for lengthAdjust.

Here's an example SVG:

<svg width="240" height="40" viewBox="0 0 240 40" xmlns="http://www.w3.org/2000/svg">
<text fill="black"><tspan x="10" y="20" textLength="20" lengthAdjust="spacingAndGlyphs">short</tspan> <tspan textLength="200" lengthAdjust="spacingAndGlyphs">long</tspan></text>
</svg>

Expected:
image

Result:
image

@liZe liZe added the feature New feature that should be supported label Jul 24, 2023
@liZe
Copy link
Member

liZe commented Jul 24, 2023

That would be a nice feature to have!

@oshmoun
Copy link
Contributor Author

oshmoun commented Jul 24, 2023

That would be a nice feature to have!

Oh definitely 😄
So if I'm understanding correctly, Weasyprint is currently lacking support of this. Would you think it's doable with the current state of Weasyprint or would it require too many things to change? I'll dig into the code and hopefully figure that out 😁

@liZe
Copy link
Member

liZe commented Jul 24, 2023

So if I'm understanding correctly, Weasyprint is currently lacking support of this.

That’s right.

Would you think it's doable with the current state of Weasyprint or would it require too many things to change?

It’s doable, but probably a bit complicated.

A "simple" proof of concept would be to use the width value returned here and change the letter_spacing according to the textLength value and the number of glyphs in the text string.

Don’t be afraid to try, we’re available to help if needed!

(Of course, that’s just a first step. We then would have to handle other "details" such as nested tspan tags 😱 and lengthAdjust! Let’s keep these for later…)

@oshmoun
Copy link
Contributor Author

oshmoun commented Jul 24, 2023

A "simple" proof of concept would be to use the width value returned here and change the letter_spacing according to the textLength value and the number of glyphs in the text string.

Oh that sounds quite straightforward, so basically calculate the difference between width and textLength and space the letters out so they're evenly distributed. That would implement textLength and lengthAdjust=spacing simultaneously, only spacingAndGlyphs would remain as it's probably a complex matter, as you suggest.
I'm new to this rendering stuff, guess this is a good start 😁

@oshmoun
Copy link
Contributor Author

oshmoun commented Jul 24, 2023

Implemented the PoC, here's the result for the SVG in the description:
image

while it works quite nicely, the spacing strategy has the unintended consequence of introducing whitespace within the words, making them unsearchable. Not sure how that could be avoided 😓

@oshmoun
Copy link
Contributor Author

oshmoun commented Jul 25, 2023

Ok I ended up also implementing spacingAndGlyphs, which avoids the whitespace issue observed in the previous comment 🥳

works pretty well!
image

@liZe
Copy link
Member

liZe commented Jul 25, 2023

That’s great, thanks a lot! 😍💜

I’ll clean a few things and get back to you soon.

If you have some motivation left, you can add some tests in test_text.py. Other tests in this file should be enough for you to understand how they work 😄. You’ll find extra tips about how to launch tests in the doc if needed! (And of course, I can add the tests myself if you don’t want to, no problem at all.)

@oshmoun
Copy link
Contributor Author

oshmoun commented Jul 25, 2023

If you have some motivation left, you can add some tests in test_text.py. Other tests in this file should be enough for you to understand how they work 😄. You’ll find extra tips about how to launch tests in the doc if needed! (And of course, I can add the tests myself if you don’t want to, no problem at all.)

I'll look into the tests today, and push them to the PR when ready 😃

@liZe
Copy link
Member

liZe commented Aug 19, 2023

Fixed by #1922.

@liZe liZe closed this as completed Aug 19, 2023
@liZe liZe added this to the 60.0 milestone Aug 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature that should be supported
Projects
None yet
Development

No branches or pull requests

2 participants