Skip to content

Commit

Permalink
Merge pull request #10394 from Pokechu22/alpha-1-blend
Browse files Browse the repository at this point in the history 
Revert TEV alpha lerp change and special-case alpha=1 in blending
  • Loading branch information
@JMC47
JMC47 authored Feb 11, 2022
2 parents a6f9dd5 + 444f6fd commit 30a9777
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 21 deletions.
15 changes: 14 additions & 1 deletion Source/Core/VideoBackends/Software/Tev.cpp
View file
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
View file
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
View file
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

0 comments on commit 30a9777

Please sign in to comment.