-
-
Notifications
You must be signed in to change notification settings - Fork 121
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
Support Unicode 16 octants #2820
Conversation
whoa! |
i haven't looked yet, but some questions i'll be keeping in mind:
i'm very excited, and look forward to reviewing this! update: turns out |
ahhh shit, i salute my homie from PITP! did you know annie ray by any chance? i was writing a length essay involving canadian politics (particularly the "Western alienation" of the 90s) when i saw your PR. though i see you are german, by way of...LSU? huh |
i notice with a very superficial survey that:
with that said, we don't need to wait for everyone else. just investigating. |
the relevant glyphs do render, however. so this isn't really applicable.
this remains true, but of rather less significance. |
and sextants are not the place to demonstrate your design virtuosity. To | ||
inspect your environment's rendering of drawing characters, run | ||
rather than loading them from a font. This is generally desirable. Quadrants, | ||
sextants, and octants are not the place to demonstrate your design virtuosity. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i'm guessing your methodology for this kind of thing involved grepping for at a minimum sextant
. i'm trying to think of other stuff we'd want to check. NCBLIT
is broad but probably a good one. i'll run these checks, just thinking aloud.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct. I looked for sextant
, but not for NCBLIT
.
doc/man/man3/notcurses_visual.3.md
Outdated
aspect-preserving **NCBLIT_2x1** will be returned. If sextants are available | ||
(see **notcurses_cansextant**), this will be **NCBLIT_3x2**, or otherwise | ||
**NCBLIT_2x2**. | ||
**NCBLIT_1x1**. If octants are available, (see **notcurses_canictant**), the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
canoctant
i think you mean. and i don't think 4x2
is aspect-preserving? am i being stupid?
the whole reason why 2x1
preserves aspect (mostly) is that a cell is (usually) (roughly) twice as tall as it is wide. so when you put two pixels in there, on top of one anotther, you suddenly have pixels be squares, like they (usually) are in your image. but, 4x2
is a linear map from 2x1
, so yeah, i guess that works too. good deal. nice to have another aspect-preserving blitter!
i'm not sure about it being the default for NCSCALE_NONE
and NCSCALE_SCALE
, though. 2x1
allows full color fidelity. remember, you only have a background and foreground for each cell. so with two source pixels mapped to the cell, you can do both colors losslessly. there's no way to losslessly pack 8 pixels into 2 cell colors, though. so you're going to lose some image quality for certain images, and ruin pathological ones.
so i think we stick with 2x1
.
now, maybe it should be the default for NCSCALE_STRETCH
(which is currently 3x2
, which is honestly a garbage blitter).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't quite follow the reasoning. The 4x2
aspect-preserving mode is never chosen when the caller wants to preserve the aspect, but it might be chosen when the caller explicitly allows non-aspect preserving mapping? That seems a bit confusing.
I'd be fine with never making 4x2
the default (at the moment it's new and untested). In the future, maybe there could be a new flag to preserving colour accuracy?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i'm thinking of octants as basically a more attractive, predictable braille. i.e. both are just implementing a 4x2 blitter, but you pretty much know what octants are going to look like. so octants have the same advantages and disadvantages otherwise that braille does -- the advantage is that you can get higher resolution in a fixed cell geometry, and the disadvantage is that you stand to lose color fidelity (which can in certain cases make the image effectively unrecognizable).
4x2 is chosen when the user requests 4x2, whether they choose _SCALE
, _NONE
, or _STRETCH
. this only affects the case where they choose NCBLIT_DEFAULT
together with _SCALE
or _NONE
. if you choose 4x2 explicitly, you get 4x2 (unless we can't do it). does that make sense?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I agree.
"\U0001CEA3\U0001CD36\U0001CD37\U0001CD38\U0001CD39\U0001CD3A\U0001CD3B\U0001CD3C"\ | ||
"\U0001CD3D\U0001CD3E\U0001CD3F\U0001CD40\U0001CD41\U0001CD42\U0001CD43\U0001CD44"\ | ||
"\U00002596\U0001CD45\U0001CD46\U0001CD47\U0001CD48\U0000258C\U0001CD49\U0001CD4A"\ | ||
"\U0001CD4B\U0001CD4C\U0000259E\U0001CD4D\U0001CD4E\U0001CD4F\U0001CD50\U0000259B"\ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
my eyes
"\U0001CDCB\U0001CDCC\U0001CDCD\U0001CDCE\U0001CDCF\U0001CDD0\U0001CDD1\U0001CDD2"\ | ||
"\U0001CDD3\U0001CDD4\U0001CDD5\U0001CDD6\U0001CDD7\U0001CDD8\U0001CDD9\U0001CDDA"\ | ||
"\U00002584\U0001CDDB\U0001CDDC\U0001CDDD\U0001CDDE\U00002599\U0001CDDF\U0001CDE0"\ | ||
"\U0001CDE1\U0001CDE2\U0000259F\U0001CDE3\U00002586\U0001CDE4\U0001CDE5\U00002588" | ||
#define NCBRAILLEEGCS \ | ||
L"\u2800\u2801\u2808\u2809\u2802\u2803\u280a\u280b\u2810\u2811\u2818\u2819\u2812\u2813\u281a\u281b"\ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh apparently i pulled this shit too, hah
README.md
Outdated
@@ -309,7 +309,8 @@ If things break or seem otherwise lackluster, **please** consult the | |||
Notcurses will not make use of bitmap protocols unless the terminal positively | |||
indicates support for them, even if <code>NCBLIT_PIXEL</code> has been | |||
requested. Likewise, sextants (<code>NCBLIT_3x2</code>) won't be used without | |||
Unicode 13 support, etc. <code>ncvisual_blit()</code> will use the best blitter | |||
Unicode 13 support, octants (<code>NCBLIT_4x2</code>) won't be used without | |||
Unicode 17 support, etc. <code>ncvisual_blit()</code> will use the best blitter |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
have we traveled to the future in a Delorean? unicode 17 does not yet exist afaik
See also wez/wezterm#6502. |
src/lib/blit.c
Outdated
octant_blit(ncplane* nc, int linesize, const void* data, int leny, int lenx, | ||
const blitterargs* bargs){ | ||
const unsigned nointerpolate = bargs->flags & NCVISUAL_OPTION_NOINTERPOLATE; | ||
const bool blendcolors = bargs->flags & NCVISUAL_OPTION_BLEND; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there's a lot of cut-and-paste going on here. i understand that, but one thing i was hoping to do was to come back and parameterize a general geometric character blitter. if you think that's beyond your abilities / the scope of this pr, that's fine, but it's something i'd like to do sooner rather than later.
maybe i ought do a parameterization, and then fold your patch into it? unsure.
reviewing this is definitely....dicey.
i think at a minimum i'd like to see this share the braille solver code. it seems like they ought be the exact same logic, just with two distinct sets of EGCs, no? i think that's a requirement for getting this in, unless you can show me why they'd be different solvers. what do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I currently replicated the search-over-everything logic from the sextant solver. I can switch to using the Braille solver logic. The EGCs for octants are not ordered regularly, so there will need to be a lookup table. If you're fine with using a lookup table for Braille as well then I can combine the solvers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah let's use the braille code plus a lookup table. if it's easiest, we can just use a lookup table for the braille, too, and pass them in as parameters.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here's the generic blitter.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
beautiful. i'll try to look at this today, but it might not be until tomorrow. thanks a lot for your work here. fwiw, i think this will likely be the largest contribution to the notcurses c core by someone who is not me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, i looked over this and dropped some thoughts. overall, i like it, and thanks a ton for writing this. i hope it wasn't too difficult.
the two things i'd like to see before merging it:
1: i think this and the braille code ought share solver code, distinguished by two sets of EGCs. i'm going to gate merging this on either (a) explaining why i'm wrong or (b) doing so.
2: i think 2x1 ought remain the default for NCSCALE_NONE
/NCSCALE_SCALE
, for reasons of color fidelity explained in a comment.
other than those two things, this looks awesome!
Both ideas look reasonable. It should also be possible to combine this with the sextant code, or to have a generic solver for any size. |
alright i think i'm going to go ahead and merge this, and then make some changes on top of it. i don't want to hold back your pr, though. do you mind me asking you for review of my changes? |
I'd be happy to have a look at your changes. |
thanks a lot for this contribution to Notcurses, @eschnett ! i'm delighted, and you've inspired me to get some longstanding work done. huzzah! |
@dankamongmen I never answered: No, I don't know Annie Ray. (I don't spend much time at IQC.) The world is small... |
This is a first stab at supporting Unicode 16 octants #2669 . Comments welcome.