wader / ansisvg

Convert ANSI to SVG
MIT License
89 stars 9 forks source link

svgscreen rewrite #23

Closed patrislav1 closed 9 months ago

patrislav1 commented 9 months ago

Work in progress

:white_check_mark: Replace background rects with full block characters :white_check_mark: Get rid of explicit X coordinates for character positioning :white_check_mark: Consolidate consecutive tspans with same style (for smaller/cleaner SVG file) :white_check_mark: Support for bold, italic, underline, strikethrough :grimacing: Not sure if all relevant fonts support full block U+2588 (█) :grimacing: Copy/paste of background blocks "suppressed" by putting them on top of the SVG; not sure if there's a better way :x: Still needs guesswork for font width (charboxsize); by default the svg canvas is too narrow (see testdata/*.ansi.svg) :x: tspan consolidation sometimes un-monospaces the font when there's a ligature (see ff in ansitosvg/testdata/fq.ansi.svg). Maybe there's a CSS flag to fix that?

patrislav1 commented 9 months ago

We can set global font size/family and can use ch and em units then!

<svg width="5ch" height="2em" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" xml:space="preserve">
    <style>
        * {
            font-family: Courier, monospace;
            font-size: 14px;
        }
        tspan, text {
            dominant-baseline: text-before-edge;
            alignment-baseline: text-before-edge;
            white-space: pre;
            fill: #bbbbbb;
        }
    </style>
    <rect width="100%" height="100%" x="0" y="0" style="fill: #000000"/>
<text x="0ch" y="0em"><tspan>hello</tspan></text>
<text x="0ch" y="1em"><tspan>world</tspan></text>
</svg>

hello

This looks good in Firefox. Inkscape however, messes up the background rect and X spacing with powerline fonts. :-(

patrislav1 commented 9 months ago

Looks like Inkscape cannot use width and height in ch and em units and falls back to 300x150px as default.

image

patrislav1 commented 9 months ago

I think I have found the reason why SVGs look bad (misaligned) in inkscape with some fonts. Turns out that the powerline webfont I'm using has a "leftSideBearing" of -12 for some symbols. Inkscape moves the character to the left by that little amount, so all the subsequent characters are moved to the left as well. After like 3 or 4 such symbols the misalignment against the background blocks accumulates so much that it's noticeable. When replacing these symbols with plain whitespace everything's fine again. Chrome and Firefox seem to ignore this offset or at least do not accumulate it, so it does not have visible effect. Also when I replace the woff2 font in Inkscape with a similar ttf font on my system the problem disappears.

I wonder if this offset is a random bug in the font, of if it's possible with some kind of CSS flag to make Inkscape ignore it.

image

patrislav1 commented 9 months ago

When we add an infinitely small shadow to the background boxes, it eliminates the ugly little spaces between them (before/after):

colortest ansi_old colortest ansi_new

(ok on Firefox the one with the shadow looks better, on Chrome and Safari it actually looks worse?!? have to play with the parameters I guess)

patrislav1 commented 9 months ago

tspan consolidation sometimes un-monospaces the font when there's a ligature (see ff in ansitosvg/testdata/fq.ansi.svg). Maybe there's a CSS flag to fix that?

font-variant-ligatures: none; fixes the ligatures problem. :tada:

fq ansi

wader commented 9 months ago

We can set global font size/family and can use ch and em units then! ...

Huh surprised it worked! so to make it work you have to style the root svg element otherwise it will use some default font setting for ch, em etc?

patrislav1 commented 9 months ago

We can set global font size/family and can use ch and em units then! ...

Huh surprised it worked! so to make it work you have to style the root svg element otherwise it will use some default font setting for ch, em etc?

Exactly! And Inkscape will ignore it none the less. ¯\_(ツ)_/¯

wader commented 9 months ago

I think I have found the reason why SVGs look bad (misaligned) in inkscape with some fonts. Turns out that the powerline webfont I'm using has a "leftSideBearing" of -12 for some symbols. Inkscape moves the character to the left by that little amount, so all the subsequent characters are moved to the left as well. After like 3 or 4 such symbols the misalignment against the background blocks accumulates so much that it's noticeable. When replacing these symbols with plain whitespace everything's fine again. Chrome and Firefox seem to ignore this offset or at least do not accumulate it, so it does not have visible effect. Also when I replace the woff2 font in Inkscape with a similar ttf font on my system the problem disappears.

I wonder if this offset is a random bug in the font, of if it's possible with some kind of CSS flag to make Inkscape ignore

Nice find, have no idea :( maybe if we can't figure it out it's ok to document the behaviour for now?

patrislav1 commented 9 months ago

I wonder if this offset is a random bug in the font, of if it's possible with some kind of CSS flag to make Inkscape ignore

Nice find, have no idea :( maybe if we can't figure it out it's ok to document the behaviour for now?

After spending a obscene amount of time fiddling with font-feature-settings, font-kerning etc. to no avail, I believe that wernight/powerline-web-fonts is just buggy in that regard and I'll just roll my own web font ¯\_(ツ)_/¯

Since it's a edge case that only pops up when using powerline and colored background and Inkscape and such a "buggy" webfont, I believe it's okay to ignore and just document it.

wader commented 9 months ago

When we add an infinitely small shadow to the background boxes, it eliminates the ugly little spaces between them (before/after):

...

(ok on Firefox the one with the shadow looks better, on Chrome and Safari it actually looks worse?!? have to play with the parameters I guess)

Only tried in chrome, think i see some faint boxes for both before and after?

Unrelated to this but i noticed that when opening the images in a new window on github it seems to hit from CSP limitation:

Refused to apply inline style because it violates the following Content Security Policy directive: "default-src 'none'". Either the 'unsafe-inline' keyword, a hash ('sha256-q6PgY17P2FZaxqvBsFI1LMr5rPJa6kwkVECROGBUWoo='), or a nonce ('nonce-...') is required to enable inline execution. Note that hashes do not apply to event handlers, style attributes and javascript: navigations unless the 'unsafe-hashes' keyword is present. Note also that 'style-src' was not explicitly set, so 'default-src' is used as a fallback.

Maybe we will have to define CSS classes for some things? but im a bit confused, why did we hit this before? is it some specific css property it does not like? we currently use style="filled: ... " for example and it seems to be ok.

patrislav1 commented 9 months ago

Unrelated to this but i noticed that when opening the images in a new window on github it seems to hit from CSP limitation:

Yes it chokes on that when "open image in a new tab" from a issue or PR conversation like this. When you navigate to the repo's main page and do the same for the SVG embedded in the readme, it is no problem. "view page as source" shows identical code in both cases.

wader commented 9 months ago

tspan consolidation sometimes un-monospaces the font when there's a ligature (see ff in ansitosvg/testdata/fq.ansi.svg). Maybe there's a CSS flag to fix that?

font-variant-ligatures: none; fixes the ligatures problem. 🎉

Oh haven't noticed, maybe ligatures are not usually used by default fonts? but yeah it seems to make sense to disable for us? also i guess there would be edge cases like if the characters involved a lingature is style differently?

But it do seems to be a thing https://github.com/tonsky/FiraCode :)

wader commented 9 months ago

Unrelated to this but i noticed that when opening the images in a new window on github it seems to hit from CSP limitation:

Yes it chokes on that when "open image in a new tab" from a issue or PR conversation like this. When you navigate to the repo's main page and do the same for the SVG embedded in the readme, it is no problem. "view page as source" shows identical code in both cases.

Aha i see, good. Hmm i wonder why

patrislav1 commented 9 months ago

Oh haven't noticed, maybe ligatures are not usually used by default fonts?

Didn't pop up before, b/c back then each character had a <tspan> of its own. In my rewrite branch, text is lumped together and then the font renderer tries to get smart and detect ligatures like ff or fi, even in default monospace font, regardless of the monospace setting. :roll_eyes:

wader commented 9 months ago

I did some digging into fonts thinkings if one option could be to embed default woff2 fonts with ansisvg somehow and then also support parsing them to get character width metadata. Ended up writing a start of a woff2 decoder https://github.com/wader/fq/tree/woff2 :)

Conclusion: probably possible but quite complex, lots of work per font format, would produce big output unless you filter the font file somehow? ...and it would still be nice to support output that works somewhat good with web fonts.

patrislav1 commented 9 months ago

I'm kinda in favor of keeping the ch/em dimension for the SVG width/height when the charboxsize is not set. When the intention is to work with Inkscape one can just set the charboxsize explicitly and then we can set SVG width/height in px to make inkscape happy.

Remember it's only about total SVG dimensions; the positioning of the text with font-relative units will work in every case.

wader commented 9 months ago

Impressive progress! trying to keep up as much as i can, busy with some other things :) my experience juggling with svg before is that you have to bang your head into the wall for a while figuring out whats available/working/possible and hope you can combine something into something useful :)

Do you peek anything at the specifications? https://www.w3.org/TR/SVG11/ and https://www.w3.org/TR/SVG2/ they are quite verbose but quite useful

wader commented 9 months ago

Hope your having a nice and restfull holyday!

patrislav1 commented 9 months ago

Hope your having a nice and restfull holyday!

thx, same to you!

patrislav1 commented 9 months ago

I wonder if this offset is a random bug in the font, of if it's possible with some kind of CSS flag to make Inkscape ignore

Nice find, have no idea :( maybe if we can't figure it out it's ok to document the behaviour for now?

After spending a obscene amount of time fiddling with font-feature-settings, font-kerning etc. to no avail, I believe that wernight/powerline-web-fonts is just buggy in that regard and I'll just roll my own web font ¯_(ツ)_/¯

Since it's a edge case that only pops up when using powerline and colored background and Inkscape and such a "buggy" webfont, I believe it's okay to ignore and just document it.

BTW I think that this behavior is a side effect of Inkscape not being able to load external fonts, falling back to some defaults. AFAICS it cannot load neither external fonts from URL nor embedded fonts. The only way to make it display powerline stuff correctly is to have the font installed system wide as .ttf and pointing to it with ansisvg -fontname option.

There seems to be no straight forward way to make SVGs with custom fonts that render correctly in both inkscape and web browsers.

wader commented 9 months ago

BTW I think that this behavior is a side effect of Inkscape not being able to load external fonts, falling back to some defaults. AFAICS it cannot load neither external fonts from URL nor embedded fonts. The only way to make it display powerline stuff correctly is to have the font installed system wide as .ttf and pointing to it with ansisvg -fontname option.

There seems to be no straight forward way to make SVGs with custom fonts that render correctly in both inkscape and web browsers.

Okok, then i guess we should add some disclaimer about compatibility and custom fonts. Should the default output mode still produce output that don't relay on css unit etc for maximum compatibility? haven't really thought much about it yet

patrislav1 commented 9 months ago

Now it considers charboxsize and forces pixel units if it's set. It uses font-relative ch/em units if it's not set (default).

Upside is, by default it doesn't try to guess font dimensions and the output always looks good in browsers. Downside is, the default output doesn't work too well in Inkscape (SVG size / background rect is off). The inkscape issue can easily be worked around by Ctrl-Shift-R (resize page to content).

So I'm leaning towards keeping the charboxsize unset by default, if there's no further issues.

wader commented 9 months ago

Now it considers charboxsize and forces pixel units if it's set. It uses font-relative ch/em units if it's not set (default).

Upside is, by default it doesn't try to guess font dimensions and the output always looks good in browsers. Downside is, the default output doesn't work too well in Inkscape (SVG size / background rect is off). The inkscape issue can easily be worked around by Ctrl-Shift-R (resize page to content).

So I'm leaning towards keeping the charboxsize unset by default, if there's no further issues.

👍 Yeap i think we could make it default. Just have to document what it means. Maybe could even have a section in README.md about usage with inkscape? could probably even come up with a inkscape batch mode resize if that is something that is useful?

patrislav1 commented 9 months ago

svg width/height seems correct but i guess text consolidation with a char box size won't work hmm.

Yes I'll need to re-introduce explicit positioning of every single character (let's call it "grid mode" or similar), b/c using consolidated tspans relies on actual monospace glyphs for all characters involved; this breaks ansitosvg/testdata/charboxfontsize.ansi but also some edge cases where the rendering engine cannot find the glyph in the selected font and uses a fallback font for a single character that may have different dimensions and breaks the whole alignment.

See codepen, turns out that Ubuntu Mono Nerd Font contains every icon on earth, but not "↱" (U+21B1)! :roll_eyes:

patrislav1 commented 9 months ago

Implemented "grid mode" (default on, disable with -nogrid), see https://github.com/wader/ansisvg/pull/23/commits/7f0d125c94643ba822a62d31df80a04f54a12b93

Fixes issues with non-monospace and fallback fonts, see here (nogrid vs grid; note how the ↱ from the fallback font messes up not only horizontal but also vertical alignment in the former case)

test_nogrid

test_grid

wader commented 9 months ago

Implemented "grid mode" (default on, disable with -nogrid), see 7f0d125

Mm wonder what we should call it? another option i guess would be the inverted, -consolidate, -combine etc?

Were you also considering "no grid" to be default? it produces so much nicer and smaller output :)

