wader / ansisvg

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

Wrong foreground/background alignment for some glyphs and fonts #21

Closed wader closed 9 months ago

wader commented 10 months ago
Screenshot 2023-12-14 at 23 58 09
wader commented 10 months ago

@patrislav1 Let's continue here

wader commented 10 months ago

BTW thanks for helping fix and stress ansisvg! to honest i've mostly used it for demo and slide pictures of https://github.com/wader/fq output which might explain hitting lots of bugs :)

patrislav1 commented 10 months ago

And yet it seems to be the best tool for that job. The only other I could find was some JS based thing that you can’t even invoke from the command line.

wader commented 10 months ago

I think you summarised well why i ended up writing my own, "how hard can it be?" 😬

wader commented 9 months ago

Played around a bit more with different element attribute and css properties, no luck :( the only one that i noticed affected only x coordinate was to set direction: rtl then it looks to align better but things gets offseted one latter hmm. I do wonder how iterm2, kitty etc can make this correct? maybe should read some source code.

patrislav1 commented 9 months ago

I had a completely different idea today. Instead of guessing where to draw background rectangles, why not let the svg font rendering engine do that work? We can use unicode full block β€œβ–ˆβ€ (U+2588) as background and color it accordingly. Then put another layer of text on top of it, containing the actual text:

test-fullblock

wader commented 9 months ago

Interesting! also don't seem to interfere with copy/paste, unless one wants the arrows included? guess not? but i did notice it's a bit random what fonts has a filled glyph for the full block codepoint, some seem to have stroked rectangle hmm.

Nice with some progress, hopefully we can poke around and understand this enough to get something working, hopefully without adding more cli arguments :)

A bit busy this weekend but maybe i will get some time

patrislav1 commented 9 months ago

I made a little proof of concept to explore this background rendering approach. It also doesn't need the housekeeping of used background colors anymore. Would be nice if the excess linefeeds in the generated SVG could be eliminated, I couldn't do it myself for lack of familiarity with the templating engine. I also would like to consolidate <tspan> elements with the same style/fill attribute sometime ...

test1

but i did notice it's a bit random what fonts has a filled glyph for the full block codepoint, some seem to have stroked rectangle hmm.

Ok that's a bummer :disappointed: Are there many like that, so many that it's a deal breaker?

patrislav1 commented 9 months ago

This is how I would like the SVG output to be: https://codepen.io/patrislav1/pen/xxMovKW

That way it doesn't even need X coordinates or the charboxsize anymore, it only needs the line spacing.

patrislav1 commented 9 months ago

I also would like to consolidate <tspan> elements with the same style/fill attribute sometime ...

Ok finally implemented tspan consolidation, now the SVG output is lean and doesn't use X coordinates for individual letters anymore: https://codepen.io/patrislav1/pen/yLZmJLm

here's a diff of my changes, please be patient as I never wrote Go before (maybe you have suggestions for improvements): https://github.com/patrislav1/ansisvg/compare/8daed6c16cac6f42f271d2806ac83c2150b0307c..fa03c63f85a2c2668235b6bdd3601a387071e60d#diff-5948cb3358520edabb7269cc310a861b2e2223a57e38e697aac25aeee34fd856

