Commit 712fa19c authored by ccameron's avatar ccameron Committed by Commit bot

Fix content scaling bug on Mac

The root cause was that ImageTransportSurfaceFBO was using the pixel
size of the surface to early-out of surface reallocation. This is a problem
when scale factor changed and pixel size doesn't -- the change in scale
factor is never propagated to the underlying CALayer.

Move all logic for changing the size of the surface and for skipping
changes in the size of the surface to AllocateOrResizeFramebuffer.

Rename ambiguous sizes to be labeled as pixel_size or dip_size.

Remove a stray ScopedCAActionDisabler that does nothing (found by
inspection of nearby code).

Add myself as an OWNER for this path.

BUG=424713

Review URL: https://codereview.chromium.org/696823002

Cr-Commit-Position: refs/heads/master@{#302363}
parent 4a058243
ccameron@chromium.org
jbauman@chromium.org jbauman@chromium.org
kbr@chromium.org kbr@chromium.org
......
...@@ -131,9 +131,6 @@ bool CALayerStorageProvider::AllocateColorBufferStorage( ...@@ -131,9 +131,6 @@ bool CALayerStorageProvider::AllocateColorBufferStorage(
} }
glFlush(); glFlush();
// Disable the fade-in animation as the layer is changed.
ScopedCAActionDisabler disabler;
// Set the parameters that will be used to allocate the CALayer to draw the // Set the parameters that will be used to allocate the CALayer to draw the
// texture into. // texture into.
share_group_context_.reset(CGLRetainContext(context)); share_group_context_.reset(CGLRetainContext(context));
......
...@@ -88,7 +88,7 @@ class ImageTransportSurfaceFBO ...@@ -88,7 +88,7 @@ class ImageTransportSurfaceFBO
// ImageTransportSurface implementation // ImageTransportSurface implementation
void OnBufferPresented( void OnBufferPresented(
const AcceleratedSurfaceMsg_BufferPresented_Params& params) override; const AcceleratedSurfaceMsg_BufferPresented_Params& params) override;
void OnResize(gfx::Size size, float scale_factor) override; void OnResize(gfx::Size pixel_size, float scale_factor) override;
void SetLatencyInfo(const std::vector<ui::LatencyInfo>&) override; void SetLatencyInfo(const std::vector<ui::LatencyInfo>&) override;
void WakeUpGpu() override; void WakeUpGpu() override;
...@@ -100,7 +100,8 @@ class ImageTransportSurfaceFBO ...@@ -100,7 +100,8 @@ class ImageTransportSurfaceFBO
void AdjustBufferAllocation(); void AdjustBufferAllocation();
void DestroyFramebuffer(); void DestroyFramebuffer();
void CreateFramebuffer(); void AllocateOrResizeFramebuffer(
const gfx::Size& pixel_size, float scale_factor);
scoped_ptr<StorageProvider> storage_provider_; scoped_ptr<StorageProvider> storage_provider_;
...@@ -116,8 +117,8 @@ class ImageTransportSurfaceFBO ...@@ -116,8 +117,8 @@ class ImageTransportSurfaceFBO
// Weak pointer to the context that this was last made current to. // Weak pointer to the context that this was last made current to.
gfx::GLContext* context_; gfx::GLContext* context_;
gfx::Size size_; gfx::Size pixel_size_;
gfx::Size rounded_size_; gfx::Size rounded_pixel_size_;
float scale_factor_; float scale_factor_;
// Whether or not we've successfully made the surface current once. // Whether or not we've successfully made the surface current once.
......
...@@ -75,7 +75,7 @@ bool ImageTransportSurfaceFBO::OnMakeCurrent(gfx::GLContext* context) { ...@@ -75,7 +75,7 @@ bool ImageTransportSurfaceFBO::OnMakeCurrent(gfx::GLContext* context) {
if (made_current_) if (made_current_)
return true; return true;
OnResize(gfx::Size(1, 1), 1.f); AllocateOrResizeFramebuffer(gfx::Size(1, 1), 1.f);
made_current_ = true; made_current_ = true;
return true; return true;
...@@ -110,7 +110,7 @@ void ImageTransportSurfaceFBO::AdjustBufferAllocation() { ...@@ -110,7 +110,7 @@ void ImageTransportSurfaceFBO::AdjustBufferAllocation() {
has_complete_framebuffer_) { has_complete_framebuffer_) {
DestroyFramebuffer(); DestroyFramebuffer();
} else if (backbuffer_suggested_allocation_ && !has_complete_framebuffer_) { } else if (backbuffer_suggested_allocation_ && !has_complete_framebuffer_) {
CreateFramebuffer(); AllocateOrResizeFramebuffer(pixel_size_, scale_factor_);
} }
} }
...@@ -122,7 +122,7 @@ bool ImageTransportSurfaceFBO::SwapBuffers() { ...@@ -122,7 +122,7 @@ bool ImageTransportSurfaceFBO::SwapBuffers() {
// It is the responsibility of the storage provider to send the swap IPC. // It is the responsibility of the storage provider to send the swap IPC.
is_swap_buffers_send_pending_ = true; is_swap_buffers_send_pending_ = true;
storage_provider_->SwapBuffers(size_, scale_factor_); storage_provider_->SwapBuffers(pixel_size_, scale_factor_);
return true; return true;
} }
...@@ -155,7 +155,7 @@ bool ImageTransportSurfaceFBO::SupportsPostSubBuffer() { ...@@ -155,7 +155,7 @@ bool ImageTransportSurfaceFBO::SupportsPostSubBuffer() {
} }
gfx::Size ImageTransportSurfaceFBO::GetSize() { gfx::Size ImageTransportSurfaceFBO::GetSize() {
return size_; return pixel_size_;
} }
void* ImageTransportSurfaceFBO::GetHandle() { void* ImageTransportSurfaceFBO::GetHandle() {
...@@ -172,17 +172,15 @@ void ImageTransportSurfaceFBO::OnBufferPresented( ...@@ -172,17 +172,15 @@ void ImageTransportSurfaceFBO::OnBufferPresented(
storage_provider_->SwapBuffersAckedByBrowser(params.disable_throttling); storage_provider_->SwapBuffersAckedByBrowser(params.disable_throttling);
} }
void ImageTransportSurfaceFBO::OnResize(gfx::Size size, void ImageTransportSurfaceFBO::OnResize(gfx::Size pixel_size,
float scale_factor) { float scale_factor) {
TRACE_EVENT2("gpu", "ImageTransportSurfaceFBO::OnResize", TRACE_EVENT2("gpu", "ImageTransportSurfaceFBO::OnResize",
"old_width", size_.width(), "new_width", size.width()); "old_size", pixel_size_.ToString(),
"new_size", pixel_size.ToString());
// Caching |context_| from OnMakeCurrent. It should still be current. // Caching |context_| from OnMakeCurrent. It should still be current.
DCHECK(context_->IsCurrent(this)); DCHECK(context_->IsCurrent(this));
size_ = size; AllocateOrResizeFramebuffer(pixel_size, scale_factor);
scale_factor_ = scale_factor;
CreateFramebuffer();
} }
void ImageTransportSurfaceFBO::SetLatencyInfo( void ImageTransportSurfaceFBO::SetLatencyInfo(
...@@ -229,18 +227,29 @@ void ImageTransportSurfaceFBO::DestroyFramebuffer() { ...@@ -229,18 +227,29 @@ void ImageTransportSurfaceFBO::DestroyFramebuffer() {
has_complete_framebuffer_ = false; has_complete_framebuffer_ = false;
} }
void ImageTransportSurfaceFBO::CreateFramebuffer() { void ImageTransportSurfaceFBO::AllocateOrResizeFramebuffer(
gfx::Size new_rounded_size = storage_provider_->GetRoundedSize(size_); const gfx::Size& new_pixel_size, float new_scale_factor) {
gfx::Size new_rounded_pixel_size =
storage_provider_->GetRoundedSize(new_pixel_size);
// Only recreate surface when the rounded up size has changed. // Only recreate the surface's storage when the rounded up size has changed,
if (has_complete_framebuffer_ && new_rounded_size == rounded_size_) // or the scale factor has changed.
return; bool needs_new_storage =
!has_complete_framebuffer_ ||
new_rounded_pixel_size != rounded_pixel_size_ ||
new_scale_factor != scale_factor_;
TRACE_EVENT2("gpu", "ImageTransportSurfaceFBO::CreateFramebuffer", // Save the new storage parameters.
"width", new_rounded_size.width(), pixel_size_ = new_pixel_size;
"height", new_rounded_size.height()); rounded_pixel_size_ = new_rounded_pixel_size;
scale_factor_ = new_scale_factor;
if (!needs_new_storage)
return;
rounded_size_ = new_rounded_size; TRACE_EVENT2("gpu", "ImageTransportSurfaceFBO::AllocateOrResizeFramebuffer",
"width", new_rounded_pixel_size.width(),
"height", new_rounded_pixel_size.height());
// GL_TEXTURE_RECTANGLE_ARB is the best supported render target on // GL_TEXTURE_RECTANGLE_ARB is the best supported render target on
// Mac OS X and is required for IOSurface interoperability. // Mac OS X and is required for IOSurface interoperability.
...@@ -286,7 +295,8 @@ void ImageTransportSurfaceFBO::CreateFramebuffer() { ...@@ -286,7 +295,8 @@ void ImageTransportSurfaceFBO::CreateFramebuffer() {
glBindRenderbufferEXT(GL_RENDERBUFFER_EXT, glBindRenderbufferEXT(GL_RENDERBUFFER_EXT,
depth_stencil_renderbuffer_id_); depth_stencil_renderbuffer_id_);
glRenderbufferStorageEXT(GL_RENDERBUFFER_EXT, GL_DEPTH24_STENCIL8_EXT, glRenderbufferStorageEXT(GL_RENDERBUFFER_EXT, GL_DEPTH24_STENCIL8_EXT,
rounded_size_.width(), rounded_size_.height()); rounded_pixel_size_.width(),
rounded_pixel_size_.height());
glFramebufferRenderbufferEXT(GL_FRAMEBUFFER_EXT, glFramebufferRenderbufferEXT(GL_FRAMEBUFFER_EXT,
GL_STENCIL_ATTACHMENT_EXT, GL_STENCIL_ATTACHMENT_EXT,
GL_RENDERBUFFER_EXT, GL_RENDERBUFFER_EXT,
...@@ -301,7 +311,7 @@ void ImageTransportSurfaceFBO::CreateFramebuffer() { ...@@ -301,7 +311,7 @@ void ImageTransportSurfaceFBO::CreateFramebuffer() {
bool allocated_color_buffer = storage_provider_->AllocateColorBufferStorage( bool allocated_color_buffer = storage_provider_->AllocateColorBufferStorage(
static_cast<CGLContextObj>(context_->GetHandle()), texture_id_, static_cast<CGLContextObj>(context_->GetHandle()), texture_id_,
rounded_size_, scale_factor_); rounded_pixel_size_, scale_factor_);
if (!allocated_color_buffer) { if (!allocated_color_buffer) {
DLOG(ERROR) << "Failed to allocate color buffer storage."; DLOG(ERROR) << "Failed to allocate color buffer storage.";
DestroyFramebuffer(); DestroyFramebuffer();
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment