Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix viewport overflow issues #43

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

MadLittleMods
Copy link

@MadLittleMods MadLittleMods commented Feb 19, 2024

Fix viewport overflow issues

Fix marionebl/svg-term-cli#73
Fix marionebl/svg-term-cli#78

Can be reviewed commit-by-commit (has a description of each fix)

(click to enlarge SVGs)

  • Notice as the second line of the first command is being typed out, the "chrom" overflow isn't cut-off on the right-side and there isn't an artifact from the previous frame on the left-side.
  • Notice that the terminal prompt is visible after each command (last line isn't cut-off)
Before After

Testing strategy

Download demo.cast

# Clone `svg-term`
$ git clone git@github.com:marionebl/svg-term.git
$ cd svg-term
$ git fetch origin refs/pull/43/head:pr-43
$ git checkout pr-43
$ npm install
$ npm run build
$ npm link
$ cd ..

# Now clone `svg-term-cli`
$ git clone git@github.com:marionebl/svg-term-cli.git
$ cd svg-term-cli
$ npm install
$ npm link svg-term
$ npm run build
$ npm link

# Generate a SVG
$ cat demo/demo.cast | svg-term --out demo/demo.svg --window --width=80 --height=24

Fix marionebl/svg-term-cli#73

This problem only manifested when an explicit `--height`
was given.

Because `options.height` and `cast.height` differ, using
the wrong one will result in an off-by-one error. Note:
those height values only differ because it errantly seems to be
[incremented by one when we load the cast](https://github.com/marionebl/svg-term/blob/f761fcac7dc77520cb6d7c8e28618cd266f74538/src/load-cast.ts#L14).
We should probably remove that increment (see next commit).
This just throws off target height. Looking at git blame
to try to find where this came from, it's just from the beginning
of the repo so the reason seems to be lost to history.
Perhaps a crutch/workaround for a different problem that no
longer exists.
Part of marionebl/svg-term-cli#78

All of the frames in a long horizontal strip. Because the frame
text slightly overflows each frame, the content from the last
frame was slightly visible in the current frame.

Essentially, we're adding `overflow: hidden` but in SVG form
so regardless of content in frames, they can't affect each
other.
Other part of marionebl/svg-term-cli#78

Essentially, we were previously just multiplying `1.67` (`fontSize`)
by `0.6` to try to cancel it back down to `1`. But since these values
don't match perfectly, they gave horribly off-pixel and inaccurate values.
For example at a column width of `80`, this calculation gives `80.16`
pixels and puts each character on an off-grid position. It also means each
line overflows by 1.6px and was the root cause of
marionebl/svg-term-cli#78

Since we just want each character to take up 1 unit of width space,
we don't need to do any calculation, the `x` value should just be
the character index in the line.
(removed usage in earlier commit)
`width` and `displayWidth` are the same value but
since it's more appropriate to use `height={data.displayHeight}`
it makes more sense to be consistent and also use
`width={data.displayWidth}`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Last column is partially cut Last line of terminal cast goes off-screen
1 participant