(Considering it's my first attempt at writing Go, it was a good experience, maybe I like the language?)

I had to move a lot of logic from the template to the svgscreen type, but I also think it's cleaner that way. WDYT?

wader commented 9 months ago

I made a little proof of concept to explore this background rendering approach. It also doesn't need the housekeeping of used background colors anymore. Would be nice if the excess linefeeds in the generated SVG could be eliminated, I couldn't do it myself for lack of familiarity with the templating engine. I also would like to consolidate <tspan> elements with the same style/fill attribute sometime ...

The template language is https://pkg.go.dev/html/template. I've thought about skip the whole template thing and just use go instead. If we end up with something as simple as in your branch I think it would make sense?

Excess linefeeds/whitespace can be controlled in the template language using a leading/ending - on template tags {{- ... }} but it's not that intuitive how the whitespace gets collapsed :)

Yeap both skipping use of xlink:ref and do consolidation would be great if possible.

test1

but i did notice it's a bit random what fonts has a filled glyph for the full block codepoint, some seem to have stroked rectangle hmm.

Ok that's a bummer 😞 Are there many like that, so many that it's a deal breaker?

I think it would be good if it by default outputs something that works with the standard "web safe fonts". What that output is I think we will have to figure out. If we can't figure out something that can work with what powerline and similar things use I guess we need some argument to enable it, haven't thought about what that would be.

This is how I would like the SVG output to be: https://codepen.io/patrislav1/pen/xxMovKW

That way it doesn't even need X coordinates or the charboxsize anymore, it only needs the line spacing.

That would be great if possible

I also would like to consolidate <tspan> elements with the same style/fill attribute sometime ...

Ok finally implemented tspan consolidation, now the SVG output is lean and doesn't use X coordinates for individual letters anymore: https://codepen.io/patrislav1/pen/yLZmJLm

here's a diff of my changes, please be patient as I never wrote Go before (maybe you have suggestions for improvements): https://github.com/patrislav1/ansisvg/compare/8daed6c16cac6f42f271d2806ac83c2150b0307c..fa03c63f85a2c2668235b6bdd3601a387071e60d#diff-5948cb3358520edabb7269cc310a861b2e2223a57e38e697aac25aeee34fd856

(Considering it's my first attempt at writing Go, it was a good experience, maybe I like the language?)

No worries! happy to help with go related questions also :) it's a nice language and the tooling is great, I do miss some fancy language features but I think it makes up for it in other ways.

Had a quick look at the tests and I seem to break some things. Haven't look deeper yet. I usually do something like this to quickly get an overview:

# this rewrites the *.ansi.svg files with new output
$ go test ./... -update 
# generate all.html, open in favorite browser
$ for i in ansitosvg/testdata/*.ansi.svg; do echo "$i<br><img src=\"$i\"/><br>" ; done  > all.html
$ 

Screenshot 2023-12-18 at 10 39 52

Broken with 14px font but seems to work fine with 16px for me both with monospace and courier.

Also wondering what one would expect when copy/pasting from the example above. I guess only the the "text". Now it seems the full blocks gets included hmm.

I had to move a lot of logic from the template to the svgscreen type, but I also think it's cleaner that way. WDYT?

Yeap could make sense. From the start I had some vague idea that divide it into a packages that decodes ansi, a svg "screen" rendered and then a third package to glue them together. But it turned out it was not that clear where some things belong and when you want to optimise for small output etc it gets even more murky :) so there is no clear divide, do things where it's convenient... and the api:s are all internal anyway, maybe should even move them into a internal directory to make it clear? (in go all packages are public unless in a "internal" directory)

Will play around more with the later, and again, thanks for helping out!

patrislav1 commented 9 months ago

Also wondering what one would expect when copy/pasting from the example above. I guess only the the "text". Now it seems the full blocks gets included hmm.

I made some more commits in my playground branch; turns out that when the SVG first runs all background and only then the foreground, then the copy / paste only selects the foreground! Not sure if this is implementation/browser specific though, would be better if there was something like a tag preventing copying from a text element, however not sure if that exists in SVG.

patrislav1 commented 9 months ago

Had a quick look at the tests and I seem to break some things. Broken with 14px font but seems to work fine with 16px for me both with monospace and courier.

From what I can see the problem is the explicitly given svg width is too narrow,

<svg viewBox="0 0 224 128" width="224" height="128" ...
                  ^^^            ^^^^^

again, due to small difference between assumed font dimensions (CharBoxSize) and actual rendered font size.

Since we don't know the actual rendered text width in advance (at least not if we're not bundling a font file), it probably needs some JS inside the SVG that determines it and sets the image width on the fly. Other than that it's probably down to guesswork with charboxsize ...

patrislav1 commented 9 months ago

I tried to write some JS adapting the SVG width to the actual width, it works in codepen but not embedded in the SVG ...

https://codepen.io/patrislav1/pen/yLZmJLm?editors=1010

wader commented 9 months ago

I made some more commits in my playground branch; turns out that when the SVG first runs all background and only then the foreground, then the copy / paste only selects the foreground! Not sure if this is implementation/browser specific though, would be better if there was something like a tag preventing copying from a text element, however not sure if that exists in SVG.

Maybe there is something in CSS but might be shaky for SVG. The SVG specification seems to have some notes about how it should behave https://www.w3.org/TR/SVG11/implnote.html#TextSelectionImplementationNotes

wader commented 9 months ago

From what I can see the problem is the explicitly given svg width is too narrow,

<svg viewBox="0 0 224 128" width="224" height="128" ...
                  ^^^            ^^^^^

Ah yeap, and the black lines between the rows was just that default font size is 14 but character box is 8x16 🀦

again, due to small difference between assumed font dimensions (CharBoxSize) and actual rendered font size.

Since we don't know the actual rendered text width in advance (at least not if we're not bundling a font file), it probably needs some JS inside the SVG that determines it and sets the image width on the fly. Other than that it's probably down to guesswork with charboxsize ...

Using JS might be problematic as you noticed in the next comment. Not sure if it's always disabled for embedding or if this is github using CSP (Content-Security-Policy) to disable it for security reasons. Either way it's probably good to not relay on it i think.

BTW there is CSS length unit called ch that might be interesting, ex: x="4ch", it seem to be what we want. Problem still is that i don't think use can use that with viewPort or svg width/height, also if i remember correctly inkscape had issues with it but maybe the support has improved?

wader commented 9 months ago

Do you want to turn your branch into a PR even if it is very work in progress? would make it easier to talk about the code changes i think

patrislav1 commented 9 months ago

Ok, will clean up the commits and open a PR :+1:

wader commented 9 months ago

@patrislav1 Close issue or something left?

patrislav1 commented 9 months ago

Close πŸ‘