Fixes issues with non-monospace and fallback fonts, see here (nogrid vs grid; note how the ↱ from the fallback font messes up not only horizontal but also vertical alignment in the former case)

👍 see horizontal but no vertical align issue but maybe browser/font backend issue again

patrislav1 commented 9 months ago

Implemented "grid mode" (default on, disable with -nogrid), see 7f0d125

Mm wonder what we should call it? another option i guess would be the inverted, -consolidate, -combine etc?

I went with "grid" b/c it emphasizes the explicit positioning. But I don't have a strong opinion how to call it.

Were you also considering "no grid" to be default? it produces so much nicer and smaller output :)

Also have no strong opinion whether it should be default. I went with grid as default b/c it has a better chance at producing a good looking graphics. We can as well make "nogrid" or "consolidate" mode the default, as it works equally well in 99% of the use cases not involving exotic glyphs, and mention in the README to set -grid if some alignment is off. WDYT?

Fixes issues with non-monospace and fallback fonts, see here (nogrid vs grid; note how the ↱ from the fallback font messes up not only horizontal but also vertical alignment in the former case)

👍 see horizontal but no vertical align issue but maybe browser/font backend issue again

Yes it depends on what your browser happens to use as a fallback font.

wader commented 9 months ago

Were you also considering "no grid" to be default? it produces so much nicer and smaller output :)

