terminal
terminal copied to clipboard
Added experimental.pixelShaderImagePath
I realize I might be one of the few developers that care about custom shader support in terminal but I thought it's worth proposing it and see what you think.
This is to support custom shaders with custom textures.
I was thinking of exposing the background image to the shader but that felt complicated after looking into it.
I have tested exploratively. I think the texture loader is possible to unit test so that is a possible improvement.
The error reporting (as with other custom pixel shader code) is not very good. That is also an area that I could improve upon.
I do think the risk of adding this is rather low as the new code is only executed when experimental.pixelShaderImagePath is set.
Details
Only added to the Atlas engine. Unsure if needs adding in DXEngine?
Instead I load the texture using WIC into a shader resource view. When binding shader resources I test for presence of custom texture and bind it to register t1.
The image loading code was found in the D3D Texture documentation. It's a mouthful but seems rather robust.
Tested setting: "experimental.pixelShaderImagePath"
- Tested not specifying it.
- Tested setting it.
- Tested changing it (the changes are picked up)
- Tested invalid path
- Tested a custom shader that made use of the custom texture.
The blazor logo is a custom shader image.
I get a bunch of spell check issues in code I haven't modified. What should I do about them?
Finally all checks are green
Exciting!
Thanks for the review. I am a little bit short in time during weekdays, but I think I can address some of your concerns over the weekend.
I have to confess as a user, I'm unsure what the difference between this and the already existing shader support is. I don't think we should have 2 settings regarding shaders. That'll be very confusing. If there's some type of support missing from the other shader code, could you simply merge the 2 instead so we only have a single user facing option to specify a shader file?
@WSLUser I don't mind merging the options but this setting is to support loading a shader texture like a PNG that the shader source code can make use of. I am testing with these settings:
"defaults":
{
"experimental.pixelShaderImagePath": "C:\\temp\\cake.png",
"experimental.pixelShaderPath": "C:\\code\\github\\windows-terminal-shaders\\cake.hlsl"
},
So the pixelShaderImagePath
points to a PNG loaded as a texture which the shader pointed out by pixelShaderPath
uses.
So in my example it looks like this:
The background is computed by the shader but cake logo comes from the PNG which the shader overlays over the background.
How would you prefer the configuration option to look like?
Ok I think I see what you mean. That makes more sense but this will definitely need some documentation to avoid the confusion I had. In this instance I see that 2 settings will in fact be needed but they should be grouped together as this one requires the other one to be set (so the shader path should be set as a hard dependency before trying to fetch a ShaderImage path). I think you might be able to use the json schema for this, which will need updating regardless.
@lhecker in response to your comment on looking at DirectXTK for texture loading code I made the change that I added that repo as a submodule. I noticed you had submodules already so I figured it wouldn't be too much of a paradigm shift to add another one. Not sure this is a how you like it done but this is my current proposal.
If we are to go with the submodule I would like some advice on best way on how to include the source code from it. The approach taken works it seems but is it a good enough solution?
So because of the "controversial" addition of a submodule I consider this PR to be a proposal with need of more testing cleanup to be merged. Seemed to work fine in my smoketests
Bump. Still looking for feedback if the latest approach is acceptable (ie bring in DirectXTK as a submodule)
@mrange I think it would've been fine if you just copied DirectXTK's source code into this project. But since it's just a submodule, it's easy to add/remove it and so I'm also fine with your new approach. I think this is good to be merged as is. π
Spellchecker still complains about:
#### Unrecognized words (1)
otepad
But didn't find any otepad
strings in git repo.
(Thanks for being patient with us. I know that it's been some time. I wanted myself assigned so that I could vet the dependency and the risk we'd take on if we further brought it into Windows. We probably wouldn't be taking on any risk, but I just want to make sure.)
If you are concerned about bringing in the dependency into windows builds I could put some #ifdef
in there
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.
It's not exactly fair for me to take a while to respond and then let the bot call you slow, so I'm poking it along to remove the "no recent activity" tag :)
Hope to get some time coming weekend to look into comments. The weekends before been buzy.
Hey we're trying to clean out our PR backlog: I'm gonna tag this up for discussion next team sync and figure out if we're bringing this in or not. The change Dustin called out above was simple enough that we could probably do it on your behalf, but Leonard had some other thoughts on another way we could go about this.... something about comments and shadertoy and I honestly didn't understand all he said π
To add more notes we said after I sent that: Maybe we just pull this in as-is, because it handles a lot of the plumbing for loading images and passing them down. THEN, as a follow up, we remove this setting, and replace it with the ShaderToy thing. That's the great thing with it being an experimental setting - we can just remove it later if we have a better idea π
(there's also probably some work that needs doing to update this for the latest version of the Atlas Engine (which went through an entire re-write since this PR was opened)
@mrange you are the Terminal shader master though, so let us know what you're thinking. Sorry for letting this stagnate so long! Let's get this in for 1.21 βΊοΈ
Hi sorry for not answering. I have been travelling and doing shader presentations for .NET developers. Anyway, I will try to have an answer after work.
So I obviously like to see this merged and I think it will enable some cool things for us. The obvious thing is ofc an image like a logo but it will also enable importing noise images which are very useful things as it can be costly to do a compute for.
When I wrote it after I got the tip about using the external library I felt pretty confident that this was a rather safe change as it simplified the code quite much.
What is this shadertoy thing you are discussing?
Re-reading the PR. Most changes are just forwarding data and some spelling errors. The actual works is mostly handled by DirectXTk or rather simple (like checking if we have an extra texture and in that case include).
I think personally it's rather low risk especially since this is new experimental setting but obviously you might disagree
What is this shadertoy thing you are discussing?
I honestly don't know. @lhecker mentioned something about being able to pass a texture to the shader using a comment in the body of the shader itself, that shadertoy would then load as a texture for you? That was at the end of the day though and I probably misunderstood π
I can handle updating to main, and pulling in DirectXTK
as a raw include if you'd like. That'll get this in and get us moving βΊοΈ
EDIT: I may have spoke too quickly on the "I can merge it" thing. I forgot that Atlas engine was basically entirely re-written since this was opened. This is more than a trivial merge, but I'll give it a shot.
shadertoy has a very constrained template of things you can do in a shader: https://www.shadertoy.com/howto ...but it allows you to do some super impressive stuff: https://www.shadertoy.com/view/ssjyWc
I figured we could - over time - offer an API that is inspired by shadertoy's, but with the web interface configuration for the 4 channels and the buffer stages replaced with comments or similar that can be parsed during shader loading, or by having an additional .json file that contains such metadata.
I know that FX11 would fit quite well here, but it's not actively maintained, nor are there plans to port it to D3D12.
@check-spelling-bot Report
:red_circle: Please review
See the :open_file_folder: files view, the :scroll:action log, or :memo: job summary for details.
Unrecognized words (1)
BGRA
Previously acknowledged words that are now absent
AAAa ABORTIFHUNG Batang bgra chcbpat CLUSTERMAP COLORMATRIX COLR CUsers DBlob dcompile DESTINATIONNAME dxguid FFrom Geddy GETKEYSTATE GFEh hicon inputrc IWIC kcub kcud kcuf kcuu khome MAPVIRTUALKEY Mbxy Mip NOTIMEOUTIFNOTHUNG nto QUESTIONMARK reallocs reamapping Resequence RTFTo SFolder srv subresource tracelog TRIANGLESTRIP Uniscribe VKKEYSCAN waitable wic wincodec WScript XColors xff XMFLOAT π«₯To accept these unrecognized words as correct and remove the previously acknowledged and now absent words, you could run the following commands
... in a clone of the [email protected]:mrange/terminal.git repository
on the mrange/shader_bitmap
branch (:information_source: how do I use this?):
curl -s -S -L 'https://raw.githubusercontent.com/check-spelling/check-spelling/v0.0.22/apply.pl' |
perl - 'https://github.com/microsoft/terminal/actions/runs/8008136479/attempts/1'
Available :books: dictionaries could cover words (expected and unrecognized) not in the :blue_book: dictionary
This includes both expected items (2246) from .github/actions/spelling/expect/04cdb9b77d6827c0202f51acd4205b017015bfff.txt .github/actions/spelling/expect/alphabet.txt .github/actions/spelling/expect/expect.txt .github/actions/spelling/expect/web.txt and unrecognized words (1)
Dictionary | Entries | Covers | Uniquely |
---|---|---|---|
cspell:swift/src/swift.txt | 53 | 1 | 1 |
cspell:gaming-terms/dict/gaming-terms.txt | 59 | 1 | 1 |
cspell:monkeyc/src/monkeyc_keywords.txt | 123 | 1 | 1 |
cspell:cryptocurrencies/cryptocurrencies.txt | 125 | 1 | 1 |
cspell:scala/dict/scala.txt | 153 | 1 | 1 |
Consider adding them (in .github/workflows/spelling2.yml
) for uses: check-spelling/[email protected]
in its with
:
with:
extra_dictionaries:
cspell:swift/src/swift.txt
cspell:gaming-terms/dict/gaming-terms.txt
cspell:monkeyc/src/monkeyc_keywords.txt
cspell:cryptocurrencies/cryptocurrencies.txt
cspell:scala/dict/scala.txt
To stop checking additional dictionaries, add (in .github/workflows/spelling2.yml
) for uses: check-spelling/[email protected]
in its with
:
check_extra_dictionaries: ''
Errors (1)
See the :open_file_folder: files view, the :scroll:action log, or :memo: job summary for details.
:x: Errors | Count |
---|---|
:x: ignored-expect-variant | 4 |
See :x: Event descriptions for more information.
:pencil2: 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.
If the listed items are:
- ... misspelled, then please correct them instead of using the command.
- ... names, please add them to
.github/actions/spelling/allow/names.txt
. - ... APIs, you can add them to a file in
.github/actions/spelling/allow/
. - ... just things you're using, please add them to an appropriate file in
.github/actions/spelling/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/spelling/patterns/
.
See the README.md
in each directory for more information.
:microscope: 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. :wink:
If the flagged items are :exploding_head: false positives
If items relate to a ...
-
binary file (or some other file you wouldn't want to check at all).
Please add a file path to the
excludes.txt
file matching the containing file.File paths are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your files.
^
refers to the file's path from the root of the repository, so^README\.md$
would exclude README.md (on whichever branch you're using). -
well-formed pattern.
If you can write a pattern that would match it, try adding it to the
patterns.txt
file.Patterns are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your lines.
Note that patterns can't match multiline strings.
shadertoy has a very constrained template of things you can do in a shader: https://www.shadertoy.com/howto ...but it allows you to do some super impressive stuff: https://www.shadertoy.com/view/ssjyWc
I figured we could - over time - offer an API that is inspired by shadertoy's, but with the web interface configuration for the 4 channels and the buffer stages replaced with comments or similar that can be parsed during shader loading, or by having an additional .json file that contains such metadata.
I know that FX11 would fit quite well here, but it's not actively maintained, nor are there plans to port it to D3D12.
I have used shadertoy quite much :) (322 public shaders). The comment made me curious ofc.
Not surprisingly I would like to see more shader capabilities. While multipasses (and retaining the previous frame pass) is great I personally would think I would love some kind of live editing before to simplify development.
Imagine for example that windows terminal reloads the shader code automatically when changed. If something goes wrong catch the error and show it as the shader texture.
That way I could sit and develop shaders real-time in VIM. That would be a very cool demo! And also helps productivity.
Also would like to control the pixelshader version.
Then ofc capturing the windows sound and provide it as a texture with both the signal and the FFT opens up for cool stuff.
And so on :)
As for deploying shaders what about a folder with all the content in (also zip)?
In the folder images, Common.fx, BufferA.fx, BufferB.fx, BufferC.fx and Main.fx (only Main.fx required). Some kind of way to configure the input textures (not sure how).
Perhaps look at https://github.com/Gargaj/Bonzomatic for how they store "projects". Seems like pretty smart and supports DX. Bonzomatic is the goto tool for shader competitions and jams.