reymond-group / smilesDrawer

A small, highly performant JavaScript component for parsing and drawing SMILES strings. Released under the MIT license.
https://smilesdrawer.rocks
MIT License
434 stars 68 forks source link

Canvas from SVG #133

Closed daenuprobst closed 2 years ago

daenuprobst commented 2 years ago
swamidass commented 2 years ago

With changes this big, should you be at least incrementing the minor version and pushing to the package registries? Without doing so, we don't get the updates.

daenuprobst commented 2 years ago

Will happen soon.👍🏻

swamidass commented 2 years ago

Unfortunately, it seems like that change to the text parsing created some problems with misplaced labels.

daenuprobst commented 2 years ago

@swamidass Yeah, I noticed it as well. Depending on a combination of font size, browser and DPI ("retina" et al). I'm currently invastigating... Edit: Regarding the npm package, I will deploy this as a minor update in preparation of reaction drawing as soon as the current bugs are ironed out

swamidass commented 2 years ago

Rolling back to the prior CSS could work, but the key thing is to get rid of the RTL, which isn't too difficult to do by (1) explicitely reordering text characters, and (2) appropriately changing the text start css.

If at all possible, avoid calling bbox? That doesn't work for server side rendering...

daenuprobst commented 2 years ago

Yea... the RTL was bugged in firefox for the up direction. I also wasn't too convinced by the placement with CSS text properties. What could be a solution is to use paths instead of text (I'll check out whether there are any good solutions) then the text placement could be done pixel-perfect without the corrections in the previous (CSS) version. The server-side rendering issue could be solved with PhantomJS, although this dependency might be a bit overkill...

swamidass commented 2 years ago

PhantomJS is no longer active. Both PhantomJS and puppeteer are really slow. They are not viable solutions for SSR.

What about converting a font into SVG glyphs (e.g. https://cloudconvert.com/ttf-to-svg), and then committing the SVG glyphs to the repo. This might sound crazy, but I think it will produce the most reliable results. It would not require using bbox, so it wouldn't break server side rendering.

swamidass commented 2 years ago

This might be useful too:

https://github.com/danmarshall/google-font-to-svg-path

swamidass commented 2 years ago

Actually, this library might make the getting the font metrics fairly straightforward:

https://github.com/foliojs/fontkit

One approach might be to use bbox in the the browser, but this library on the server.

daenuprobst commented 2 years ago

fontkit is a bit heavy, I was looking into this last week, there's also opentype.js and a smaller one (I think typr.js). The issue there is, that they generally need the font file, so this would mean that the font has to be loaded from a server (no access browser-side of course). So the user would be forced to use a (self-)hosted webfont. I might be wrong about this, but that's what I'm gathered from looking into these libs.

However, I found another way that, as far as I could see (Chrome, Edge, Firefox) works pretty well. I use canvas (non-rendered) to get the width of the font (and some other hackery). Not the cleanest solution but if it works... See PR #135 and let me know whether it fixes things for you

daenuprobst commented 2 years ago

PhantomJS is no longer active. Both PhantomJS and puppeteer are really slow. They are not viable solutions for SSR.

What about converting a font into SVG glyphs (e.g. https://cloudconvert.com/ttf-to-svg), and then committing the SVG glyphs to the repo. This might sound crazy, but I think it will produce the most reliable results. It would not require using bbox, so it wouldn't break server side rendering.

I actually spend some time doing that but then had the idea with the in-memory canvas and just using relative em-positioning for vertical offsets (https://github.com/reymond-group/smilesDrawer/blob/4f407865592e9bd3ae03eb96c175579322bdba16/src/SvgWrapper.js#L628).

swamidass commented 2 years ago

If that works, great! If we do revisit this, using fontkit, I think, will be the most robust and platform independent approach. (Or maybe SVG.js???)

Also, I did some searching for the right SVG library. Here are a few options:

https://github.com/svgdotjs/svgdom https://svgjs.dev/docs/3.0/ https://github.com/adobe-webplatform/Snap.svg/

My suggestion is to replace the current SVG Wrapper with calls to SVG.js. That should, transparently, work very fast both on the server and on the browser. It seems that SVG.js pollyfills with svg-dom on node, but on the browser it uses native elements. If that's correct, that seems like its the right appraoch.

On top of that, it can actually get the font metrics for you too.

swamidass commented 2 years ago

The issue there is, that they generally need the font file, so this would mean that the font has to be loaded from a server (no access browser-side of course). So the user would be forced to use a (self-)hosted webfont.

I think SVG.js solves this trade off. On the server, it loads the font (but that isn't a problem on the server). On the browser, it should just the standard browser API (which of course, also is loading the font, but in a more efficient way).

daenuprobst commented 2 years ago

If that works, great! If we do revisit this, using fontkit, I think, will be the most robust and platform independent approach. (Or maybe SVG.js???)

Also, I did some searching for the right SVG library. Here are a few options:

https://github.com/svgdotjs/svgdom https://svgjs.dev/docs/3.0/ https://github.com/adobe-webplatform/Snap.svg/

My suggestion is to replace the current SVG Wrapper with calls to SVG.js. That should, transparently, work very fast both on the server and on the browser. It seems that SVG.js pollyfills with svg-dom on node, but on the browser it uses native elements. If that's correct, that seems like its the right appraoch.

On top of that, it can actually get the font metrics for you too.

Definitely. I have used snap.svg in the past, but svg.js seems to be the way to go, especially performance-wise. What is really interesting there is also the interactivity and animations (some really interesting options for data visualization).

I'll see how the implementation of the reaction drawing goes and then decide how to proceed in regard to the library.