bo3b / 3Dmigoto

Chiri's DX11 wrapper to enable fixing broken stereoscopic effects.
Other
760 stars 119 forks source link

Change giant numbers to FLT_MAX #52

Open bo3b opened 7 years ago

bo3b commented 7 years ago

Right now if the Decompiler hits a poorly dumped constant of: 340282346638528860000000000000000000000.000000

it will crash the game. That number is FLT_MAX and the disassembler is just doing a poor job with that. We need to catch these and convert them to 3.402823466e+38F.

Apparently there is no FLT_MAX macro or intrinsic for HLSL.

DarkStarSword commented 7 years ago

define FLT_MAX asfloat(0x7f7fffff)

We can also pass defines in to D3DCompile(), which I want to do for a few other things:

bo3b commented 7 years ago

define FLT_MAX asfloat(0x7f7fffff)

That seems like the best way to go.

define something to indicate what kind of shader it is (VERTEX_SHADER, PIXEL_SHADER, etc), allowing a single hlsl file to contain the main functions of multiple shaders stages in a custom shader (this approach is used by Unity and other engines in the real world).

I'd be hesitant to add something like this to our shader handling. Our 'customers' tend to be people who are not software developers, and making it more abstract and complicated to understand will lower the number of people who can ShaderHack. I think we need to keep it dead simple so that even novices can contribute.

DarkStarSword commented 7 years ago

define something to indicate what kind of shader it is (VERTEX_SHADER, PIXEL_SHADER, etc), allowing a single hlsl file to contain the main functions of multiple shaders stages in a custom shader (this approach is used by Unity and other engines in the real world).

I'd be hesitant to add something like this to our shader handling. Our 'customers' tend to be people who are not software developers, and making it more abstract and complicated to understand will lower the number of people who can ShaderHack. I think we need to keep it dead simple so that even novices can contribute.

The idea there is just for custom shaders - things like the 3dvision2sbs, software mouse, upscale and debug shaders - replaced shaders from the game wouldn't (and couldn't) use this. The reason is because the typical way to pass values between shader stages is to use a structure and have both vertex and pixel shaders defined in one file, as a pretty stupid but simple demonstration of this:

struct v2p {
    float4 pos : SV_POSITION;
    float4 uv : TEXCOORD0;
    float3 ray : TEXCOORD1;
};

#ifdef VERTEX_SHADER
v2p main (float4 vertex : POSITION, float3 normal : NORMAL)
{
    v2p o;
    o.pos = 0;
    o.uv = 1;
    o.ray = 2;

    return o;
}
#endif

#ifdef PIXEL_SHADER
float4 main (v2p i) : SV_Target
{
    return i.uv;
}
#endif

That makes it easier to change the values passed between shaders because it guarantees that they are always consistent with each other, and it means less custom hlsl files to ship and puts everything relevant to a custom shader in one spot. This also makes it easier to import existing third party effects that are already structured like this. All that would happen is that the custom shader section would specify the same hlsl file for multiple stages:

[CustomShaderFoo]
vs = ShaderFixes/foo.hlsl
ps = ShaderFixes/foo.hlsl
bo3b commented 7 years ago

That sounds good. As long as we have a simple path for our beginners, I'm happy. And our more pro users have a lot of flexibility.


Would this also work for custom value sharing from VS to PS, in normal shaders, not custom shaders? Some of our more advanced people are already doing this, by manually adding extra outputs to the VS HLSL and then inputs to PS to pass matrices they want in PS.

This technique is not quite for that scenario because of the more random input/output nature of a game shader combo, but presumably they could do this now in a similar way. Not sure it's worth that effort though, and generally speaking I haven't seen our hackers try to simplify stuff like this, they just brute force through.

BTW, I do something similar in my HeadLander fix, where I put a common stereoization function, so that I can just call it wherever. #include the helper returns. ( not in github, was just hacking around and shipped what I had. Didn't really think anyone would care about HeadLander except me.)

I've thought a couple of times about making something like common routines, so that people don't have to always do float4 stereo = StereoParams.Load(0); The more confusing ones are the non-0 Params. But it's all boilerplate and could be simplified.

Maybe something like:

float separation = StereoParams.Load(0).x;
float convergence = StereoParams.Load(0).y;
...
o0.x += separation * (o0.w - convergence);

Inlined would be easy enough to add to any shader generated. Could also be done via an include file like my HeadLander file, but I'm a little hesitant to add another file dependency.

Our brute force people won't care one way or the other, but it might be a nice simplification/clarification for our beginners. We still occasionally get questions about stereo.x on the forums, and I use that as a metric for whether we have a clear enough solution/example.

DarkStarSword commented 7 years ago

Would this also work for custom value sharing from VS to PS, in normal shaders, not custom shaders? Some of our more advanced people are already doing this, by manually adding extra outputs to the VS HLSL and then inputs to PS to pass matrices they want in PS.

Theoretically yeah, but that's a little more tricky because we can't see the relationship between VS and PS until the game actually uses a combination of the two together, and I'm not sure how we could really make the decompiled text clean. I guess the starting point would be to have the decompiler start generating structs for inputs and outputs, and maybe place the main function inside an #ifdef for the shader type. After that it's not as clear - perhaps a shaderhacker could change the pixel shader to #include the vertex shader and pick up the structure definition from there, or maybe have both #include a common file with the structure definition. I guess at a pinch we could generate separate hlsl files for each distinct structure we see so that the shaders start out including something already and the shaderhacker then has a more obvious path to modify it.

