From 95cbc86adea462c5437289c61be1ba3442a17467 Mon Sep 17 00:00:00 2001 From: Adam Barth Date: Fri, 23 Jan 2015 21:20:14 -0800 Subject: [PATCH] Sky shouldn't crash on device rotate There were two problems (both fixed in this CL): 1) When we were resized by the view manager, we forgot to deflate by the device-pixel-ratio when converting to engine types. That caused use to allocate a backing texture that was 9x what we needed. 2) When the surfaces system returned textures to us for re-use, we'd put them into the cache even if they were the old size. That caused us to thrash the texture cache. In this CL, we make the size of the textures in the cache explicit. R=eseidel@chromium.org BUG=449001 Review URL: https://codereview.chromium.org/868263002 --- compositor/BUILD.gn | 2 ++ compositor/resource_manager.cc | 17 +++++----------- compositor/resource_manager.h | 6 +++--- compositor/texture_cache.cc | 37 ++++++++++++++++++++++++++++++++++ compositor/texture_cache.h | 35 ++++++++++++++++++++++++++++++++ viewer/document_view.cc | 5 +++-- 6 files changed, 85 insertions(+), 17 deletions(-) create mode 100644 compositor/texture_cache.cc create mode 100644 compositor/texture_cache.h diff --git a/compositor/BUILD.gn b/compositor/BUILD.gn index fdb51678278..27f59b0a1d2 100644 --- a/compositor/BUILD.gn +++ b/compositor/BUILD.gn @@ -26,6 +26,8 @@ source_set("compositor") { "surface_allocator.h", "surface_holder.cc", "surface_holder.h", + "texture_cache.cc", + "texture_cache.h", ] deps = [ diff --git a/compositor/resource_manager.cc b/compositor/resource_manager.cc index 0169380c982..9a5eecd249f 100644 --- a/compositor/resource_manager.cc +++ b/compositor/resource_manager.cc @@ -31,16 +31,9 @@ ResourceManager::~ResourceManager() { scoped_ptr ResourceManager::CreateTexture( const gfx::Size& size) { - if (!available_textures_.empty()) { - scoped_ptr texture(available_textures_.back()); - available_textures_.back() = nullptr; - available_textures_.pop_back(); - if (texture->size() == size) - return texture.Pass(); - // Currently we only support caching textures of a constant size. - available_textures_.clear(); - } - + scoped_ptr texture = texture_cache_.GetTexture(size); + if (texture) + return texture.Pass(); gl_context_->MakeCurrent(); return make_scoped_ptr(new mojo::GLTexture( gl_context_, mojo::TypeConverter::Convert(size))); @@ -89,11 +82,11 @@ void ResourceManager::ReturnResources( auto iter = resource_to_texture_map_.find(resource->id); if (iter == resource_to_texture_map_.end()) continue; - mojo::GLTexture* texture = iter->second; + scoped_ptr texture(iter->second); DCHECK_NE(0u, texture->texture_id()); resource_to_texture_map_.erase(iter); glWaitSyncPointCHROMIUM(resource->sync_point); - available_textures_.push_back(texture); + texture_cache_.PutTexture(texture.Pass()); } } diff --git a/compositor/resource_manager.h b/compositor/resource_manager.h index cd1ebdde7ee..e6fd329dd4e 100644 --- a/compositor/resource_manager.h +++ b/compositor/resource_manager.h @@ -10,6 +10,7 @@ #include "base/memory/scoped_vector.h" #include "base/memory/weak_ptr.h" #include "mojo/services/surfaces/public/interfaces/surfaces.mojom.h" +#include "sky/compositor/texture_cache.h" namespace gfx { class Size; @@ -38,12 +39,11 @@ class ResourceManager { base::WeakPtr gl_context_; uint32_t next_resource_id_; base::hash_map resource_to_texture_map_; - - ScopedVector available_textures_; + TextureCache texture_cache_; DISALLOW_COPY_AND_ASSIGN(ResourceManager); }; -} // namespace examples +} // namespace sky #endif // SKY_COMPOSITOR_RESOURCE_MANAGER_H_ diff --git a/compositor/texture_cache.cc b/compositor/texture_cache.cc new file mode 100644 index 00000000000..05fa7d58ebc --- /dev/null +++ b/compositor/texture_cache.cc @@ -0,0 +1,37 @@ +// Copyright 2015 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "sky/compositor/texture_cache.h" + +#include "mojo/converters/geometry/geometry_type_converters.h" +#include "mojo/gpu/gl_texture.h" + +namespace sky { + +TextureCache::TextureCache() { +} + +TextureCache::~TextureCache() { +} + +scoped_ptr TextureCache::GetTexture(const gfx::Size& size) { + if (size != size_) { + available_textures_.clear(); + size_ = size; + } + if (available_textures_.empty()) + return nullptr; + scoped_ptr texture(available_textures_.back()); + available_textures_.back() = nullptr; + available_textures_.pop_back(); + return texture.Pass(); +} + +void TextureCache::PutTexture(scoped_ptr texture) { + if (texture->size() != size_) + return; + available_textures_.push_back(texture.release()); +} + +} // namespace sky diff --git a/compositor/texture_cache.h b/compositor/texture_cache.h new file mode 100644 index 00000000000..056112e0fa5 --- /dev/null +++ b/compositor/texture_cache.h @@ -0,0 +1,35 @@ +// Copyright 2015 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef SKY_COMPOSITOR_TEXTURE_CACHE_H_ +#define SKY_COMPOSITOR_TEXTURE_CACHE_H_ + +#include "base/memory/scoped_ptr.h" +#include "base/memory/scoped_vector.h" +#include "ui/gfx/geometry/size.h" + +namespace mojo { +class GLTexture; +} + +namespace sky { + +class TextureCache { + public: + TextureCache(); + ~TextureCache(); + + scoped_ptr GetTexture(const gfx::Size& size); + void PutTexture(scoped_ptr texture); + + private: + gfx::Size size_; + ScopedVector available_textures_; + + DISALLOW_COPY_AND_ASSIGN(TextureCache); +}; + +} // namespace sky + +#endif // SKY_COMPOSITOR_TEXTURE_CACHE_H_ diff --git a/viewer/document_view.cc b/viewer/document_view.cc index cf48567326b..ab1516d0d53 100644 --- a/viewer/document_view.cc +++ b/viewer/document_view.cc @@ -270,8 +270,9 @@ void DocumentView::OnViewBoundsChanged(mojo::View* view, const mojo::Rect& old_bounds, const mojo::Rect& new_bounds) { DCHECK_EQ(view, root_); - gfx::Size size = new_bounds.To().size(); - web_view_->resize(size); + float device_pixel_ratio = GetDevicePixelRatio(); + web_view_->resize(blink::WebSize(new_bounds.width / device_pixel_ratio, + new_bounds.height / device_pixel_ratio)); } void DocumentView::OnViewFocusChanged(mojo::View* gained_focus,