This CL addresses multiple issues:
- A logic issue where the wide gamut color mode was not set at the right time
- The presence of scRGB surfaces was not detected
- The sRGB to Display P3 matrix was transposed
- The color matrix was applied in gamma space instead of linear space*
- The GPU code path was doing a division by w for each pixel when a
color transform is set, which shouldn't be necessary (the code now
checks that the matrix has the expected form)
- Incorrect comment
- Dead code in Description.cpp (mDirtyUniforms was never used)
- Screenshots were taken using the last render engine configuration,
the configuration is now properly set every time
* This can in theory cause a discrepancy when we switch to/from hardware
composer mode. I was not able to create a visible discrepancy in practice
using Night Light, color blindness compensation modes and color inversion.
More importantly, how/where the hardware composer applies the color
transform is not specified: it could be gamma or linear space, before or
after the hardware color LUT. We'll address this in a future CL if needed.
In addition, this code assumes that fp16 surfaces are encoded in non-linear
space (scRGB-nl instead of scRGB) but we do not have EGL/Vulkan extensions
to specify this behavior. We need to address this as well
This CL also fixes potential divides by 0 in the GPU code path.
Bug:
29940137
Test: CtsUiRenderingTestsCases, CtsGraphicsTestCases
Change-Id: I9ae15850f8b9d48c39ebc2724ca3da202be9b008
uint32_t getStride() const;
// size of allocated memory in bytes
size_t getSize() const;
+ android_dataspace getDataSpace() const;
};
// ---------------------------------------------------------------------------
return mBuffer.stride * mBuffer.height * bytesPerPixel(mBuffer.format);
}
+android_dataspace ScreenshotClient::getDataSpace() const {
+ return mBuffer.dataSpace;
+}
+
// ----------------------------------------------------------------------------
}; // namespace android
template <typename MATRIX>
String8 asString(const MATRIX& m) {
String8 s;
- for (size_t c = 0; c < MATRIX::col_size(); c++) {
+ for (size_t c = 0; c < MATRIX::COL_SIZE; c++) {
s.append("| ");
- for (size_t r = 0; r < MATRIX::row_size(); r++) {
+ for (size_t r = 0; r < MATRIX::ROW_SIZE; r++) {
s.appendFormat("%7.2f ", m[r][c]);
}
s.append("|\n");
namespace android {
-Description::Description() :
- mUniformsDirty(true) {
+Description::Description() {
mPlaneAlpha = 1.0f;
mPremultipliedAlpha = false;
mOpaque = true;
}
void Description::setPlaneAlpha(GLclampf planeAlpha) {
- if (planeAlpha != mPlaneAlpha) {
- mUniformsDirty = true;
- mPlaneAlpha = planeAlpha;
- }
+ mPlaneAlpha = planeAlpha;
}
void Description::setPremultipliedAlpha(bool premultipliedAlpha) {
- if (premultipliedAlpha != mPremultipliedAlpha) {
- mPremultipliedAlpha = premultipliedAlpha;
- }
+ mPremultipliedAlpha = premultipliedAlpha;
}
void Description::setOpaque(bool opaque) {
- if (opaque != mOpaque) {
- mOpaque = opaque;
- }
+ mOpaque = opaque;
}
void Description::setTexture(const Texture& texture) {
mTexture = texture;
mTextureEnabled = true;
- mUniformsDirty = true;
}
void Description::disableTexture() {
mColor[1] = green;
mColor[2] = blue;
mColor[3] = alpha;
- mUniformsDirty = true;
}
void Description::setProjectionMatrix(const mat4& mtx) {
mProjectionMatrix = mtx;
- mUniformsDirty = true;
}
void Description::setColorMatrix(const mat4& mtx) {
return mColorMatrix;
}
+void Description::setWideGamut(bool wideGamut) {
+ mIsWideGamut = wideGamut;
+}
} /* namespace android */
bool mColorMatrixEnabled;
mat4 mColorMatrix;
+ bool mIsWideGamut;
+
public:
Description();
~Description();
void setProjectionMatrix(const mat4& mtx);
void setColorMatrix(const mat4& mtx);
const mat4& getColorMatrix() const;
-
-private:
- bool mUniformsDirty;
+ void setWideGamut(bool wideGamut);
};
} /* namespace android */
// Display-P3 only.
mat3 srgbToP3 = ColorSpaceConnector(ColorSpace::sRGB(), ColorSpace::DisplayP3()).getTransform();
- // color transform needs to be transposed and expanded to 4x4
- // to be what the shader wants
+ // color transform needs to be expanded to 4x4 to be what the shader wants
// mat has an initializer that expands mat3 to mat4, but
// not an assignment operator
- mat4 gamutTransform(transpose(srgbToP3));
+ mat4 gamutTransform(srgbToP3);
mSrgbToDisplayP3 = gamutTransform;
}
#endif
Description wideColorState = mState;
if (mDataSpace != HAL_DATASPACE_DISPLAY_P3) {
wideColorState.setColorMatrix(mState.getColorMatrix() * mSrgbToDisplayP3);
+ wideColorState.setWideGamut(true);
ALOGV("drawMesh: gamut transform applied");
}
ProgramCache::getInstance().useProgram(wideColorState);
.set(Key::OPACITY_MASK,
description.mOpaque ? Key::OPACITY_OPAQUE : Key::OPACITY_TRANSLUCENT)
.set(Key::COLOR_MATRIX_MASK,
- description.mColorMatrixEnabled ? Key::COLOR_MATRIX_ON : Key::COLOR_MATRIX_OFF);
+ description.mColorMatrixEnabled ? Key::COLOR_MATRIX_ON : Key::COLOR_MATRIX_OFF)
+ .set(Key::WIDE_GAMUT_MASK,
+ description.mIsWideGamut ? Key::WIDE_GAMUT_ON : Key::WIDE_GAMUT_OFF);
return needs;
}
if (needs.hasColorMatrix()) {
fs << "uniform mat4 colorMatrix;";
}
+ if (needs.hasColorMatrix()) {
+ // When in wide gamut mode, the color matrix will contain a color space
+ // conversion matrix that needs to be applied in linear space
+ // When not in wide gamut, we can simply no-op the transfer functions
+ // and let the shader compiler get rid of them
+ if (needs.isWideGamut()) {
+ fs << R"__SHADER__(
+ float OETF_sRGB(const float linear) {
+ return linear <= 0.0031308 ?
+ linear * 12.92 : (pow(linear, 1.0 / 2.4) * 1.055) - 0.055;
+ }
+
+ vec3 OETF_sRGB(const vec3 linear) {
+ return vec3(OETF_sRGB(linear.r), OETF_sRGB(linear.g), OETF_sRGB(linear.b));
+ }
+
+ vec3 OETF_scRGB(const vec3 linear) {
+ return sign(linear.rgb) * OETF_sRGB(abs(linear.rgb));
+ }
+
+ float EOTF_sRGB(float srgb) {
+ return srgb <= 0.04045 ? srgb / 12.92 : pow((srgb + 0.055) / 1.055, 2.4);
+ }
+
+ vec3 EOTF_sRGB(const vec3 srgb) {
+ return vec3(EOTF_sRGB(srgb.r), EOTF_sRGB(srgb.g), EOTF_sRGB(srgb.b));
+ }
+
+ vec3 EOTF_scRGB(const vec3 srgb) {
+ return sign(srgb.rgb) * EOTF_sRGB(abs(srgb.rgb));
+ }
+ )__SHADER__";
+ } else {
+ fs << R"__SHADER__(
+ vec3 OETF_scRGB(const vec3 linear) {
+ return linear;
+ }
+
+ vec3 EOTF_scRGB(const vec3 srgb) {
+ return srgb;
+ }
+ )__SHADER__";
+ }
+ }
fs << "void main(void) {" << indent;
if (needs.isTexturing()) {
fs << "gl_FragColor = texture2D(sampler, outTexCoords);";
if (needs.hasColorMatrix()) {
if (!needs.isOpaque() && needs.isPremultiplied()) {
// un-premultiply if needed before linearization
- fs << "gl_FragColor.rgb = gl_FragColor.rgb/gl_FragColor.a;";
+ // avoid divide by 0 by adding 0.5/256 to the alpha channel
+ fs << "gl_FragColor.rgb = gl_FragColor.rgb / (gl_FragColor.a + 0.0019);";
}
- fs << "vec4 transformed = colorMatrix * vec4(gl_FragColor.rgb, 1);";
- fs << "gl_FragColor.rgb = transformed.rgb/transformed.a;";
+ fs << "vec4 transformed = colorMatrix * vec4(EOTF_scRGB(gl_FragColor.rgb), 1);";
+ // We assume the last row is always {0,0,0,1} and we skip the division by w
+ fs << "gl_FragColor.rgb = OETF_scRGB(transformed.rgb);";
if (!needs.isOpaque() && needs.isPremultiplied()) {
// and re-premultiply if needed after gamma correction
- fs << "gl_FragColor.rgb = gl_FragColor.rgb*gl_FragColor.a;";
+ fs << "gl_FragColor.rgb = gl_FragColor.rgb * (gl_FragColor.a + 0.0019);";
}
}
COLOR_MATRIX_OFF = 0x00000000,
COLOR_MATRIX_ON = 0x00000020,
COLOR_MATRIX_MASK = 0x00000020,
+
+ WIDE_GAMUT_OFF = 0x00000000,
+ WIDE_GAMUT_ON = 0x00000040,
+ WIDE_GAMUT_MASK = 0x00000040,
};
inline Key() : mKey(0) { }
inline bool hasColorMatrix() const {
return (mKey & COLOR_MATRIX_MASK) == COLOR_MATRIX_ON;
}
+ inline bool isWideGamut() const {
+ return (mKey & WIDE_GAMUT_MASK) == WIDE_GAMUT_ON;
+ }
// this is the definition of a friend function -- not a method of class Needs
friend inline int strictly_order_type(const Key& lhs, const Key& rhs) {
- return (lhs.mKey < rhs.mKey) ? 1 : 0;
+ return (lhs.mKey < rhs.mKey) ? 1 : 0;
}
};
if (a == HAL_DATASPACE_DISPLAY_P3 || b == HAL_DATASPACE_DISPLAY_P3) {
return HAL_DATASPACE_DISPLAY_P3;
}
+ if (a == HAL_DATASPACE_V0_SCRGB_LINEAR || b == HAL_DATASPACE_V0_SCRGB_LINEAR) {
+ return HAL_DATASPACE_DISPLAY_P3;
+ }
+ if (a == HAL_DATASPACE_V0_SCRGB || b == HAL_DATASPACE_V0_SCRGB) {
+ return HAL_DATASPACE_DISPLAY_P3;
+ }
+
return HAL_DATASPACE_V0_SRGB;
}
ALOGV("hasClientComposition");
#ifdef USE_HWC2
- mRenderEngine->setColorMode(displayDevice->getActiveColorMode());
mRenderEngine->setWideColor(displayDevice->getWideColorSupport());
+ mRenderEngine->setColorMode(displayDevice->getActiveColorMode());
#endif
if (!displayDevice->makeCurrent(mEGLDisplay, mEGLContext)) {
ALOGW("DisplayDevice::makeCurrent failed. Aborting surface composition for display %s",
// apply a color matrix
n = data.readInt32();
if (n) {
- // color matrix is sent as mat3 matrix followed by vec3
- // offset, then packed into a mat4 where the last row is
- // the offset and extra values are 0
+ // color matrix is sent as a row-major mat4 matrix
for (size_t i = 0 ; i < 4; i++) {
for (size_t j = 0; j < 4; j++) {
mColorMatrix[i][j] = data.readFloat();
} else {
mColorMatrix = mat4();
}
+
+ // Check that supplied matrix's last row is {0,0,0,1} so we can avoid
+ // the division by w in the fragment shader
+ float4 lastRow(transpose(mColorMatrix)[3]);
+ if (any(greaterThan(abs(lastRow - float4{0, 0, 0, 1}), float4{1e-4f}))) {
+ ALOGE("The color transform's last row must be (0, 0, 0, 1)");
+ }
+
invalidateHwcGeometry();
repaintEverything();
return NO_ERROR;
ALOGE("Invalid crop rect: b = %d (> %d)", sourceCrop.bottom, hw_h);
}
+#ifdef USE_HWC2
+ engine.setWideColor(hw->getWideColorSupport());
+ engine.setColorMode(hw->getActiveColorMode());
+#endif
+
// make sure to clear all GL error flags
engine.checkErrors();
err |= native_window_set_scaling_mode(window, NATIVE_WINDOW_SCALING_MODE_SCALE_TO_WINDOW);
err |= native_window_set_buffers_format(window, HAL_PIXEL_FORMAT_RGBA_8888);
err |= native_window_set_usage(window, usage);
+ err |= native_window_set_buffers_data_space(window, hw->getWideColorSupport()
+ ? HAL_DATASPACE_DISPLAY_P3 : HAL_DATASPACE_V0_SRGB);
if (err == NO_ERROR) {
ANativeWindowBuffer* buffer;
OSDN Git Service
