Rotation into Video Layer + Content Transform

cc/layers/video_layer_impl.cc

 // Copyright 2011 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 "cc/layers/video_layer_impl.h"
 
 #include "base/bind.h"
 #include "base/logging.h"
 #include "cc/layers/video_frame_provider_client_impl.h"
 #include "cc/quads/io_surface_draw_quad.h"
@@ skipping 19 matching lines @@
     int id,
     VideoFrameProvider* provider) {
   scoped_ptr<VideoLayerImpl> layer(new VideoLayerImpl(tree_impl, id));
   layer->SetProviderClientImpl(VideoFrameProviderClientImpl::Create(provider));
   DCHECK(tree_impl->proxy()->IsImplThread());
   DCHECK(tree_impl->proxy()->IsMainThreadBlocked());
   return layer.Pass();
 }
 
 VideoLayerImpl::VideoLayerImpl(LayerTreeImpl* tree_impl, int id)
-    : LayerImpl(tree_impl, id),
-      frame_(NULL) {}
+    : LayerImpl(tree_impl, id), frame_(NULL) {
+}
 
 VideoLayerImpl::~VideoLayerImpl() {
   if (!provider_client_impl_->Stopped()) {
     // In impl side painting, we may have a pending and active layer
     // associated with the video provider at the same time. Both have a ref
     // on the VideoFrameProviderClientImpl, but we stop when the first
     // LayerImpl (the one on the pending tree) is destroyed since we know
     // the main thread is blocked for this commit.
     DCHECK(layer_tree_impl()->proxy()->IsImplThread());
     DCHECK(layer_tree_impl()->proxy()->IsMainThreadBlocked());
     provider_client_impl_->Stop();
   }
 }
 
 scoped_ptr<LayerImpl> VideoLayerImpl::CreateLayerImpl(
     LayerTreeImpl* tree_impl) {
-  return scoped_ptr<LayerImpl>(new VideoLayerImpl(tree_impl, id()));
+  VideoLayerImpl* impl = new VideoLayerImpl(tree_impl, id());
+  impl->set_video_rotation(video_rotation_);

[enne (OOO), 2014/07/21 23:42:49]

Properties should get pushed in push properties, not in this create constructor.

[suderman, 2014/07/22 00:05:20]

ditto

+  return scoped_ptr<LayerImpl>(impl);
 }
 
 void VideoLayerImpl::PushPropertiesTo(LayerImpl* layer) {
   LayerImpl::PushPropertiesTo(layer);
 
   VideoLayerImpl* other = static_cast<VideoLayerImpl*>(layer);
   other->SetProviderClientImpl(provider_client_impl_);
 }
 
 void VideoLayerImpl::DidBecomeActive() {
@@ skipping 58 matching lines @@
 void VideoLayerImpl::AppendQuads(
     RenderPass* render_pass,
     const OcclusionTracker<LayerImpl>& occlusion_tracker,
     AppendQuadsData* append_quads_data) {
   DCHECK(frame_.get());
 
   SharedQuadState* shared_quad_state =
       render_pass->CreateAndAppendSharedQuadState();
   PopulateSharedQuadState(shared_quad_state);
 
+  gfx::Transform transform = draw_transform();

[enne (OOO), 2014/07/22 21:10:39]

I think you also need to move the draw transform out of AppendQuads.  It is too
late in the pipeline.  DamageTracker::ExtendDamageForLayer uses layer transform
and bounds, and will not take this rotation into consideration.

I think the right place to do this would actually be to do this during
VideoLayer::PushPropertiesTo, and clobber the transform that's set on the
VideoLayerImpl with this modified version after calling the
Layer::PushProperties base class method.  Then, none of the code that deals with
LayerImpl has to know anything about this rotation and can just treat it as a
transformed layer (which it already knows how to do).

Additionally, you will not need a NoteLayerPropertyChanged since that'll happen
automatically as the layer is created for the first time.

[suderman, 2014/07/22 21:48:16]

I'll move the transform into push properties and (along with the rotation
setting) and check functionality.

[suderman, 2014/07/22 22:53:59]

Unfortunately draw_transform is not set via PushProperties and is not set when
PushProperties is executed. If I move the rotation transform to PushProperties
it is wiped out before append quads.

[enne (OOO), 2014/07/22 22:55:37]

Sorry, I was trying to suggest that you modify LayerImpl::transform_.

+  transform.Scale(content_bounds().width(), content_bounds().height());
+
+  switch (video_rotation_) {
+    case media::VIDEO_ROTATION_90:
+      transform.Rotate(90.0);
+      transform.Translate(0.0, -1.0);
+      break;
+    case media::VIDEO_ROTATION_180:
+      transform.Rotate(180.0);
+      transform.Translate(-1.0, -1.0);
+      break;
+    case media::VIDEO_ROTATION_270:
+      transform.Rotate(270.0);
+      transform.Translate(-1.0, 0);
+    case media::VIDEO_ROTATION_0:
+      break;
+  }
+
+  shared_quad_state->SetAll(transform,
+                            content_bounds(),
+                            visible_content_rect(),

[enne (OOO), 2014/07/21 23:42:49]

This seems a bit wrong.  Quad rects and visible content rects are in the space
of content bounds.  You can't change quad rects to be 1x1 but not change content
bounds or visible content rect without breaking some assumptions.

It seems like you could do this without the scale, if you just translated by
-width or -height rather than -1?

[suderman, 2014/07/22 00:05:20]

Previous version used width/height translation w/o scaling however it was found
to be confusing due to 90/270 degree rotations swapping width/height. We could
change to use a 1,1 content/visible rect to match or use the width/height
translation. I have alternated between these before so I am willing to use
either once there is a final say.

+                            clip_rect(),
+                            is_clipped(),
+                            draw_opacity(),
+                            blend_mode(),
+                            sorting_context_id());
+
   AppendDebugBorderQuad(
       render_pass, content_bounds(), shared_quad_state, append_quads_data);
 
-  gfx::Rect quad_rect(content_bounds());
+  gfx::Rect quad_rect(1, 1);
   gfx::Rect opaque_rect(contents_opaque() ? quad_rect : gfx::Rect());
   gfx::Rect visible_rect = frame_->visible_rect();
   gfx::Size coded_size = frame_->coded_size();
 
-  gfx::Rect visible_quad_rect = occlusion_tracker.UnoccludedContentRect(
-      quad_rect, draw_properties().target_space_transform);
+  gfx::Rect visible_quad_rect =
+      occlusion_tracker.UnoccludedContentRect(quad_rect, transform);
   if (visible_quad_rect.IsEmpty())
     return;
 
   // Pixels for macroblocked formats.
   const float tex_width_scale =
       static_cast<float>(visible_rect.width()) / coded_size.width();
   const float tex_height_scale =
       static_cast<float>(visible_rect.height()) / coded_size.height();
   const float tex_x_offset =
       static_cast<float>(visible_rect.x()) / coded_size.width();
@@ skipping 175 matching lines @@
 void VideoLayerImpl::SetProviderClientImpl(
     scoped_refptr<VideoFrameProviderClientImpl> provider_client_impl) {
   provider_client_impl_ = provider_client_impl;
 }
 
 const char* VideoLayerImpl::LayerTypeAsString() const {
   return "cc::VideoLayerImpl";
 }
 
 }  // namespace cc