drm_hwcomposer: Stop using HWC2 interface for CTM
Add a new function HwcDisplay::SetColorTransformMatrix to set the
display's CTM. Add a new function to validate the CTM before passing it
into HwcDisplay.
Change-Id: I3258f61c95fc4053aecd9fcc4b7e948e49d4a7f6
Signed-off-by: Drew Davenport <ddavenport@google.com>
diff --git a/hwc2_device/HwcDisplay.cpp b/hwc2_device/HwcDisplay.cpp
index 2df3b4f..4068e71 100644
--- a/hwc2_device/HwcDisplay.cpp
+++ b/hwc2_device/HwcDisplay.cpp
@@ -21,6 +21,8 @@
#include <cinttypes>
+#include <xf86drmMode.h>
+
#include <hardware/gralloc.h>
#include <ui/GraphicBufferAllocator.h>
#include <ui/GraphicBufferMapper.h>
@@ -41,6 +43,56 @@
namespace android {
namespace {
+
+constexpr int kCtmRows = 3;
+constexpr int kCtmCols = 3;
+
+constexpr std::array<float, 16> kIdentityMatrix = {
+ 1.0F, 0.0F, 0.0F, 0.0F, 0.0F, 1.0F, 0.0F, 0.0F,
+ 0.0F, 0.0F, 1.0F, 0.0F, 0.0F, 0.0F, 0.0F, 1.0F,
+};
+
+uint64_t To3132FixPt(float in) {
+ constexpr uint64_t kSignMask = (1ULL << 63);
+ constexpr uint64_t kValueMask = ~(1ULL << 63);
+ constexpr auto kValueScale = static_cast<float>(1ULL << 32);
+ if (in < 0)
+ return (static_cast<uint64_t>(-in * kValueScale) & kValueMask) | kSignMask;
+ return static_cast<uint64_t>(in * kValueScale) & kValueMask;
+}
+
+auto ToColorTransform(const std::array<float, 16> &color_transform_matrix) {
+ /* HAL provides a 4x4 float type matrix:
+ * | 0 1 2 3|
+ * | 4 5 6 7|
+ * | 8 9 10 11|
+ * |12 13 14 15|
+ *
+ * R_out = R*0 + G*4 + B*8 + 12
+ * G_out = R*1 + G*5 + B*9 + 13
+ * B_out = R*2 + G*6 + B*10 + 14
+ *
+ * DRM expects a 3x3 s31.32 fixed point matrix:
+ * out matrix in
+ * |R| |0 1 2| |R|
+ * |G| = |3 4 5| x |G|
+ * |B| |6 7 8| |B|
+ *
+ * R_out = R*0 + G*1 + B*2
+ * G_out = R*3 + G*4 + B*5
+ * B_out = R*6 + G*7 + B*8
+ */
+ auto color_matrix = std::make_shared<drm_color_ctm>();
+ for (int i = 0; i < kCtmCols; i++) {
+ for (int j = 0; j < kCtmRows; j++) {
+ constexpr int kInCtmRows = 4;
+ color_matrix->matrix[i * kCtmRows + j] = To3132FixPt(
+ color_transform_matrix[j * kInCtmRows + i]);
+ }
+ }
+ return color_matrix;
+}
+
// Allocate a black buffer that can be used for an initial modeset when there.
// is no appropriate client buffer available to be used.
// Caller must free the returned buffer with GraphicBufferAllocator::free.
@@ -178,6 +230,24 @@
}
}
+void HwcDisplay::SetColorTransformMatrix(
+ const std::array<float, 16> &color_transform_matrix) {
+ auto almost_equal = [](auto a, auto b) {
+ const float epsilon = 0.001F;
+ return std::abs(a - b) < epsilon;
+ };
+ const bool is_identity = std::equal(color_transform_matrix.begin(),
+ color_transform_matrix.end(),
+ kIdentityMatrix.begin(), almost_equal);
+ color_transform_hint_ = is_identity ? HAL_COLOR_TRANSFORM_IDENTITY
+ : HAL_COLOR_TRANSFORM_ARBITRARY_MATRIX;
+ if (color_transform_hint_ == is_identity) {
+ SetColorMatrixToIdentity();
+ } else {
+ color_matrix_ = ToColorTransform(color_transform_matrix);
+ }
+}
+
void HwcDisplay::SetColorMatrixToIdentity() {
color_matrix_ = std::make_shared<drm_color_ctm>();
for (int i = 0; i < kCtmCols; i++) {
@@ -939,17 +1009,6 @@
return HWC2::Error::None;
}
-#include <xf86drmMode.h>
-
-static uint64_t To3132FixPt(float in) {
- constexpr uint64_t kSignMask = (1ULL << 63);
- constexpr uint64_t kValueMask = ~(1ULL << 63);
- constexpr auto kValueScale = static_cast<float>(1ULL << 32);
- if (in < 0)
- return (static_cast<uint64_t>(-in * kValueScale) & kValueMask) | kSignMask;
- return static_cast<uint64_t>(in * kValueScale) & kValueMask;
-}
-
HWC2::Error HwcDisplay::SetColorTransform(const float *matrix, int32_t hint) {
if (hint < HAL_COLOR_TRANSFORM_IDENTITY ||
hint > HAL_COLOR_TRANSFORM_CORRECT_TRITANOPIA)
@@ -972,37 +1031,14 @@
break;
case HAL_COLOR_TRANSFORM_ARBITRARY_MATRIX:
// Without HW support, we cannot correctly process matrices with an offset.
- for (int i = 12; i < 14; i++) {
- if (matrix[i] != 0.F)
- return HWC2::Error::Unsupported;
- }
-
- /* HAL provides a 4x4 float type matrix:
- * | 0 1 2 3|
- * | 4 5 6 7|
- * | 8 9 10 11|
- * |12 13 14 15|
- *
- * R_out = R*0 + G*4 + B*8 + 12
- * G_out = R*1 + G*5 + B*9 + 13
- * B_out = R*2 + G*6 + B*10 + 14
- *
- * DRM expects a 3x3 s31.32 fixed point matrix:
- * out matrix in
- * |R| |0 1 2| |R|
- * |G| = |3 4 5| x |G|
- * |B| |6 7 8| |B|
- *
- * R_out = R*0 + G*1 + B*2
- * G_out = R*3 + G*4 + B*5
- * B_out = R*6 + G*7 + B*8
- */
- color_matrix_ = std::make_shared<drm_color_ctm>();
- for (int i = 0; i < kCtmCols; i++) {
- for (int j = 0; j < kCtmRows; j++) {
- constexpr int kInCtmRows = 4;
- color_matrix_->matrix[i * kCtmRows + j] = To3132FixPt(matrix[j * kInCtmRows + i]);
+ {
+ for (int i = 12; i < 14; i++) {
+ if (matrix[i] != 0.F)
+ return HWC2::Error::Unsupported;
}
+ std::array<float, 16> aidl_matrix = kIdentityMatrix;
+ memcpy(aidl_matrix.data(), matrix, aidl_matrix.size() * sizeof(float));
+ color_matrix_ = ToColorTransform(aidl_matrix);
}
break;
default:
diff --git a/hwc2_device/HwcDisplay.h b/hwc2_device/HwcDisplay.h
index acefff8..19b4caa 100644
--- a/hwc2_device/HwcDisplay.h
+++ b/hwc2_device/HwcDisplay.h
@@ -52,6 +52,9 @@
HwcDisplay(const HwcDisplay &) = delete;
~HwcDisplay();
+ void SetColorTransformMatrix(
+ const std::array<float, 16> &color_transform_matrix);
+
/* SetPipeline should be carefully used only by DrmHwcTwo hotplug handlers */
void SetPipeline(std::shared_ptr<DrmDisplayPipeline> pipeline);
@@ -260,8 +263,6 @@
uint16_t virtual_disp_width_{};
uint16_t virtual_disp_height_{};
int32_t color_mode_{};
- static constexpr int kCtmRows = 3;
- static constexpr int kCtmCols = 3;
std::shared_ptr<drm_color_ctm> color_matrix_;
android_color_transform_t color_transform_hint_{};
int32_t content_type_{};
diff --git a/hwc3/ComposerClient.cpp b/hwc3/ComposerClient.cpp
index b7c92a8..0b73e36 100644
--- a/hwc3/ComposerClient.cpp
+++ b/hwc3/ComposerClient.cpp
@@ -60,8 +60,12 @@
namespace aidl::android::hardware::graphics::composer3::impl {
namespace {
+constexpr int kCtmRows = 4;
+constexpr int kCtmColumns = 4;
+constexpr int kCtmSize = kCtmRows * kCtmColumns;
+
// clang-format off
-constexpr std::array<float, 16> kIdentityMatrix = {
+constexpr std::array<float, kCtmSize> kIdentityMatrix = {
1.0F, 0.0F, 0.0F, 0.0F,
0.0F, 1.0F, 0.0F, 0.0F,
0.0F, 0.0F, 1.0F, 0.0F,
@@ -89,12 +93,8 @@
}
std::optional<BufferColorSpace> AidlToColorSpace(
- const std::optional<ParcelableDataspace>& dataspace) {
- if (!dataspace) {
- return std::nullopt;
- }
-
- int32_t standard = static_cast<int32_t>(dataspace->dataspace) &
+ const common::Dataspace& dataspace) {
+ int32_t standard = static_cast<int32_t>(dataspace) &
static_cast<int32_t>(common::Dataspace::STANDARD_MASK);
switch (standard) {
case static_cast<int32_t>(common::Dataspace::STANDARD_BT709):
@@ -116,13 +116,17 @@
}
}
-std::optional<BufferSampleRange> AidlToSampleRange(
+std::optional<BufferColorSpace> AidlToColorSpace(
const std::optional<ParcelableDataspace>& dataspace) {
if (!dataspace) {
return std::nullopt;
}
+ return AidlToColorSpace(dataspace->dataspace);
+}
- int32_t sample_range = static_cast<int32_t>(dataspace->dataspace) &
+std::optional<BufferSampleRange> AidlToSampleRange(
+ const common::Dataspace& dataspace) {
+ int32_t sample_range = static_cast<int32_t>(dataspace) &
static_cast<int32_t>(common::Dataspace::RANGE_MASK);
switch (sample_range) {
case static_cast<int32_t>(common::Dataspace::RANGE_FULL):
@@ -137,6 +141,14 @@
}
}
+std::optional<BufferSampleRange> AidlToSampleRange(
+ const std::optional<ParcelableDataspace>& dataspace) {
+ if (!dataspace) {
+ return std::nullopt;
+ }
+ return AidlToSampleRange(dataspace->dataspace);
+}
+
bool IsSupportedCompositionType(
const std::optional<ParcelableComposition> composition) {
if (!composition) {
@@ -161,6 +173,27 @@
}
}
+hwc3::Error ValidateColorTransformMatrix(
+ const std::optional<std::vector<float>>& color_transform_matrix) {
+ if (!color_transform_matrix) {
+ return hwc3::Error::kNone;
+ }
+
+ if (color_transform_matrix->size() != kCtmSize) {
+ ALOGE("Expected color transform matrix of size %d, got size %d.", kCtmSize,
+ (int)color_transform_matrix->size());
+ return hwc3::Error::kBadParameter;
+ }
+
+ // Without HW support, we cannot correctly process matrices with an offset.
+ constexpr int kOffsetIndex = kCtmColumns * 3;
+ for (int i = kOffsetIndex; i < kOffsetIndex + 3; i++) {
+ if (color_transform_matrix.value()[i] != 0.F)
+ return hwc3::Error::kUnsupported;
+ }
+ return hwc3::Error::kNone;
+}
+
bool ValidateLayerBrightness(const std::optional<LayerBrightness>& brightness) {
if (!brightness) {
return true;
@@ -169,6 +202,19 @@
std::isnan(brightness->brightness));
}
+std::optional<std::array<float, kCtmSize>> AidlToColorTransformMatrix(
+ const std::optional<std::vector<float>>& aidl_color_transform_matrix) {
+ if (!aidl_color_transform_matrix ||
+ aidl_color_transform_matrix->size() < kCtmSize) {
+ return std::nullopt;
+ }
+
+ std::array<float, kCtmSize> color_transform_matrix = kIdentityMatrix;
+ std::copy(aidl_color_transform_matrix->begin(),
+ aidl_color_transform_matrix->end(), color_transform_matrix.begin());
+ return color_transform_matrix;
+}
+
std::optional<HWC2::Composition> AidlToCompositionType(
const std::optional<ParcelableComposition> composition) {
if (!composition) {
@@ -591,7 +637,8 @@
void ComposerClient::ExecuteDisplayCommand(const DisplayCommand& command) {
const int64_t display_id = command.display;
- if (hwc_->GetDisplay(display_id) == nullptr) {
+ HwcDisplay* display = hwc_->GetDisplay(display_id);
+ if (display == nullptr) {
cmd_result_writer_->AddError(hwc3::Error::kBadDisplay);
return;
}
@@ -602,14 +649,18 @@
return;
}
+ hwc3::Error error = ValidateColorTransformMatrix(
+ command.colorTransformMatrix);
+ if (error != hwc3::Error::kNone) {
+ ALOGE("Invalid color transform matrix.");
+ cmd_result_writer_->AddError(error);
+ return;
+ }
+
for (const auto& layer_cmd : command.layers) {
DispatchLayerCommand(command.display, layer_cmd);
}
- if (command.colorTransformMatrix) {
- ExecuteSetDisplayColorTransform(command.display,
- *command.colorTransformMatrix);
- }
if (command.clientTarget) {
ExecuteSetDisplayClientTarget(command.display, *command.clientTarget);
}
@@ -617,6 +668,13 @@
ExecuteSetDisplayOutputBuffer(command.display,
*command.virtualDisplayOutputBuffer);
}
+
+ std::optional<std::array<float, kCtmSize>> ctm = AidlToColorTransformMatrix(
+ command.colorTransformMatrix);
+ if (ctm) {
+ display->SetColorTransformMatrix(ctm.value());
+ }
+
if (command.validateDisplay) {
ExecuteValidateDisplay(command.display, command.expectedPresentTime);
}
@@ -1316,29 +1374,6 @@
return err;
}
-void ComposerClient::ExecuteSetDisplayColorTransform(
- uint64_t display_id, const std::vector<float>& matrix) {
- auto* display = GetDisplay(display_id);
- if (display == nullptr) {
- cmd_result_writer_->AddError(hwc3::Error::kBadDisplay);
- return;
- }
-
- auto almost_equal = [](auto a, auto b) {
- const float epsilon = 0.001F;
- return std::abs(a - b) < epsilon;
- };
- const bool is_identity = std::equal(matrix.begin(), matrix.end(),
- kIdentityMatrix.begin(), almost_equal);
-
- const int32_t hint = is_identity ? HAL_COLOR_TRANSFORM_IDENTITY
- : HAL_COLOR_TRANSFORM_ARBITRARY_MATRIX;
-
- auto error = Hwc2toHwc3Error(display->SetColorTransform(matrix.data(), hint));
- if (error != hwc3::Error::kNone) {
- cmd_result_writer_->AddError(error);
- }
-}
void ComposerClient::ExecuteSetDisplayClientTarget(
uint64_t display_id, const ClientTarget& command) {
auto* display = GetDisplay(display_id);
diff --git a/hwc3/ComposerClient.h b/hwc3/ComposerClient.h
index 5938ceb..3fe8250 100644
--- a/hwc3/ComposerClient.h
+++ b/hwc3/ComposerClient.h
@@ -168,8 +168,6 @@
// Display commands
void ExecuteDisplayCommand(const DisplayCommand& command);
- void ExecuteSetDisplayColorTransform(uint64_t display_id,
- const std::vector<float>& matrix);
void ExecuteSetDisplayClientTarget(uint64_t display_id,
const ClientTarget& command);
void ExecuteSetDisplayOutputBuffer(uint64_t display_id, const Buffer& buffer);