photopea / Typr.js

Typr.js - process fonts in Javascript
MIT License
914 stars 73 forks source link

Erroneous rendering #14

Closed adriaanmeuris closed 6 years ago

adriaanmeuris commented 6 years ago

We're using this library to render fonts on Canvas - works great and really fast.

In some scenario's, font rendering is off. We can't pinpoint exactly when this happens, but seems to happen when text is changed or when the size is changed (when this happens, we immediately recalculate the entire string using the glyphToPath utility for every character) - our guess is this might be a too heavy calculation since we've only been successful reproducing this in mobile browsers like iOS Safari.

normal correct rendering

strange erroneous rendering

In this case we're working with Arial, but we've also managed to see this with other fonts (e.g. Google Fonts' Chewy)

Do you have any idea, or directions where we can start looking?

photopea commented 6 years ago

Hi Adrian,

are you sure you are using the latest version of Typr.js ? I would like to reproduce your bug. Could you please load your font into this demo: https://photopea.github.io/Typr.js/ , then type your text at the bottom and see if it is rendered correctly. If not, please, attach your font here, so I can look into it.

adriaanmeuris commented 6 years ago

No, let me give it a try with the latest version and get back to you

photopea commented 6 years ago

Any news?

adriaanmeuris commented 6 years ago

we're currently in progress to use the library with a module loader, but we need to contain it in an UMD-style loader. Next, we'll test again on mobile. I'm pretty sure the error was due to an outdated version. I'll get back to you asap.

Any idea if the lib will become available on NPM so it can be easily installed and maintained? I'd be happy to contribute if you don't have the time!

photopea commented 6 years ago

Sure. Could you put it onto NPM, while keeping the GitHub version as it is? And can you update it when I update the version here? Only Typr.js and Typr.U.js are useful files.

adriaanmeuris commented 6 years ago

I'd be happy to, but for the NPM package to be linked to this repo (and not a fork of it), I need access to the repository (else npm publish fails due to You do not have permission to publish "typr".)

Can you add me as a collaborator?

photopea commented 6 years ago

Ok I added you. Could you please let me review any change you make to the content, before you push it?

There is some redundant code (Typr.js is a concatenation of all files in "tabs"). Could you please keep the code usable in a browser as is, without requiring any additional "compilation"?

adriaanmeuris commented 6 years ago

Thanks! Yes, I'll push any changes to a separate branch and confirm this issue as resolved asap. Can I contact you directly at support@photopea.com for any feedback on progress?

photopea commented 6 years ago

Sure, or we can discuss it here.

adriaanmeuris commented 6 years ago

I've just pushed all changes to the branch feature/npm. Can you take a look?

As requested, there's still a static build for browser usage, in the build directory. It contains 1 javascript file which makes Typr and Typr.U. I've updated the demo page + readme file which contains information on how to build the file if you're using npm.

If you're happy with the changes, we can merge them in master and I'll set up the npm-package. Looking forward to your feedback!

photopea commented 6 years ago

I am not familiar with npm and other tools that you use. I am afraid, that if we merge it, I will not be able to work on this project anymore, since it would seem too messy for me.

adriaanmeuris commented 6 years ago

you only once need to: install npm + install the dependencies using npm install.

Next, you simply build using npm run build instead of the bash script.

advantages: -code is automatically concatenated (like the bash script) and minified (results in an even smaller file than now: 35 kB vs. 60 kb)

If you're not open to this I'm also happy to create a fork. But since your feedback will be of great help understanding the code and suggest improvements, we'd prefer to maintain the code in one repository - hence we can include & update in in different projects.

what do you think?

photopea commented 6 years ago

BTW. some guy put UPNG.js to npm just by adding package.json, see https://github.com/photopea/UPNG.js . Could we do it this way, too? Also, I can add a few lines into the code (checking the "require" function).

adriaanmeuris commented 6 years ago

Absolutely, please do so I can publish it. You’ll then only need to up to version number when publishing again after doing changes, but your build process will stay the same. Please ping me when you’ve added the code. Thanks!

photopea commented 6 years ago

Could you add the package.json and two extra lines, since you understand npm?

adriaanmeuris commented 6 years ago

done, I've created a pull request with the changes. Little bit more complicated than UPNG, since you're merging multiple files whereas UPNG is a single source file.