Mir

Code review comment for lp:~cemil-azizoglu/mir/improve-raii

Revision history for this message
Kevin DuBois (kdub) wrote :

Seems like (more or less) an even trade-off to me as to what we had before.
typing:
auto b = bundle->compositor_acquire(...);
b->buffer()->methods();
saves on having to update the Temporary*Buffer adapter objects, but is more awkward to type, compared to:
auto b = bundle->compositor_acquire(...);
b->methods();

84 +mc::BufferHandle::BufferHandle(BufferBundle* bundle,
We used to preserve ownership of the BufferBundle in the temporary buffers, seems like something that could cause problems around the client's destruction if we don't have BufferBundle ownership, and are trying to return in the destructor of BufferHandle.

169 + explicit CompositorBufferHandle(BufferBundle* bundle,
177 + explicit SnapshotBufferHandle(BufferBundle* bundle,
spacing

161 + std::shared_ptr<graphics::Buffer> buffer);
170 + std::shared_ptr<graphics::Buffer> buffer);
178 + std::shared_ptr<graphics::Buffer> buffer);
const&

We already have a "native handle", might be over-using the word "Handle" in mc::BufferHandle. (I'm mostly just trying to avoid writing: mg::BufferHandle handle{...}; handle->buffer()->native_buffer_handle() )

257 + std::shared_ptr<mc::BufferHandle> const acquired_buffer =
258 + std::make_shared<mc::SnapshotBufferHandle>(this, buffer_for(current_compositor_buffer, buffers));
259 +
260 + return acquired_buffer;
could just:
return std::make_shared<mc::SnapshotBufferHandle(...)

review: Needs Fixing

« Back to merge proposal