Also have no strong opinion whether it should be default. I went with grid as default b/c it has a better chance at producing a good looking graphics. We can as well make "nogrid" or "consolidate" mode the default, as it works equally well in 99% of the use cases not involving exotic glyphs, and mention in the README to set -grid if some alignment is off. WDYT?

Yeap let's try with that

patrislav1 commented 9 months ago

dominant-baseline: middle seems to improve the vertical alignment and get rid of (most of) the black line, (also much nicer to edit in Inkscape), pls check how it renders at your end

image

wader commented 9 months ago

dominant-baseline: middle seems to improve the vertical alignment and get rid of (most of) the black line, (also much nicer to edit in Inkscape), pls check how it renders at your end

chrome macOS chrome-macos-Screenshot 2024-01-06 at 16 41 00

firefox macOS firefox-macos-Screenshot 2024-01-06 at 16 41 43

Did two IE tests using https://www.browserling.com/ but not sure how representative of real IE they are Screenshot 2024-01-06 at 16 45 40

Screenshot 2024-01-06 at 16 45 27

wonder also if it's possible to get rid of the "grid" that show up at some zoom levels hmm

patrislav1 commented 9 months ago

chrome macOS

uff that's strange, these examples have completely different dimensions before/after. which diff are you looking at?

patrislav1 commented 9 months ago

if your screenshot is from the diff of this PR, then it's really strange. it looks like this on FF/linux:

image

so the dimensions changed because of moving from charboxsize to font-relative coordinates, OK, but it's weird that your "before" image looks so different from mine

edit: this is chrome on linux image

wader commented 9 months ago

chrome macOS

uff that's strange, these examples have completely different dimensions before/after. which diff are you looking at?

Yes they all change dimension for me, but i guess is some monospace difference?

Screenshot 2024-01-06 at 16 59 44

patrislav1 commented 9 months ago

Yes they all change dimension for me, but i guess is some monospace difference?

