Revert "Fix video quad rect and surface output rect"
This reverts commit 1dfd4418faf858a63bd901ef0485c7e54e5e69fb.
Reason for revert: <INSERT REASONING HERE>
Original change's description:
> Fix video quad rect and surface output rect
>
> Video surface output rect didn't take rotation into account so when
> SurfaceAggregator would stretch content to fill bounds, it would end up
> with the wrong scaling factors (squash in one dimension and expand in
> another) since the surface quad's bounds (in the embedder) were rotated.
>
> To work around this (or by accident), VideoFrameResourceProvider was
> passing the rotated size as the quad's rect which doesn't make sense
> intuitively. This also worked by accident with DirectComposition
> overlays because of applying another scaling which would fix the aspect
> ratio again.
>
> This change makes it possible to use the |quad_to_target_transform| as
> is, without having to apply an aspect ratio correcting scaling, and to
> assume that the quad's rect is in the pre-transform space.
>
> Bug: 904035
> Change-Id: Ia55e44f1f2b49b8d368a97af54f3ce9d90a81234
> Reviewed-on: https://chromium-review.googlesource.com/c/1334971
> Reviewed-by: enne <enne@chromium.org>
> Reviewed-by: Kenneth Russell <kbr@chromium.org>
> Reviewed-by: Frank Liberato <liberato@chromium.org>
> Commit-Queue: Sunny Sachanandani <sunnyps@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#608495}
TBR=kbr@chromium.org,enne@chromium.org,sunnyps@chromium.org,liberato@chromium.org
Change-Id: I99727abc125468581caa0cf9e2ddea74351dd1f9
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 904035
Reviewed-on: https://chromium-review.googlesource.com/c/1338206
Reviewed-by: Ella Ge <eirage@chromium.org>
Commit-Queue: Ella Ge <eirage@chromium.org>
Cr-Commit-Position: refs/heads/master@{#608501}diff --git a/cc/layers/video_layer_impl.cc b/cc/layers/video_layer_impl.cc
index 84c9913b..c356a8f6 100644
--- a/cc/layers/video_layer_impl.cc
+++ b/cc/layers/video_layer_impl.cc
@@ -122,8 +122,6 @@
DCHECK(frame_.get());
gfx::Transform transform = DrawTransform();
- // bounds() is in post-rotation space so quad rect in content space must be
- // in pre-rotation space
gfx::Size rotated_size = bounds();
switch (video_rotation_) {
@@ -145,19 +143,20 @@
break;
}
- gfx::Rect quad_rect(rotated_size);
Occlusion occlusion_in_video_space =
draw_properties()
.occlusion_in_content_space.GetOcclusionWithGivenDrawTransform(
transform);
gfx::Rect visible_quad_rect =
- occlusion_in_video_space.GetUnoccludedContentRect(quad_rect);
+ occlusion_in_video_space.GetUnoccludedContentRect(
+ gfx::Rect(rotated_size));
if (visible_quad_rect.IsEmpty())
return;
- updater_->AppendQuads(
- render_pass, frame_, transform, quad_rect, visible_quad_rect, clip_rect(),
- is_clipped(), contents_opaque(), draw_opacity(), GetSortingContextId());
+ updater_->AppendQuads(render_pass, frame_, transform, rotated_size,
+ visible_quad_rect, clip_rect(), is_clipped(),
+ contents_opaque(), draw_opacity(),
+ GetSortingContextId(), visible_quad_rect);
}
void VideoLayerImpl::DidDraw(viz::ClientResourceProvider* resource_provider) {
diff --git a/media/renderers/video_resource_updater.cc b/media/renderers/video_resource_updater.cc
index 91bac5fe..ffac9c8dd 100644
--- a/media/renderers/video_resource_updater.cc
+++ b/media/renderers/video_resource_updater.cc
@@ -404,24 +404,26 @@
void VideoResourceUpdater::AppendQuads(viz::RenderPass* render_pass,
scoped_refptr<VideoFrame> frame,
gfx::Transform transform,
- gfx::Rect quad_rect,
- gfx::Rect visible_quad_rect,
+ gfx::Size rotated_size,
+ gfx::Rect visible_layer_rect,
gfx::Rect clip_rect,
bool is_clipped,
bool contents_opaque,
float draw_opacity,
- int sorting_context_id) {
+ int sorting_context_id,
+ gfx::Rect visible_quad_rect) {
DCHECK(frame.get());
viz::SharedQuadState* shared_quad_state =
render_pass->CreateAndAppendSharedQuadState();
- shared_quad_state->SetAll(transform, quad_rect, visible_quad_rect, clip_rect,
- is_clipped, contents_opaque, draw_opacity,
- SkBlendMode::kSrcOver, sorting_context_id);
+ gfx::Rect rotated_size_rect(rotated_size);
+ shared_quad_state->SetAll(
+ transform, rotated_size_rect, visible_layer_rect, clip_rect, is_clipped,
+ contents_opaque, draw_opacity, SkBlendMode::kSrcOver, sorting_context_id);
- bool needs_blending = !contents_opaque;
-
+ gfx::Rect quad_rect(rotated_size);
gfx::Rect visible_rect = frame->visible_rect();
+ bool needs_blending = !contents_opaque;
gfx::Size coded_size = frame->coded_size();
const float tex_width_scale =
diff --git a/media/renderers/video_resource_updater.h b/media/renderers/video_resource_updater.h
index 795a42f..9ea3f50 100644
--- a/media/renderers/video_resource_updater.h
+++ b/media/renderers/video_resource_updater.h
@@ -105,13 +105,14 @@
void AppendQuads(viz::RenderPass* render_pass,
scoped_refptr<VideoFrame> frame,
gfx::Transform transform,
- gfx::Rect quad_rect,
- gfx::Rect visible_quad_rect,
+ gfx::Size rotated_size,
+ gfx::Rect visible_layer_rect,
gfx::Rect clip_rect,
bool is_clipped,
bool context_opaque,
float draw_opacity,
- int sorting_context_id);
+ int sorting_context_id,
+ gfx::Rect visible_quad_rect);
// TODO(kylechar): This is only public for testing, make private.
VideoFrameExternalResources CreateExternalResourcesFromVideoFrame(
diff --git a/third_party/blink/renderer/platform/graphics/video_frame_resource_provider.cc b/third_party/blink/renderer/platform/graphics/video_frame_resource_provider.cc
index 96036bb..382cbf02 100644
--- a/third_party/blink/renderer/platform/graphics/video_frame_resource_provider.cc
+++ b/third_party/blink/renderer/platform/graphics/video_frame_resource_provider.cc
@@ -72,6 +72,28 @@
DCHECK(resource_updater_);
DCHECK(resource_provider_);
+ gfx::Transform transform = gfx::Transform();
+ gfx::Size rotated_size = frame->coded_size();
+
+ switch (rotation) {
+ case media::VIDEO_ROTATION_90:
+ rotated_size = gfx::Size(rotated_size.height(), rotated_size.width());
+ transform.Rotate(90.0);
+ transform.Translate(0.0, -rotated_size.height());
+ break;
+ case media::VIDEO_ROTATION_180:
+ transform.Rotate(180.0);
+ transform.Translate(-rotated_size.width(), -rotated_size.height());
+ break;
+ case media::VIDEO_ROTATION_270:
+ rotated_size = gfx::Size(rotated_size.height(), rotated_size.width());
+ transform.Rotate(270.0);
+ transform.Translate(-rotated_size.width(), 0);
+ break;
+ case media::VIDEO_ROTATION_0:
+ break;
+ }
+
// When obtaining frame resources, we end up having to wait. See
// https://crbug/878070.
// Unfortunately, we have no idea if blocking is allowed on the current thread
@@ -86,37 +108,21 @@
resource_updater_->ObtainFrameResources(frame);
}
- gfx::Transform transform = gfx::Transform();
- // The quad's rect is in pre-transform space so that applying the transform on
- // it will produce the bounds in target space.
- gfx::Rect quad_rect = gfx::Rect(frame->natural_size());
-
- switch (rotation) {
- case media::VIDEO_ROTATION_90:
- transform.Rotate(90.0);
- transform.Translate(0.0, -quad_rect.height());
- break;
- case media::VIDEO_ROTATION_180:
- transform.Rotate(180.0);
- transform.Translate(-quad_rect.width(), -quad_rect.height());
- break;
- case media::VIDEO_ROTATION_270:
- transform.Rotate(270.0);
- transform.Translate(-quad_rect.width(), 0);
- break;
- case media::VIDEO_ROTATION_0:
- break;
- }
-
- gfx::Rect visible_quad_rect = quad_rect;
- gfx::Rect clip_rect;
+ // TODO(lethalantidote) : update with true value;
+ gfx::Rect visible_layer_rect = gfx::Rect(rotated_size);
+ gfx::Rect clip_rect = gfx::Rect(frame->coded_size());
bool is_clipped = false;
float draw_opacity = 1.0f;
int sorting_context_id = 0;
- resource_updater_->AppendQuads(
- render_pass, std::move(frame), transform, quad_rect, visible_quad_rect,
- clip_rect, is_clipped, is_opaque, draw_opacity, sorting_context_id);
+ // Internal to this compositor frame, this video quad is never occluded,
+ // thus the full quad is visible.
+ gfx::Rect visible_quad_rect = gfx::Rect(rotated_size);
+
+ resource_updater_->AppendQuads(render_pass, std::move(frame), transform,
+ rotated_size, visible_layer_rect, clip_rect,
+ is_clipped, is_opaque, draw_opacity,
+ sorting_context_id, visible_quad_rect);
}
void VideoFrameResourceProvider::ReleaseFrameResources() {
diff --git a/third_party/blink/renderer/platform/graphics/video_frame_submitter.cc b/third_party/blink/renderer/platform/graphics/video_frame_submitter.cc
index bca500d..d737574f 100644
--- a/third_party/blink/renderer/platform/graphics/video_frame_submitter.cc
+++ b/third_party/blink/renderer/platform/graphics/video_frame_submitter.cc
@@ -242,22 +242,16 @@
if (!compositor_frame_sink_ || !ShouldSubmit())
return false;
- gfx::Size frame_size(video_frame->natural_size());
- if (rotation_ == media::VIDEO_ROTATION_90 ||
- rotation_ == media::VIDEO_ROTATION_270) {
- frame_size = gfx::Size(frame_size.height(), frame_size.width());
- }
- if (frame_size_ != frame_size) {
+ if (frame_size_ != gfx::Rect(video_frame->coded_size())) {
if (!frame_size_.IsEmpty())
child_local_surface_id_allocator_.GenerateId();
- frame_size_ = frame_size;
+ frame_size_ = gfx::Rect(video_frame->coded_size());
}
viz::CompositorFrame compositor_frame;
std::unique_ptr<viz::RenderPass> render_pass = viz::RenderPass::Create();
- render_pass->SetNew(1, gfx::Rect(frame_size_), gfx::Rect(frame_size_),
- gfx::Transform());
+ render_pass->SetNew(1, frame_size_, frame_size_, gfx::Transform());
render_pass->filters = cc::FilterOperations();
resource_provider_->AppendQuads(render_pass.get(), video_frame, rotation_,
is_opaque_);
@@ -308,8 +302,7 @@
compositor_frame.metadata.may_contain_video = true;
std::unique_ptr<viz::RenderPass> render_pass = viz::RenderPass::Create();
- render_pass->SetNew(1, gfx::Rect(frame_size_), gfx::Rect(frame_size_),
- gfx::Transform());
+ render_pass->SetNew(1, frame_size_, frame_size_, gfx::Transform());
compositor_frame.render_pass_list.push_back(std::move(render_pass));
compositor_frame_sink_->SubmitCompositorFrame(
diff --git a/third_party/blink/renderer/platform/graphics/video_frame_submitter.h b/third_party/blink/renderer/platform/graphics/video_frame_submitter.h
index 22728db..3b4fcbf 100644
--- a/third_party/blink/renderer/platform/graphics/video_frame_submitter.h
+++ b/third_party/blink/renderer/platform/graphics/video_frame_submitter.h
@@ -142,7 +142,7 @@
// Size of the video frame being submitted. It is set the first time a frame
// is submitted. Every time there is a change in the video frame size, the
// child component of the LocalSurfaceId will be updated.
- gfx::Size frame_size_;
+ gfx::Rect frame_size_;
// Used to updated the LocalSurfaceId when detecting a change in video frame
// size.
diff --git a/third_party/blink/renderer/platform/graphics/video_frame_submitter_test.cc b/third_party/blink/renderer/platform/graphics/video_frame_submitter_test.cc
index 3eabd60..5e4b0ff 100644
--- a/third_party/blink/renderer/platform/graphics/video_frame_submitter_test.cc
+++ b/third_party/blink/renderer/platform/graphics/video_frame_submitter_test.cc
@@ -54,10 +54,6 @@
: binding_(this, std::move(*request)) {}
~MockCompositorFrameSink() override = default;
- const viz::CompositorFrame& last_submitted_compositor_frame() const {
- return last_submitted_compositor_frame_;
- }
-
MOCK_METHOD1(SetNeedsBeginFrame, void(bool));
MOCK_METHOD0(SetWantsAnimateOnlyBeginFrames, void());
@@ -68,8 +64,7 @@
viz::CompositorFrame frame,
viz::mojom::blink::HitTestRegionListPtr hit_test_region_list,
uint64_t submit_time) override {
- last_submitted_compositor_frame_ = std::move(frame);
- DoSubmitCompositorFrame(id, &last_submitted_compositor_frame_);
+ DoSubmitCompositorFrame(id, &frame);
}
void SubmitCompositorFrameSync(
const viz::LocalSurfaceId& id,
@@ -77,8 +72,7 @@
viz::mojom::blink::HitTestRegionListPtr hit_test_region_list,
uint64_t submit_time,
const SubmitCompositorFrameSyncCallback callback) override {
- last_submitted_compositor_frame_ = std::move(frame);
- DoSubmitCompositorFrame(id, &last_submitted_compositor_frame_);
+ DoSubmitCompositorFrame(id, &frame);
}
MOCK_METHOD1(DidNotProduceFrame, void(const viz::BeginFrameAck&));
@@ -99,8 +93,6 @@
private:
mojo::Binding<viz::mojom::blink::CompositorFrameSink> binding_;
- viz::CompositorFrame last_submitted_compositor_frame_;
-
DISALLOW_COPY_AND_ASSIGN(MockCompositorFrameSink);
};
@@ -810,7 +802,7 @@
EXPECT_EQ(11u, local_surface_id.parent_sequence_number());
EXPECT_EQ(viz::kInitialChildSequenceNumber,
local_surface_id.child_sequence_number());
- EXPECT_EQ(gfx::Size(8, 8), submitter_->frame_size_);
+ EXPECT_EQ(gfx::Rect(8, 8), submitter_->frame_size_);
}
EXPECT_CALL(*video_frame_provider_, GetCurrentFrame())
@@ -835,112 +827,7 @@
EXPECT_EQ(11u, local_surface_id.parent_sequence_number());
EXPECT_EQ(viz::kInitialChildSequenceNumber + 1,
local_surface_id.child_sequence_number());
- EXPECT_EQ(gfx::Size(2, 2), submitter_->frame_size_);
- }
-}
-
-TEST_F(VideoFrameSubmitterTest, VideoRotationOutputRect) {
- MakeSubmitter();
- EXPECT_CALL(*sink_, SetNeedsBeginFrame(true));
- submitter_->StartRendering();
- EXPECT_TRUE(submitter_->Rendering());
-
- gfx::Size coded_size(1280, 720);
- gfx::Size natural_size(1280, 1024);
- gfx::Size rotated_size(1024, 1280);
-
- {
- submitter_->SetRotation(media::VideoRotation::VIDEO_ROTATION_90);
-
- EXPECT_CALL(*video_frame_provider_, UpdateCurrentFrame(_, _))
- .WillOnce(Return(true));
- EXPECT_CALL(*video_frame_provider_, GetCurrentFrame())
- .WillOnce(Return(media::VideoFrame::CreateFrame(
- media::PIXEL_FORMAT_YV12, coded_size, gfx::Rect(coded_size),
- natural_size, base::TimeDelta())));
- EXPECT_CALL(*sink_, DoSubmitCompositorFrame(_, _));
- EXPECT_CALL(*video_frame_provider_, PutCurrentFrame());
- EXPECT_CALL(*resource_provider_,
- AppendQuads(_, _, media::VideoRotation::VIDEO_ROTATION_90, _));
- EXPECT_CALL(*resource_provider_, PrepareSendToParent(_, _));
- EXPECT_CALL(*resource_provider_, ReleaseFrameResources());
-
- viz::BeginFrameArgs args = begin_frame_source_->CreateBeginFrameArgs(
- BEGINFRAME_FROM_HERE, now_src_.get());
- submitter_->OnBeginFrame(args);
- scoped_task_environment_.RunUntilIdle();
-
- EXPECT_EQ(sink_->last_submitted_compositor_frame().size_in_pixels(),
- rotated_size);
-
- submitter_->DidReceiveFrame();
-
- WTF::Vector<viz::ReturnedResource> resources;
- EXPECT_CALL(*resource_provider_, ReceiveReturnsFromParent(_));
- submitter_->DidReceiveCompositorFrameAck(resources);
- }
-
- {
- submitter_->SetRotation(media::VideoRotation::VIDEO_ROTATION_180);
-
- EXPECT_CALL(*video_frame_provider_, UpdateCurrentFrame(_, _))
- .WillOnce(Return(true));
- EXPECT_CALL(*video_frame_provider_, GetCurrentFrame())
- .WillOnce(Return(media::VideoFrame::CreateFrame(
- media::PIXEL_FORMAT_YV12, coded_size, gfx::Rect(coded_size),
- natural_size, base::TimeDelta())));
- EXPECT_CALL(*sink_, DoSubmitCompositorFrame(_, _));
- EXPECT_CALL(*video_frame_provider_, PutCurrentFrame());
- EXPECT_CALL(*resource_provider_,
- AppendQuads(_, _, media::VideoRotation::VIDEO_ROTATION_180, _));
- EXPECT_CALL(*resource_provider_, PrepareSendToParent(_, _));
- EXPECT_CALL(*resource_provider_, ReleaseFrameResources());
-
- viz::BeginFrameArgs args = begin_frame_source_->CreateBeginFrameArgs(
- BEGINFRAME_FROM_HERE, now_src_.get());
- submitter_->OnBeginFrame(args);
- scoped_task_environment_.RunUntilIdle();
-
- // 180 deg rotation has same size.
- EXPECT_EQ(sink_->last_submitted_compositor_frame().size_in_pixels(),
- natural_size);
-
- submitter_->DidReceiveFrame();
-
- WTF::Vector<viz::ReturnedResource> resources;
- EXPECT_CALL(*resource_provider_, ReceiveReturnsFromParent(_));
- submitter_->DidReceiveCompositorFrameAck(resources);
- }
-
- {
- submitter_->SetRotation(media::VideoRotation::VIDEO_ROTATION_270);
-
- EXPECT_CALL(*video_frame_provider_, UpdateCurrentFrame(_, _))
- .WillOnce(Return(true));
- EXPECT_CALL(*video_frame_provider_, GetCurrentFrame())
- .WillOnce(Return(media::VideoFrame::CreateFrame(
- media::PIXEL_FORMAT_YV12, coded_size, gfx::Rect(coded_size),
- natural_size, base::TimeDelta())));
- EXPECT_CALL(*sink_, DoSubmitCompositorFrame(_, _));
- EXPECT_CALL(*video_frame_provider_, PutCurrentFrame());
- EXPECT_CALL(*resource_provider_,
- AppendQuads(_, _, media::VideoRotation::VIDEO_ROTATION_270, _));
- EXPECT_CALL(*resource_provider_, PrepareSendToParent(_, _));
- EXPECT_CALL(*resource_provider_, ReleaseFrameResources());
-
- viz::BeginFrameArgs args = begin_frame_source_->CreateBeginFrameArgs(
- BEGINFRAME_FROM_HERE, now_src_.get());
- submitter_->OnBeginFrame(args);
- scoped_task_environment_.RunUntilIdle();
-
- EXPECT_EQ(sink_->last_submitted_compositor_frame().size_in_pixels(),
- rotated_size);
-
- submitter_->DidReceiveFrame();
-
- WTF::Vector<viz::ReturnedResource> resources;
- EXPECT_CALL(*resource_provider_, ReceiveReturnsFromParent(_));
- submitter_->DidReceiveCompositorFrameAck(resources);
+ EXPECT_EQ(gfx::Rect(2, 2), submitter_->frame_size_);
}
}
