Implemented support for user provided pixel shaders

[mrange]

Summary of the Pull Request

Allows users to provide pixel shader effects or pick one from a few presets such as the "retroII" preset.

Preliminary blog post on how to enable pixel shaders (private GIST): https://gist.github.com/mrange/1fdcfb7293f0299ecf994928f84d30b9

This built upon the existing experimental retro effect feature which lacked tests which means no tests were changed but none have been added (yet).

Testing was done exploratively with the different terminal settings.

References

#7013

#6191

PR Checklist

(These are not completed yet, will do obviously if the PR seems interesting)

• Closes: Feature Request: Allow background image to be pixel/fragment shader specified by me #7013
• CLA signed. If not, go over here and sign the CLA
• Tests added/passed
• Documentation updated. If checked, please file a pull request on our docs repo and link it here: #xxx
• Schema updated.
• I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #xxx

Detailed Description of the Pull Request / Additional comments

Awesome things can be done with pixel shaders that can spice up
the terminal such as retro looks, fractals, raytracers or star
scroller backgrounds.

This feature deprecates the old retro effect config option but
supports the retro effect through the new config option:

"experimental.pixelShaderEffect": "RETRO"

A new retro effect has been added as well:

"experimental.pixelShaderEffect": "RETROII"

It's also possible for users to implement their own pixel shader
and load them like so:

"experimental.pixelShaderEffect" : "C:\temp\RetroIII.hlsl"

Note: The escaping of paths is needed to construct a valid JSON string.

Error handling is changed so that if the shader fails to load for any
reason the exception is logged normally and it defaults to an error
shader showing the user a pattern that indicates that a pixel shader
effect was attempted but didn't work out.

Some simple example shaders are provided under: samples/PixelShaders/

Validation Steps Performed

Manual steps:

1. Enabling retro effect "experimental.pixelShaderEffect": "RETROII"
2. Checking the retro effects looks as expected.
3. Checking that settings is reloaded live
4. Enabling broken retro effect "experimental.pixelShaderEffect": "BROKEN"
5. Checking the terminal loads but display error pattern

[ghost]

All CLA requirements met.

[mrange]

Semantics changed so that this function now also updates the resource. This is because this is needed to be updated in more than one place now.

[zadjii-msft]

oh man, you gotta share screenshots

[mrange]

Current look of retroII:

[mrange]

How it looks when a shader fails to load:

[mrange]

Some TODOs remains in the code and I know the mispelling bot will complain.

[mrange]

@zadjii-msft pictures added.

[essenbee]

Looks great!

[WSLUser]

So I just realized this needs the Command Palette updated too. The retro Terminal is configured there and would clearly break if it wasn't adjusted to go to this new setting instead (it could also possibly give a crash if someone attempted to enable it). It can be a follow-up PR but it should be added not long afterwards so when the next release is out for Preview, we don't encounter unforeseen issues. @zadjii-msft would likely address it as the Command Palette is his creation.

[mrange]

If I implement support for the old experimental flag would the command palette still work? I am also unsure where the command palette is located so any pointers are welcome.

[WSLUser]

I would have the old setting value match the new value for 'Retro'. So experimental.retroTerminalEffect: true = experimental.pixelShaderEffect: "RETRO"

[zadjii-msft]

Okay so just a heads up - it might be a few days before anyone on the team has time to really take a good look at this PR. It's an awesome idea, and I'm excited to see what's possible with it.

My top-of-the-head concern is that we'd need to keep the old experimental.retroTerminalEffect setting around, but we'd probably use that as an alias for "experimental.pixelShaderEffect": "RETRO". So both settings would do the same thing, so users with the old setting aren't broken by the new feature.

IMO, if the shader doesn't work, I'd probably have the renderer default to just not doing anything with the shared at all, rather than showing an error pattern. I'm not the graphics expert on the team, so someone more qualified could overrule me.

As far as the command palette is concerned - the only real concern I have is the "toggleRetroEffect" action. Currently, that setting simply toggles the retro effect on/off, but with this feature, there's now a more complex matrix of features. Maybe that'll need to be similarly changed to toggleShaderEffect?

What happens when someone hits toggleRetroEffect with pixelShaderEffect set to RETRO? RETROII? some-path.hlsl?

TBH, we only introduced toggleRetroEffect like one release ago, so if we remove it for toggleShaderEffect, that's not the end of the world IMO

[zadjii-msft]

Also I'm seeing alot of std::optional<std::wstring> and some std::string_view. Can't we use FMT instead? Usage details here.

