Commit 5391302f authored by Fredrik Hubinette's avatar Fredrik Hubinette Committed by Commit Bot

color: avoid negative pow

Avoid calling pow() on negative numbers as that
causes undefined behavior in the shader.

Bug: 821967
Change-Id: Iab3dc9840c17d90aa5952adf17b8b24e2890d055
Reviewed-on: https://chromium-review.googlesource.com/962969Reviewed-by: default avatarccameron <ccameron@chromium.org>
Commit-Queue: Fredrik Hubinette <hubbe@chromium.org>
Cr-Commit-Position: refs/heads/master@{#543263}
parent 670c4688
...@@ -473,14 +473,9 @@ class ColorTransformSkTransferFn : public ColorTransformPerChannelTransferFn { ...@@ -473,14 +473,9 @@ class ColorTransformSkTransferFn : public ColorTransformPerChannelTransferFn {
if (std::abs(fn_.fE) > kEpsilon) if (std::abs(fn_.fE) > kEpsilon)
nonlinear = nonlinear + " + " + Str(fn_.fE); nonlinear = nonlinear + " + " + Str(fn_.fE);
// Add both parts, skipping the if clause if possible. *result << " if (v < " << Str(fn_.fD) << ")" << endl;
if (fn_.fD > kEpsilon) { *result << " return " << linear << ";" << endl;
*result << " if (v < " << Str(fn_.fD) << ")" << endl; *result << " return " << nonlinear << ";" << endl;
*result << " return " << linear << ";" << endl;
*result << " return " << nonlinear << ";" << endl;
} else {
*result << " return " << nonlinear << ";" << endl;
}
} }
private: private:
......
...@@ -380,55 +380,32 @@ TEST(SimpleColorSpace, DefaultToSRGB) { ...@@ -380,55 +380,32 @@ TEST(SimpleColorSpace, DefaultToSRGB) {
EXPECT_EQ(t2->NumberOfStepsForTesting(), 0u); EXPECT_EQ(t2->NumberOfStepsForTesting(), 0u);
} }
// This tests to make sure that we don't emit the "if" or "pow" parts of a // This tests to make sure that we don't emit "pow" parts of a
// transfer function unless necessary. // transfer function unless necessary.
TEST(SimpleColorSpace, ShaderSourceTrFnOptimizations) { TEST(SimpleColorSpace, ShaderSourceTrFnOptimizations) {
SkMatrix44 primaries; SkMatrix44 primaries;
gfx::ColorSpace::CreateSRGB().GetPrimaryMatrix(&primaries); gfx::ColorSpace::CreateSRGB().GetPrimaryMatrix(&primaries);
SkColorSpaceTransferFn fn_no_pow_no_if = { SkColorSpaceTransferFn fn_no_pow = {
1.f, 2.f, 0.f, 1.f, 0.f, 0.f, 0.f, 1.f, 2.f, 0.f, 1.f, 0.f, 0.f, 0.f,
}; };
SkColorSpaceTransferFn fn_no_pow_yes_if = { SkColorSpaceTransferFn fn_yes_pow = {
1.f, 2.f, 0.f, 1.f, 0.5f, 0.f, 0.f,
};
SkColorSpaceTransferFn fn_yes_pow_no_if = {
2.f, 2.f, 0.f, 1.f, 0.f, 0.f, 0.f, 2.f, 2.f, 0.f, 1.f, 0.f, 0.f, 0.f,
}; };
SkColorSpaceTransferFn fn_yes_pow_yes_if = {
2.f, 2.f, 0.f, 1.f, 0.5f, 0.f, 0.f,
};
gfx::ColorSpace src; gfx::ColorSpace src;
gfx::ColorSpace dst = gfx::ColorSpace::CreateXYZD50(); gfx::ColorSpace dst = gfx::ColorSpace::CreateXYZD50();
std::string shader_string; std::string shader_string;
src = gfx::ColorSpace::CreateCustom(primaries, fn_no_pow_no_if); src = gfx::ColorSpace::CreateCustom(primaries, fn_no_pow);
shader_string = ColorTransform::NewColorTransform( shader_string = ColorTransform::NewColorTransform(
src, dst, ColorTransform::Intent::INTENT_PERCEPTUAL) src, dst, ColorTransform::Intent::INTENT_PERCEPTUAL)
->GetShaderSource(); ->GetShaderSource();
EXPECT_EQ(shader_string.find("if ("), std::string::npos);
EXPECT_EQ(shader_string.find("pow("), std::string::npos); EXPECT_EQ(shader_string.find("pow("), std::string::npos);
src = gfx::ColorSpace::CreateCustom(primaries, fn_no_pow_yes_if); src = gfx::ColorSpace::CreateCustom(primaries, fn_yes_pow);
shader_string = ColorTransform::NewColorTransform(
src, dst, ColorTransform::Intent::INTENT_PERCEPTUAL)
->GetShaderSource();
EXPECT_NE(shader_string.find("if ("), std::string::npos);
EXPECT_EQ(shader_string.find("pow("), std::string::npos);
src = gfx::ColorSpace::CreateCustom(primaries, fn_yes_pow_no_if);
shader_string = ColorTransform::NewColorTransform(
src, dst, ColorTransform::Intent::INTENT_PERCEPTUAL)
->GetShaderSource();
EXPECT_EQ(shader_string.find("if ("), std::string::npos);
EXPECT_NE(shader_string.find("pow("), std::string::npos);
src = gfx::ColorSpace::CreateCustom(primaries, fn_yes_pow_yes_if);
shader_string = ColorTransform::NewColorTransform( shader_string = ColorTransform::NewColorTransform(
src, dst, ColorTransform::Intent::INTENT_PERCEPTUAL) src, dst, ColorTransform::Intent::INTENT_PERCEPTUAL)
->GetShaderSource(); ->GetShaderSource();
EXPECT_NE(shader_string.find("if ("), std::string::npos);
EXPECT_NE(shader_string.find("pow("), std::string::npos); EXPECT_NE(shader_string.find("pow("), std::string::npos);
} }
...@@ -456,6 +433,8 @@ TEST(SimpleColorSpace, MAYBE_SampleShaderSource) { ...@@ -456,6 +433,8 @@ TEST(SimpleColorSpace, MAYBE_SampleShaderSource) {
" return pow(9.47867334e-01 * v + 5.21326549e-02, 2.40000010e+00);\n" " return pow(9.47867334e-01 * v + 5.21326549e-02, 2.40000010e+00);\n"
"}\n" "}\n"
"float TransferFn3(float v) {\n" "float TransferFn3(float v) {\n"
" if (v < 0.00000000e+00)\n"
" return v;\n"
" return pow(v, 3.57142866e-01);\n" " return pow(v, 3.57142866e-01);\n"
"}\n" "}\n"
"vec3 DoColorConversion(vec3 color) {\n" "vec3 DoColorConversion(vec3 color) {\n"
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment