App: cleanup name/id mapping and its reverse

* Single name/id mapping in id_alloc.
* Add Proxy methods to App.
* Disable old API functions to activate surfaces by ID.

Signed-off-by: Marcus Fritzsch <marcus_fritzsch@mentor.com>

2 files changed, 54 insertions, 74 deletions

diff --git a/src/app.cpp b/src/app.cpp
index b8d3a86..6a3e2e6 100644
--- a/src/app.cpp
+++ b/src/app.cpp
@@ -128,7 +128,6 @@ App::App(wl::display *d)
      config(),
      layouts(),
      layers(),
-     name_mapping(),
      id_alloc{},
      last_active() {
    assert(g_app == nullptr);
@@ -357,14 +356,25 @@ void App::surface_set_layout(uint32_t surface_id) {
    if (surface_id == static_cast<unsigned>(this->layers.main_surface)) {
       logdebug("Activating main_surface (%d)", surface_id);
 
-      this->activate_surface(surface_id);
+      this->activate_surface(this->lookup_name(surface_id).value_or("unknown-name").c_str());
    }
 
    logdebug("Surface %u now on layer %u with rect { %d, %d, %d, %d }",
             surface_id, layer_id, x, y, w, h);
 }
 
-char const *App::activate_surface(uint32_t surface_id) {
+char const *App::activate_surface(char const *drawing_name) {
+   int surface_id = -1;
+
+   {
+      auto oid = this->lookup_id(drawing_name);
+      if (oid) {
+         surface_id = oid.value();
+      } else {
+         return "Surface does not exist";
+      }
+   }
+
    if (!this->controller->surface_exists(surface_id)) {
       return "Surface does not exist";
    }
@@ -400,7 +410,18 @@ char const *App::activate_surface(uint32_t surface_id) {
    return nullptr;
 }
 
-char const *App::deactivate_surface(uint32_t surface_id) {
+char const *App::deactivate_surface(char const *drawing_name) {
+   int surface_id = -1;
+
+   {
+      auto oid = this->lookup_id(drawing_name);
+      if (oid) {
+         surface_id = oid.value();
+      } else {
+         return "Surface does not exist";
+      }
+   }
+
    if (surface_id == this->layers.main_surface) {
       return "Cannot deactivate main_surface";
    }
@@ -426,9 +447,9 @@ char const *App::deactivate_surface(uint32_t surface_id) {
 
    if (! this->last_active.empty()) {
       // Should be active already, shouldn't it?
-      this->activate_surface(this->last_active.front());
+      this->activate_surface(this->lookup_name(this->last_active.front()).value_or("unknown-name").c_str());
    } else {
-      this->activate_surface(this->layers.main_surface);
+      this->activate_surface(this->layers.main_surface_name.c_str());
    }
 
    return nullptr;
@@ -500,7 +521,7 @@ result<int> App::request_surface(char const *drawing_name) {
       if (!this->layers.main_surface_name.empty() &&
           this->layers.main_surface_name == drawing_name) {
          this->layers.main_surface = id;
-         this->activate_surface(id);
+         this->activate_surface(drawing_name);
          logdebug("Set main_surface id to %u", id);
       }
 
@@ -511,36 +532,6 @@ result<int> App::request_surface(char const *drawing_name) {
    return Err<int>("Surface already present");
 }
 
-char const *App::activate_surface(char const *drawing_name) {
-   auto osid = this->id_alloc.lookup(drawing_name);
-
-   if (osid) {
-      logdebug("ativate surface with name %s and id %u", drawing_name,
-               osid.value());
-      auto ret = this->activate_surface(osid.value());
-      if (!ret) {
-         this->emit_activated(drawing_name);
-      }
-      return ret;
-   }
-
-   logerror("surface %s unknown", drawing_name);
-   return "Surface unknown";
-}
-
-char const *App::deactivate_surface(char const *drawing_name) {
-   auto osid = this->id_alloc.lookup(drawing_name);
-
-   if (osid) {
-      logdebug("deativate surface with name %s and id %u", drawing_name,
-               osid.value());
-      return this->deactivate_surface(osid.value());
-   }
-
-   logerror("surface %s unknown", drawing_name);
-   return "Surface unknown";
-}
-
 //  _     _           _ _                            _   _                 _
 // | |__ (_)_ __   __| (_)_ __   __ _     __ _ _ __ (_) (_)_ __ ___  _ __ | |
 // | '_ \| | '_ \ / _` | | '_ \ / _` |   / _` | '_ \| | | | '_ ` _ \| '_ \| |
@@ -582,7 +573,7 @@ binding_api::result_type binding_api::enddraw(char const* drawing_name) {
 
 binding_api::result_type binding_api::list_drawing_names() {
    logdebug("%s", __func__);
-   json j = this->app->id_alloc.names;
+   json j = this->app->id_alloc.name2id;
    return Ok(json_tokener_parse(j.dump().c_str()));
 }
 
@@ -612,22 +603,12 @@ binding_api::result_type binding_api::debug_terminate() {
    return Ok(json_object_new_object());
 }
 
-binding_api::result_type binding_api::demo_activate_surface(
-   uint32_t surfaceid) {
-   char const *e = this->app->activate_surface(surfaceid);
-   if (e != nullptr) {
-      return Err<json_object *>(e);
-   }
-   return Ok(json_object_new_object());
+binding_api::result_type binding_api::demo_activate_surface(uint32_t s) {
+   return Err<json_object *>("not implemented");
 }
 
 binding_api::result_type binding_api::demo_activate_all() {
-   for (auto &s : this->app->controller->surfaces) {
-      s.second->set_visibility(1);
-   }
-   this->app->controller->commit_changes();
-   this->app->display->flush();
-   return Ok(json_object_new_object());
+   return Err<json_object *>("not implemented");
 }
 
 //                  _             _ _            _                 _
diff --git a/src/app.hpp b/src/app.hpp
index 6921865..370c65d 100644
--- a/src/app.hpp
+++ b/src/app.hpp
@@ -48,9 +48,9 @@ struct id_allocator {
    unsigned next = 1;
 
    // Surfaces that where requested but not yet created
-   std::unordered_map<unsigned, std::string> surfaces;
+   std::unordered_map<unsigned, std::string> id2name;
    // std::unordered_set<unsigned> pending_surfaces;
-   std::unordered_map<std::string, unsigned> names;
+   std::unordered_map<std::string, unsigned> name2id;
 
    id_allocator(id_allocator const &) = delete;
    id_allocator(id_allocator &&) = delete;
@@ -60,40 +60,40 @@ struct id_allocator {
    // Insert and return a new ID
    unsigned generate_id(std::string const &name) {
       unsigned sid = this->next++;
-      this->surfaces[sid] = name;
+      this->id2name[sid] = name;
       // this->pending_surfaces.insert({sid});
-      this->names[name] = sid;
+      this->name2id[name] = sid;
       logdebug("allocated new id %u with name %s", sid, name.c_str());
       return sid;
    }
 
    // Lookup by ID or by name
    optional<unsigned> lookup(std::string const &name) const {
-      auto i = this->names.find(name);
-      return i == this->names.end() ? nullopt : optional<unsigned>(i->second);
+      auto i = this->name2id.find(name);
+      return i == this->name2id.end() ? nullopt : optional<unsigned>(i->second);
    }
 
    optional<std::string> lookup(unsigned id) const {
-      auto i = this->surfaces.find(id);
-      return i == this->surfaces.end() ? nullopt
+      auto i = this->id2name.find(id);
+      return i == this->id2name.end() ? nullopt
                                        : optional<std::string>(i->second);
    }
 
    // Remove a surface id and name
    // I don't think I will need this, do I?
    void remove_id(std::string const &name) {
-      auto i = this->names.find(name);
-      if (i != this->names.end()) {
-         this->surfaces.erase(i->second);
-         this->names.erase(i);
+      auto i = this->name2id.find(name);
+      if (i != this->name2id.end()) {
+         this->id2name.erase(i->second);
+         this->name2id.erase(i);
       }
    }
 
    void remove_id(unsigned id) {
-      auto i = this->surfaces.find(id);
-      if (i != this->surfaces.end()) {
-         this->names.erase(i->second);
-         this->surfaces.erase(i);
+      auto i = this->id2name.find(id);
+      if (i != this->id2name.end()) {
+         this->name2id.erase(i->second);
+         this->id2name.erase(i);
       }
    }
 };
@@ -113,12 +113,13 @@ struct App {
    layouts_type layouts;
    layer_map layers;
 
-   typedef std::pair<char const *, std::function<void()>> name_task_pair;
-
-   typedef std::map<std::string, int> drawing_name_map;
-   drawing_name_map name_mapping;
-
    struct id_allocator id_alloc;
+   optional<unsigned> lookup_id(char const *name) {
+      return this->id_alloc.lookup(std::string(name));
+   }
+   optional<std::string> lookup_name(unsigned id) {
+      return this->id_alloc.lookup(id);
+   }
 
    std::deque<unsigned> last_active;
 
@@ -136,8 +137,6 @@ struct App {
    int dispatch_events();
 
    void surface_set_layout(uint32_t surface_id);
-   char const *activate_surface(uint32_t surface_id);
-   char const *deactivate_surface(uint32_t surface_id);
 
    // Allocate a surface ID for this role
    result<int> request_surface(char const *drawing_name);