Fog.
Merge request reports
Activity
added 7 commits
- bc8eafb9 - vkd3d-shader/spirv: Allow vector SSA values.
- 3d050c66 - vkd3d-shader/spirv: Allow using a scalar SSA value as a source in a vector instruction.
- 1ee03683 - vkd3d-shader: Allow controlling fog through parameters.
- aed7fbd9 - vkd3d-shader: Add a couple of traces for signature remapping.
- 4f5f3170 - vkd3d-shader: Allow controlling the fog source through a parameter.
- 3fbc0f50 - vkd3d-shader: Add a natural exponent instruction.
- 31b1b6e6 - vkd3d-shader: Implement exponential fog.
Toggle commit listSubject: [PATCH 1/7] vkd3d-shader/spirv: Allow vector SSA values.
What are we doing here exactly? Vector SSA values are already allowed, as far as I can tell. This seems to be lifting the restriction that SSA parameters need a scalar/replicate swizzle, allowing identity swizzles as well.
if (reg->type == VKD3DSPR_SSA) - return spirv_compiler_emit_load_ssa_reg(compiler, reg, component_type, swizzle); + { + val_id = spirv_compiler_emit_load_ssa_reg(compiler, reg, component_type, swizzle); + if (reg->dimension == VSIR_DIMENSION_SCALAR && component_count > 1) + { + uint32_t components[VKD3D_VEC4_SIZE]; + + type_id = vkd3d_spirv_get_type_id(builder, component_type, component_count); + for (unsigned int i = 0; i < VKD3D_VEC4_SIZE; ++i) + components[i] = val_id; + val_id = vkd3d_spirv_build_op_composite_construct(builder, type_id, components, component_count); + } + return val_id; + }
Is there a reason we couldn't use spirv_compiler_emit_swizzle() here? More broadly, if we're going to make SSA values behave more like regular registers (and I wouldn't be opposed to that), I'd be tempted to suggest constructing the "reg_info" variable for SSA parameters, and then just letting spirv_compiler_emit_load_reg() run its coarse. Note that we already do something similar for VKD3DSPR_TEMP in spirv_compiler_get_register_info().
+static bool find_signature_element(const struct shader_signature *signature, + const char *name, uint32_t index, uint32_t *signature_idx) +{ + for (unsigned int i = 0; i < signature->element_count; ++i) + { + const struct signature_element *e = &signature->elements[i]; + + if (!ascii_strcasecmp(e->semantic_name, name) && e->semantic_index == index) + { + *signature_idx = i; + return true; + } + } + + return false; +}
That's essentially vsir_signature_find_element_by_name(), right?
+ for (unsigned int i = 0; i < signature->element_count; ++i) + { + e = &signature->elements[i]; + + if (!ascii_strcasecmp(e->semantic_name, "FOG") && !e->semantic_index) + return VKD3D_OK; + register_idx = max(register_idx, e->register_index + 1); + }
As is this.
+ if (!(new_elements = vkd3d_realloc(signature->elements, + (signature->element_count + 1) * sizeof(*signature->elements)))) + return VKD3D_ERROR_OUT_OF_MEMORY; + signature->elements = new_elements; + e = &signature->elements[signature->element_count++]; + memset(e, 0, sizeof(*e)); + e->semantic_name = vkd3d_strdup("FOG"); + e->sysval_semantic = VKD3D_SHADER_SV_NONE; + e->component_type = VKD3D_SHADER_COMPONENT_FLOAT; + e->register_count = 1; + e->mask = VKD3DSP_WRITEMASK_0; + e->used_mask = VKD3DSP_WRITEMASK_0; + e->register_index = register_idx; + e->target_location = register_idx; + e->interpolation_mode = VKD3DSIM_LINEAR;
It might be time to introduce a helper function to initialise signature elements. Doesn't need to be in this series.
+ {VKD3D_SHADER_PARAMETER_NAME_ALPHA_TEST_REF, {.f = 0.0}, "alpha_test_ref"}, + {VKD3D_SHADER_PARAMETER_NAME_FOG_END, {.f = 1.0}, "fog_end"}, + {VKD3D_SHADER_PARAMETER_NAME_FOG_SCALE, {.f = 1.0}, "fog_scale"},
Technically those are f64 literals. IIRC gcc doesn't, but some compilers warn about assigning those to f32 variables/fields.
It's not a requirement for merging this series, but adding "fog" support to the GL and Metal runners shouldn't be much harder than adding it to the Vulkan runner, right?
+ * Together with other fog parameters, this parameter can be used to + * implement fixed function fog, as present in Direct3D versions up to 9, + * if the target environment does not support shade mode as part of its own + * fixed-function API (as Vulkan and core OpenGL).
"fog" instead of "shade mode"?
+ if (source == VKD3D_SHADER_FOG_SOURCE_FOG_OR_SPECULAR_W) + has_specular = find_signature_element(&program->output_signature, "COLOR", 1, &specular_signature_idx); + + if (!vsir_signature_find_sysval(&program->output_signature, VKD3D_SHADER_SV_POSITION, 0, &position_signature_idx)) + { + vkd3d_shader_error(ctx->message_context, &no_loc, VKD3D_SHADER_ERROR_VSIR_MISSING_SEMANTIC, + "Shader does not write position."); + return VKD3D_ERROR_INVALID_SHADER; + } + source_signature_idx = position_signature_idx; + + if (!find_signature_element(&program->output_signature, "FOG", 0, &fog_signature_idx)) + { + ERR("Fog output not found.\n"); + return VKD3D_ERROR_INVALID_SHADER; + } + + if (source == VKD3D_SHADER_FOG_SOURCE_FOG_OR_SPECULAR_W) + { + if (program->has_fog || !has_specular) + return VKD3D_OK; + source_signature_idx = specular_signature_idx; + }
That's a little messy. Could we do something like this instead?
if (source == VKD3D_SHADER_FOG_SOURCE_FOG_OR_SPECULAR_W) { if (program->has_fog || !find_signature_element(&program->output_signature, "COLOR", 1, &source_signature_idx); return VKD3D_OK; } else { if (!vsir_signature_find_sysval(&program->output_signature, VKD3D_SHADER_SV_POSITION, 0, &source_signature_idx)) { vkd3d_shader_error(ctx->message_context, &no_loc, VKD3D_SHADER_ERROR_VSIR_MISSING_SEMANTIC, "Shader does not write position."); return VKD3D_ERROR_INVALID_SHADER; } } if (!find_signature_element(&program->output_signature, "FOG", 0, &fog_signature_idx)) { ERR("Fog output not found.\n"); return VKD3D_ERROR_INVALID_SHADER; }
+ /* We generate the following code: + * + * mul sr0, FOG_DENSITY, vFOG.x + * exp_sat srFACTOR, -sr0 + */
We actually generate "exp_natural" tough. Note that if we wanted, we could also define FOG_FRAGMENT_EXP as "exp2(-d * c)", and then pass "d * log2(e)" instead of "d" for FOG_DENSITY. I.e.,
. I'm not sure it matters all that much for modern GPUs, but if we think it's worth the trouble to define FOG_FRAGMENT_LINEAR as "(e - c) * k", we might as well do something similar for FOG_FRAGMENT_EXP and FOG_FRAGMENT_EXP2.@@ -10335,6 +10336,7 @@ static int spirv_compiler_handle_instruction(struct spirv_compiler *compiler, case VKD3DSIH_DMAX: case VKD3DSIH_DMIN: case VKD3DSIH_EXP: + case VKD3DSIH_EXP_NATURAL: case VKD3DSIH_FIRSTBIT_HI: case VKD3DSIH_FIRSTBIT_LO: case VKD3DSIH_FIRSTBIT_SHI:
That looks like it belongs in 6/7.
Subject: [PATCH 1/7] vkd3d-shader/spirv: Allow vector SSA values.
What are we doing here exactly? Vector SSA values are already allowed, as far as I can tell. This seems to be lifting the restriction that SSA parameters need a scalar/replicate swizzle, allowing identity swizzles as well.
Poor wording, sorry. Yes, that is what the patch does.
if (reg->type == VKD3DSPR_SSA) - return spirv_compiler_emit_load_ssa_reg(compiler, reg, component_type, swizzle); + { + val_id = spirv_compiler_emit_load_ssa_reg(compiler, reg, component_type, swizzle); + if (reg->dimension == VSIR_DIMENSION_SCALAR && component_count > 1) + { + uint32_t components[VKD3D_VEC4_SIZE]; + + type_id = vkd3d_spirv_get_type_id(builder, component_type, component_count); + for (unsigned int i = 0; i < VKD3D_VEC4_SIZE; ++i) + components[i] = val_id; + val_id = vkd3d_spirv_build_op_composite_construct(builder, type_id, components, component_count); + } + return val_id; + }
Is there a reason we couldn't use spirv_compiler_emit_swizzle() here? More broadly, if we're going to make SSA values behave more like regular registers (and I wouldn't be opposed to that), I'd be tempted to suggest constructing the "reg_info" variable for SSA parameters, and then just letting spirv_compiler_emit_load_reg() run its coarse. Note that we already do something similar for VKD3DSPR_TEMP in spirv_compiler_get_register_info().
Yes, I guess there's no reason to keep the separate path.
+static bool find_signature_element(const struct shader_signature *signature, + const char *name, uint32_t index, uint32_t *signature_idx) +{ + for (unsigned int i = 0; i < signature->element_count; ++i) + { + const struct signature_element *e = &signature->elements[i]; + + if (!ascii_strcasecmp(e->semantic_name, name) && e->semantic_index == index) + { + *signature_idx = i; + return true; + } + } + + return false; +}
That's essentially vsir_signature_find_element_by_name(), right?
Oops, and I even introduced that function. I think I wrote most of the fog pass before I wrote e781abc3, but the better excuse is I just forgot :S
+ for (unsigned int i = 0; i < signature->element_count; ++i) + { + e = &signature->elements[i]; + + if (!ascii_strcasecmp(e->semantic_name, "FOG") && !e->semantic_index) + return VKD3D_OK; + register_idx = max(register_idx, e->register_index + 1); + }
As is this.
Though in that case we need the loop to look for a new register index to assign anyway. I guess I'll at least still use vsir_signature_find_element_by_name().
+ if (!(new_elements = vkd3d_realloc(signature->elements, + (signature->element_count + 1) * sizeof(*signature->elements)))) + return VKD3D_ERROR_OUT_OF_MEMORY; + signature->elements = new_elements; + e = &signature->elements[signature->element_count++]; + memset(e, 0, sizeof(*e)); + e->semantic_name = vkd3d_strdup("FOG"); + e->sysval_semantic = VKD3D_SHADER_SV_NONE; + e->component_type = VKD3D_SHADER_COMPONENT_FLOAT; + e->register_count = 1; + e->mask = VKD3DSP_WRITEMASK_0; + e->used_mask = VKD3DSP_WRITEMASK_0; + e->register_index = register_idx; + e->target_location = register_idx; + e->interpolation_mode = VKD3DSIM_LINEAR;
It might be time to introduce a helper function to initialise signature elements. Doesn't need to be in this series.
Yeah, this thought had crossed my mind as well. I probably would have done it earlier, but I wasn't quite sure how e781abc3 (and fog) would shake out and how similar the code would actually end up being.
It's not a requirement for merging this series, but adding "fog" support to the GL and Metal runners shouldn't be much harder than adding it to the Vulkan runner, right?
Yes, and the same for other parameters. I had had it in the back of my mind that I would need to take care of that eventually...
I'll add it after this series.
+ if (source == VKD3D_SHADER_FOG_SOURCE_FOG_OR_SPECULAR_W) + has_specular = find_signature_element(&program->output_signature, "COLOR", 1, &specular_signature_idx); + + if (!vsir_signature_find_sysval(&program->output_signature, VKD3D_SHADER_SV_POSITION, 0, &position_signature_idx)) + { + vkd3d_shader_error(ctx->message_context, &no_loc, VKD3D_SHADER_ERROR_VSIR_MISSING_SEMANTIC, + "Shader does not write position."); + return VKD3D_ERROR_INVALID_SHADER; + } + source_signature_idx = position_signature_idx; + + if (!find_signature_element(&program->output_signature, "FOG", 0, &fog_signature_idx)) + { + ERR("Fog output not found.\n"); + return VKD3D_ERROR_INVALID_SHADER; + } + + if (source == VKD3D_SHADER_FOG_SOURCE_FOG_OR_SPECULAR_W) + { + if (program->has_fog || !has_specular) + return VKD3D_OK; + source_signature_idx = specular_signature_idx; + }
That's a little messy. Could we do something like this instead?
if (source == VKD3D_SHADER_FOG_SOURCE_FOG_OR_SPECULAR_W) { if (program->has_fog || !find_signature_element(&program->output_signature, "COLOR", 1, &source_signature_idx); return VKD3D_OK; } else { if (!vsir_signature_find_sysval(&program->output_signature, VKD3D_SHADER_SV_POSITION, 0, &source_signature_idx)) { vkd3d_shader_error(ctx->message_context, &no_loc, VKD3D_SHADER_ERROR_VSIR_MISSING_SEMANTIC, "Shader does not write position."); return VKD3D_ERROR_INVALID_SHADER; } } if (!find_signature_element(&program->output_signature, "FOG", 0, &fog_signature_idx)) { ERR("Fog output not found.\n"); return VKD3D_ERROR_INVALID_SHADER; }
Yes, I think the current state is something of an artifact of being written a different way, or that I didn't try to simplify it quite hard enough.
+ /* We generate the following code: + * + * mul sr0, FOG_DENSITY, vFOG.x + * exp_sat srFACTOR, -sr0 + */
We actually generate "exp_natural" tough. Note that if we wanted, we could also define FOG_FRAGMENT_EXP as "exp2(-d * c)", and then pass "d * log2(e)" instead of "d" for FOG_DENSITY. I.e.,
. I'm not sure it matters all that much for modern GPUs, but if we think it's worth the trouble to define FOG_FRAGMENT_LINEAR as "(e - c) * k", we might as well do something similar for FOG_FRAGMENT_EXP and FOG_FRAGMENT_EXP2.
I'll make that change then. I have no context for these kinds of performance concerns and was just following the GLSL backend.
added 5 commits
- da33985e - vkd3d-shader/spirv: Handle SSA registers in spirv_compiler_get_register_info().
- 26baf449 - vkd3d-shader: Allow controlling fog through parameters.
- dec62262 - vkd3d-shader: Add a couple of traces for signature remapping.
- 345fe966 - vkd3d-shader: Allow controlling the fog source through a parameter.
- 45df102b - vkd3d-shader: Implement exponential fog.
Toggle commit list@@ -7501,7 +7464,9 @@ static void spirv_compiler_emit_mov(struct spirv_compiler *compiler, general_implementation: write_mask = dst->write_mask; - if (src->reg.type == VKD3DSPR_IMMCONST64 && !data_type_is_64_bit(dst->reg.data_type)) + if ((src->reg.type == VKD3DSPR_IMMCONST64 + || (data_type_is_64_bit(src->reg.data_type) && src->reg.dimension == VSIR_DIMENSION_SCALAR)) + && !data_type_is_64_bit(dst->reg.data_type)) write_mask = vsir_write_mask_64_from_32(write_mask); else if (!data_type_is_64_bit(src->reg.data_type) && data_type_is_64_bit(dst->reg.data_type)) write_mask = vsir_write_mask_32_from_64(write_mask);
Should the condition there just be "if (data_type_is_64_bit(src->reg.data_type) && !data_type_is_64_bit(dst->reg.data_type))"? You'd expect 64-bit immediate constants to have a 64-bit data type, and it's not clear to me that scalar sources should be special here. (Is this just left over from the previous version?)
+ for (unsigned int i = 0; i < signature->element_count; ++i) + { + e = &signature->elements[i]; + + if (!ascii_strcasecmp(e->semantic_name, "FOG") && !e->semantic_index) + return VKD3D_OK; + register_idx = max(register_idx, e->register_index + 1); + } + + if (!(new_elements = vkd3d_realloc(signature->elements, + (signature->element_count + 1) * sizeof(*signature->elements)))) + return VKD3D_ERROR_OUT_OF_MEMORY; + signature->elements = new_elements; + e = &signature->elements[signature->element_count++]; + memset(e, 0, sizeof(*e)); + e->semantic_name = vkd3d_strdup("FOG"); + e->sysval_semantic = VKD3D_SHADER_SV_NONE; + e->component_type = VKD3D_SHADER_COMPONENT_FLOAT; + e->register_count = 1; + e->mask = VKD3DSP_WRITEMASK_0; + e->used_mask = VKD3DSP_WRITEMASK_0; + e->register_index = register_idx; + e->target_location = register_idx; + e->interpolation_mode = VKD3DSIM_LINEAR; + + return VKD3D_OK;
"vsir_signature_find_element_by_name(signature, "FOG", 0)" and "add_signature_element(signature, "FOG", 0, VKD3DSP_WRITEMASK_0, register_index, VKD3DSIM_LINEAR)", I'd say.
+ /** + * The fog interpolation factor is 2^(-(k * c)²). + */ + VKD3D_SHADER_FOG_FRAGMENT_EXP2 = 0x2,
"-(k * c)²" isn't as unambiguous as it perhaps should be. (I.e., is that "(-(k * c))²" or "-((k * c)²)"?)
/** - * Scale value for linear fog. + * Scale value for fog. * See VKD3D_SHADER_PARAMETER_NAME_FOG_FRAGMENT_MODE for documentation of * fog. * - * In order to emulate fixed-function pipelines which use start and end - * coordinates, this value can be set to 1 / (end - start). + * In order to emulate Direct3D and OpenGL fog, which use start and end + * coordinates for linear fog and density for exponential fog, this value + * can be set as follows: + * + * - For linear fog, set the scale to 1 / (end - start). + * + * - For exponential fog, set the scale to (density * log₂(e)). + * + * - For square-exponential fog, set the scale to (density * √(log₂(e))).
That works. It does spread the documentation a bit between FOG_FRAGMENT_MODE and FOG_SCALE though. Alternatively, we could do something like this:
/** * The fog interpolation factor is (e - c) * k. * * In order to implement traditional linear fog * * f = (end - c) / (end - start) * * set * * e = end * k = 1 / (end - start) */ VKD3D_SHADER_FOG_FRAGMENT_LINEAR = 0x3,
added 5 commits
- 8603124f - vkd3d-shader/spirv: Handle SSA registers in spirv_compiler_get_register_info().
- 01b1a1c6 - vkd3d-shader: Allow controlling fog through parameters.
- c4737d7d - vkd3d-shader: Add a couple of traces for signature remapping.
- e8899d7a - vkd3d-shader: Allow controlling the fog source through a parameter.
- 0a29c3c3 - vkd3d-shader: Implement exponential fog.
Toggle commit list@@ -7501,7 +7464,9 @@ static void spirv_compiler_emit_mov(struct spirv_compiler *compiler, general_implementation: write_mask = dst->write_mask; - if (src->reg.type == VKD3DSPR_IMMCONST64 && !data_type_is_64_bit(dst->reg.data_type)) + if ((src->reg.type == VKD3DSPR_IMMCONST64 + || (data_type_is_64_bit(src->reg.data_type) && src->reg.dimension == VSIR_DIMENSION_SCALAR)) + && !data_type_is_64_bit(dst->reg.data_type)) write_mask = vsir_write_mask_64_from_32(write_mask); else if (!data_type_is_64_bit(src->reg.data_type) && data_type_is_64_bit(dst->reg.data_type)) write_mask = vsir_write_mask_32_from_64(write_mask);
Should the condition there just be "if (data_type_is_64_bit(src->reg.data_type) && !data_type_is_64_bit(dst->reg.data_type))"? You'd expect 64-bit immediate constants to have a 64-bit data type, and it's not clear to me that scalar sources should be special here. (Is this just left over from the previous version?)
Yes, your prescription is the correct version, and this change is still necessary (not left over). I had it this way because of an incorrect understanding of how 64-bit swizzles are written in vsir—they're disassembled in the "32-bit" format but I didn't realize they're not actually encoded that way; they're actually encoded the more useful way.
+ for (unsigned int i = 0; i < signature->element_count; ++i) + { + e = &signature->elements[i]; + + if (!ascii_strcasecmp(e->semantic_name, "FOG") && !e->semantic_index) + return VKD3D_OK; + register_idx = max(register_idx, e->register_index + 1); + } + + if (!(new_elements = vkd3d_realloc(signature->elements, + (signature->element_count + 1) * sizeof(*signature->elements)))) + return VKD3D_ERROR_OUT_OF_MEMORY; + signature->elements = new_elements; + e = &signature->elements[signature->element_count++]; + memset(e, 0, sizeof(*e)); + e->semantic_name = vkd3d_strdup("FOG"); + e->sysval_semantic = VKD3D_SHADER_SV_NONE; + e->component_type = VKD3D_SHADER_COMPONENT_FLOAT; + e->register_count = 1; + e->mask = VKD3DSP_WRITEMASK_0; + e->used_mask = VKD3DSP_WRITEMASK_0; + e->register_index = register_idx; + e->target_location = register_idx; + e->interpolation_mode = VKD3DSIM_LINEAR; + + return VKD3D_OK;
"vsir_signature_find_element_by_name(signature, "FOG", 0)" and "add_signature_element(signature, "FOG", 0, VKD3DSP_WRITEMASK_0, register_index, VKD3DSIM_LINEAR)", I'd say.
Yep, yet another case of me forgetting that we have helpers for that, or to apply them everywhere.
+ /** + * The fog interpolation factor is 2^(-(k * c)²). + */ + VKD3D_SHADER_FOG_FRAGMENT_EXP2 = 0x2,
"-(k * c)²" isn't as unambiguous as it perhaps should be. (I.e., is that "(-(k * c))²" or "-((k * c)²)"?)
Fixed.
/** - * Scale value for linear fog. + * Scale value for fog. * See VKD3D_SHADER_PARAMETER_NAME_FOG_FRAGMENT_MODE for documentation of * fog. * - * In order to emulate fixed-function pipelines which use start and end - * coordinates, this value can be set to 1 / (end - start). + * In order to emulate Direct3D and OpenGL fog, which use start and end + * coordinates for linear fog and density for exponential fog, this value + * can be set as follows: + * + * - For linear fog, set the scale to 1 / (end - start). + * + * - For exponential fog, set the scale to (density * log₂(e)). + * + * - For square-exponential fog, set the scale to (density * √(log₂(e))).
That works. It does spread the documentation a bit between FOG_FRAGMENT_MODE and FOG_SCALE though. Alternatively, we could do something like this:
/** * The fog interpolation factor is (e - c) * k. * * In order to implement traditional linear fog * * f = (end - c) / (end - start) * * set * * e = end * k = 1 / (end - start) */ VKD3D_SHADER_FOG_FRAGMENT_LINEAR = 0x3,
That's a good idea, I've changed it accordingly.
added 109 commits
-
0a29c3c3...7d281924 - 104 commits from branch
wine:master
- a1de406d - vkd3d-shader/spirv: Handle SSA registers in spirv_compiler_get_register_info().
- f86d1e72 - vkd3d-shader/ir: Allow controlling fog through parameters.
- fc98cb48 - vkd3d-shader/ir: Add a couple of traces for signature remapping.
- 1fbbc82f - vkd3d-shader/ir: Allow controlling the fog source through a parameter.
- d56601c8 - vkd3d-shader/ir: Implement exponential fog.
Toggle commit list-
0a29c3c3...7d281924 - 104 commits from branch