It's the difference between px and ch/em units.

patrislav1 commented 9 months ago

wonder also if it's possible to get rid of the "grid" that show up at some zoom levels hmm

I guess we have to go back to <rect> after all :grimacing:

wader commented 9 months ago

It's the difference between px and ch/em units.

Ah good point

wader commented 9 months ago

I guess we have to go back to <rect> after all 😬

The joy! 😅 ... maybe also has a greater chance at working in IE etc? non-font relate stuff seems way more reliable 😬

patrislav1 commented 9 months ago

Made a quick and dirty reimplementation of <rect>. Added a 0.5px stroke to get rid of the faint black grid in the background. First had an issue with vertical alignment for powerline symbols again, but then found out that if we replace dominant-baseline: middle; with dominant-baseline: central; it lines up again nicely - at least with my standard UbuntuMono powerline font. Crossing my fingers that it also works with other fonts...!

patrislav1 commented 9 months ago

Added testdata for powerline. With custom font and grid mode. When combining all testdata SVG's to the all.html page with your one-line script, the powerline testcase doesn't show up with the correct font. But "open image in new tab" will show the same SVG with the custom font correctly. I believe it's due to every SVG setting the global font properties (in order to make font-relative units work for the SVG size), so the SVGs shown after the powerline one will just override it. So in order to enable embedding multiple SVGs with different font properties in the same page, we must not set global font properties, and hence cannot use font-relative units for SVG size :exploding_head: :fire:

wader commented 9 months ago

Made a quick and dirty reimplementation of <rect>. Added a 0.5px stroke to get rid of the faint black grid in the background. First had an issue with vertical alignment for powerline symbols again, but then found out that if we replace dominant-baseline: middle; with dominant-baseline: central; it lines up again nicely - at least with my standard UbuntuMono powerline font. Crossing my fingers that it also works with other fonts...!

Ok, and i don't see any grid lines with chrome or firefox on macOS now 👍

wader commented 9 months ago

Added testdata for powerline. With custom font and grid mode. When combining all testdata SVG's to the all.html page with your one-line script, the powerline testcase doesn't show up with the correct font. But "open image in new tab" will show the same SVG with the custom font correctly. I believe it's due to every SVG setting the global font properties (in order to make font-relative units work for the SVG size), so the SVGs shown after the powerline one will just override it. So in order to enable embedding multiple SVGs with different font properties in the same page, we must not set global font properties, and hence cannot use font-relative units for SVG size 🤯 🔥

Hmm yes that is strange, feels nearly like browser bug if there is some state affecting other images on a page?

But not sure i see it with chrome or firefox on macOS. If i understand it correctly it should work if the svg is alone one a page?

in all.html

Screenshot 2024-01-06 at 22 26 55

in html page with one image

Screenshot 2024-01-06 at 22 27 10

When i search around a bit it seems to not be allowed to refer to external fonts when a svg is used in a img on a page, i thought it would be ok as long as it was ok CORS-wise? i tried to change to url(UbuntuMonoNerdFontMono-Regular.woff2); and put the font next to the svg, did not work neither with file:// and thru a local web server hmm.

patrislav1 commented 9 months ago

Please forget about the hypothetical browser bug keeping state between SVGs theory, I was totally wrong about that.

Turns out in reality the problem is: "To use fonts with img tags you will need to embed the fonts, nothing else works."

When I embed the font in the powerline test (manually), the "all" page turns out fine: image

Now the problem is I can't embed the woff2 file in ansitosvg/testdata/powerline.ansi.svg.json.

Do you think we can extend -fontfile to detect web URLs and download the font if necessary?

wader commented 9 months ago

Please forget about the hypothetical browser bug keeping state between SVGs theory, I was totally wrong about that.

👍

Turns out in reality the problem is: "To use fonts with img tags you will need to embed the fonts, nothing else works."

Yeap was my conclusion also and after thinking a bit more about it maybe it can see how it makes sense for the html standard to require image resources to be "standalone", can imagine caching etc could be a mess if external dependencies could affect rendering. Maybe this is something to document also? maybe README.md should have common use cases, "I want to use svg:s on a webpage": do this and don't his

