cc: Add support for non-2D texture targets to YUVVideoQuad.

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 166 matching lines @@
   gfx::Rect visible_quad_rect =
       occlusion_in_video_space.GetUnoccludedContentRect(quad_rect);
   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();
-  const float tex_y_offset =
-      static_cast<float>(visible_rect.y()) / coded_size.height();
 
   switch (frame_resource_type_) {
     // TODO(danakj): Remove this, hide it in the hardware path.
     case VideoFrameExternalResources::SOFTWARE_RESOURCE: {
       DCHECK_EQ(frame_resources_.size(), 0u);
       DCHECK_EQ(software_resources_.size(), 1u);
       if (software_resources_.size() < 1u)
         break;
       bool premultiplied_alpha = true;
       gfx::PointF uv_top_left(0.f, 0.f);
@@ skipping 34 matching lines @@
           frame_->format(), media::VideoFrame::kUPlane, coded_size);
       DCHECK(uv_tex_size ==
              media::VideoFrame::PlaneSize(
                  frame_->format(), media::VideoFrame::kVPlane, coded_size));
       if (frame_resources_.size() > 3) {
         DCHECK(ya_tex_size ==
                media::VideoFrame::PlaneSize(
                    frame_->format(), media::VideoFrame::kAPlane, coded_size));
       }
 
-      gfx::RectF tex_coord_rect(
-          tex_x_offset, tex_y_offset, tex_width_scale, tex_height_scale);
+      // Compute the UV sub-sampling factor based on the ratio between
+      // |ya_tex_size| and |uv_tex_size|.
+      float uv_subsampling_factor_x =
+          static_cast<float>(ya_tex_size.width()) / uv_tex_size.width();
+      float uv_subsampling_factor_y =
+          static_cast<float>(ya_tex_size.height()) / uv_tex_size.height();
+      gfx::RectF ya_tex_coord_rect(visible_rect);

[danakj, 2015/05/12 16:32:12]

if these are in pixels not in [0,1] they aren't really tex coord rects anymore?
our texel space is [0,1]

i think you should do the math you have in GLRenderer over here to get this
right, eg:

ya_vertex_tex_translate_x = quad->ya_tex_coord_rect.x() * ya_tex_scale.width()

Instead you should divide the ya_tex_coord_rect.x here by the ya_tex_size.width

Is there a reason you can't do that math here so you have [0,1] coords?

[reveman, 2015/05/12 17:09:35]

If tex coords are normalized or not depend on the target. tex_coord_rect for
quads that support targets with non-normalized coordinates (e.g. TileDrawQuad)
currently use a non-normalized tex_coord_rect and convert to normalized
coordinates in GLRenderer when needed. I prefer not doing things differently in
the VideoDrawQuad case but I guess we can change it everywhere in a follow up
patch if you think it's better to always pass normalized coords to the quads and
convert to non-normalized in the Renderer impl. 

[danakj, 2015/05/12 17:22:11]

Maybe we're a bit inconsistent but this used to be normalized and most places
are using normalized tex coords, I think Tiles may be the exception.

TextureLayerImpl:
https://code.google.com/p/chromium/codesearch#chromium/src/cc/layers/texture_...

RenderSurfaceImpl:
https://code.google.com/p/chromium/codesearch#chromium/src/cc/layers/render_s...

Video RGB:
https://code.google.com/p/chromium/codesearch#chromium/src/cc/layers/video_la...

For PictureLayerImpl it calls it texture_rect (not tex_coord) fwiw, it's
explicitly non-normalized. But I think this should be consistent with everything
else?

[reveman, 2015/05/12 17:40:35]

It's "tex_coord_rect" here:
https://code.google.com/p/chromium/codesearch#chromium/src/cc/quads/tile_draw...
it just happens that texture_rect is what we should use in the case of
PictureLayerImpl.

I'd really prefer if we didn't handle TEXTURE_RECTANGLE differently in
GLRenderer for VideoDrawQuad and TileDrawQuad.

I can add TODOs here and follow up with a patch that tries to make this
consistent across all quad types. Not sure normalized is preferred over
non-normalized though. I'd probably go with the latter and avoid the conversion
to normalized coords unless necessary.

+      gfx::RectF uv_tex_coord_rect(
+          visible_rect.x() / uv_subsampling_factor_x,
+          visible_rect.y() / uv_subsampling_factor_y,
+          visible_rect.width() / uv_subsampling_factor_x,
+          visible_rect.height() / uv_subsampling_factor_y);
+
       YUVVideoDrawQuad* yuv_video_quad =
           render_pass->CreateAndAppendDrawQuad<YUVVideoDrawQuad>();
       yuv_video_quad->SetNew(
           shared_quad_state, quad_rect, opaque_rect, visible_quad_rect,
-          tex_coord_rect, ya_tex_size, uv_tex_size, frame_resources_[0],
-          frame_resources_[1], frame_resources_[2],
+          ya_tex_coord_rect, uv_tex_coord_rect, ya_tex_size, uv_tex_size,
+          frame_resources_[0], frame_resources_[1], frame_resources_[2],
           frame_resources_.size() > 3 ? frame_resources_[3] : 0, color_space);
       ValidateQuadResources(yuv_video_quad);
       break;
     }
     case VideoFrameExternalResources::RGB_RESOURCE: {
       DCHECK_EQ(frame_resources_.size(), 1u);
       if (frame_resources_.size() < 1u)
         break;
       bool premultiplied_alpha = true;
       gfx::PointF uv_top_left(0.f, 0.f);
@@ skipping 113 matching lines @@
 void VideoLayerImpl::SetNeedsRedraw() {
   SetUpdateRect(gfx::UnionRects(update_rect(), gfx::Rect(bounds())));
   layer_tree_impl()->SetNeedsRedraw();
 }
 
 const char* VideoLayerImpl::LayerTypeAsString() const {
   return "cc::VideoLayerImpl";
 }
 
 }  // namespace cc