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

Revert TEV alpha lerp change and special-case alpha=1 in blending #10394

Merged
merged 2 commits into from
Feb 11, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
15 changes: 14 additions & 1 deletion Source/Core/VideoBackends/Software/Tev.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,7 @@ void Tev::DrawAlphaRegular(const TevStageCombiner::AlphaCombiner& ac, const Inpu

s32 temp = InputReg.a * (256 - c) + (InputReg.b * c);
temp <<= m_ScaleLShiftLUT[u32(ac.scale.Value())];
temp += (ac.scale != TevScale::Divide2) ? 0 : (ac.op == TevOp::Sub) ? 127 : 128;
temp += (ac.scale == TevScale::Divide2) ? 0 : (ac.op == TevOp::Sub) ? 127 : 128;
temp = ac.op == TevOp::Sub ? (-temp >> 8) : (temp >> 8);

s32 result =
Expand Down Expand Up @@ -714,6 +714,19 @@ void Tev::Draw()
if (!TevAlphaTest(output[ALP_C]))
return;

// Hardware testing indicates that an alpha of 1 can pass an alpha test,
// but doesn't do anything in blending
// This situation is important for Mario Kart Wii's menus (they will render incorrectly if the
// alpha test for the FMV in the background fails, since they depend on depth for drawing a yellow
// border) and Fortune Street's gameplay (where a rectangle with an alpha value of 1 is drawn over
// the center of the screen several times, but those rectangles shouldn't be visible).
// Blending seems to result in no changes to the output with an alpha of 1, even if the input
// color is white.
// TODO: Investigate this further: we might be handling blending incorrectly in general (though
// there might not be any good way of changing blending behavior)
if (output[ALP_C] == 1)
output[ALP_C] = 0;

// z texture
if (bpmem.ztex2.op != ZTexOp::Disabled)
{
Expand Down
24 changes: 19 additions & 5 deletions Source/Core/VideoCommon/PixelShaderGen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -852,7 +852,7 @@ uint WrapCoord(int coord, uint wrap, int size) {{
static void WriteStage(ShaderCode& out, const pixel_shader_uid_data* uid_data, int n,
APIType api_type, bool stereo);
static void WriteTevRegular(ShaderCode& out, std::string_view components, TevBias bias, TevOp op,
bool clamp, TevScale scale, bool alpha);
bool clamp, TevScale scale);
static void WriteAlphaTest(ShaderCode& out, const pixel_shader_uid_data* uid_data, APIType api_type,
bool per_pixel_depth, bool use_dual_source);
static void WriteFog(ShaderCode& out, const pixel_shader_uid_data* uid_data);
Expand Down Expand Up @@ -1234,6 +1234,18 @@ ShaderCode GeneratePixelShaderCode(APIType api_type, const ShaderHostConfig& hos
use_dual_source || use_shader_blend);
}

// This situation is important for Mario Kart Wii's menus (they will render incorrectly if the
// alpha test for the FMV in the background fails, since they depend on depth for drawing a yellow
// border) and Fortune Street's gameplay (where a rectangle with an alpha value of 1 is drawn over
// the center of the screen several times, but those rectangles shouldn't be visible).
// Blending seems to result in no changes to the output with an alpha of 1, even if the input
// color is white.
// TODO: Investigate this further: we might be handling blending incorrectly in general (though
// there might not be any good way of changing blending behavior)
out.Write("\t// Hardware testing indicates that an alpha of 1 can pass an alpha test,\n"
"\t// but doesn't do anything in blending\n"
"\tif (prev.a == 1) prev.a = 0;\n");

if (uid_data->zfreeze)
{
out.SetConstantsUsed(C_ZSLOPE, C_ZSLOPE);
Expand Down Expand Up @@ -1677,7 +1689,7 @@ static void WriteStage(ShaderCode& out, const pixel_shader_uid_data* uid_data, i
out.Write("\t{} = clamp(", tev_c_output_table[cc.dest]);
if (cc.bias != TevBias::Compare)
{
WriteTevRegular(out, "rgb", cc.bias, cc.op, cc.clamp, cc.scale, false);
WriteTevRegular(out, "rgb", cc.bias, cc.op, cc.clamp, cc.scale);
}
else
{
Expand Down Expand Up @@ -1710,7 +1722,7 @@ static void WriteStage(ShaderCode& out, const pixel_shader_uid_data* uid_data, i
out.Write("\t{} = clamp(", tev_a_output_table[ac.dest]);
if (ac.bias != TevBias::Compare)
{
WriteTevRegular(out, "a", ac.bias, ac.op, ac.clamp, ac.scale, true);
WriteTevRegular(out, "a", ac.bias, ac.op, ac.clamp, ac.scale);
}
else
{
Expand Down Expand Up @@ -1742,7 +1754,7 @@ static void WriteStage(ShaderCode& out, const pixel_shader_uid_data* uid_data, i
}

static void WriteTevRegular(ShaderCode& out, std::string_view components, TevBias bias, TevOp op,
bool clamp, TevScale scale, bool alpha)
bool clamp, TevScale scale)
{
static constexpr Common::EnumMap<const char*, TevScale::Divide2> tev_scale_table_left{
"", // Scale1
Expand Down Expand Up @@ -1780,12 +1792,14 @@ static void WriteTevRegular(ShaderCode& out, std::string_view components, TevBia
// - c is scaled from 0..255 to 0..256, which allows dividing the result by 256 instead of 255
// - if scale is bigger than one, it is moved inside the lerp calculation for increased accuracy
// - a rounding bias is added before dividing by 256
// TODO: Is the rounding bias still added when the scale is divide by 2? Currently we do not
// apply it.
out.Write("(((tevin_d.{}{}){})", components, tev_bias_table[bias], tev_scale_table_left[scale]);
out.Write(" {} ", tev_op_table[op]);
out.Write("(((((tevin_a.{0}<<8) + "
"(tevin_b.{0}-tevin_a.{0})*(tevin_c.{0}+(tevin_c.{0}>>7))){1}){2})>>8)",
components, tev_scale_table_left[scale],
((scale == TevScale::Divide2) == alpha) ? tev_lerp_bias[op] : "");
(scale != TevScale::Divide2) ? tev_lerp_bias[op] : "");
out.Write("){}", tev_scale_table_right[scale]);
}

Expand Down
35 changes: 20 additions & 15 deletions Source/Core/VideoCommon/UberShaderPixel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -334,8 +334,8 @@ ShaderCode GenPixelShader(APIType api_type, const ShaderHostConfig& host_config,
const auto WriteTevLerp = [&out](std::string_view components) {
out.Write(
"// TEV's Linear Interpolate, plus bias, add/subtract and scale\n"
"int{0} tevLerp{0}(int{0} A, int{0} B, int{0} C, int{0} D, uint bias, bool op, bool alpha, "
"uint shift) {{\n"
"int{0} tevLerp{0}(int{0} A, int{0} B, int{0} C, int{0} D, uint bias, bool op, "
"uint scale) {{\n"
" // Scale C from 0..255 to 0..256\n"
" C += C >> 7;\n"
"\n"
Expand All @@ -344,12 +344,14 @@ ShaderCode GenPixelShader(APIType api_type, const ShaderHostConfig& host_config,
" else if (bias == 2u) D -= 128;\n"
"\n"
" int{0} lerp = (A << 8) + (B - A)*C;\n"
" if (shift != 3u) {{\n"
" lerp = lerp << shift;\n"
" D = D << shift;\n"
" if (scale != 3u) {{\n"
" lerp = lerp << scale;\n"
" D = D << scale;\n"
" }}\n"
"\n"
" if ((shift == 3u) == alpha)\n"
" // TODO: Is this rounding bias still added when the scale is divide by 2? Currently we "
"do not apply it.\n"
" if (scale != 3u)\n"
" lerp = lerp + (op ? 127 : 128);\n"
"\n"
" int{0} result = lerp >> 8;\n"
Expand All @@ -360,9 +362,9 @@ ShaderCode GenPixelShader(APIType api_type, const ShaderHostConfig& host_config,
" else // Add\n"
" result = D + result;\n"
"\n"
" // Most of the Shift was moved inside the lerp for improved precision\n"
" // Most of the Scale was moved inside the lerp for improved precision\n"
" // But we still do the divide by 2 here\n"
" if (shift == 3u)\n"
" if (scale == 3u)\n"
" result = result >> 1;\n"
" return result;\n"
"}}\n\n",
Expand Down Expand Up @@ -810,13 +812,13 @@ ShaderCode GenPixelShader(APIType api_type, const ShaderHostConfig& host_config,
BitfieldExtract<&TevStageCombiner::ColorCombiner::op>("ss.cc"));
out.Write(" bool color_clamp = bool({});\n",
BitfieldExtract<&TevStageCombiner::ColorCombiner::clamp>("ss.cc"));
out.Write(" uint color_shift = {};\n",
out.Write(" uint color_scale = {};\n",
BitfieldExtract<&TevStageCombiner::ColorCombiner::scale>("ss.cc"));
out.Write(" uint color_dest = {};\n",
BitfieldExtract<&TevStageCombiner::ColorCombiner::dest>("ss.cc"));

out.Write(
" uint color_compare_op = color_shift << 1 | uint(color_op);\n"
" uint color_compare_op = color_scale << 1 | uint(color_op);\n"
"\n"
" int3 color_A = selectColorInput(s, ss, {0}colors_0, {0}colors_1, color_a) & "
"int3(255, 255, 255);\n"
Expand All @@ -831,8 +833,8 @@ ShaderCode GenPixelShader(APIType api_type, const ShaderHostConfig& host_config,
out.Write(
" int3 color;\n"
" if (color_bias != 3u) {{ // Normal mode\n"
" color = tevLerp3(color_A, color_B, color_C, color_D, color_bias, color_op, false, "
"color_shift);\n"
" color = tevLerp3(color_A, color_B, color_C, color_D, color_bias, color_op, "
"color_scale);\n"
" }} else {{ // Compare mode\n"
" // op 6 and 7 do a select per color channel\n"
" if (color_compare_op == 6u) {{\n"
Expand Down Expand Up @@ -880,13 +882,13 @@ ShaderCode GenPixelShader(APIType api_type, const ShaderHostConfig& host_config,
BitfieldExtract<&TevStageCombiner::AlphaCombiner::op>("ss.ac"));
out.Write(" bool alpha_clamp = bool({});\n",
BitfieldExtract<&TevStageCombiner::AlphaCombiner::clamp>("ss.ac"));
out.Write(" uint alpha_shift = {};\n",
out.Write(" uint alpha_scale = {};\n",
BitfieldExtract<&TevStageCombiner::AlphaCombiner::scale>("ss.ac"));
out.Write(" uint alpha_dest = {};\n",
BitfieldExtract<&TevStageCombiner::AlphaCombiner::dest>("ss.ac"));

out.Write(
" uint alpha_compare_op = alpha_shift << 1 | uint(alpha_op);\n"
" uint alpha_compare_op = alpha_scale << 1 | uint(alpha_op);\n"
"\n"
" int alpha_A;\n"
" int alpha_B;\n"
Expand All @@ -904,7 +906,7 @@ ShaderCode GenPixelShader(APIType api_type, const ShaderHostConfig& host_config,
" int alpha;\n"
" if (alpha_bias != 3u) {{ // Normal mode\n"
" alpha = tevLerp(alpha_A, alpha_B, alpha_C, alpha_D, alpha_bias, alpha_op, "
"true, alpha_shift);\n"
"alpha_scale);\n"
" }} else {{ // Compare mode\n"
" if (alpha_compare_op == 6u) {{\n"
" // TevCompareMode::A8, TevComparison::GT\n"
Expand Down Expand Up @@ -1030,6 +1032,9 @@ ShaderCode GenPixelShader(APIType api_type, const ShaderHostConfig& host_config,
" }}\n"
"\n");

out.Write(" // Hardware testing indicates that an alpha of 1 can pass an alpha test,\n"
" // but doesn't do anything in blending\n"
" if (TevResult.a == 1) TevResult.a = 0;\n");
// =========
// Dithering
// =========
Expand Down