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

Implement user-specified pixel shaders, redux #8565

Merged
13 commits merged into from
Dec 15, 2020

Conversation

zadjii-msft
Copy link
Member

@zadjii-msft zadjii-msft commented Dec 11, 2020

Co-authored-by: mrange [email protected]

I loved the pixel shaders in #7058, but that PR needed a bit of polish
to be ready for ingestion. This PR is almost exactly that PR, with
some small changes.

  • It adds a new pre-profile setting "experimental.pixelShaderPath",
    which lets the user set a pixel shader to use with the Terminal.
  • It adds a bunch of sample shaders in samples/shaders. Included:
    • A NOP shader as a base to build from.
    • An "invert" shader that inverts the colors, as a simple example
    • An "grayscale" shader that converts all colors to grayscale, as a
      simple example
    • An "raster bars" shader that draws some colored bars on the screen
      with a drop shadow, as a more involved example
    • The original retro terminal effects, as a more involved example
    • It also includes a broken shader, as an example of what heppens
      when the shader fails to compile
    • CHANGED FROM Implemented support for user provided pixel shaders #7058: It does not add the "retroII" shader we were
      all worried about.
  • When a shader fails to be found or fails to compile, we'll display an
    error dialog to the user with a relevant error message.
  • Renames the toggleRetroEffect action to toggleShaderEffect.
    (toggleRetroEffect is now an alias to toggleShaderEffect). This
    action will turn the shader OR the retro effects on/off.

toggleShaderEffect works the way you'd expect it to, but the mental
math on how is a little weird. The logic is basically:

useShader = shaderEffectsEnabled ? 
                (pixelShaderProvided ? 
                    pixelShader : 
                    (retroEffectEnabled ? 
                        retroEffect : null
                    )
                ) : 
                null

and toggleShaderEffect toggles shaderEffectsEnabled.

  • If you've got both a shader and retro enabled, toggleShaderEffect
    will toggle between the shader on/off.
  • If you've got a shader and retro disabled, toggleShaderEffect will
    toggle between the shader on/off.

References #6191
References #7058

Closes #7013

Closes #3930 "Add setting to retro terminal shader to control blur
radius, color"
Closes #3929 "Add setting to retro terminal shader to enable drawing
scanlines"
- At this point, just roll your own version of the shader.

mrange and others added 5 commits December 10, 2020 12:30
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 commit adds the capability for the user to create their
own pixel shader effects.

In addition a new retro effect has been added as built-in pixel
shader:
"experimental.pixelShaderEffect": "RETROII"

The existing retro effect is possible to enable as well:
"experimental.pixelShaderEffect": "RETRO"

In order to for a user to create their own pixel shader effects
specify a path rather than a built-in effect:
"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.

Example shaders are provided under: samples/PixelShaders/

A new command has been added: toggleTerminalEffects
This command turns on and off configured terminal effects, if no
terminal effects are configured this command does nothing.

The existing command: toggleRetroEffect
is changed to have the same semantices as the new command. This is a
change in behaviour that was discussed with the maintainers.

The RETROII pixel shader relies on a ray-sphere intersection function
developed by Inigo Quilez and released with the MIT license.
For an example of the ray-sphere function in action:
https://www.shadertoy.com/view/4d2XWV
@ghost ghost added Area-Extensibility A feature that would ideally be fulfilled by us having an extension model. Area-Rendering Text rendering, emoji, complex glyph & font-fallback issues Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. Issue-Task It's a feature request, but it doesn't really need a major design. Product-Terminal The new Windows Terminal. labels Dec 11, 2020
doc/cascadia/profiles.schema.json Outdated Show resolved Hide resolved
src/cascadia/TerminalControl/TermControl.cpp Outdated Show resolved Hide resolved
src/renderer/dx/DxRenderer.cpp Outdated Show resolved Hide resolved
src/renderer/dx/DxRenderer.cpp Show resolved Hide resolved
@ghost ghost added the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Dec 11, 2020
@WSLUser
Copy link
Contributor

WSLUser commented Dec 11, 2020

So https://stackoverflow.com/questions/15095909/from-rgb-to-hsv-in-opengl-glsl had an update for the license after requesting it. It's under https://en.wikipedia.org/wiki/WTFPL. Can this be looked into legally if it can be ingested like it originally was going to be? It would be extremely useful if it could be. If it's able to be, that can be a separate PR. We've already waited long enough for this.

@DHowett
Copy link
Member

DHowett commented Dec 11, 2020

@WSLUser Thanks for that! The rgb-hsv routines weren't my only concern, but this helps.

