Make shaded block glyphs look even betterer #16760
Conversation
| const auto invalidationTime = _sourceCodeInvalidationTime.load(std::memory_order_relaxed); | ||
|
|
||
| if (invalidationTime == INT64_MAX || invalidationTime > std::chrono::steady_clock::now().time_since_epoch().count()) | ||
| #if ATLAS_DEBUG_SHADER_HOT_RELOAD |
There was a problem hiding this comment.
Suppress whitespace changes here: https://github.com/microsoft/terminal/pull/16760/files?w=1
|
Before After |
|
Have you considered rendering these glyphs in a solid color? Something like |
|
I have, but I figured that if someone wrote a ░ ▒ ▓ it must've been a conscious choice to get a "textured" look. After all, if they wanted an untextured block glyph, they could just blend the colors themselves and emit a whitespace character with an appropriate background color. Do you see this differently? |
|
It seems to me that texture is losing its relevance here due to hi-DPI monitors. In addition, the Win32 Console API is limited to a palette of 16 colors and it will not be possible to mix any color there. |
I was still intending to make the pattern size a bit larger.
That's a good point. I haven't considered that before. (I was only thinking about modern VT apps with their full 24-bit color access.) @j4james if you don't mind, I'd also be interested in what you think about this, due to your knowledge about terminal history. Should we implement shaded glyphs with simple semi-transparent overlays in your opinion? It'd be a simple change for me. 🙂 |
|
Also, solid color rendering would be compatible with gdi mode rendering. |
|
I thought deeper and realized that TUI elements drawn in a mixed solid color (without texture) may not be consistent with other UI elements that have a clear fg color. |
|
I am all for us keeping the textures. James has in the past brought up the concern that the textured shade glyphs look different under reverse video than the equivalent semitransparent or blended fills, and applications can use this effect to their advantage. |
I am very much in favor of keeping the textured effect, and I know I'm not alone in this because I've seen people asking for this on terminals that take the other approach. That Turbo Vision demo is a great example of why this is so appealing to some of us. It's a distinctive look from the 90s that just doesn't have the same effect when rendered with a solid block.
+1 to this as well. I love your implementation as it is, but I do think it would be nicer on hi-DPI monitors if the pattern size could be made larger.
The reverse issue I brought up before (assuming we're remembering the same thing) was regarding the character |
|
I've been trying to replicate an up/down pattern similar to the one tusharsnx posted here for hours now: #16729 (comment) Well, this morning my wife left a random "Isn't that just a horizontally stretched checkerboard pattern?" remark which gave me the most shell-shocked "Ah." moment of my life. So with all credit to her: With the dots going up and down like that, it looks way way more pleasing when the entire background is filled: As you can see there, I've also made the dot size a bit larger. At 12pt on my 150% scale display this will result in 2px per dot. I've updated my initial comment above with the new screenshot. I don't know what's up with today, but I've also been able to condense that into the most disgusting shader code known to (terminal-)mankind. Look how tiny that is! abs(step(frac(dot(floor(position.xy / dotSize), float2(stretch * -0.25f + 0.5f, 0.5f))), 0) - invert);Let's just pretend the |
This is maybe my favorite thing ever. Right next to the look of that stippling of course 😃 |
| float stretched = step(frac(dot(pos, float2(glyph.r * -0.25f + 0.5f, 0.5f))), 0) * glyph.a; | ||
| // Thankfully the remaining 2 operations are a lot simpler. | ||
| float inverted = abs(glyph.g - stretched); | ||
| float filled = max(glyph.b, inverted); | ||
|
|
||
| color = premultiplyColor(data.color) * filled; |
There was a problem hiding this comment.
nit:
Can we do this instead (if this is legal in hlsl)?
int on = step(frac(dot(pos, float2(glyph.r * -0.25f + 0.5f, 0.5f))), 0) * glyph.a;
on ^= glyph.g;
on |= glyph.b;
color = premultiplyColor(data.color) * on;There was a problem hiding this comment.
It is legal in hlsl, but this won't work here. glyph.b may not just be 0 or 1, because some of the box drawing and powerline glyphs (╳, etc.) are anti-aliased and have semi-transparent pixels.
Also, the abs() and max() calls are super efficient on GPUs.
Side note: The most expensive part by far in the shader (like probably 10x over the above 4 lines) is the switch/case because GPUs don't seem to support computed jumps (or jump tables). That means that reaching the 7th case label needs at a minimum 7 jumps which are not fast. On AMD cards it needs twice that (you can test that with https://shader-playground.timjones.io/ and by setting the compiler to the Radeon GPU Analyzer).
We have the option to stop using an "uber shader" like this and instead use multiple distinct pixel shaders that each do one (or few) of the ShadingTypes.
A middle ground could be to manually split up the switch/case into multiple smaller switches. For instance, our pixel shader can clearly be split into 3 parts: "depends on the background texture", "depends on the glyphAtlas texture" and "depends on nothing". I've experimented with that and it lead to a significant size reduction of the shader, but I've not tested how much power we can save that way. (On my desktop GPU I found that it's impossible to measure that, because it's just way too fast for our pixel shader either way.)
There was a problem hiding this comment.
Isn't this ░ ▒ ▓ specific code (especially, the step(frac(dot(...))) part)? If we're not going to work with floats (either in future or now), I would recommend to just simplify it to use int instead of float as it makes the code more readable 😅
(we can also get rid of the extra glyph.a multiplication in the stretched calc as it is always going to be 1)
There was a problem hiding this comment.
Initially, I used the new ShadingType only for the 3 shaded glyphs ░ ▒ ▓ and I used the ShadingType::TextGrayscale for the other box drawing glyphs. This allowed me to use integer operations, just like you suggested it. But I just didn't like this approach in the end for two reasons:
- 4 of the "Symbols for Legacy Computing" glyphs require both, shaded textures and anti-aliasing. They're U+1FB9C to U+1FB9F. For instance: https://www.compart.com/en/unicode/U+1FB9C
- The floating point logic generates more compact HLSL assembly and often runs faster on older/slower hardware too.
So, now the new shading type is used for all builtin glyphs and uses floating point, as this solves the 1st point. It's also why I need to multiply by glyph.a. It turns the 0/1 values that the step() call spits out into a 0/alpha value. I'm using a multiplication there, because this results in better assembly.
I would recommend to just simplify it to use int instead of float as it makes the code more readable 😅
In particular the 2nd point is important to me, even more so than for our CPU-side code. On the currently upcoming 8K@60Hz displays this shader is supposed to churn out 2B pixels per second. The float math I wrote compiles down to more efficient instructions and saves us a couple cycles. It's not much but I think every bit helps. (...which is also why I'll probably have to investigate my "side note" above at some point or another.)
There was a problem hiding this comment.
Only after reading this, I realised shading type TextBuiltinGlyph is used for ALL builtin glyphs and not just the 3 shaded block 🤦
| // .r = stretch | ||
| // 0: #_#_#_#_ | ||
| // _#_#_#_# | ||
| // #_#_#_#_ | ||
| // _#_#_#_# | ||
| // | ||
| // 1: #___#___ | ||
| // __#___#_ | ||
| // #___#___ | ||
| // __#___#_ | ||
| // | ||
| // .g = invert | ||
| // 0: #_#_#_#_ | ||
| // _#_#_#_# | ||
| // #_#_#_#_ | ||
| // _#_#_#_# | ||
| // | ||
| // 1: _#_#_#_# | ||
| // #_#_#_#_ | ||
| // _#_#_#_# | ||
| // #_#_#_#_ | ||
| // | ||
| // .r = fill | ||
| // 0: #_#_#_#_ | ||
| // _#_#_#_# | ||
| // #_#_#_#_ | ||
| // _#_#_#_# | ||
| // | ||
| // 1: ######## | ||
| // ######## | ||
| // ######## | ||
| // ######## |
There was a problem hiding this comment.
Did we just encode the entire pattern in just THREE bits??? Wow.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
Shaded glyphs (U+2591..3, etc.) all have one problem in common:
The cell size may not be evenly divisible by the pixel/dot size in
the glyph. This either results in blurring, or in moiré-like patterns
at the edges of the cells with its neighbors, because they happen to
start with a pattern that overlaps with the end of the previous cell.
This PR solves the issue by moving the pixel/dot pattern generation
into the shader. That way the pixel/dot location can be made dependent
on the viewport-position of the actual underlying pixels, which avoids
repeating patterns between cells.
The PR contains some additional modifications, all of which either
extend or improve the existing debug facilities in AtlasEngine.
Suppressing whitespaces changes makes the diff way smaller.