Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 88d37dda authored by Romain Guy's avatar Romain Guy
Browse files

Various fixes for wide color gamut rendering 

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
parent 177759a9

include/gui/SurfaceComposerClient.h

+1 −0
Original line number Diff line number Diff line
@@ -271,6 +271,7 @@ public: 
    uint32_t getStride() const;

    // size of allocated memory in bytes

    size_t getSize() const;

    android_dataspace getDataSpace() const;

};



// ---------------------------------------------------------------------------

libs/gui/SurfaceComposerClient.cpp

+4 −0
Original line number Diff line number Diff line
@@ -1080,5 +1080,9 @@ size_t ScreenshotClient::getSize() const { 
    return mBuffer.stride * mBuffer.height * bytesPerPixel(mBuffer.format);

}



android_dataspace ScreenshotClient::getDataSpace() const {

    return mBuffer.dataSpace;

}



// ----------------------------------------------------------------------------

}; // namespace android

libs/math/include/math/TMatHelpers.h

+2 −2
Original line number Diff line number Diff line
@@ -342,9 +342,9 @@ TQuaternion<typename MATRIX::value_type> extractQuat(const MATRIX& mat) { 
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");

services/surfaceflinger/RenderEngine/Description.cpp

+7 −15
Original line number Diff line number Diff line
@@ -26,8 +26,7 @@ 


namespace android {



Description::Description() :

    mUniformsDirty(true) {

Description::Description() {

    mPlaneAlpha = 1.0f;

    mPremultipliedAlpha = false;

    mOpaque = true;
@@ -41,28 +40,20 @@ Description::~Description() { 
}



void Description::setPlaneAlpha(GLclampf planeAlpha) {

    if (planeAlpha != mPlaneAlpha) {

        mUniformsDirty = true;

    mPlaneAlpha = planeAlpha;

}

}



void Description::setPremultipliedAlpha(bool premultipliedAlpha) {

    if (premultipliedAlpha != mPremultipliedAlpha) {

    mPremultipliedAlpha = premultipliedAlpha;

}

}



void Description::setOpaque(bool opaque) {

    if (opaque != mOpaque) {

    mOpaque = opaque;

}

}



void Description::setTexture(const Texture& texture) {

    mTexture = texture;

    mTextureEnabled = true;

    mUniformsDirty = true;

}



void Description::disableTexture() {
@@ -74,12 +65,10 @@ void Description::setColor(GLclampf red, GLclampf green, GLclampf blue, GLclampf 
    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) {
@@ -92,5 +81,8 @@ const mat4& Description::getColorMatrix() const { 
    return mColorMatrix;

}



void Description::setWideGamut(bool wideGamut) {

    mIsWideGamut = wideGamut;

}



} /* namespace android */

services/surfaceflinger/RenderEngine/Description.h

+3 −3
Original line number Diff line number Diff line
@@ -54,6 +54,8 @@ class Description { 
    bool mColorMatrixEnabled;

    mat4 mColorMatrix;



    bool mIsWideGamut;



public:

    Description();

    ~Description();
@@ -67,9 +69,7 @@ public: 
    void setProjectionMatrix(const mat4& mtx);

    void setColorMatrix(const mat4& mtx);

    const mat4& getColorMatrix() const;



private:

    bool mUniformsDirty;

    void setWideGamut(bool wideGamut);

};



} /* namespace android */