Merge lp:~alan-griffiths/mir/client-side-buffer-age into lp:~raof/mir/client-side-buffer-age

[Alan Griffiths]

client: Suggestion to make code easier to follow

client: Suggestion to make code easier to follow

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:470
http://jenkins.qa.ubuntu.com/job/mir-ci/27/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-quantal-amd64-ci/27//console

Click here to trigger a rebuild:
http://jenkins.qa.ubuntu.com/job/mir-ci/27//rebuild/?

review: Approve (continuous-integration)

[Chris Halse Rogers (raof)]

I didn't write it this way originally because we're unnecessarily iterating through the buffer list twice in a once-pre-frame function.

That said, this *is* much clearer¹, we should only have at most 3 buffers in the BufferMap, and if we need to speed things up we can always come back and revisit the performance/obviousness trade.

¹: Except for the "auto current = next++;" - I know what it does, but I need to *think* about what it does. ☺

=== modified file 'src/client/gbm/gbm_client_buffer_depository.cpp'
--- src/client/gbm/gbm_client_buffer_depository.cpp	2013-03-08 01:29:56 +0000
+++ src/client/gbm/gbm_client_buffer_depository.cpp	2013-03-08 15:01:20 +0000
@@ -28,43 +28,47 @@
 
 mclg::GBMClientBufferDepository::GBMClientBufferDepository(
         std::shared_ptr<DRMFDHandler> const& drm_fd_handler)
-    : drm_fd_handler{drm_fd_handler}
+    : current_buffer(buffers.end()), drm_fd_handler{drm_fd_handler}
 {
 }
 
 void mclg::GBMClientBufferDepository::deposit_package(std::shared_ptr<mir_toolkit::MirBufferPackage>&& package, int id, geometry::Size size, geometry::PixelFormat pf)
 {
-    auto buffer_it = buffers.begin();
-    while (buffer_it != buffers.end())
-    {
-        // C++ guarantees that buffers.erase() will only invalidate the iterator
-        // we erase. Move to the next buffer before we potentially invalidate our
-        // iterator.
-        auto current = buffer_it;
-        ++buffer_it;
-
-        if (current->first == current_buffer)
-        {
-            current->second->mark_as_submitted();
-        }
-        else
-        {
-            if (current->second->age() >= 2 && (current->first != static_cast<uint32_t>(id)))
-            {
-                buffers.erase(current);
-            }
-            else
-            {
-                current->second->increment_age();
-            }
-        }
-    }
-    if (buffers.count(id) == 0)
-        buffers[id] = std::make_shared<mclg::GBMClientBuffer>(drm_fd_handler, std::move(package), size, pf);
-    current_buffer = id;
+    for (auto next = buffers.begin(); next != buffers.end();)
+    {
+        // C++ guarantees that buffers.erase() will only invalidate the iterator we
+        // erase. Move to the next buffer before we potentially invalidate our iterator.
+        auto current = next++;
+
+        if (current != current_buffer &&
+            current->first != id &&
+            current->second->age() >= 2)
+        {
+            buffers.erase(current);
+        }
+    }
+
+    for (auto& current : buffers)
+    {
+        current.second->increment_age();
+    }
+
+    if (current_buffer != buffers.end())
+    {
+        current_buffer->second->mark_as_submitted();
+    }
+
+    current_buffer = buffers.find(id);
+
+    if (current_buffer == buffers.end())
+    {
+        auto new_buffer = std::make_shared<mclg::GBMClientBuffer>(drm_fd_handler, std::move(package), size, pf);
+
+        current_buffer = buffers.insert(current_buffer, std::make_pair(id, new_buffer));
+    }
 }
 
 std::shared_ptr<mcl::ClientBuffer> mclg::GBMClientBufferDepository::access_current_buffer()
 {
-    return buffers[current_buffer];
+    return current_buffer->second;
 }

=== modified file 'src/client/gbm/gbm_client_buffer_depository.h'
--- src/client/gbm/gbm_client_buffer_depository.h	2013-03-08 01:29:56 +0000
+++ src/client/gbm/gbm_client_buffer_depository.h	2013-03-08 15:01:20 +0000
@@ -46,8 +46,9 @@
 
     std::shared_ptr<ClientBuffer> access_current_buffer();
 private:
-    std::map<uint32_t, std::shared_ptr<GBMClientBuffer>> buffers;
-    uint32_t current_buffer;
+    typedef std::map<int, std::shared_ptr<GBMClientBuffer>> BufferMap;
+    BufferMap buffers;
+    BufferMap::iterator current_buffer;
     std::shared_ptr<DRMFDHandler> drm_fd_handler;
 };