Now the problem is I can't embed the woff2 file in ansitosvg/testdata/powerline.ansi.svg.json.

Do you think we can extend -fontfile to detect web URLs and download the font if necessary?

In the CLI or for tests? both? hmm feels a bit feature bloaty to support it. For testing -fontfile i think we can add the font file to testdata? (but currently there is no way to read the file, see below)

For the tests i've thought about maybe rewrite the cli to have a "OS" interface that then could also be implemented by the tests. That way the tests will also test the cli interface and probably can test more things. I've done this in some other projects with good results.

It would be nice if we could make this PR into something mergeable soonish so that we can more easily work parallell. What do you think? fine with me to skip some things and do them separately

patrislav1 commented 9 months ago

Maybe this is something to document also? maybe README.md should have common use cases, "I want to use svg:s on a webpage": do this and don't his

Yes absolutely, this kind of stuff is beyond common sense or basic knowledge ;)

Do you think we can extend -fontfile to detect web URLs and download the font if necessary?

In the CLI or for tests? both? hmm feels a bit feature bloaty to support it. For testing -fontfile i think we can add the font file to testdata? (but currently there is no way to read the file, see below)

Yes it'd be in grey area towards scope creep, hence reasonable to leave it out, esp. if the font can be included in the tests by other means.

For the tests i've thought about maybe rewrite the cli to have a "OS" interface that then could also be implemented by the tests. That way the tests will also test the cli interface and probably can test more things. I've done this in some other projects with good results.

:+1: Would make a lot of sense, considering that the tests in ansisvg/testdata are more like functional tests rather than unit tests.

It would be nice if we could make this PR into something mergeable soonish so that we can more easily work parallell. What do you think? fine with me to skip some things and do them separately

For me it's just a matter of squashing the fixup commits, then we can totally merge it, if you're fine with it. I have a few more optimizations/refactorings in mind, but they can be done separately.

patrislav1 commented 9 months ago

Squashed / cleaned up the commits.

wader commented 9 months ago

Maybe this is something to document also? maybe README.md should have common use cases, "I want to use svg:s on a webpage": do this and don't his

Yes absolutely, this kind of stuff is beyond common sense or basic knowledge ;)

Very much so, working on ansisvg have learned me a lot about web standards and whatnot

For the tests i've thought about maybe rewrite the cli to have a "OS" interface that then could also be implemented by the tests. That way the tests will also test the cli interface and probably can test more things. I've done this in some other projects with good results.

👍 Would make a lot of sense, considering that the tests in ansisvg/testdata are more like functional tests rather than unit tests.

Yeap will look into it once this PR is merged

It would be nice if we could make this PR into something mergeable soonish so that we can more easily work parallell. What do you think? fine with me to skip some things and do them separately

For me it's just a matter of squashing the fixup commits, then we can totally merge it, if you're fine with it. I have a few more optimizations/refactorings in mind, but they can be done separately.

Great! i noticed one thing about some grid artifact showing up, see comment, hope we can tweak something for that. Otherwise after i think we're good to merge, something more you want to fix?

Also tested some IE via browserling again and rect seems to have fix things and it looks reasonable good

patrislav1 commented 9 months ago

Implemented line-wise consolidation of consecutive background rects. Should fix grid artifacts between horizontal neighbors, the 0.5px stroke should take care of the vertical ones ...?

Not sure why CI is suddenly failing.

wader commented 9 months ago

Implemented line-wise consolidation of consecutive background rects. Should fix grid artifacts between horizontal neighbors, the 0.5px stroke should take care of the vertical ones ...?

Now it looks good 🥳

Not sure why CI is suddenly failing.

Seem to be test output difference, maybe forgot add new test update? easiest is usually to look at the raw action log, the github log ui is not very nice

patrislav1 commented 9 months ago

Now it looks good 🥳

:tada:

Seem to be test output difference, maybe forgot add new test update?

Yep, somehow the last testdata change slipped through, now it's fine again. You're welcome to merge now. :rocket:

wader commented 9 months ago

BTW i filed a inkscape bug regarding using em/ch units on root svg element https://gitlab.com/inkscape/inkscape/-/issues/4737