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

Generation hangs with long titles #25

Closed
cwhite92 opened this issue Jan 19, 2024 · 5 comments · Fixed by #31
Closed

Generation hangs with long titles #25

cwhite92 opened this issue Jan 19, 2024 · 5 comments · Fixed by #31

Comments

@cwhite92
Copy link

Thank you for the package! I'm using it on my Jigsaw static site to generate OG images for blog posts, and seeing an issue with certain post titles.

The following snippet hangs indefinitely on my machine and pegs a PHP process at 100% CPU usage:

return (new Image())
    ->accentColor('#cc0000')
    ->border()
    ->url($item->getUrl())
    ->title('Adding Unique Field to MySQL Table With Existing Records')
    ->description('Test description')
    ->background(Background::JustWaves, 0.2)
    ->toString();

Shortening the title to:

->title('Adding Unique Field to MySQL')

generates the image almost instantly.

Interestingly, it doesn't seem to just be an issue with title length, because this is successful:

->title('foo bar foo bar foo bar foo bar foo bar foo bar foo bar foo bar foo bar foo bar foo bar foo bar')

However, removing spaces causes the hanging issue:

->title('foobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobar')

I source dived a bit and think I tracked the problem down to the while() loop in getFinalTextBox() in the TextBox class. It seems to have an issue fitting this specific text in the text box and loops indefinitely.

I'm happy to prepare a PR to fix this, but the code around here is pretty complicated. If you could point me in the right directly perhaps I can understand it better and propose the correct fix?

Thanks!

@simonhamp
Copy link
Owner

Thanks for sharing your code. This will be a good test case!

I'm certain it is that while loop that's to blame. It looks like you've found an edge case where the text cannot be trimmed further but still fails to fit the box.

I suspect it's to do with word length not fitting the box horizontally for the given font and font size.

The problem is that I've chosen not to split words (tho I think wordwrap can) because I believe that makes for awkward presentation, at least in an OG image context.

The solution I have in mind is to steadily make the font size smaller until the long word fits the width of the containing box.

That's probably not a perfect solution either to be honest, but should be workable in most scenarios.

@RemiHin
Copy link

RemiHin commented May 29, 2024

@simonhamp any updates on this? Currently running in to the same issue

@askonomm
Copy link

askonomm commented Jul 9, 2024

I ran into the same issue, and started poking around in the code. To me it looks like the 1.8 heuristic used to divide the character width of M polygon (if I understand this correctly) is perhaps too high:

$text = wordwrap($text, intval(floor($this->box->width() / ($modifier->boxSize('M')->width() / 1.8))));

I was successfully able to render all types of long text with a value lower than that, like say 1.5. I'm not sure why the 1.8 was picked, but I figure it was to make up for whitespace between characters because otherwise the text is too condensed and do not fill up the whole horizontal axis.

So then I started thinking that if we're doing that to accommodate for the space, why don't we just take the space char width and reduce it from the character (M) width? Something like this:

$spaceCharWidth = $modifier->boxSize(' ')->width();
$baseCharWidth = $modifier->boxSize('M')->width();
$charWidth = $baseCharWidth - $spaceCharWidth;
$length = intval(floor($this->box->width() / $charWidth));
$text = wordwrap($text, $length);

That way we don't have a hardcoded heuristic, but one that (hopefully) changes based on font used. Now granted, I have zero experience with image rendering of any kind, so this is just my $0.02 and what seemed to have fixed the issue for me at least.

And then for languages that use a many words in one long word (Estonian, German, many others) such that a single word can never fit in a single line, it would be nice if the cut_long_words could be changed on the user-space level, because it alone would solve that problem, while you can still keep the aesthetic the way you want in English. A hyphenation algo could solve these issues though, but that's very complex to do especially for many languages, so perhaps another option would be to run passes. If the first iteration did not pass, go through words in $text, find the longest word, break it in half from the middle with a hyphen - and a newline \n, and keep doing this until all long words are no longer long. Poor mans hyphenation so to speak.

Looking forward to thoughts on this.

@simonhamp
Copy link
Owner

@askonomm thanks for doing some investigation and writing up your thoughts. I agree, your approach could work... but at best it will just make the sizing of the widest character more accurate.

The 1.8 can be thought of as an approximation of the "average character width" in an "average word" (although statistically it's probably nowhere near average - and there is no real average word).

Your suggestion doesn't move away from this, it only refines it ever so slightly by giving it a known character width to work with. But that will only work well for fonts where the space character is somehow proportional to other characters in the font... if it's not, then this could go wildly wrong.

I have already come to the conclusion that the solution I've already proposed makes the most sense:

I've chosen not to split words (tho I think wordwrap can) because I believe that makes for awkward presentation, at least in an OG image context.

The solution I have in mind is to steadily make the font size smaller until the long word fits the width of the containing box.

This wouldn't require any kind of hyphenation, would work reasonably well for most long words regardless of font/language and would most likely completely remove the need for arbitrary character dimension calculations.

I also don't think it would be that difficult to implement, I just haven't had time to pick this up again.

@askonomm
Copy link

askonomm commented Jul 10, 2024

@simonhamp Gotcha! I tried quickly to do something like that as well, to basically in each iteration of the while loop:

$modifier->font->setSize($modifier->font->size() - 1);

But I could not get that to work. In any case my previous solution seemed to have fixed it for me at least, so if someone else is stuck with this meanwhile can give that a go maybe as a temporary work-around. But yeah, will most likely break with fonts where the space char is not proportional (e.g monospace fonts).

If you can give me a hint to the font size strategy (as in where or what did I do wrong) I can try to fix that by doing so and open a PR myself as well.

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 a pull request may close this issue.

4 participants