Shader files not loading in 1.7 #12833

Closed
opened 2026-01-31 03:26:09 +00:00 by claunia · 7 comments
Owner

Originally created by @Nacimota on GitHub (Mar 3, 2021).

Originally assigned to: @PankajBhojwani on GitHub.

Environment

Windows build number: 10.0.19042.0
Windows Terminal version: 1.7.572.0

Steps to reproduce

  1. Build/Install Windows Terminal 1.7.572.0
  2. Set experimental.pixelShaderPath on a profile (or globally) such that it points to an appropriately structured HLSL file (such as Invert.hlsl from the samples folder)
  3. Run the terminal (with the appropriate profile if necessary)

Expected behavior

The pixel shader is loaded and executed.

Actual behavior

The pixel shader is not loaded.

Comments

I have not been able to load any shader files at all in 1.7 from the official release, nor when I build it from source. Poking around in the debugger, I can see that the shader file path is definitely there when the JSON is read in:

CascadiaSettingsSerialization.cpp 1.7 branch

However it seems to disappear at some point between being read and sent to SetPixelShaderPath(). I'm not sure if it's being deserialized correctly because I don't really understand how I'm supposed to inspect the settings object; it doesn't behave the way I'd expect it to (just C++ things, I assume).

release-1.7

DxRenderer.cpp 1.7 branch

release-1.6

DxRenderer.cpp 1.6 branch

I've tried this on other machines that have never had any version of the Terminal installed and they all exhibit the same issue.

Originally created by @Nacimota on GitHub (Mar 3, 2021). Originally assigned to: @PankajBhojwani on GitHub. # Environment ```none Windows build number: 10.0.19042.0 Windows Terminal version: 1.7.572.0 ``` # Steps to reproduce 1. Build/Install Windows Terminal 1.7.572.0 2. Set `experimental.pixelShaderPath` on a profile (or globally) such that it points to an appropriately structured HLSL file (such as `Invert.hlsl` from the samples folder) 3. Run the terminal (with the appropriate profile if necessary) # Expected behavior The pixel shader is loaded and executed. # Actual behavior The pixel shader is not loaded. # Comments I have not been able to load any shader files at all in 1.7 from the official release, nor when I build it from source. Poking around in the debugger, I can see that the shader file path is definitely there when the JSON is read in: ![CascadiaSettingsSerialization.cpp 1.7 branch](https://user-images.githubusercontent.com/651955/109766645-2d063780-7c42-11eb-9724-ffde02aebf36.png) However it seems to disappear at some point between being read and sent to `SetPixelShaderPath()`. I'm not sure if it's being deserialized correctly because I don't really understand how I'm supposed to inspect the settings object; it doesn't behave the way I'd expect it to (just C++ things, I assume). ### release-1.7 ![DxRenderer.cpp 1.7 branch](https://user-images.githubusercontent.com/651955/109764925-df88cb00-7c3f-11eb-8c78-e297cf2c4382.png) ### release-1.6 ![DxRenderer.cpp 1.6 branch](https://user-images.githubusercontent.com/651955/109765459-9d13be00-7c40-11eb-981a-e67a96545f29.png) I've tried this on other machines that have never had any version of the Terminal installed and they all exhibit the same issue.
Author
Owner

@zadjii-msft commented on GitHub (Mar 3, 2021):

Well that's batty. Maybe that's what's causing #9338?

I'm gonna real quick @DHowett because he was helping investigate another bug where the shader path would get absolutely mangled before making it into the DxRenderer.

@zadjii-msft commented on GitHub (Mar 3, 2021): Well that's _batty_. Maybe that's what's causing #9338? I'm gonna real quick @DHowett because he was helping investigate another bug where the shader path would get absolutely mangled before making it into the DxRenderer.
Author
Owner

@Nacimota commented on GitHub (Mar 3, 2021):

Well that's batty. Maybe that's what's causing #9338?

I did see that issue, but I decided to post a separate one because they're using 1.6, which has no problems for me, and they're also experiencing an error message, which I'm not getting even if use a non-existent path (because it just becomes an empty string, I guess). Could still be related though? ¯\_(ツ)_/¯