Or if we rethink things without constraining ourselves to fit in with our existing model we could dump out shaders when they are used so the combination of VS+HS+DS+GS+PS is known, and place all the related shaders together in one file. That is actually kind of compelling for anther reason, because it also gives us a more direct way to do partner shader filtering.

I've thought a couple of times about making something like common routines, so that people don't have to always do float4 stereo = StereoParams.Load(0); The more confusing ones are the non-0 Params. But it's all boilerplate and could be simplified.

I use things like this in a game-specific hud.hlsl that is included by any shader that needs it (e.g. from Dreamfall Chapters):

#define separation StereoParams.Load(0).x
#define convergence StereoParams.Load(0).y
#define stereo_eye StereoParams.Load(0).z

#define min_subtitle_depth IniParams[0].x
#define cursor_pos         IniParams[1].xy
#define texture_filter     IniParams[1].z
#define hud_shader         IniParams[1].w
#define rt_size            IniParams[4].xy
#define res_size           IniParams[4].zw

#define hud_shader_floating_icons 1
#define hud_shader_inventory_examine_icons 2
#define hud_shader_text 3

#define inventory_depth IniParams[2].x
#define inventory_depth_examine IniParams[2].x

#define selection_circle 2
// #define subtitle_text 3 // Hash proved unreliable - if we need this, try enabling hash tracking
#define loading_screen_graphic 4
#define action_icon 5
#define inventory_circle 6
#define inventory_examine_circles 7

#ifndef IS_FULLSCREEN
#define IS_FULLSCREEN false
#endif

We can change the compiler to statically generate macros for separation and convergence in new shaders easily enough, but we have to be careful if we were to pass them as options to D3DCompile that we pick names that are unlikely to be have been used - "VERTEX_SHADER", "PIXEL_SHADER" and "FLT_MAX" are probably ok, but variations of "separation" and "convergence" are less safe.

Inlined would be easy enough to add to any shader generated. Could also be done via an include file like my HeadLander file, but I'm a little hesitant to add another file dependency.

An included file might be the way to go, and that would also give me something I can include from custom shaders to pick up the StereoParams and IniParams definitions. Realistically, a lot of shaderhackers have adopted using my matrix.hlsl already, so it might be worthwhile to add a sort of 3DMigoto standard library :)

We should probably write that custom include handler first to fix reloading shaders when the included file has been modified first though - we have workarounds, but they all cause their own problems or are easy to forget (how many bugs has oomek filed now that were in some way related to working around that issue? And I know I hit similar issues, so it's not just him. If you take a look at my d3dx.ini from Dreamfall Chapters you may notice I'm using custom shader sections to force the HUD shaders to always reload, and part of (not all) the motivation for the handling=skip fixes I put in 3DMigoto were to solve problems created by that workaround).

If we want to take this further (and this may well be taking it too far - I'm just putting it out there), we might consider using a section in the d3dx.ini to handle what extra defines we pass to the compiler and simultaneously give us a way to map ini params and texture filter indexes to names. A quick mock up of what this might look like for my Dreamfall Chapters example (definitely not final - I can already see some problems with this):

[Defines]
; "separation", "convergence" and "stereo_eye" in hlsl defined to StereoParams
; Only reason to define these manually is to not break existing fixes that
; might have already used these terms in HLSL
separation = separation
convergence = convergence
stereo_eye = eye

; IniParams. Maybe this, or maybe the other way around
min_subtitle_depth      = x0
cursor_pos.x            = x1
cursor_pos.y            = y1
texture_filter          = z1
hud_shader              = w1
rt_size.x               = x4
rt_size.y               = y4
res_size.x              = z4
res_size.y              = w4
inventory_depth         = x2
inventory_depth_examine = x2

; Shader filters:
hud_shader_floating_icons          = 1
hud_shader_inventory_examine_icons = 2
hud_shader_text                    = 3

; Texture filters:
selection_circle          = 2
;subtitle_text            = 3 // Hash proved unreliable - if we need this, try enabling hash tracking
loading_screen_graphic    = 4
action_icon               = 5
inventory_circle          = 6
inventory_examine_circles = 7

; filter_index could now use the descriptive names:
[TextureOverrideIconChoices]
hash = b649d77d
filter_index = selection_circle
[TextureOverrideLoadingScreenImage]
hash = 14433742
filter_index = loading_screen_graphic
[TextureOverrideIconEyeHandEtc]
hash = 9bc17812
filter_index = action_icon
[TextureOverrideCog]
hash = 8181192a
filter_index = action_icon

; As can IniParam overrides:
[CustomShader_HUD_Analyse]
...
texture_filter = ps-t0
rt_size.x = rt_width
rt_size.y = rt_height
res_size.x = res_width
res_size.y = res_height

This would need some bike shedding, because I can already see some problems with it:

I've thought about something like this for a while, but there isn't an obvious solution that stands out to me. The first two could probably be solved fairly easily by using $variablename or similar.