Merge changes from topic "color-auto-legacy-srgb" into pi-dev
* changes:
surfaceflinger: no re-saturation when in SRGB mode
surfaceflinger: switch RE color matrices lazily
surfaceflinger: remove hwcId < 0 path in doComposeSurfaces
surfaceflinger: re-saturate all legacy dataspaces
diff --git a/services/surfaceflinger/BufferLayer.cpp b/services/surfaceflinger/BufferLayer.cpp
index 4c3844e..6feec53 100644
--- a/services/surfaceflinger/BufferLayer.cpp
+++ b/services/surfaceflinger/BufferLayer.cpp
@@ -528,11 +528,9 @@
recomputeVisibleRegions = true;
}
- // Dataspace::V0_SRGB and Dataspace::V0_SRGB_LINEAR are not legacy
- // data space, however since framework doesn't distinguish them out of
- // legacy SRGB, we have to treat them as the same for now.
- // UNKNOWN is treated as legacy SRGB when the connected api is EGL.
ui::Dataspace dataSpace = mConsumer->getCurrentDataSpace();
+ // treat modern dataspaces as legacy dataspaces whenever possible, until
+ // we can trust the buffer producers
switch (dataSpace) {
case ui::Dataspace::V0_SRGB:
dataSpace = ui::Dataspace::SRGB;
@@ -540,10 +538,17 @@
case ui::Dataspace::V0_SRGB_LINEAR:
dataSpace = ui::Dataspace::SRGB_LINEAR;
break;
- case ui::Dataspace::UNKNOWN:
- if (mConsumer->getCurrentApi() == NATIVE_WINDOW_API_EGL) {
- dataSpace = ui::Dataspace::SRGB;
- }
+ case ui::Dataspace::V0_JFIF:
+ dataSpace = ui::Dataspace::JFIF;
+ break;
+ case ui::Dataspace::V0_BT601_625:
+ dataSpace = ui::Dataspace::BT601_625;
+ break;
+ case ui::Dataspace::V0_BT601_525:
+ dataSpace = ui::Dataspace::BT601_525;
+ break;
+ case ui::Dataspace::V0_BT709:
+ dataSpace = ui::Dataspace::BT709;
break;
default:
break;
diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp
index 2802fc7..9043234 100644
--- a/services/surfaceflinger/Layer.cpp
+++ b/services/surfaceflinger/Layer.cpp
@@ -1650,9 +1650,10 @@
return true;
}
-bool Layer::isLegacySrgbDataSpace() const {
- return mDrawingState.dataSpace == ui::Dataspace::SRGB ||
- mDrawingState.dataSpace == ui::Dataspace::SRGB_LINEAR;
+bool Layer::isLegacyDataSpace() const {
+ // return true when no higher bits are set
+ return !(mDrawingState.dataSpace & (ui::Dataspace::STANDARD_MASK |
+ ui::Dataspace::TRANSFER_MASK | ui::Dataspace::RANGE_MASK));
}
void Layer::setParent(const sp<Layer>& layer) {
diff --git a/services/surfaceflinger/Layer.h b/services/surfaceflinger/Layer.h
index 0b15b67..632efbe 100644
--- a/services/surfaceflinger/Layer.h
+++ b/services/surfaceflinger/Layer.h
@@ -303,8 +303,8 @@
// desaturated in order to match what they appears like visually.
// With color management, these contents will appear desaturated, thus
// needed to be saturated so that they match what they are designed for
- // visually. When returns true, legacy SRGB data space is passed to HWC.
- bool isLegacySrgbDataSpace() const;
+ // visually.
+ bool isLegacyDataSpace() const;
// If we have received a new buffer this frame, we will pass its surface
// damage down to hardware composer. Otherwise, we must send a region with
diff --git a/services/surfaceflinger/RenderEngine/GLES20RenderEngine.cpp b/services/surfaceflinger/RenderEngine/GLES20RenderEngine.cpp
index 08cd5b0..64095dd 100644
--- a/services/surfaceflinger/RenderEngine/GLES20RenderEngine.cpp
+++ b/services/surfaceflinger/RenderEngine/GLES20RenderEngine.cpp
@@ -259,10 +259,8 @@
mState.setTexture(texture);
}
-mat4 GLES20RenderEngine::setupColorTransform(const mat4& colorTransform) {
- mat4 oldTransform = mState.getColorMatrix();
+void GLES20RenderEngine::setupColorTransform(const mat4& colorTransform) {
mState.setColorMatrix(colorTransform);
- return oldTransform;
}
void GLES20RenderEngine::disableTexturing() {
diff --git a/services/surfaceflinger/RenderEngine/GLES20RenderEngine.h b/services/surfaceflinger/RenderEngine/GLES20RenderEngine.h
index 9acd79b..c9e402d 100644
--- a/services/surfaceflinger/RenderEngine/GLES20RenderEngine.h
+++ b/services/surfaceflinger/RenderEngine/GLES20RenderEngine.h
@@ -80,7 +80,7 @@
virtual void setupLayerTexturing(const Texture& texture);
virtual void setupLayerBlackedOut();
virtual void setupFillWithColor(float r, float g, float b, float a);
- virtual mat4 setupColorTransform(const mat4& colorTransform);
+ virtual void setupColorTransform(const mat4& colorTransform);
virtual void disableTexturing();
virtual void disableBlending();
diff --git a/services/surfaceflinger/RenderEngine/RenderEngine.h b/services/surfaceflinger/RenderEngine/RenderEngine.h
index df9e6a7..d559464 100644
--- a/services/surfaceflinger/RenderEngine/RenderEngine.h
+++ b/services/surfaceflinger/RenderEngine/RenderEngine.h
@@ -112,7 +112,7 @@
virtual void setupLayerBlackedOut() = 0;
virtual void setupFillWithColor(float r, float g, float b, float a) = 0;
- virtual mat4 setupColorTransform(const mat4& /* colorTransform */) = 0;
+ virtual void setupColorTransform(const mat4& /* colorTransform */) = 0;
virtual void disableTexturing() = 0;
virtual void disableBlending() = 0;
@@ -224,7 +224,7 @@
void checkErrors() const override;
- mat4 setupColorTransform(const mat4& /* colorTransform */) override { return mat4(); }
+ void setupColorTransform(const mat4& /* colorTransform */) override {}
// internal to RenderEngine
EGLDisplay getEGLDisplay() const;
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 2e8a276..be70244 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -1872,13 +1872,13 @@
// Returns a dataspace that fits all visible layers. The returned dataspace
// can only be one of
//
-// - Dataspace::V0_SRGB
+// - Dataspace::SRGB (use legacy dataspace and let HWC saturate when colors are enhanced)
// - Dataspace::DISPLAY_P3
// - Dataspace::V0_SCRGB_LINEAR
// TODO(b/73825729) Add BT2020 data space.
ui::Dataspace SurfaceFlinger::getBestDataspace(
const sp<const DisplayDevice>& displayDevice) const {
- Dataspace bestDataspace = Dataspace::V0_SRGB;
+ Dataspace bestDataspace = Dataspace::SRGB;
for (const auto& layer : displayDevice->getVisibleLayersSortedByZ()) {
switch (layer->getDataSpace()) {
case Dataspace::V0_SCRGB:
@@ -1929,7 +1929,7 @@
break;
default:
*outMode = ColorMode::SRGB;
- *outDataSpace = Dataspace::V0_SRGB;
+ *outDataSpace = Dataspace::SRGB;
break;
}
}
@@ -2872,18 +2872,13 @@
const DisplayRenderArea renderArea(displayDevice);
const auto hwcId = displayDevice->getHwcDisplayId();
const bool hasClientComposition = getBE().mHwc->hasClientComposition(hwcId);
- const bool hasDeviceComposition = getBE().mHwc->hasDeviceComposition(hwcId);
- const bool skipClientColorTransform = getBE().mHwc->hasCapability(
- HWC2::Capability::SkipClientColorTransform);
ATRACE_INT("hasClientComposition", hasClientComposition);
- mat4 oldColorMatrix;
- mat4 legacySrgbSaturationMatrix = mLegacySrgbSaturationMatrix;
- const bool applyColorMatrix = !hasDeviceComposition && !skipClientColorTransform;
- if (applyColorMatrix) {
- oldColorMatrix = getRenderEngine().setupColorTransform(mDrawingState.colorMatrix);
- legacySrgbSaturationMatrix = mDrawingState.colorMatrix * legacySrgbSaturationMatrix;
- }
+ bool applyColorMatrix = false;
+ bool applyLegacyColorMatrix = false;
+ mat4 colorMatrix;
+ mat4 legacyColorMatrix;
+ const mat4* currentColorMatrix = nullptr;
if (hasClientComposition) {
ALOGV("hasClientComposition");
@@ -2896,6 +2891,26 @@
getBE().mRenderEngine->setDisplayMaxLuminance(
displayDevice->getHdrCapabilities().getDesiredMaxLuminance());
+ const bool hasDeviceComposition = getBE().mHwc->hasDeviceComposition(hwcId);
+ const bool skipClientColorTransform = getBE().mHwc->hasCapability(
+ HWC2::Capability::SkipClientColorTransform);
+
+ applyColorMatrix = !hasDeviceComposition && !skipClientColorTransform;
+ if (applyColorMatrix) {
+ colorMatrix = mDrawingState.colorMatrix;
+ }
+
+ applyLegacyColorMatrix = (mDisplayColorSetting == DisplayColorSetting::ENHANCED &&
+ outputDataspace != Dataspace::UNKNOWN &&
+ outputDataspace != Dataspace::SRGB);
+ if (applyLegacyColorMatrix) {
+ if (applyColorMatrix) {
+ legacyColorMatrix = colorMatrix * mLegacySrgbSaturationMatrix;
+ } else {
+ legacyColorMatrix = mLegacySrgbSaturationMatrix;
+ }
+ }
+
if (!displayDevice->makeCurrent()) {
ALOGW("DisplayDevice::makeCurrent failed. Aborting surface composition for display %s",
displayDevice->getDisplayName().string());
@@ -2957,69 +2972,57 @@
ALOGV("Rendering client layers");
const Transform& displayTransform = displayDevice->getTransform();
- if (hwcId >= 0) {
- // we're using h/w composer
- bool firstLayer = true;
- for (auto& layer : displayDevice->getVisibleLayersSortedByZ()) {
- const Region clip(bounds.intersect(
- displayTransform.transform(layer->visibleRegion)));
- ALOGV("Layer: %s", layer->getName().string());
- ALOGV(" Composition type: %s",
- to_string(layer->getCompositionType(hwcId)).c_str());
- if (!clip.isEmpty()) {
- switch (layer->getCompositionType(hwcId)) {
- case HWC2::Composition::Cursor:
- case HWC2::Composition::Device:
- case HWC2::Composition::Sideband:
- case HWC2::Composition::SolidColor: {
- const Layer::State& state(layer->getDrawingState());
- if (layer->getClearClientTarget(hwcId) && !firstLayer &&
- layer->isOpaque(state) && (state.color.a == 1.0f)
- && hasClientComposition) {
- // never clear the very first layer since we're
- // guaranteed the FB is already cleared
- layer->clearWithOpenGL(renderArea);
- }
- break;
+ bool firstLayer = true;
+ for (auto& layer : displayDevice->getVisibleLayersSortedByZ()) {
+ const Region clip(bounds.intersect(
+ displayTransform.transform(layer->visibleRegion)));
+ ALOGV("Layer: %s", layer->getName().string());
+ ALOGV(" Composition type: %s",
+ to_string(layer->getCompositionType(hwcId)).c_str());
+ if (!clip.isEmpty()) {
+ switch (layer->getCompositionType(hwcId)) {
+ case HWC2::Composition::Cursor:
+ case HWC2::Composition::Device:
+ case HWC2::Composition::Sideband:
+ case HWC2::Composition::SolidColor: {
+ const Layer::State& state(layer->getDrawingState());
+ if (layer->getClearClientTarget(hwcId) && !firstLayer &&
+ layer->isOpaque(state) && (state.color.a == 1.0f)
+ && hasClientComposition) {
+ // never clear the very first layer since we're
+ // guaranteed the FB is already cleared
+ layer->clearWithOpenGL(renderArea);
}
- case HWC2::Composition::Client: {
- // Only apply saturation matrix layer that is legacy SRGB dataspace
- // when auto color mode is on.
- bool restore = false;
- mat4 savedMatrix;
- if (mDisplayColorSetting == DisplayColorSetting::ENHANCED &&
- layer->isLegacySrgbDataSpace()) {
- savedMatrix =
- getRenderEngine().setupColorTransform(legacySrgbSaturationMatrix);
- restore = true;
- }
- layer->draw(renderArea, clip);
- if (restore) {
- getRenderEngine().setupColorTransform(savedMatrix);
- }
- break;
- }
- default:
- break;
+ break;
}
- } else {
- ALOGV(" Skipping for empty clip");
+ case HWC2::Composition::Client: {
+ // switch color matrices lazily
+ if (layer->isLegacyDataSpace()) {
+ if (applyLegacyColorMatrix && currentColorMatrix != &legacyColorMatrix) {
+ getRenderEngine().setupColorTransform(legacyColorMatrix);
+ currentColorMatrix = &legacyColorMatrix;
+ }
+ } else {
+ if (applyColorMatrix && currentColorMatrix != &colorMatrix) {
+ getRenderEngine().setupColorTransform(colorMatrix);
+ currentColorMatrix = &colorMatrix;
+ }
+ }
+
+ layer->draw(renderArea, clip);
+ break;
+ }
+ default:
+ break;
}
- firstLayer = false;
+ } else {
+ ALOGV(" Skipping for empty clip");
}
- } else {
- // we're not using h/w composer
- for (auto& layer : displayDevice->getVisibleLayersSortedByZ()) {
- const Region clip(bounds.intersect(
- displayTransform.transform(layer->visibleRegion)));
- if (!clip.isEmpty()) {
- layer->draw(renderArea, clip);
- }
- }
+ firstLayer = false;
}
- if (applyColorMatrix) {
- getRenderEngine().setupColorTransform(oldColorMatrix);
+ if (applyColorMatrix || applyLegacyColorMatrix) {
+ getRenderEngine().setupColorTransform(mat4());
}
// disable scissor at the end of the frame
diff --git a/services/surfaceflinger/tests/unittests/mock/RenderEngine/MockRenderEngine.h b/services/surfaceflinger/tests/unittests/mock/RenderEngine/MockRenderEngine.h
index 29cd2d5..93769a5 100644
--- a/services/surfaceflinger/tests/unittests/mock/RenderEngine/MockRenderEngine.h
+++ b/services/surfaceflinger/tests/unittests/mock/RenderEngine/MockRenderEngine.h
@@ -60,7 +60,7 @@
MOCK_METHOD1(setupLayerTexturing, void(const Texture&));
MOCK_METHOD0(setupLayerBlackedOut, void());
MOCK_METHOD4(setupFillWithColor, void(float, float, float, float));
- MOCK_METHOD1(setupColorTransform, mat4(const mat4&));
+ MOCK_METHOD1(setupColorTransform, void(const mat4&));
MOCK_METHOD0(disableTexturing, void());
MOCK_METHOD0(disableBlending, void());
MOCK_METHOD1(setSourceY410BT2020, void(bool));