What are you talking about? fmt is a string formatting library. The strings still need to be stored in some type, a wstring seems perfectly fine to me...

[github-actions]

New misspellings found, please review:

• DOWNSCALE
• frac
• GAUSSIAN
• glsl
• hocevar
• iquilezles
• Kode
• KODELIFE
• opengl
• psin
• reso
• RETROII
• scanbri
• shsv
• spe
• sph
• spherefunctions
• Viginetting
• wz
• xyw
• yzx

To accept these changes, run the following commands

perl -e '
my @expect_files=qw('".github/actions/spell-check/expect/alphabet.txt
.github/actions/spell-check/expect/expect.txt
.github/actions/spell-check/expect/web.txt"');
@ARGV=@expect_files;
my @stale=qw('"atg BKMK colo consoleaccessibility cso Dst emptybox FFF Filledbox fullcolor hyperlink IDefault minmax NRCS phsl popclip remoting rerendered ROWSTOSCROLL SCS shobjidl stdarg stddef stdlib terminalnuget Tofill unfocuses xab xb xbc xca xce xdd xffff "');
my $re=join "|", @stale;
my $suffix=".".time();
my $previous="";
sub maybe_unlink { unlink($_[0]) if $_[0]; }
while (<>) {
  if ($ARGV ne $old_argv) { maybe_unlink($previous); $previous="$ARGV$suffix"; rename($ARGV, $previous); open(ARGV_OUT, ">$ARGV"); select(ARGV_OUT); $old_argv = $ARGV; }
  next if /^($re)(?:$| .*)/; print;
}; maybe_unlink($previous);'
perl -e '
my $new_expect_file=".github/actions/spell-check/expect/8cebe8ea4c0a8fe121fe8f525e1890a1bad82ba5.txt";
open FILE, q{<}, $new_expect_file; chomp(my @words = <FILE>); close FILE;
my @add=qw('"DOWNSCALE dst EMPTYBOX frac GAUSSIAN glsl hocevar iquilezles Kode KODELIFE nrcs opengl psin Remoting reso RETROII scanbri Scs Shobjidl shsv spe sph spherefunctions Viginetting wz xyw yzx "');
my %items; @items{@words} = @words x (1); @items{@add} = @add x (1);
@words = sort {lc($a) cmp lc($b)} keys %items;
open FILE, q{>}, $new_expect_file; for my $word (@words) { print FILE "$word\n" if $word =~ /\w/; };
close FILE;'
git add .github/actions/spell-check/expect || echo '... you want to ensure .github/actions/spell-check/expect/8cebe8ea4c0a8fe121fe8f525e1890a1bad82ba5.txt is added to your repository...'

✏️ Contributor please read this

By default the command suggestion will generate a file named based on your commit. That's generally ok as long as you add the file to your commit. Someone can reorganize it later.

⚠️ The command is written for posix shells. You can copy the contents of each perl command excluding the outer ' marks and dropping any '"/"' quotation mark pairs into a file and then run perl file.pl from the root of the repository to run the code. Alternatively, you can manually insert the items...

If the listed items are:

• ... misspelled, then please correct them instead of using the command.
• ... names, please add them to .github/actions/spell-check/dictionary/names.txt.
• ... APIs, you can add them to a file in .github/actions/spell-check/dictionary/.
• ... just things you're using, please add them to an appropriate file in .github/actions/spell-check/expect/.
• ... tokens you only need in one place and shouldn't generally be used, you can add an item in an appropriate file in .github/actions/spell-check/patterns/.

See the README.md in each directory for more information.

🔬 You can test your commits without appending to a PR by creating a new branch with that extra change and pushing it to your fork. The :check-spelling action will run in response to your push -- it doesn't require an open pull request. By using such a branch, you can limit the number of typos your peers see you make. 😉

⚠️ Reviewers

At present, the action that triggered this message will not show its ❌ in this PR unless the branch is within this repository.
Thus, you should make sure that this comment has been addressed before encouraging the merge bot to merge this PR.

[mrange]

@zadjii-msft Yes, it is clear to me that the old experimental feature needs to be supported. I think it being an alias for "RETRO" is fine. When it comes to toggleRetroEffect Ì agree a new toggle should be introduced toggleShaderEffect and that it reasonable that they actually both do the same thing.

When it comes to broken shader code, if there's a good way to make the user aware (and possibly a shader dev) the shader load failed that should be used instead. However, I wouldn't like a pop-up myself and would in that case prefer the background to indicate the loading failed.

As a shader dev I really like the quick iteration cycles from change to seeing it (less than 1 sec typically). When I try to tinker and get my shader working with windows terminal I would like that iteration cycle to be as quick as possible. Preferably even support hot-reload on file change. Having a pop-up that I have to click away each time would irritate me.

No indication at all would possibly be confusing. Did I manage to activate the feature or did it fail? At least with a failing shader background the user knows that windows terminal tried to load the shader but something happened.

[github-actions]

New misspellings found, please review:

• DOWNSCALE
• frac
• GAUSSIAN
• glsl
• hocevar
• iquilezles
• Kode
• KODELIFE
• opengl
• precendence
• psin
• reso
• RETROII
• scanbri
• shsv
• spe
• sph
• spherefunctions
• Viginetting
• wz
• xyw
• yzx

To accept these changes, run the following commands

perl -e '
my @expect_files=qw('".github/actions/spell-check/expect/alphabet.txt
.github/actions/spell-check/expect/expect.txt
.github/actions/spell-check/expect/web.txt"');
@ARGV=@expect_files;
my @stale=qw('"atg BKMK colo consoleaccessibility cso Dst emptybox FFF Filledbox fullcolor hyperlink IDefault minmax NRCS phsl popclip remoting rerendered ROWSTOSCROLL SCS shobjidl stdarg stddef stdlib terminalnuget Tofill unfocuses xab xb xbc xca xce xdd xffff "');
my $re=join "|", @stale;
my $suffix=".".time();
my $previous="";
sub maybe_unlink { unlink($_[0]) if $_[0]; }
while (<>) {
  if ($ARGV ne $old_argv) { maybe_unlink($previous); $previous="$ARGV$suffix"; rename($ARGV, $previous); open(ARGV_OUT, ">$ARGV"); select(ARGV_OUT); $old_argv = $ARGV; }
  next if /^($re)(?:$| .*)/; print;
}; maybe_unlink($previous);'
perl -e '
my $new_expect_file=".github/actions/spell-check/expect/942f3a5c2c599e21af9249c4b402c8b5255d07ce.txt";
open FILE, q{<}, $new_expect_file; chomp(my @words = <FILE>); close FILE;
my @add=qw('"DOWNSCALE dst EMPTYBOX frac GAUSSIAN glsl hocevar iquilezles Kode KODELIFE nrcs opengl precendence psin Remoting reso RETROII scanbri Scs Shobjidl shsv spe sph spherefunctions Viginetting wz xyw yzx "');
my %items; @items{@words} = @words x (1); @items{@add} = @add x (1);
@words = sort {lc($a) cmp lc($b)} keys %items;
open FILE, q{>}, $new_expect_file; for my $word (@words) { print FILE "$word\n" if $word =~ /\w/; };
close FILE;'
git add .github/actions/spell-check/expect || echo '... you want to ensure .github/actions/spell-check/expect/942f3a5c2c599e21af9249c4b402c8b5255d07ce.txt is added to your repository...'

✏️ Contributor please read this

By default the command suggestion will generate a file named based on your commit. That's generally ok as long as you add the file to your commit. Someone can reorganize it later.

⚠️ The command is written for posix shells. You can copy the contents of each perl command excluding the outer ' marks and dropping any '"/"' quotation mark pairs into a file and then run perl file.pl from the root of the repository to run the code. Alternatively, you can manually insert the items...

If the listed items are:

• ... misspelled, then please correct them instead of using the command.
• ... names, please add them to .github/actions/spell-check/dictionary/names.txt.
• ... APIs, you can add them to a file in .github/actions/spell-check/dictionary/.
• ... just things you're using, please add them to an appropriate file in .github/actions/spell-check/expect/.
• ... tokens you only need in one place and shouldn't generally be used, you can add an item in an appropriate file in .github/actions/spell-check/patterns/.

See the README.md in each directory for more information.

🔬 You can test your commits without appending to a PR by creating a new branch with that extra change and pushing it to your fork. The :check-spelling action will run in response to your push -- it doesn't require an open pull request. By using such a branch, you can limit the number of typos your peers see you make. 😉

⚠️ Reviewers

At present, the action that triggered this message will not show its ❌ in this PR unless the branch is within this repository.
Thus, you should make sure that this comment has been addressed before encouraging the merge bot to merge this PR.

[mrange]

Another snapshot of retroII:

[mrange]

Also been experimenting with optimizing the redraw process to enable animated pixel shader backgrounds:

[AnuthaDev]

Maybe you should lock the aspect ratio to 4:3 in retro mode, for a truly retro experience (lol, jk😅😅)

[mrange]

TODO: Can noexcept be added here when this returns a value?

[mrange]

I am now more "worried" over HSV2RGB functions as no license have been specified for these functions. Since the author posted it in Stackoverflow he has consented by TOS that they are licensensed under CC BY-SA 3.0 (as his post was made in 2013, see: https://stackoverflow.com/help/licensing). CC BY-SA requires Attribution (check, I think) and ShareAlike (not sure if MIT is compatible in this regard).

I am not sure how "excerpts" works, the argument was a little hand-wavy by Jeff Atwood IMHO.

[mrange]

Hi. I now clarfied (I hope) the licensing of the external work RetroII depends on. I think the ray sphere intersection code is in the clear as it's the same license as Windows Terminal (ie MIT). See https://www.shadertoy.com/view/4d2XWV for evidence.

The HSV to RGB conversion is CC BY-SA 3.0 which I think most likely isn't compatible with MIT (not a copyright lawyer so just a cautious guess). Your legal counsel might have some good advice on how to deal with it.

Depending on the advice I could:

1. Remove HSV to RGB conversion function. Not critical but nice.
2. Remove RetroII from the PR.

I prefer 1 as it would simplify for users to start using RetroII.

The other work is done by me and I just need to clear with my employer that I can sign the CLA (I don't foresee any problems here).

[mrange]

@DHowett Just FYI; I think the ray sphere intersection code is fine but now I am more uncertain about the HSV2RGB code taken from Stackoverflow. Have you had the chance to check with copyright experts on what your stance on code from Stackoverflow is?

[skyline75489]

I remember reading somewhere about code on StackOverflow being public domain (maybe from places like https://meta.stackexchange.com/q/12527) . But I'm not sure if Creative Common/Public Domain is good for company governance of MSFT.

Come to think about it, we may want to add some kind of instruction on README, for people who wants to contribute to realize that due to the fact this is a company project and the majority of the code was originally from Windows OS source repo and will be merged back to the OS repo, the license restriction is far more strict.

CC @DHowett

[mrange]

I am not a copyright lawyer with FOSS specialization but AFAIK Creative Commons 0 (https://creativecommons.org/share-your-work/public-domain/cc0/) is close to public domain in spirit (no rights reserved). Creative Commons 4.0 Share-Alike with Attribution Required (https://creativecommons.org/licenses/by-sa/4.0/) is not public domain as this comes with terms (share alike and attribution required) and freedoms (alot given the terms are fulfilled).

I will try to contact the author of the HSV2RGB functions and see what license he is licensing them under. If I can't reach the author or I get a negative response I will change the code to not use them.

[mrange]

I've tried to reach the author of of HSV2RGB but haven't been successful so I decided to remove the functions as it's unclear if the Stackoverflow license applies and is compatible with the MIT license of Terminal.

However, when I move to master I run into this. Have anyone seen it?

Unhandled exception at 0x00007FFFFD7BB3CC (KernelBase.dll) in WindowsTerminal.exe: 0xC000027B: Ett programinternt undantagsfel har uppstått (parameters: 0x000001B79CF7E060, 0x0000000000000004).

Happens at:

while (GetMessage(&message, nullptr, 0, 0))

And unfortunately the callstack didn't tell anything. I checked if there was changes to README for updated dependencies but didn't find any. Has there been changes in SDK/VS requirments that hasn't been documented yet?

[DHowett]

Sorry about that (and for the radio silence); there’s a commit on main that is causing a failure on startup. If you want to revert it, it’s the one that “updates MUX to 2.5”.

[mrange]

Removed the dependency on HSV2RGB due to uncertainities if Stack overflow code can be safely mixed with MIT licensed code.

The ray-sphere intersection code is confirmed to be under the MIT license from discussion with the author directly as well as seen from the examples on shadertoy. IMO (not a copyright lawyer) as this is the same license as Windows Terminal I believe it should be ok to use the code in Windows Terminal.

Rebasing was difficult due to significant changes to Profile.* files so I created an entire new commit with all changes.

Due to problems with commit: "Update to a newer MUX prerelease; remove workaround for compact sizing" (#7447) 5330759c0f3d98b92209813c48e781a664fe22eb the code is based on the prior commit .

[github-actions]

New misspellings found, please review:

• intersectors
• Kodelife
• sbri
• scol
• XWV

To accept these changes, run the following commands

perl -e '
my @expect_files=qw('".github/actions/spell-check/expect/alphabet.txt
.github/actions/spell-check/expect/expect.txt
.github/actions/spell-check/expect/web.txt"');
@ARGV=@expect_files;
my @stale=qw('"Autogenerated cso debugbreak DECLL DECSMBV Inplace Kode notypeopt restrictederrorinfo Scs shsv spherefunctions Switchto Wlk wz xyw yzx "');
my $re=join "|", @stale;
my $suffix=".".time();
my $previous="";
sub maybe_unlink { unlink($_[0]) if $_[0]; }
while (<>) {
  if ($ARGV ne $old_argv) { maybe_unlink($previous); $previous="$ARGV$suffix"; rename($ARGV, $previous); open(ARGV_OUT, ">$ARGV"); select(ARGV_OUT); $old_argv = $ARGV; }
  next if /^($re)(?:$| .*)/; print;
}; maybe_unlink($previous);'
perl -e '
my $new_expect_file=".github/actions/spell-check/expect/17b7e2595db7fba5e22b4baecfdb80ed569afef2.txt";
open FILE, q{<}, $new_expect_file; chomp(my @words = <FILE>); close FILE;
my @add=qw('"autogenerated inplace intersectors Kodelife sbri scol XWV "');
my %items; @items{@words} = @words x (1); @items{@add} = @add x (1);
@words = sort {lc($a) cmp lc($b)} keys %items;
open FILE, q{>}, $new_expect_file; for my $word (@words) { print FILE "$word\n" if $word =~ /\w/; };
close FILE;'
git add .github/actions/spell-check/expect || echo '... you want to ensure .github/actions/spell-check/expect/17b7e2595db7fba5e22b4baecfdb80ed569afef2.txt is added to your repository...'

✏️ Contributor please read this

By default the command suggestion will generate a file named based on your commit. That's generally ok as long as you add the file to your commit. Someone can reorganize it later.

⚠️ The command is written for posix shells. You can copy the contents of each perl command excluding the outer ' marks and dropping any '"/"' quotation mark pairs into a file and then run perl file.pl from the root of the repository to run the code. Alternatively, you can manually insert the items...

If the listed items are:

• ... misspelled, then please correct them instead of using the command.
• ... names, please add them to .github/actions/spell-check/dictionary/names.txt.
• ... APIs, you can add them to a file in .github/actions/spell-check/dictionary/.
• ... just things you're using, please add them to an appropriate file in .github/actions/spell-check/expect/.
• ... tokens you only need in one place and shouldn't generally be used, you can add an item in an appropriate file in .github/actions/spell-check/patterns/.

See the README.md in each directory for more information.

🔬 You can test your commits without appending to a PR by creating a new branch with that extra change and pushing it to your fork. The :check-spelling action will run in response to your push -- it doesn't require an open pull request. By using such a branch, you can limit the number of typos your peers see you make. 😉

⚠️ Reviewers

At present, the action that triggered this message will not show its ❌ in this PR unless the branch is within this repository.
Thus, you should make sure that this comment has been addressed before encouraging the merge bot to merge this PR.

[mrange]

Read the CLA more closely . It seems to indicate that I should separate the MIT licensed code that is not my original work (ray sphere intersection) in a separate PR and submit that separately. That can be done.

[mrange]

Is it required to extract the MIT licensed code into a separate commit to be eligible for a merge?

[DHowett]

@mrange While I'm waiting for word -- sorry, the process has been slower than I had hoped -- it might be best if we restructure this code into two pull requests, yes. If we can get one that adds custom shader infrastructure and pulls out the existing retro effect, and then one that adds any additional shaders that will need individual licensing review, that would be the quickest path forward.
d

[ghost]

This pull request has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 7 days. It will be closed if no further activity occurs within 7 days of this comment.

[mrange]

Hi. Been swamped at work with work so haven't had much chance to react to the feedback yet.

[zadjii-msft]

Hey, I know it's been a while since there's been any movement on this PR. I really love it, and I want to ship it in the Terminal. I can understand being busy, this year more than ever, and I didn't want to have this work lost. If it's okay with you @mrange, I've taken the liberty of taking this PR, merging the last few months of changes, and polishing it over in #8565. Hopefully we can use that to get this feature in finally (with credit where credit is due to you @mrange).

Thanks for all this!