Fix VR surface attributes not being read on surface create.

The API to create VR surfaces allows attributes to be passed during
creation. Attributes can also be set/unset after creation. This CL
fixes an issue where visible and z-order attributes are not honored
when passed during creation, only when set after creation. This
required redundant IPCs and confused a lot of people using this
API.

Bug: 64127728
Test: dvr_api-test
Change-Id: Ife319bcca86115ba1bd2ece0ae93f71f15c0dd0e

diff --git a/libs/vr/libvrflinger/display_service.cpp b/libs/vr/libvrflinger/display_service.cpp
index f350762..af18e21 100644 (file)
--- a/libs/vr/libvrflinger/display_service.cpp
+++ b/libs/vr/libvrflinger/display_service.cpp
@@ -243,18 +243,16 @@ Status<display::SurfaceInfo> DisplayService::OnCreateSurface(
           surface_status.GetErrorMessage().c_str());
     return ErrorStatus(surface_status.error());
   }
+  auto surface = surface_status.take();
+  message.SetChannel(surface);
 
-  SurfaceType surface_type = surface_status.get()->surface_type();
-  display::SurfaceUpdateFlags update_flags =
-      surface_status.get()->update_flags();
-  display::SurfaceInfo surface_info{surface_status.get()->surface_id(),
-                                    surface_status.get()->visible(),
-                                    surface_status.get()->z_order()};
+  // Update the surface with the attributes supplied with the create call. For
+  // application surfaces this has the side effect of notifying the display
+  // manager of the new surface. For direct surfaces, this may trigger a mode
+  // change, depending on the value of the visible attribute.
+  surface->OnSetAttributes(message, attributes);
 
-  message.SetChannel(surface_status.take());
-
-  SurfaceUpdated(surface_type, update_flags);
-  return {surface_info};
+  return {{surface->surface_id(), surface->visible(), surface->z_order()}};
 }
 
 void DisplayService::SurfaceUpdated(SurfaceType surface_type,

diff --git a/libs/vr/libvrflinger/display_surface.cpp b/libs/vr/libvrflinger/display_surface.cpp
index 4852fab..6853781 100644 (file)
--- a/libs/vr/libvrflinger/display_surface.cpp
+++ b/libs/vr/libvrflinger/display_surface.cpp
@@ -26,14 +26,12 @@ namespace dvr {
 
 DisplaySurface::DisplaySurface(DisplayService* service,
                                SurfaceType surface_type, int surface_id,
-                               int process_id, int user_id,
-                               const display::SurfaceAttributes& attributes)
+                               int process_id, int user_id)
     : service_(service),
       surface_type_(surface_type),
       surface_id_(surface_id),
       process_id_(process_id),
       user_id_(user_id),
-      attributes_(attributes),
       update_flags_(display::SurfaceUpdateFlags::NewSurface) {}
 
 DisplaySurface::~DisplaySurface() {
@@ -471,8 +469,8 @@ Status<std::shared_ptr<DisplaySurface>> DisplaySurface::Create(
   if (direct) {
     const bool trusted = user_id == AID_ROOT || IsTrustedUid(user_id);
     if (trusted) {
-      return {std::shared_ptr<DisplaySurface>{new DirectDisplaySurface(
-          service, surface_id, process_id, user_id, attributes)}};
+      return {std::shared_ptr<DisplaySurface>{
+          new DirectDisplaySurface(service, surface_id, process_id, user_id)}};
     } else {
       ALOGE(
           "DisplaySurface::Create: Direct surfaces may only be created by "
@@ -482,7 +480,7 @@ Status<std::shared_ptr<DisplaySurface>> DisplaySurface::Create(
     }
   } else {
     return {std::shared_ptr<DisplaySurface>{new ApplicationDisplaySurface(
-        service, surface_id, process_id, user_id, attributes)}};
+        service, surface_id, process_id, user_id)}};
   }
 }
 

diff --git a/libs/vr/libvrflinger/display_surface.h b/libs/vr/libvrflinger/display_surface.h
index 7a0fb18..c8b1a07 100644 (file)
--- a/libs/vr/libvrflinger/display_surface.h
+++ b/libs/vr/libvrflinger/display_surface.h
@@ -53,8 +53,7 @@ class DisplaySurface : public pdx::Channel {
 
  protected:
   DisplaySurface(DisplayService* service, SurfaceType surface_type,
-                 int surface_id, int process_id, int user_id,
-                 const display::SurfaceAttributes& attributes);
+                 int surface_id, int process_id, int user_id);
 
   // Utility to retrieve a shared pointer to this channel as the desired derived
   // type.
@@ -119,10 +118,9 @@ class DisplaySurface : public pdx::Channel {
 class ApplicationDisplaySurface : public DisplaySurface {
  public:
   ApplicationDisplaySurface(DisplayService* service, int surface_id,
-                            int process_id, int user_id,
-                            const display::SurfaceAttributes& attributes)
+                            int process_id, int user_id)
       : DisplaySurface(service, SurfaceType::Application, surface_id,
-                       process_id, user_id, attributes) {}
+                       process_id, user_id) {}
 
   std::shared_ptr<ConsumerQueue> GetQueue(int32_t queue_id);
   std::vector<int32_t> GetQueueIds() const override;
@@ -140,12 +138,11 @@ class ApplicationDisplaySurface : public DisplaySurface {
 class DirectDisplaySurface : public DisplaySurface {
  public:
   DirectDisplaySurface(DisplayService* service, int surface_id, int process_id,
-                       int user_id,
-                       const display::SurfaceAttributes& attributes)
+                       int user_id)
       : DisplaySurface(service, SurfaceType::Direct, surface_id, process_id,
-                       user_id, attributes),
+                       user_id),
         acquired_buffers_(kMaxPostedBuffers),
-        metadata_(nullptr){}
+        metadata_(nullptr) {}
   std::vector<int32_t> GetQueueIds() const override;
   bool IsBufferAvailable();
   bool IsBufferPosted();