@ghost ghost removed the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Dec 14, 2020
src/cascadia/TerminalApp/Resources/en-US/Resources.resw Outdated Show resolved Hide resolved
src/cascadia/TerminalApp/TerminalSettings.cpp Outdated Show resolved Hide resolved
src/cascadia/TerminalControl/TermControl.cpp Outdated Show resolved Hide resolved
@@ -179,11 +179,22 @@
<value>Ctrl+Click to follow link</value>
</data>
<data name="NoticeFontNotFound" xml:space="preserve">
<value>Unable to find the selected font "{0}".
<value>Unable to find the selected font "{0}".
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i hope the whitespace change here doesn't cause a re-loc

src/cascadia/TerminalControl/TermControl.cpp Outdated Show resolved Hide resolved
src/renderer/dx/DxRenderer.cpp Outdated Show resolved Hide resolved
src/renderer/dx/DxRenderer.cpp Outdated Show resolved Hide resolved
src/renderer/dx/DxRenderer.cpp Show resolved Hide resolved
src/renderer/dx/DxRenderer.cpp Show resolved Hide resolved
src/renderer/dx/DxRenderer.cpp Outdated Show resolved Hide resolved
@ghost ghost added the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Dec 15, 2020
@zadjii-msft zadjii-msft assigned zadjii-msft and unassigned miniksa Dec 15, 2020
@ghost ghost removed the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Dec 15, 2020
@zadjii-msft zadjii-msft assigned DHowett and unassigned zadjii-msft Dec 15, 2020
zadjii-msft added a commit to MicrosoftDocs/terminal that referenced this pull request Dec 15, 2020
Copy link
Member

@DHowett DHowett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's do this.

@DHowett DHowett added the AutoMerge Marked for automatic merge by the bot when requirements are met label Dec 15, 2020
@ghost
Copy link

ghost commented Dec 15, 2020

Hello @DHowett!

Because this pull request has the AutoMerge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

@ghost ghost merged commit b140299 into main Dec 15, 2020
@ghost ghost deleted the dev/migrie/f/lets-ship-pixel-shaders branch December 15, 2020 20:40
mpela81 pushed a commit to mpela81/terminal that referenced this pull request Jan 28, 2021
Co-authored-by: mrange <[email protected]>

I loved the pixel shaders in microsoft#7058, but that PR needed a bit of polish
to be ready for ingestion. This PR is almost _exactly_ that PR, with
some small changes.

* It adds a new pre-profile setting `"experimental.pixelShaderPath"`,
  which lets the user set a pixel shader to use with the Terminal.
    - CHANGED FROM microsoft#7058: It does _not_ add any built-in shaders.
    - CHANGED FROM microsoft#7058: it will _override_
      `experimental.retroTerminalEffect`
* It adds a bunch of sample shaders in `samples/shaders`. Included: 
    - A NOP shader as a base to build from.
    - An "invert" shader that inverts the colors, as a simple example
    - An "grayscale" shader that converts all colors to grayscale, as a
      simple example
    - An "raster bars" shader that draws some colored bars on the screen
      with a drop shadow, as a more involved example
    - The original retro terminal effects, as a more involved example
    - It also includes a broken shader, as an example of what heppens
      when the shader fails to compile
    - CHANGED FROM microsoft#7058: It does _not_ add the "retroII" shader we were
      all worried about.
* When a shader fails to be found or fails to compile, we'll display an
  error dialog to the user with a relevant error message.
    - CHANGED FROM microsoft#7058: Originally, microsoft#7058 would display "error bars"
      on the screen. I've removed that, and had the Terminal disable the
      shader entirely then.
* Renames the `toggleRetroEffect` action to `toggleShaderEffect`.
  (`toggleRetroEffect` is now an alias to `toggleShaderEffect`). This
  action will turn the shader OR the retro effects on/off. 

`toggleShaderEffect` works the way you'd expect it to, but the mental
math on _how_ is a little weird. The logic is basically:

```
useShader = shaderEffectsEnabled ? 
                (pixelShaderProvided ? 
                    pixelShader : 
                    (retroEffectEnabled ? 
                        retroEffect : null
                    )
                ) : 
                null
```

and `toggleShaderEffect` toggles `shaderEffectsEnabled`.

* If you've got both a shader and retro enabled, `toggleShaderEffect`
  will toggle between the shader on/off.
* If you've got a shader and retro disabled, `toggleShaderEffect` will
  toggle between the shader on/off.

References microsoft#6191
References microsoft#7058

Closes microsoft#7013

Closes microsoft#3930 "Add setting to retro terminal shader to control blur
radius, color" 
Closes microsoft#3929 "Add setting to retro terminal shader to enable drawing
scanlines" 
     - At this point, just roll your own version of the shader.
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Extensibility A feature that would ideally be fulfilled by us having an extension model. Area-Rendering Text rendering, emoji, complex glyph & font-fallback issues AutoMerge Marked for automatic merge by the bot when requirements are met Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. 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
5 participants