compositor: Fix texture flipping for SW mirroring with surfaceless

content/browser/compositor/reflector_impl.cc

 // Copyright 2014 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 "content/browser/compositor/reflector_impl.h"
 
 #include "base/bind.h"
 #include "base/location.h"
 #include "content/browser/compositor/browser_compositor_output_surface.h"
 #include "content/browser/compositor/owned_mailbox.h"
 #include "content/common/gpu/client/gl_helper.h"
 #include "ui/compositor/layer.h"
 
 namespace content {
 
 ReflectorImpl::ReflectorImpl(
     ui::Compositor* mirrored_compositor,
     ui::Layer* mirroring_layer,
     IDMap<BrowserCompositorOutputSurface>* output_surface_map,
     base::MessageLoopProxy* compositor_thread_loop,
     int surface_id)
     : impl_unsafe_(output_surface_map),
       main_unsafe_(mirrored_compositor, mirroring_layer),
       impl_message_loop_(compositor_thread_loop),
       main_message_loop_(base::MessageLoopProxy::current()),
-      surface_id_(surface_id) {
+      surface_id_(surface_id),
+      flip_texture_(false) {
   GLHelper* helper = ImageTransportFactory::GetInstance()->GetGLHelper();
   MainThreadData& main = GetMain();
   main.mailbox = new OwnedMailbox(helper);
   impl_message_loop_->PostTask(
       FROM_HERE,
       base::Bind(
           &ReflectorImpl::InitOnImplThread, this, main.mailbox->holder()));
 }
 
 ReflectorImpl::MainThreadData::MainThreadData(
@@ skipping 139 matching lines @@
 
 void ReflectorImpl::AttachToOutputSurfaceOnImplThread(
     const gpu::MailboxHolder& mailbox_holder,
     BrowserCompositorOutputSurface* output_surface) {
   ImplThreadData& impl = GetImpl();
   if (output_surface == impl.output_surface)
     return;
   if (impl.output_surface)
     DetachFromOutputSurface();
   impl.output_surface = output_surface;
+  flip_texture_ = !output_surface->capabilities().flipped_output_surface;

[danakj, 2015/01/12 18:27:30]

This is getting the flipped-ness of the current output surface, but setting
flipped for the texture mailbox, which came from a different output surface,
right? How can you guarantee that these match?

[achaulk, 2015/01/12 18:47:35]

All that matters is the flipped-ness of the source surface, since that's where
the texture contents are coming from. This ensures that the layer contains the
same data either way (normal painting and flipped=true or inverted painting and
flipped=false). The properties of the output surface for the mirroring layer
shouldn't matter

[danakj, 2015/01/12 19:02:42]

Oh this is the source surface? OK great :) I read it as the target surface at
first!

   output_surface->context_provider()->BindToCurrentThread();
   impl.gl_helper.reset(
       new GLHelper(output_surface->context_provider()->ContextGL(),
                    output_surface->context_provider()->ContextSupport()));
   impl.texture_id = impl.gl_helper->ConsumeMailboxToTexture(
       mailbox_holder.mailbox, mailbox_holder.sync_point);
   impl.gl_helper->ResizeTexture(impl.texture_id, output_surface->SurfaceSize());
   impl.gl_helper->Flush();
   output_surface->SetReflector(this);
   // The texture doesn't have the data, so invokes full redraw now.
@@ skipping 12 matching lines @@
     main.mirroring_layer->SetTextureMailbox(
         cc::TextureMailbox(main.mailbox->holder()),
         cc::SingleReleaseCallback::Create(
             base::Bind(ReleaseMailbox, main.mailbox)),
         size);
     main.needs_set_mailbox = false;
   } else {
     main.mirroring_layer->SetTextureSize(size);
   }
   main.mirroring_layer->SetBounds(gfx::Rect(size));
+  main.mirroring_layer->SetTextureFlipped(flip_texture_);
 }
 
 void ReflectorImpl::FullRedrawOnMainThread(gfx::Size size) {
   MainThreadData& main = GetMain();
   if (!main.mirroring_layer)
     return;
   UpdateTextureSizeOnMainThread(size);
   main.mirroring_layer->SchedulePaint(main.mirroring_layer->bounds());
 }
 
 void ReflectorImpl::UpdateSubBufferOnMainThread(gfx::Size size,

[danakj, 2015/01/12 18:27:30]

while you're here, these should be const& arguments

[achaulk, 2015/01/12 18:47:35]

Done.

                                                 gfx::Rect rect) {
   MainThreadData& main = GetMain();
   if (!main.mirroring_layer)
     return;
   UpdateTextureSizeOnMainThread(size);
-  // Flip the coordinates to compositor's one.
-  int y = size.height() - rect.y() - rect.height();
-  gfx::Rect new_rect(rect.x(), y, rect.width(), rect.height());
-  main.mirroring_layer->SchedulePaint(new_rect);
+  if(flip_texture_) {
+    // Flip the coordinates to compositor's one.
+    int y = size.height() - rect.y() - rect.height();
+    gfx::Rect new_rect(rect.x(), y, rect.width(), rect.height());
+    main.mirroring_layer->SchedulePaint(new_rect);
+  } else {
+    main.mirroring_layer->SchedulePaint(rect);

[danakj, 2015/01/12 18:27:30]

How about just doing rect math inside an if/else and keep the function call
outside, so there's only one?

[achaulk, 2015/01/12 18:47:35]

Sure

+  }
 }
 
 void ReflectorImpl::FullRedrawContentOnMainThread() {
   MainThreadData& main = GetMain();
   main.mirrored_compositor->ScheduleFullRedraw();
 }
 
 }  // namespace content