drm_hwcomposer: Simplify LayerTransform
Fixes clang-analyzer-optin.core.EnumCastOutOfRange clang tidy check.
Change-Id: I0a88d1ef084848c924198e8bd3831533b6578675
Signed-off-by: Roman Stratiienko <r.stratiienko@gmail.com>
diff --git a/.ci/Makefile b/.ci/Makefile
index 3fc2e9b..f5cfa03 100644
--- a/.ci/Makefile
+++ b/.ci/Makefile
@@ -56,7 +56,6 @@
-readability-math-missing-parentheses \
-readability-avoid-unconditional-preprocessor-if \
-modernize-type-traits \
- -clang-analyzer-optin.core.EnumCastOutOfRange \
-readability-static-accessed-through-instance \
-misc-use-internal-linkage \
-performance-avoid-endl \
diff --git a/compositor/LayerData.h b/compositor/LayerData.h
index a808adc..7eb6cba 100644
--- a/compositor/LayerData.h
+++ b/compositor/LayerData.h
@@ -34,13 +34,11 @@
class DrmFbIdHandle;
/* Rotation is defined in the clockwise direction */
-enum LayerTransform : uint32_t {
- kIdentity = 0,
- kFlipH = 1 << 0,
- kFlipV = 1 << 1,
- kRotate90 = 1 << 2,
- kRotate180 = 1 << 3,
- kRotate270 = 1 << 4,
+/* The flip is done before rotation */
+struct LayerTransform {
+ bool hflip;
+ bool vflip;
+ bool rotate90;
};
struct PresentInfo {
diff --git a/drm/DrmPlane.cpp b/drm/DrmPlane.cpp
index dbb5ad6..0010742 100644
--- a/drm/DrmPlane.cpp
+++ b/drm/DrmPlane.cpp
@@ -88,22 +88,8 @@
GetPlaneProperty("zpos", zpos_property_, Presence::kOptional);
- /* DRM/KMS uses counter-clockwise rotations, while HWC API uses
- * clockwise. That's why 90 and 270 are swapped here.
- */
if (GetPlaneProperty("rotation", rotation_property_, Presence::kOptional)) {
- rotation_property_.AddEnumToMap("rotate-0", LayerTransform::kIdentity,
- transform_enum_map_);
- rotation_property_.AddEnumToMap("rotate-90", LayerTransform::kRotate270,
- transform_enum_map_);
- rotation_property_.AddEnumToMap("rotate-180", LayerTransform::kRotate180,
- transform_enum_map_);
- rotation_property_.AddEnumToMap("rotate-270", LayerTransform::kRotate90,
- transform_enum_map_);
- rotation_property_.AddEnumToMap("reflect-x", LayerTransform::kFlipH,
- transform_enum_map_);
- rotation_property_.AddEnumToMap("reflect-y", LayerTransform::kFlipV,
- transform_enum_map_);
+ rotation_property_.GetEnumMask(transform_enum_mask_);
}
GetPlaneProperty("alpha", alpha_property_, Presence::kOptional);
@@ -166,22 +152,40 @@
return ((1 << crtc.GetIndexInResArray()) & plane_->possible_crtcs) != 0;
}
+static uint64_t ToDrmRotation(LayerTransform transform) {
+ /* DRM/KMS uses counter-clockwise rotations, while HWC API uses
+ * clockwise. That's why 90 and 270 are swapped here.
+ */
+ uint64_t rotation = DRM_MODE_ROTATE_0;
+
+ if (transform.rotate90) {
+ rotation |= DRM_MODE_ROTATE_270;
+ }
+
+ if (transform.hflip) {
+ rotation |= DRM_MODE_REFLECT_X;
+ }
+
+ if (transform.vflip) {
+ rotation |= DRM_MODE_REFLECT_Y;
+ }
+
+ // TODO(nobody): Respect transform_enum_mask_ to find alternative rotation
+ // values
+
+ return rotation;
+}
+
bool DrmPlane::IsValidForLayer(LayerData *layer) {
if (layer == nullptr || !layer->bi) {
ALOGE("%s: Invalid parameters", __func__);
return false;
}
- if (!rotation_property_) {
- if (layer->pi.transform != LayerTransform::kIdentity) {
- ALOGV("No rotation property on plane %d", GetId());
- return false;
- }
- } else {
- if (transform_enum_map_.count(layer->pi.transform) == 0) {
- ALOGV("Transform is not supported on plane %d", GetId());
- return false;
- }
+ uint64_t drm_rotation = ToDrmRotation(layer->pi.transform);
+ if ((drm_rotation & transform_enum_mask_) != drm_rotation) {
+ ALOGV("Transform is not supported on plane %d", GetId());
+ return false;
}
if (!alpha_property_ && layer->pi.alpha != UINT16_MAX) {
@@ -218,27 +222,6 @@
}) != std::end(formats_);
}
-static uint64_t ToDrmRotation(LayerTransform transform) {
- uint64_t rotation = 0;
- /* DRM/KMS uses counter-clockwise rotations, while HWC API uses
- * clockwise. That's why 90 and 270 are swapped here.
- */
- if ((transform & LayerTransform::kFlipH) != 0)
- rotation |= DRM_MODE_REFLECT_X;
- if ((transform & LayerTransform::kFlipV) != 0)
- rotation |= DRM_MODE_REFLECT_Y;
- if ((transform & LayerTransform::kRotate90) != 0)
- rotation |= DRM_MODE_ROTATE_270;
- else if ((transform & LayerTransform::kRotate180) != 0)
- rotation |= DRM_MODE_ROTATE_180;
- else if ((transform & LayerTransform::kRotate270) != 0)
- rotation |= DRM_MODE_ROTATE_90;
- else
- rotation |= DRM_MODE_ROTATE_0;
-
- return rotation;
-}
-
/* Convert float to 16.16 fixed point */
static int To1616FixPt(float in) {
constexpr int kBitShift = 16;
diff --git a/drm/DrmPlane.h b/drm/DrmPlane.h
index c26a3cc..24d21c8 100644
--- a/drm/DrmPlane.h
+++ b/drm/DrmPlane.h
@@ -96,6 +96,6 @@
std::map<BufferBlendMode, uint64_t> blending_enum_map_;
std::map<BufferColorSpace, uint64_t> color_encoding_enum_map_;
std::map<BufferSampleRange, uint64_t> color_range_enum_map_;
- std::map<LayerTransform, uint64_t> transform_enum_map_;
+ uint64_t transform_enum_mask_ = DRM_MODE_ROTATE_0;
};
} // namespace android
diff --git a/drm/DrmProperty.cpp b/drm/DrmProperty.cpp
index dbd307e..76fb4f9 100644
--- a/drm/DrmProperty.cpp
+++ b/drm/DrmProperty.cpp
@@ -144,4 +144,19 @@
return {};
}
+auto DrmProperty::GetEnumMask(uint64_t &mask) -> bool {
+ if (enums_.empty()) {
+ ALOGE("No enum values for property: %s", name_.c_str());
+ return false;
+ }
+
+ mask = 0;
+
+ for (const auto &it : enums_) {
+ mask |= it.value;
+ }
+
+ return true;
+}
+
} // namespace android
diff --git a/drm/DrmProperty.h b/drm/DrmProperty.h
index 2683ad8..860f9ca 100644
--- a/drm/DrmProperty.h
+++ b/drm/DrmProperty.h
@@ -68,6 +68,8 @@
auto AddEnumToMapReverse(const std::string &name, E value,
std::map<uint64_t, E> &map) -> bool;
+ auto GetEnumMask(uint64_t &mask) -> bool;
+
explicit operator bool() const {
return id_ != 0;
}
diff --git a/hwc2_device/hwc2_device.cpp b/hwc2_device/hwc2_device.cpp
index 98f5953..c7bce5f 100644
--- a/hwc2_device/hwc2_device.cpp
+++ b/hwc2_device/hwc2_device.cpp
@@ -300,27 +300,12 @@
GET_DISPLAY(display);
GET_LAYER(layer);
- uint32_t l_transform = 0;
-
- // 270* and 180* cannot be combined with flips. More specifically, they
- // already contain both horizontal and vertical flips, so those fields are
- // redundant in this case. 90* rotation can be combined with either horizontal
- // flip or vertical flip, so treat it differently
- if (transform == HWC_TRANSFORM_ROT_270) {
- l_transform = LayerTransform::kRotate270;
- } else if (transform == HWC_TRANSFORM_ROT_180) {
- l_transform = LayerTransform::kRotate180;
- } else {
- if ((transform & HWC_TRANSFORM_FLIP_H) != 0)
- l_transform |= LayerTransform::kFlipH;
- if ((transform & HWC_TRANSFORM_FLIP_V) != 0)
- l_transform |= LayerTransform::kFlipV;
- if ((transform & HWC_TRANSFORM_ROT_90) != 0)
- l_transform |= LayerTransform::kRotate90;
- }
-
HwcLayer::LayerProperties layer_properties;
- layer_properties.transform = static_cast<LayerTransform>(l_transform);
+ layer_properties.transform = {
+ .hflip = (transform & HAL_TRANSFORM_FLIP_H) != 0,
+ .vflip = (transform & HAL_TRANSFORM_FLIP_V) != 0,
+ .rotate90 = (transform & HAL_TRANSFORM_ROT_90) != 0,
+ };
ilayer->SetLayerProperties(layer_properties);
return 0;
diff --git a/hwc3/ComposerClient.cpp b/hwc3/ComposerClient.cpp
index 90043d8..13371fc 100644
--- a/hwc3/ComposerClient.cpp
+++ b/hwc3/ComposerClient.cpp
@@ -324,28 +324,16 @@
return std::nullopt;
}
- uint32_t transform = LayerTransform::kIdentity;
- // 270* and 180* cannot be combined with flips. More specifically, they
- // already contain both horizontal and vertical flips, so those fields are
- // redundant in this case. 90* rotation can be combined with either horizontal
- // flip or vertical flip, so treat it differently
- if (aidl_transform->transform == common::Transform::ROT_270) {
- transform = LayerTransform::kRotate270;
- } else if (aidl_transform->transform == common::Transform::ROT_180) {
- transform = LayerTransform::kRotate180;
- } else {
- auto aidl_transform_bits = static_cast<uint32_t>(aidl_transform->transform);
- if ((aidl_transform_bits &
- static_cast<uint32_t>(common::Transform::FLIP_H)) != 0)
- transform |= LayerTransform::kFlipH;
- if ((aidl_transform_bits &
- static_cast<uint32_t>(common::Transform::FLIP_V)) != 0)
- transform |= LayerTransform::kFlipV;
- if ((aidl_transform_bits &
- static_cast<uint32_t>(common::Transform::ROT_90)) != 0)
- transform |= LayerTransform::kRotate90;
- }
- return static_cast<LayerTransform>(transform);
+ using aidl::android::hardware::graphics::common::Transform;
+
+ return (LayerTransform){
+ .hflip = (int32_t(aidl_transform->transform) &
+ int32_t(Transform::FLIP_H)) != 0,
+ .vflip = (int32_t(aidl_transform->transform) &
+ int32_t(Transform::FLIP_V)) != 0,
+ .rotate90 = (int32_t(aidl_transform->transform) &
+ int32_t(Transform::ROT_90)) != 0,
+ };
}
} // namespace