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

make NCBLIT_3x2 the default for NCSCALE_STRETCH once it works in most terminals #1114

Closed
dankamongmen opened this issue Nov 13, 2020 · 10 comments
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@dankamongmen
Copy link
Owner

NCBLIT_3x2 is superior to NCBLIT_2x2 when there's proper font support (it's stictly superior regarding aspect ratio as well as resolution, 1.5:1 instead of 2:1). While we probably don't want to make it the NCBLIT_DEFAULT choice for NCBLIT_STRETCH just yet, due to font problems (see e.g. #1099), it ought eventually become the default. Reminder to investigate regularly, and switch when satisfied.

@dankamongmen dankamongmen added the enhancement New feature or request label Nov 13, 2020
@dankamongmen dankamongmen self-assigned this Nov 13, 2020
@dankamongmen
Copy link
Owner Author

kitty 0.19.2 came out today in Debian Unstable, but my fix for sextants isn't yet present (it'll be in 0.19.3, not yet released). I don't think we can make this switch until that's at least in Unstable.

@dankamongmen
Copy link
Owner Author

Sooooooooooooooooooooooooooooooooo i'm wondering whether we might not check for an environment variable, and if that environment variable is set, use NCBLIT_3x2 for NCBLIT_DEFAULT+NCSCALE_STRETCH. This environment variable won't be necessary for use of sextants (NCBLIT_3x2 will always get you sextants if they're available, and we'll still change the default at some point), but it'll get them. Maybe.

@dankamongmen
Copy link
Owner Author

We also need wait for everyone to have glibc 2.32, which brings in necessary Unicode 13 changes.

@dankamongmen
Copy link
Owner Author

Since we're now already doing grisly per-emulator stuff (see kitty fix from #1117), we might as well turn this on for terminals we know to support it. Cat's out of the bag on this one. First thing we'll need do is move ncvisual_default_blitter() into the library (it's currently an inline).

@dankamongmen
Copy link
Owner Author

We also need wait for everyone to have glibc 2.32, which brings in necessary Unicode 13 changes.

this has been invalidated by the move to direct blitting.

i'm thinking i'll turn it on for any TERM containing vte or kitty, maybe also gnome (though I think the proper TERM for gnome-terminal is vte-256color).

@dankamongmen
Copy link
Owner Author

urk, there's no pretty way to do this, since ncvisual_default_blitter() doesn't take any big state objects (notcurses/ncdirect), and this is clearly a stateful decision. we'd need be able to pass in a third argument (new function so as not to break compat) to indicate whether we'd decided sextants were good in this environment. i guess that's not such an uncouth thing...

dankamongmen added a commit that referenced this issue Dec 25, 2020
Add a new member 'sextants' to the terminfo cache (both
notcurses and ncvisual contain one of these, and both
initialize it the same way -- interrogate_terminfo()).
Add a new function, 'notcurses_media_defblitter()', and
deprecate 'ncvisual_default_blitter()' (the latter didn't
receive enough information to return NCBLIT_3x2). Update
all callers. Add new *internal* function rgba_default_blitter(),
so this logic can be freely changed in the future. If
sextants are available, and we're scaling, return NCBLIT_3x2.
Once we detect sextant availability, we'll have sexblitter
as a default -- stay tuned! #1114
@dankamongmen
Copy link
Owner Author

About to turn this on...

@dankamongmen
Copy link
Owner Author

looks like i busted the rust wrappers. i can fix this, but @joseluis seems to be doing some Christmas Day hacking of his own, so i'll leave it to him for the moment: https://drone.dsscaw.com:4443/dankamongmen/notcurses/5124

dankamongmen added a commit that referenced this issue Dec 25, 2020
If the TERM string contains either of "kitty", "vte", or
"gnome", enable sextants. Fuck it, we'll do it live!
@joseluis
Copy link
Collaborator

joseluis commented Dec 25, 2020

happy hacking christmas @dankamongmen , I broke the compilation earlier before by forgetting to include a couple of files with necessary changes.

@dankamongmen
Copy link
Owner Author

Alright, 2.1.2 distributed this for kitty and VTE. We'll enable others as they come available, and make it the general default once most everything supports it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants