Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(666)

Unified Diff: content/browser/compositor/buffer_queue.cc

Issue 1423843003: Make content::BufferQueue use scoped ptrs for AllocatedSurfaces (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@surfaceless_only
Patch Set: More feedback Created 5 years ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « content/browser/compositor/buffer_queue.h ('k') | content/browser/compositor/buffer_queue_unittest.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: content/browser/compositor/buffer_queue.cc
diff --git a/content/browser/compositor/buffer_queue.cc b/content/browser/compositor/buffer_queue.cc
index c82741654b59cf4a259091a154d6e7324c7a0b49..15cfcc613dc2e47fbc0553e732ffcbb09cbb6f04 100644
--- a/content/browser/compositor/buffer_queue.cc
+++ b/content/browser/compositor/buffer_queue.cc
@@ -4,6 +4,7 @@
#include "content/browser/compositor/buffer_queue.h"
+#include "base/containers/adapters.h"
#include "content/browser/compositor/image_transport_factory.h"
#include "content/browser/gpu/browser_gpu_memory_buffer_manager.h"
#include "content/common/gpu/client/context_provider_command_buffer.h"
@@ -51,10 +52,12 @@ void BufferQueue::BindFramebuffer() {
gpu::gles2::GLES2Interface* gl = context_provider_->ContextGL();
gl->BindFramebuffer(GL_FRAMEBUFFER, fbo_);
- if (!current_surface_.texture) {
+ if (!current_surface_)
current_surface_ = GetNextSurface();
+
+ if (current_surface_) {
gl->FramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0,
- texture_target_, current_surface_.texture, 0);
+ texture_target_, current_surface_->texture, 0);
}
}
@@ -69,33 +72,41 @@ void BufferQueue::CopyBufferDamage(int texture,
}
void BufferQueue::UpdateBufferDamage(const gfx::Rect& damage) {
- displayed_surface_.damage.Union(damage);
- for (size_t i = 0; i < available_surfaces_.size(); i++)
- available_surfaces_[i].damage.Union(damage);
-
- for (std::deque<AllocatedSurface>::iterator it =
- in_flight_surfaces_.begin();
- it != in_flight_surfaces_.end();
- ++it)
- it->damage.Union(damage);
+ if (displayed_surface_)
+ displayed_surface_->damage.Union(damage);
+ for (auto& surface : available_surfaces_)
+ surface->damage.Union(damage);
+ for (auto& surface : in_flight_surfaces_) {
+ if (surface)
+ surface->damage.Union(damage);
+ }
}
void BufferQueue::SwapBuffers(const gfx::Rect& damage) {
- if (!damage.IsEmpty() && damage != gfx::Rect(size_)) {
- // We must have a frame available to copy from.
- DCHECK(!in_flight_surfaces_.empty() || displayed_surface_.texture);
- unsigned int texture_id = !in_flight_surfaces_.empty()
- ? in_flight_surfaces_.back().texture
- : displayed_surface_.texture;
-
- CopyBufferDamage(current_surface_.texture, texture_id, damage,
- current_surface_.damage);
+ if (current_surface_) {
+ if (!damage.IsEmpty() && damage != gfx::Rect(size_)) {
+ // Copy damage from the most recently swapped buffer. In the event that
+ // the buffer was destroyed and failed to recreate, pick from the most
+ // recently available buffer.
+ unsigned int texture_id = 0;
+ for (auto& surface : base::Reversed(in_flight_surfaces_)) {
+ if (surface) {
+ texture_id = surface->texture;
+ break;
+ }
+ }
+ if (!texture_id && displayed_surface_)
+ texture_id = displayed_surface_->texture;
+
+ if (texture_id) {
+ CopyBufferDamage(current_surface_->texture, texture_id, damage,
+ current_surface_->damage);
+ }
+ }
+ current_surface_->damage = gfx::Rect();
}
UpdateBufferDamage(damage);
- current_surface_.damage = gfx::Rect();
- in_flight_surfaces_.push_back(current_surface_);
- current_surface_.texture = 0;
- current_surface_.image = 0;
+ in_flight_surfaces_.push_back(std::move(current_surface_));
// Some things reset the framebuffer (CopySubBufferDamage, some GLRenderer
// paths), so ensure we restore it here.
context_provider_->ContextGL()->BindFramebuffer(GL_FRAMEBUFFER, fbo_);
@@ -108,7 +119,7 @@ void BufferQueue::Reshape(const gfx::Size& size, float scale_factor) {
// is cause for concern or if it is benign.
// http://crbug.com/524624
#if !defined(OS_MACOSX)
- DCHECK(!current_surface_.texture);
+ DCHECK(!current_surface_);
#endif
size_ = size;
@@ -126,80 +137,76 @@ void BufferQueue::RecreateBuffers() {
// presentable on the device anymore.
// Unused buffers can be freed directly, they will be re-allocated as needed.
// Any in flight, current or displayed surface must be replaced.
- for (size_t i = 0; i < available_surfaces_.size(); i++)
- FreeSurface(&available_surfaces_[i]);
available_surfaces_.clear();
+ // Recreate all in-flight surfaces and put the recreated copies in the queue.
for (auto& surface : in_flight_surfaces_)
- surface = RecreateBuffer(&surface);
+ surface = RecreateBuffer(std::move(surface));
- current_surface_ = RecreateBuffer(&current_surface_);
- displayed_surface_ = RecreateBuffer(&displayed_surface_);
+ current_surface_ = RecreateBuffer(std::move(current_surface_));
+ displayed_surface_ = RecreateBuffer(std::move(displayed_surface_));
- if (current_surface_.texture) {
+ if (current_surface_) {
// If we have a texture bound, we will need to re-bind it.
gpu::gles2::GLES2Interface* gl = context_provider_->ContextGL();
gl->BindFramebuffer(GL_FRAMEBUFFER, fbo_);
gl->FramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0,
- texture_target_, current_surface_.texture, 0);
+ texture_target_, current_surface_->texture, 0);
}
}
-BufferQueue::AllocatedSurface BufferQueue::RecreateBuffer(
- AllocatedSurface* surface) {
- if (!surface->texture)
- return *surface;
- AllocatedSurface new_surface = GetNextSurface();
- new_surface.damage = surface->damage;
+scoped_ptr<BufferQueue::AllocatedSurface> BufferQueue::RecreateBuffer(
+ scoped_ptr<AllocatedSurface> surface) {
+ if (!surface)
+ return nullptr;
+
+ scoped_ptr<AllocatedSurface> new_surface(GetNextSurface());
+ if (!new_surface)
+ return nullptr;
+
+ new_surface->damage = surface->damage;
// Copy the entire texture.
- CopyBufferDamage(new_surface.texture, surface->texture, gfx::Rect(),
+ CopyBufferDamage(new_surface->texture, surface->texture, gfx::Rect(),
gfx::Rect(size_));
-
- FreeSurface(surface);
return new_surface;
}
void BufferQueue::PageFlipComplete() {
DCHECK(!in_flight_surfaces_.empty());
-
- if (displayed_surface_.texture)
- available_surfaces_.push_back(displayed_surface_);
-
- displayed_surface_ = in_flight_surfaces_.front();
+ if (displayed_surface_)
+ available_surfaces_.push_back(std::move(displayed_surface_));
+ displayed_surface_ = std::move(in_flight_surfaces_.front());
in_flight_surfaces_.pop_front();
}
void BufferQueue::FreeAllSurfaces() {
- FreeSurface(&displayed_surface_);
- FreeSurface(&current_surface_);
+ displayed_surface_.reset();
+ current_surface_.reset();
// This is intentionally not emptied since the swap buffers acks are still
// expected to arrive.
for (auto& surface : in_flight_surfaces_)
- FreeSurface(&surface);
-
- for (size_t i = 0; i < available_surfaces_.size(); i++)
- FreeSurface(&available_surfaces_[i]);
-
+ surface = nullptr;
available_surfaces_.clear();
}
-void BufferQueue::FreeSurface(AllocatedSurface* surface) {
- if (surface->texture) {
- gpu::gles2::GLES2Interface* gl = context_provider_->ContextGL();
- gl->BindTexture(texture_target_, surface->texture);
- gl->ReleaseTexImage2DCHROMIUM(texture_target_, surface->image);
- gl->DeleteTextures(1, &surface->texture);
- gl->DestroyImageCHROMIUM(surface->image);
- surface->image = 0;
- surface->texture = 0;
- allocated_count_--;
- }
+void BufferQueue::FreeSurfaceResources(AllocatedSurface* surface) {
+ if (!surface->texture)
+ return;
+
+ gpu::gles2::GLES2Interface* gl = context_provider_->ContextGL();
+ gl->BindTexture(texture_target_, surface->texture);
+ gl->ReleaseTexImage2DCHROMIUM(texture_target_, surface->image);
+ gl->DeleteTextures(1, &surface->texture);
+ gl->DestroyImageCHROMIUM(surface->image);
+ surface->buffer.reset();
+ allocated_count_--;
}
-BufferQueue::AllocatedSurface BufferQueue::GetNextSurface() {
+scoped_ptr<BufferQueue::AllocatedSurface> BufferQueue::GetNextSurface() {
if (!available_surfaces_.empty()) {
- AllocatedSurface surface = available_surfaces_.back();
+ scoped_ptr<AllocatedSurface> surface =
+ std::move(available_surfaces_.back());
available_surfaces_.pop_back();
return surface;
}
@@ -208,7 +215,7 @@ BufferQueue::AllocatedSurface BufferQueue::GetNextSurface() {
gpu::gles2::GLES2Interface* gl = context_provider_->ContextGL();
gl->GenTextures(1, &texture);
if (!texture)
- return AllocatedSurface();
+ return nullptr;
// We don't want to allow anything more than triple buffering.
DCHECK_LT(allocated_count_, 4U);
@@ -221,33 +228,39 @@ BufferQueue::AllocatedSurface BufferQueue::GetNextSurface() {
if (!buffer.get()) {
gl->DeleteTextures(1, &texture);
DLOG(ERROR) << "Failed to allocate GPU memory buffer";
- return AllocatedSurface();
+ return nullptr;
}
unsigned int id = gl->CreateImageCHROMIUM(
buffer->AsClientBuffer(), size_.width(), size_.height(),
internal_format_);
-
if (!id) {
LOG(ERROR) << "Failed to allocate backing image surface";
gl->DeleteTextures(1, &texture);
- return AllocatedSurface();
+ return nullptr;
}
+
allocated_count_++;
gl->BindTexture(texture_target_, texture);
gl->BindTexImage2DCHROMIUM(texture_target_, id);
- return AllocatedSurface(buffer.Pass(), texture, id, gfx::Rect(size_));
+ return make_scoped_ptr(new AllocatedSurface(this, std::move(buffer), texture,
+ id, gfx::Rect(size_)));
}
-BufferQueue::AllocatedSurface::AllocatedSurface() : texture(0), image(0) {}
-
BufferQueue::AllocatedSurface::AllocatedSurface(
+ BufferQueue* buffer_queue,
scoped_ptr<gfx::GpuMemoryBuffer> buffer,
unsigned int texture,
unsigned int image,
const gfx::Rect& rect)
- : buffer(buffer.release()), texture(texture), image(image), damage(rect) {}
-
-BufferQueue::AllocatedSurface::~AllocatedSurface() {}
+ : buffer_queue(buffer_queue),
+ buffer(buffer.release()),
+ texture(texture),
+ image(image),
+ damage(rect) {}
+
+BufferQueue::AllocatedSurface::~AllocatedSurface() {
+ buffer_queue->FreeSurfaceResources(this);
+}
} // namespace content
« no previous file with comments | « content/browser/compositor/buffer_queue.h ('k') | content/browser/compositor/buffer_queue_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698