Pomax / lib-font

This library adds a new Font() object to the JavaScript toolbox, similar to new Image() for images
MIT License
734 stars 72 forks source link

Timing out due to empty character #28

Closed Wildhoney closed 4 years ago

Wildhoney commented 9 years ago

It seems that the printChar it uses for testing the font is an empty string ("").

On line 425:

printChar = String.fromCharCode(endChar);

Where endChar is 129 for the following Optima TTF font.

I assume the fix in this instance is to detect if the yielded printChar is empty, and if so, to revert to the printChar default of A.

Pomax commented 9 years ago

There is already the check to see if the endChar is a printable characters, see https://github.com/Pomax/Font.js/blob/master/Font.js#L412, and from the looks of https://github.com/Pomax/Font.js/blob/master/Font.js#L402 it seems that list is simply missing 0x81 and 0x82 (which are control codes, not actually printable characters) so extending that list with the two codes that are missing should help towards fixing the problem you're experiencing.

Pomax commented 9 years ago

https://github.com/Pomax/Font.js/commit/3c8658627c0709e297cc32702fdf01db04a41307 should have fixed this, can you verify this is no longer a problem?

Wildhoney commented 9 years ago

Nope – that didn't fix it. It's now using an endChar of 144.

Could you check the font please to ensure it's not doing something odd?

Wildhoney commented 9 years ago

Works when you add 0x0090 to the list:

return [0x0009,0x000A,0x000B,0x000C,0x000D,0x0020,0x0081,0x0082,
                  0x0085,0x0090,0x00A0,0x1680,0x180E,0x2000,0x2001,0x2002,
                  0x2003,0x2004,0x2005,0x2006,0x2007,0x2008,0x2009,0x200A,
                  0x2028,0x2029,0x202F,0x205F,0x3000].indexOf(chr) === -1; }

Although I'm not sure if that's recommended. With 0x0090 added, the endChar is 255 and everything proceeds just fine.

Pomax commented 9 years ago

it looks like there's a fair number of control codes missing. https://github.com/Pomax/Font.js/commit/710ea5c7287c41c9cc994c813ecf782110d175f2 adds all reasonable control codes, so with that, things should be fine.

Wildhoney commented 9 years ago

:+1: Could you add a bower.json when you release next, please?

Pomax commented 9 years ago

I'm not a fan of bower, but I can throw it up on npm quite easily (and if you have bower, you have npm already)

Wildhoney commented 9 years ago

Don't worry. I'll keep installing it via Bower without the bower.json file.

Vadimus commented 9 years ago

Got similar issue, chr = 29. Just added condition chr > 32 to printable

Pomax commented 9 years ago

Except that is a plain wrong condition, as just being "abover the space" in no way guarantees a character is printable. Have you tried the latest Font.js from this repo?

Pomax commented 4 years ago

obsolete with the 2019 rewrite