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

Add a path parameter to toggleShaderEffects #9898

Open
JSTee787 opened this issue Apr 19, 2021 · 8 comments
Open

Add a path parameter to toggleShaderEffects #9898

JSTee787 opened this issue Apr 19, 2021 · 8 comments
Labels
Area-Rendering Text rendering, emoji, complex glyph & font-fallback issues Area-Settings Issues related to settings and customizability, for console or terminal Help Wanted We encourage anyone to jump in on these. Issue-Task It's a feature request, but it doesn't really need a major design. Product-Terminal The new Windows Terminal.
Milestone

Comments

@JSTee787
Copy link

Add an "enablePixelShader" setting, which can be set to true or false.

If set to "false", but a valid pixelShaderPath was specified, this would enable Windows Terminal to be started without the pixel shader visible, but available at the press of a key combination.

@JSTee787 JSTee787 added the Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. label Apr 19, 2021
@ghost ghost added Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Tag-Fix Doesn't match tag requirements labels Apr 19, 2021
@zadjii-msft
Copy link
Member

You know, this isn't a bad idea, but maybe we should turn it around. Rather than adding a setting to start with the shader disabled, and then use toggleShaderEffects to enable it, what if we let toggleShaderEffects accept a path parameter? Then you could press a key and enable one shader, then press a different key and enable a totally different shader. Then, the profile wouldn't have a shader set in it's own settings, but the keybindings could apply to any profile.

@zadjii-msft zadjii-msft changed the title Add an "enablePixerShader" setting Add a path parameter to toggleShaderEffects Apr 20, 2021
@zadjii-msft zadjii-msft added Area-Rendering Text rendering, emoji, complex glyph & font-fallback issues Area-Settings Issues related to settings and customizability, for console or terminal good first issue This is a fix that might be easier for someone to do as a first contribution Help Wanted We encourage anyone to jump in on these. Issue-Task It's a feature request, but it doesn't really need a major design. Product-Terminal The new Windows Terminal. and removed Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. labels Apr 20, 2021
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Apr 20, 2021
@zadjii-msft zadjii-msft added this to the Terminal Backlog milestone Apr 20, 2021
@JSTee787
Copy link
Author

Ok, I like that idea, in addition toggleShaderEffects would have to have some means of specifying that pixel shader should be turned off, either "null" or an empty string for the path (I'm not sure what best fits your philosophy).

Keep the existing pixelShaderPath setting, which could be included in "defaults", and overridden in a particular profile.

The final touch would be to have a list populated with the available shader names, and then have a mechanism and UI similar to the "next tab" mechanism, where you could scroll through the different shaders.

@zadjii-msft
Copy link
Member

I would love to be able to auto-populate the list of available shaders, but that might not be possible - they could really be anywhere in the OS.

That being said, I also love the idea of previewing the shaders. It would fit in nice with #9818. You'd probably have to manually specify a list of toggleShaderEffects commands with each of the possible shaders, but then we could definitely live preview them. You'd basically get a, but with shaders.

We'd definitely need to work out the edge cases here.

  • toggleShaderEffects(path=null) would probably still need to act the same as it currently does - toggle on and off.
  • toggleShaderEffects(path="") would... always disable? We actually don't have a good way of differentiating between null and "", so that might need to behave like the previous entry.
  • toggleShaderEffects(path="foo.hlsl") If foo.hlsl is the current shader path, and shader effects are enabled, disable the shader effects. Otherwise, set the shader path to foo.hlsl and enable the effects.

okay that was easier than I expected

@JSTee787
Copy link
Author

JSTee787 commented Apr 20, 2021

What about a "pixelShaders" section similar to the "schemes" section

"pixelShaders":
[
{
"pixelShaderPath": "path.to.pixel.shader1.hlsl",
"name": "Crt"
},
{
"pixelShaderPath": "path.to.pixel.shader2.hlsl",
"name": "invert"
}
]

This would make the list available and elsewhere the shaders could just be referred to by name.

@zadjii-msft zadjii-msft removed good first issue This is a fix that might be easier for someone to do as a first contribution Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels Apr 29, 2021
@zadjii-msft zadjii-msft modified the milestones: Terminal Backlog, Backlog Jan 4, 2022
@Welding-Torch
Copy link

@zadjii-msft So when are we getting a Select Shader... entry in the command palette? That'd be really cool and make shaders a lot more accessible.

@zadjii-msft
Copy link
Member

When someone contributes the code 😜 As it currently stands, this probably isn't something that the core team will have dedicated time to get around to. But I'd be more than happy to help guide someone who wanted to contribute this on their own.

@Welding-Torch
Copy link

I see. So it's not a priority for the core team? That's a bit sad, it could add a lot of fun to Windows Terminal.

@zadjii-msft
Copy link
Member

Alas, no. We've got to make pretty hard cuts on what we can actually prioritize. I sure agree, shaders are the kind of thing that sparks joy. But we've only got so many dev hours a week to go around. We tend to prioritize requests vaguely around 👍's. So go ahead and hit that like and subscribe to help prioritize this 😉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Rendering Text rendering, emoji, complex glyph & font-fallback issues Area-Settings Issues related to settings and customizability, for console or terminal Help Wanted We encourage anyone to jump in on these. Issue-Task It's a feature request, but it doesn't really need a major design. Product-Terminal The new Windows Terminal.
Projects
None yet
Development

No branches or pull requests

3 participants