From 2edff36f503d52289ca5306a9ae876f637fe55c3 Mon Sep 17 00:00:00 2001 From: Rahul Singhal Date: Fri, 23 Sep 2011 15:16:28 +0530 Subject: [PATCH] Add restrictions on adding a surface to a layer This prevents: 1. A surface being added to multiple layers. 2. A surface being added to the same layer multiple times. API Calls: Added: unsigned int Surface::getContainingLayerId() const Added: void Surface::setContainingLayerId(unsigned int id) Added: inline void Layer::removeAllSurfaces() Added: bool Scene::isLayerInCurrentRenderOrder(const uint id) Removed: void Scene::removeSurfaceFromAllLayers(Surafce* surface) Change-Id: I788e716aabf322b9b800157d1c498ca62e468b90 --- .../ilmClient/include/ilm_client_priv.h | 4 +-- .../src/LayerAddSurfaceCommand.cpp | 18 +++++++--- LayerManagerCommands/src/ScreenShotCommand.cpp | 39 ++++------------------ .../src/SetOrderWithinLayerCommand.cpp | 17 +++++++--- LayerManagerService/include/Layer.h | 29 +++++++++++++--- LayerManagerService/include/Scene.h | 2 +- LayerManagerService/include/Surface.h | 13 ++++++++ LayerManagerService/src/Scene.cpp | 39 +++++++++++++--------- 8 files changed, 97 insertions(+), 64 deletions(-) diff --git a/LayerManagerClient/ilmClient/include/ilm_client_priv.h b/LayerManagerClient/ilmClient/include/ilm_client_priv.h index 1e08b39..3109922 100644 --- a/LayerManagerClient/ilmClient/include/ilm_client_priv.h +++ b/LayerManagerClient/ilmClient/include/ilm_client_priv.h @@ -45,14 +45,14 @@ static t_ilm_bool g_ilm_init = ILM_FALSE; static t_ilm_client* g_ilm_client; #define ILM_ERROR(method,error) \ - fprintf (stderr,"[ILM_CLIENT][%s] %s",method,error) + fprintf (stderr,"[ILM_CLIENT][%s] %s\n",method,error) #define ILM_CHECK_METHOD_ERROR(message) \ if (message){ \ t_ilm_int messageType = dbus_message_get_type(message); \ if (messageType == DBUS_MESSAGE_TYPE_ERROR) \ { \ - fprintf (stderr,"[ILM_CLIENT][%s] DBUS ERROR: %s",__func__,dbus_message_get_error_name(message) ); \ + fprintf (stderr,"[ILM_CLIENT][%s] DBUS ERROR: %s\n",__func__,dbus_message_get_error_name(message) ); \ return ILM_FAILED; \ } \ } diff --git a/LayerManagerCommands/src/LayerAddSurfaceCommand.cpp b/LayerManagerCommands/src/LayerAddSurfaceCommand.cpp index 15f1eec..4239402 100644 --- a/LayerManagerCommands/src/LayerAddSurfaceCommand.cpp +++ b/LayerManagerCommands/src/LayerAddSurfaceCommand.cpp @@ -36,12 +36,20 @@ ExecutionResult LayerAddSurfaceCommand::execute(ICommandExecutor* executor) Layer* layer = scene.getLayer(m_layerid); Surface* surface = scene.getSurface(m_surfaceid); - if (layer != NULL && surface != NULL ) + if (layer != NULL && surface != NULL) { - LOG_DEBUG("LayerAddSurfaceCommand","add surface(" <getID() << " to layer(" << m_layerid << ") " << layer->getID()); - layer->addSurface(surface); - LOG_DEBUG("LayerAddSurfaceCommand", "Layer now has #surfaces:" << layer->getAllSurfaces().size()); - result = ExecutionSuccessRedraw; + unsigned int layer_id = surface->getContainingLayerId(); + if (layer_id != GraphicalObject::INVALID_ID) + { + LOG_WARNING("LayerAddSurfaceCommand","surface : id [ " << m_surfaceid << " ] already belongs to layer : id [ " << layer_id << " ]"); + } + else + { + LOG_DEBUG("LayerAddSurfaceCommand","add surface(" << m_surfaceid << ")" << surface->getID() << " to layer(" << m_layerid << ") " << layer->getID()); + layer->addSurface(surface); + LOG_DEBUG("LayerAddSurfaceCommand", "Layer now has #surfaces:" << layer->getAllSurfaces().size()); + result = ExecutionSuccessRedraw; + } } return result; diff --git a/LayerManagerCommands/src/ScreenShotCommand.cpp b/LayerManagerCommands/src/ScreenShotCommand.cpp index a7c4695..fd0b33a 100644 --- a/LayerManagerCommands/src/ScreenShotCommand.cpp +++ b/LayerManagerCommands/src/ScreenShotCommand.cpp @@ -36,7 +36,7 @@ ExecutionResult ScreenShotCommand::execute(ICommandExecutor* executor) bool status = false; unsigned int layer_id = 0; - LOG_INFO("LayerManager","making screenshot, output file: " << m_filename); + LOG_INFO("ScreenShotCommand","making screenshot, output file: " << m_filename); // check for invalid screen, surface or layer ids switch (m_type) @@ -48,19 +48,10 @@ ExecutionResult ScreenShotCommand::execute(ICommandExecutor* executor) case ScreenshotOfLayer: if (scene.getLayer(m_id)) { - LayerListConstIterator iter = scene.getCurrentRenderOrder().begin(); - LayerListConstIterator iterEnd = scene.getCurrentRenderOrder().end(); - for (; iter != iterEnd; ++iter) - { - if ((*iter)->getID() == m_id) - { - status = true; - break; - } - } + status = scene.isLayerInCurrentRenderOrder(m_id); if (!status) { - LOG_WARNING("LayerManager","Requested layer: " << m_id << " does not belong to the current render order"); + LOG_WARNING("ScreenShotCommand","Requested layer: " << m_id << " does not belong to the current render order"); } } break; @@ -68,29 +59,11 @@ ExecutionResult ScreenShotCommand::execute(ICommandExecutor* executor) case ScreenshotOfSurface: if (scene.getSurface(m_id)) { - LayerListConstIterator iter = scene.getCurrentRenderOrder().begin(); - LayerListConstIterator iterEnd = scene.getCurrentRenderOrder().end(); - for (; iter != iterEnd; ++iter) - { - SurfaceListConstIterator s_iter = (*iter)->getAllSurfaces().begin(); - SurfaceListConstIterator s_iterEnd = (*iter)->getAllSurfaces().end(); - for (; s_iter != s_iterEnd; ++s_iter) - { - if ((*s_iter)->getID() == m_id) - { - layer_id = (*iter)->getID(); - status = true; - break; - } - } - if (status) - { - break; - } - } + layer_id = scene.getSurface(m_id)->getContainingLayerId(); + status = scene.isLayerInCurrentRenderOrder(layer_id); if (!status) { - LOG_WARNING("LayerManager","Requested surface: " << m_id << " does not belong to a layer which is part of the current render order"); + LOG_WARNING("ScreenShotCommand","Requested surface: " << m_id << " does not belong to a layer which is part of the current render order"); } } break; diff --git a/LayerManagerCommands/src/SetOrderWithinLayerCommand.cpp b/LayerManagerCommands/src/SetOrderWithinLayerCommand.cpp index 4807a06..3646cec 100644 --- a/LayerManagerCommands/src/SetOrderWithinLayerCommand.cpp +++ b/LayerManagerCommands/src/SetOrderWithinLayerCommand.cpp @@ -38,7 +38,8 @@ ExecutionResult SetOrderWithinLayerCommand::execute(ICommandExecutor* executor) if (layer) { - layer->getAllSurfaces().clear(); + layer->removeAllSurfaces(); + result = ExecutionSuccessRedraw; for (unsigned int surfaceIndex = 0; surfaceIndex < m_length; ++surfaceIndex) { @@ -46,9 +47,17 @@ ExecutionResult SetOrderWithinLayerCommand::execute(ICommandExecutor* executor) if (surface) { - LOG_DEBUG("SetOrderWithinLayerCommand","add surface " << surface->getID() << " to renderorder of layer " << m_layerid); - layer->getAllSurfaces().push_back(surface); - result = ExecutionSuccessRedraw; + unsigned int layer_id = surface->getContainingLayerId(); + if (layer_id != GraphicalObject::INVALID_ID) + { + LOG_WARNING("SetOrderWithinLayerCommand","surface : id [ " << m_array[surfaceIndex] << " ] already belongs to layer : id [ " << layer_id << " ]"); + } + else + { + LOG_DEBUG("SetOrderWithinLayerCommand","add surface " << surface->getID() << " to renderorder of layer " << m_layerid); + layer->addSurface(surface); + result = ExecutionSuccessRedraw; + } } } } diff --git a/LayerManagerService/include/Layer.h b/LayerManagerService/include/Layer.h index 04f6a01..a19ac4c 100644 --- a/LayerManagerService/include/Layer.h +++ b/LayerManagerService/include/Layer.h @@ -45,6 +45,7 @@ public: void addSurface(Surface* s); void removeSurface(Surface* s); SurfaceList& getAllSurfaces(); + void removeAllSurfaces(); private: SurfaceList m_surfaces; @@ -96,14 +97,22 @@ inline uint Layer::getCapabilities() const inline void Layer::addSurface(Surface* s) { - renderPropertyChanged = true; - m_surfaces.push_back(s); + if (s->getContainingLayerId() == INVALID_ID) + { + m_surfaces.push_back(s); + s->setContainingLayerId(getID()); + renderPropertyChanged = true; + } } inline void Layer::removeSurface(Surface* s) { - renderPropertyChanged = true; - m_surfaces.remove(s); + if (s->getContainingLayerId() == getID()) + { + m_surfaces.remove(s); + s->setContainingLayerId(INVALID_ID); + renderPropertyChanged = true; + } } inline SurfaceList& Layer::getAllSurfaces() @@ -111,4 +120,16 @@ inline SurfaceList& Layer::getAllSurfaces() return m_surfaces; } +inline void Layer::removeAllSurfaces() +{ + SurfaceListConstIterator iter = m_surfaces.begin(); + SurfaceListConstIterator iterEnd = m_surfaces.end(); + for (; iter != iterEnd; ++iter) + { + (*iter)->setContainingLayerId(GraphicalObject::INVALID_ID); + } + m_surfaces.clear(); + renderPropertyChanged = true; +} + #endif /* _LAYER_H_ */ diff --git a/LayerManagerService/include/Scene.h b/LayerManagerService/include/Scene.h index 7024e94..24453dc 100644 --- a/LayerManagerService/include/Scene.h +++ b/LayerManagerService/include/Scene.h @@ -81,11 +81,11 @@ public: void removeLayerGroup(LayerGroup *layer); const SurfaceMap getAllSurfaces() const; Surface* getSurfaceAt(unsigned int *x, unsigned int *y, double minOpacity); + bool isLayerInCurrentRenderOrder(const uint id); private: const LayerMap getAllLayers() const; void removeLayerFromAllLayerGroups(Layer* layer); - void removeSurfaceFromAllLayers(Surface* surface); void removeSurfaceFromAllSurfaceGroups(Surface* surface); private: diff --git a/LayerManagerService/include/Surface.h b/LayerManagerService/include/Surface.h index 390ebb3..46c3e66 100644 --- a/LayerManagerService/include/Surface.h +++ b/LayerManagerService/include/Surface.h @@ -42,6 +42,16 @@ public: pixformat = pf; } + unsigned int getContainingLayerId() const + { + return layerId; + } + + void setContainingLayerId(unsigned int id) + { + layerId = id; + } + /** * Platform specific Object containing surface information specific to a used platform. @@ -56,13 +66,16 @@ private: Surface() : GraphicalSurface(TypeSurface), platform(NULL), frameCounter(0) { + layerId = INVALID_ID; } Surface(int id) : GraphicalSurface(id, TypeSurface), platform(NULL), frameCounter(0) { + layerId = INVALID_ID; } friend class Scene; PixelFormat pixformat; + unsigned int layerId; }; #endif /* _SURFACE_H_ */ diff --git a/LayerManagerService/src/Scene.cpp b/LayerManagerService/src/Scene.cpp index b978012..25c6bfc 100644 --- a/LayerManagerService/src/Scene.cpp +++ b/LayerManagerService/src/Scene.cpp @@ -212,6 +212,7 @@ void Scene::removeLayer(Layer* layer) { if (layer != NULL) { + layer->removeAllSurfaces(); m_currentRenderOrder.remove(layer); m_layerMap.erase(layer->getID()); removeLayerFromAllLayerGroups(layer); @@ -232,29 +233,24 @@ void Scene::removeSurfaceFromAllSurfaceGroups(Surface* surface) } } -/// \brief remove surface from all layers it might be on -void Scene::removeSurfaceFromAllLayers(Surface* surface) -{ - LayerMapIterator iter = m_layerMap.begin(); - LayerMapIterator iterEnd = m_layerMap.end(); - - for (; iter != iterEnd; ++iter) - { - Layer *l = iter->second; - l->removeSurface(surface); - } -} - /// \brief take surface out of list of surfaces void Scene::removeSurface(Surface* surface) { if (surface != NULL) { uint surfaceId = surface->getID(); + uint layerId = surface->getContainingLayerId(); - m_surfaceMap.erase(surfaceId); + if (layerId != GraphicalObject::INVALID_ID) + { + Layer* layer = getLayer(layerId); + if (layer != NULL) + { + layer->removeSurface(surface); + } + } - removeSurfaceFromAllLayers(surface); + m_surfaceMap.erase(surfaceId); removeSurfaceFromAllSurfaceGroups(surface); delete surface; @@ -441,3 +437,16 @@ Surface* Scene::getSurfaceAt(unsigned int *x, unsigned int *y, double minOpacity return surf; } +bool Scene::isLayerInCurrentRenderOrder(const uint id) +{ + LayerListConstIterator iter = m_currentRenderOrder.begin(); + LayerListConstIterator iterEnd = m_currentRenderOrder.end(); + + for (; iter != iterEnd; ++iter) + { + if ((*iter)->getID() == id) + return true; + } + + return false; +} -- 2.7.4