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>

.ci/Makefile

@@ -56,7 +56,6 @@ TIDY_CHECKS_FINE := *                                   \
     -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                             \

compositor/LayerData.h

@@ -34,13 +34,11 @@ namespace android {
 class DrmFbIdHandle;
 
 /* Rotation is defined in the clockwise direction */
-enum LayerTransform : uint32_t {
+/* The flip is done before rotation */
-  kIdentity = 0,
+struct LayerTransform {
-  kFlipH = 1 << 0,
+  bool hflip;
-  kFlipV = 1 << 1,
+  bool vflip;
-  kRotate90 = 1 << 2,
+  bool rotate90;
-  kRotate180 = 1 << 3,
-  kRotate270 = 1 << 4,
 };
 
 struct PresentInfo {

drm/DrmPlane.cpp

@@ -88,22 +88,8 @@ int DrmPlane::Init() {
 
   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,
+    rotation_property_.GetEnumMask(transform_enum_mask_);
-                                    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_);
   }
 
   GetPlaneProperty("alpha", alpha_property_, Presence::kOptional);
@@ -166,22 +152,40 @@ bool DrmPlane::IsCrtcSupported(const DrmCrtc &crtc) const {
   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_) {
+  uint64_t drm_rotation = ToDrmRotation(layer->pi.transform);
-    if (layer->pi.transform != LayerTransform::kIdentity) {
+  if ((drm_rotation & transform_enum_mask_) != drm_rotation) {
-      ALOGV("No rotation property on plane %d", GetId());
+    ALOGV("Transform is not supported on plane %d", GetId());
-      return false;
+    return false;
-    }
-  } else {
-    if (transform_enum_map_.count(layer->pi.transform) == 0) {
-      ALOGV("Transform is not supported on plane %d", GetId());
-      return false;
-    }
   }
 
   if (!alpha_property_ && layer->pi.alpha != UINT16_MAX) {
@@ -218,27 +222,6 @@ bool DrmPlane::HasNonRgbFormat() const {
                           }) != 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;

drm/DrmPlane.h

@@ -96,6 +96,6 @@ class DrmPlane : public PipelineBindable<DrmPlane> {
   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

drm/DrmProperty.cpp

@@ -144,4 +144,19 @@ std::optional<std::string> DrmProperty::GetEnumNameFromValue(
   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

drm/DrmProperty.h

@@ -68,6 +68,8 @@ class DrmProperty {
   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;
   }

hwc2_device/hwc2_device.cpp

@@ -300,27 +300,12 @@ static int32_t SetLayerTransform(hwc2_device_t *device, hwc2_display_t display,
   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;

hwc3/ComposerClient.cpp

@@ -324,28 +324,16 @@ std::optional<LayerTransform> AidlToLayerTransform(
     return std::nullopt;
   }
 
-  uint32_t transform = LayerTransform::kIdentity;
+  using aidl::android::hardware::graphics::common::Transform;
-  // 270* and 180* cannot be combined with flips. More specifically, they
+
-  // already contain both horizontal and vertical flips, so those fields are
+  return (LayerTransform){
-  // redundant in this case. 90* rotation can be combined with either horizontal
+      .hflip = (int32_t(aidl_transform->transform) &
-  // flip or vertical flip, so treat it differently
+                int32_t(Transform::FLIP_H)) != 0,
-  if (aidl_transform->transform == common::Transform::ROT_270) {
+      .vflip = (int32_t(aidl_transform->transform) &
-    transform = LayerTransform::kRotate270;
+                int32_t(Transform::FLIP_V)) != 0,
-  } else if (aidl_transform->transform == common::Transform::ROT_180) {
+      .rotate90 = (int32_t(aidl_transform->transform) &
-    transform = LayerTransform::kRotate180;
+                   int32_t(Transform::ROT_90)) != 0,
-  } 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);
 }
 
 }  // namespace