@Nacimota commented on GitHub (Mar 3, 2021): > Well that's _batty_. Maybe that's what's causing #9338? I did see that issue, but I decided to post a separate one because they're using 1.6, which has no problems for me, and they're also experiencing an error message, which I'm not getting even if use a non-existent path (because it just becomes an empty string, I guess). Could still be related though? ¯\\\_(ツ)\_/¯
Author
Owner

@zadjii-msft commented on GitHub (Mar 3, 2021):

Yea, after re-reading #9338, they do definitely seem different. The warning dialog in that one's really throwing me for a loop. I mean, this one's crazy too. They're both crazy, just, each their own unique kind of crazy 😄

@zadjii-msft commented on GitHub (Mar 3, 2021): Yea, after re-reading #9338, they do definitely seem different. The warning dialog in that one's really throwing me for a loop. I mean, this one's crazy too. They're both crazy, just, each their own unique kind of crazy 😄
Author
Owner

@DHowett commented on GitHub (Mar 3, 2021):

@PankajBhojwani Check this out!

TerminalSettings.h:117

        GETSET_PROPERTY(hstring, PixelShaderPath);

But look at all the other settings ...

        GETSET_SETTING(TerminalApp::TerminalSettings, bool, RetroTerminalEffect, false);
        GETSET_SETTING(TerminalApp::TerminalSettings, bool, ForceFullRepaintRendering, false);
        GETSET_SETTING(TerminalApp::TerminalSettings, bool, SoftwareRendering, false);
        GETSET_SETTING(TerminalApp::TerminalSettings, bool, ForceVTInput, false);

When we create a new TerminalSettings leaf object, it has no PixelShaderPath because it is not an inheritable setting.

The danger of macros with a bunch of identical names.

@DHowett commented on GitHub (Mar 3, 2021): @PankajBhojwani Check this out! TerminalSettings.h:117 ``` GETSET_PROPERTY(hstring, PixelShaderPath); ``` But look at all the other settings ... ``` GETSET_SETTING(TerminalApp::TerminalSettings, bool, RetroTerminalEffect, false); GETSET_SETTING(TerminalApp::TerminalSettings, bool, ForceFullRepaintRendering, false); GETSET_SETTING(TerminalApp::TerminalSettings, bool, SoftwareRendering, false); GETSET_SETTING(TerminalApp::TerminalSettings, bool, ForceVTInput, false); ``` When we create a new TerminalSettings leaf object, it has no PixelShaderPath because _it is not an inheritable setting_. The danger of macros with a bunch of identical names.
Author
Owner

@zadjii-msft commented on GitHub (Mar 3, 2021):

Maybe we should GET rid of the GETSET_ prefix, because that's not really relevant anymore. Or maybe just change GETSET_SETTING to SETTING. That would be at least visually distinct and mentally distinct from GETSET_PROPERTY

@zadjii-msft commented on GitHub (Mar 3, 2021): Maybe we should `GET` rid of the `GETSET_` prefix, because that's not really relevant anymore. Or maybe just change `GETSET_SETTING` to `SETTING`. That would be at least visually distinct and mentally distinct from `GETSET_PROPERTY`
Author
Owner

@ghost commented on GitHub (Apr 14, 2021):

:tada:This issue was addressed in #9371, which has now been successfully released as Windows Terminal v1.7.1033.0.🎉

Handy links:

@ghost commented on GitHub (Apr 14, 2021): :tada:This issue was addressed in #9371, which has now been successfully released as `Windows Terminal v1.7.1033.0`.:tada: Handy links: * [Release Notes](https://github.com/microsoft/terminal/releases/tag/v1.7.1033.0) * [Store Download](https://www.microsoft.com/store/apps/9n8g5rfz9xk3?cid=storebadge&ocid=badge)
Author
Owner

@ghost commented on GitHub (Apr 14, 2021):

:tada:This issue was addressed in #9371, which has now been successfully released as Windows Terminal Preview v1.8.1032.0.🎉

Handy links:

@ghost commented on GitHub (Apr 14, 2021): :tada:This issue was addressed in #9371, which has now been successfully released as `Windows Terminal Preview v1.8.1032.0`.:tada: Handy links: * [Release Notes](https://github.com/microsoft/terminal/releases/tag/v1.8.1032.0) * [Store Download](https://www.microsoft.com/store/apps/9n8g5rfz9xk3?cid=storebadge&ocid=badge)
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/terminal#12833