exo: Fix issues with viewports and transforms
The crop rectangle is set via the Wayland interface in post-transform
buffer coordinates. To generate UV coordinates, the crop rectangle must
be transformed by the inverse transform.
The forward transformation from "buffer" pixel space to target space
also turned out to be incorrect when there was a crop, there was no
viewport, and the buffer transformation was 90 or 270. The construction
of that transformation matrix now makes sense, unlike before.
The fixed up code shares a common buffer transformation matrix (rotating
in normalized coordinates around 0.5, 0.5), but that is all that can be
shared. The two transformations otherwise apply different scaling and
translations to get to their respective coordinate spaces otherwise.
Correct functionality was verified with a test app that sets a crop
rectangle while also setting/disabling the viewport rectangle, while
cycling through buffers at different rotations. The test app was run on
Chrome OS as well as under the Weston reference implementation.
BUG=751232
Change-Id: Ibca12b6d722891076db4cb053d133d128ee78889
Reviewed-on: https://chromium-review.googlesource.com/653494
Commit-Queue: Lloyd Pique <[email protected]>
Reviewed-by: David Reveman <[email protected]>
Cr-Commit-Position: refs/heads/master@{#502440}diff --git a/components/exo/surface.cc b/components/exo/surface.cc
index ce4684d5..8765e65 100644
--- a/components/exo/surface.cc
+++ b/components/exo/surface.cc
@@ -435,6 +435,10 @@
pending_state_.blend_mode != state_.blend_mode ||
pending_state_.alpha != state_.alpha;
+ bool needs_update_buffer_transform =
+ pending_state_.buffer_scale != state_.buffer_scale ||
+ pending_state_.buffer_transform != state_.buffer_transform;
+
state_ = pending_state_;
pending_state_.only_visible_on_secure_output = false;
@@ -450,6 +454,9 @@
needs_update_resource_ = true;
}
+ if (needs_update_buffer_transform)
+ UpdateBufferTransform();
+
// Move pending frame callbacks to the end of frame_callbacks.
frame_callbacks->splice(frame_callbacks->end(), pending_frame_callbacks_);
@@ -702,13 +709,33 @@
}
}
+void Surface::UpdateBufferTransform() {
+ SkMatrix buffer_matrix;
+ switch (state_.buffer_transform) {
+ case Transform::NORMAL:
+ buffer_matrix.setIdentity();
+ break;
+ case Transform::ROTATE_90:
+ buffer_matrix.setSinCos(-1, 0, 0.5f, 0.5f);
+ break;
+ case Transform::ROTATE_180:
+ buffer_matrix.setSinCos(0, -1, 0.5f, 0.5f);
+ break;
+ case Transform::ROTATE_270:
+ buffer_matrix.setSinCos(1, 0, 0.5f, 0.5f);
+ break;
+ }
+ buffer_matrix.postIDiv(state_.buffer_scale, state_.buffer_scale);
+ buffer_transform_ = gfx::Transform(buffer_matrix);
+}
+
void Surface::AppendContentsToFrame(const gfx::Point& origin,
float device_scale_factor,
cc::CompositorFrame* frame) {
const std::unique_ptr<viz::RenderPass>& render_pass =
frame->render_pass_list.back();
gfx::Rect output_rect(origin, content_size_);
- gfx::Rect quad_rect(current_resource_.size);
+ gfx::Rect quad_rect(0, 0, 1, 1);
// Surface uses DIP, but the |render_pass->damage_rect| uses pixels, so we
// need scale it beased on the |device_scale_factor|.
@@ -717,36 +744,20 @@
render_pass->damage_rect.Union(
gfx::ConvertRectToPixel(device_scale_factor, damage_rect));
- // Create a transformation matrix that maps buffer coordinates to target by
- // inverting the transform and scale of buffer.
- SkMatrix buffer_to_target_matrix;
- switch (state_.buffer_transform) {
- case Transform::NORMAL:
- buffer_to_target_matrix.setIdentity();
- break;
- case Transform::ROTATE_90:
- buffer_to_target_matrix.setSinCos(-1, 0);
- buffer_to_target_matrix.postTranslate(0, output_rect.height());
- break;
- case Transform::ROTATE_180:
- buffer_to_target_matrix.setSinCos(0, -1);
- buffer_to_target_matrix.postTranslate(output_rect.width(),
- output_rect.height());
- break;
- case Transform::ROTATE_270:
- buffer_to_target_matrix.setSinCos(1, 0);
- buffer_to_target_matrix.postTranslate(output_rect.width(), 0);
- break;
- }
- gfx::SizeF transformed_buffer_size(
- ToTransformedSize(current_resource_.size, state_.buffer_transform));
- if (!transformed_buffer_size.IsEmpty()) {
- buffer_to_target_matrix.preScale(
- output_rect.width() / transformed_buffer_size.width(),
- output_rect.height() / transformed_buffer_size.height());
- }
- buffer_to_target_matrix.postTranslate(origin.x(), origin.y());
- buffer_to_target_matrix.postScale(device_scale_factor, device_scale_factor);
+ // Compute the total transformation from post-transform buffer coordinates to
+ // target coordinates.
+ SkMatrix viewport_to_target_matrix;
+ // Scale and offset the normalized space to fit the content size rectangle.
+ viewport_to_target_matrix.setScale(
+ content_size_.width() * state_.buffer_scale,
+ content_size_.height() * state_.buffer_scale);
+ viewport_to_target_matrix.postTranslate(origin.x(), origin.y());
+ // Convert from DPs to pixels.
+ viewport_to_target_matrix.postScale(device_scale_factor, device_scale_factor);
+
+ gfx::Transform quad_to_target_transform(buffer_transform_);
+ quad_to_target_transform.ConcatTransform(
+ gfx::Transform(viewport_to_target_matrix));
bool are_contents_opaque =
!current_resource_has_alpha_ || state_.blend_mode == SkBlendMode::kSrc ||
@@ -755,25 +766,27 @@
viz::SharedQuadState* quad_state =
render_pass->CreateAndAppendSharedQuadState();
quad_state->SetAll(
- gfx::Transform(buffer_to_target_matrix),
- gfx::Rect(content_size_) /* quad_layer_rect */,
+ quad_to_target_transform, gfx::Rect(content_size_) /* quad_layer_rect */,
output_rect /* visible_quad_layer_rect */, gfx::Rect() /* clip_rect */,
false /* is_clipped */, are_contents_opaque, state_.alpha /* opacity */,
SkBlendMode::kSrcOver /* blend_mode */, 0 /* sorting_context_id */);
if (current_resource_.id) {
- gfx::PointF uv_top_left(0.f, 0.f);
- gfx::PointF uv_bottom_right(1.f, 1.f);
+ gfx::RectF uv_crop(gfx::SizeF(1, 1));
if (!state_.crop.IsEmpty()) {
- gfx::SizeF scaled_buffer_size(
- gfx::ScaleSize(transformed_buffer_size, 1.0f / state_.buffer_scale));
- uv_top_left = state_.crop.origin();
- uv_top_left.Scale(1.f / scaled_buffer_size.width(),
- 1.f / scaled_buffer_size.height());
- uv_bottom_right = state_.crop.bottom_right();
- uv_bottom_right.Scale(1.f / scaled_buffer_size.width(),
- 1.f / scaled_buffer_size.height());
+ // The crop rectangle is a post-transformation rectangle. To get the UV
+ // coordinates, we need to convert it to normalized buffer coordinates and
+ // pass them through the inverse of the buffer transformation.
+ uv_crop = gfx::RectF(state_.crop);
+ gfx::Size transformed_buffer_size(
+ ToTransformedSize(current_resource_.size, state_.buffer_transform));
+ if (!transformed_buffer_size.IsEmpty())
+ uv_crop.Scale(1.f / transformed_buffer_size.width(),
+ 1.f / transformed_buffer_size.height());
+
+ buffer_transform_.TransformRectReverse(&uv_crop);
}
+
// Texture quad is only needed if buffer is not fully transparent.
if (state_.alpha) {
viz::TextureDrawQuad* texture_quad =
@@ -782,9 +795,10 @@
texture_quad->SetNew(
quad_state, quad_rect, quad_rect, !are_contents_opaque,
- current_resource_.id, true /* premultiplied_alpha */, uv_top_left,
- uv_bottom_right, SK_ColorTRANSPARENT /* background_color */,
- vertex_opacity, false /* y_flipped */, false /* nearest_neighbor */,
+ current_resource_.id, true /* premultiplied_alpha */,
+ uv_crop.origin(), uv_crop.bottom_right(),
+ SK_ColorTRANSPARENT /* background_color */, vertex_opacity,
+ false /* y_flipped */, false /* nearest_neighbor */,
state_.only_visible_on_secure_output);
if (current_resource_.is_overlay_candidate)
texture_quad->set_resource_size_in_pixels(current_resource_.size);
diff --git a/components/exo/surface.h b/components/exo/surface.h
index f73180b..76f72b5 100644
--- a/components/exo/surface.h
+++ b/components/exo/surface.h
@@ -23,6 +23,7 @@
#include "ui/aura/window.h"
#include "ui/gfx/geometry/rect.h"
#include "ui/gfx/native_widget_types.h"
+#include "ui/gfx/transform.h"
namespace base {
namespace trace_event {
@@ -260,6 +261,9 @@
// will be called.
void UpdateResource(LayerTreeFrameSinkHolder* frame_sink_holder);
+ // Updates buffer_transform_ to match the current buffer parameters.
+ void UpdateBufferTransform();
+
// Puts the current surface into a draw quad, and appends the draw quads into
// the |frame|.
void AppendContentsToFrame(const gfx::Point& origin,
@@ -341,6 +345,10 @@
// This is true if UpdateResources() should be called.
bool needs_update_resource_ = true;
+ // The current buffer transform matrix. It specifies the transformation from
+ // normalized buffer coordinates to post-tranform buffer coordinates.
+ gfx::Transform buffer_transform_;
+
// This is set when the compositing starts and passed to active frame
// callbacks when compositing successfully ends.
base::TimeTicks last_compositing_start_time_;
diff --git a/components/exo/surface_unittest.cc b/components/exo/surface_unittest.cc
index 53f047fd..b036849 100644
--- a/components/exo/surface_unittest.cc
+++ b/components/exo/surface_unittest.cc
@@ -423,6 +423,189 @@
frame.render_pass_list.back()->damage_rect);
}
+TEST_P(SurfaceTest, SetCropAndBufferTransform) {
+ gfx::Size buffer_size(128, 64);
+ auto buffer = base::MakeUnique<Buffer>(
+ exo_test_helper()->CreateGpuMemoryBuffer(buffer_size));
+ auto surface = base::MakeUnique<Surface>();
+ auto shell_surface = base::MakeUnique<ShellSurface>(surface.get());
+
+ const gfx::RectF crop_0(
+ gfx::SkRectToRectF(SkRect::MakeLTRB(0.03125f, 0.1875f, 0.4375f, 0.25f)));
+ const gfx::RectF crop_90(
+ gfx::SkRectToRectF(SkRect::MakeLTRB(0.875f, 0.0625f, 0.90625f, 0.875f)));
+ const gfx::RectF crop_180(
+ gfx::SkRectToRectF(SkRect::MakeLTRB(0.5625f, 0.75f, 0.96875f, 0.8125f)));
+ const gfx::RectF crop_270(
+ gfx::SkRectToRectF(SkRect::MakeLTRB(0.09375f, 0.125f, 0.125f, 0.9375f)));
+ const gfx::Rect target_with_no_viewport(ToPixel(gfx::Rect(gfx::Size(52, 4))));
+ const gfx::Rect target_with_viewport(ToPixel(gfx::Rect(gfx::Size(128, 64))));
+
+ surface->Attach(buffer.get());
+ gfx::Size crop_size(52, 4);
+ surface->SetCrop(gfx::RectF(gfx::PointF(4, 12), gfx::SizeF(crop_size)));
+ surface->SetBufferTransform(Transform::NORMAL);
+ surface->Commit();
+
+ RunAllPendingInMessageLoop();
+
+ {
+ const cc::CompositorFrame& frame = GetFrameFromSurface(shell_surface.get());
+ ASSERT_EQ(1u, frame.render_pass_list.size());
+ const viz::QuadList& quad_list = frame.render_pass_list[0]->quad_list;
+ ASSERT_EQ(1u, quad_list.size());
+ const viz::TextureDrawQuad* quad =
+ viz::TextureDrawQuad::MaterialCast(quad_list.front());
+ EXPECT_EQ(crop_0.origin(), quad->uv_top_left);
+ EXPECT_EQ(crop_0.bottom_right(), quad->uv_bottom_right);
+ EXPECT_EQ(
+ ToPixel(gfx::Rect(0, 0, 52, 4)),
+ cc::MathUtil::MapEnclosingClippedRect(
+ quad->shared_quad_state->quad_to_target_transform, quad->rect));
+ }
+
+ surface->SetBufferTransform(Transform::ROTATE_90);
+ surface->Commit();
+
+ RunAllPendingInMessageLoop();
+
+ {
+ const cc::CompositorFrame& frame = GetFrameFromSurface(shell_surface.get());
+ ASSERT_EQ(1u, frame.render_pass_list.size());
+ const viz::QuadList& quad_list = frame.render_pass_list[0]->quad_list;
+ ASSERT_EQ(1u, quad_list.size());
+ const viz::TextureDrawQuad* quad =
+ viz::TextureDrawQuad::MaterialCast(quad_list.front());
+ EXPECT_EQ(crop_90.origin(), quad->uv_top_left);
+ EXPECT_EQ(crop_90.bottom_right(), quad->uv_bottom_right);
+ EXPECT_EQ(
+ ToPixel(gfx::Rect(0, 0, 52, 4)),
+ cc::MathUtil::MapEnclosingClippedRect(
+ quad->shared_quad_state->quad_to_target_transform, quad->rect));
+ }
+
+ surface->SetBufferTransform(Transform::ROTATE_180);
+ surface->Commit();
+
+ RunAllPendingInMessageLoop();
+
+ {
+ const cc::CompositorFrame& frame = GetFrameFromSurface(shell_surface.get());
+ ASSERT_EQ(1u, frame.render_pass_list.size());
+ const viz::QuadList& quad_list = frame.render_pass_list[0]->quad_list;
+ ASSERT_EQ(1u, quad_list.size());
+ const viz::TextureDrawQuad* quad =
+ viz::TextureDrawQuad::MaterialCast(quad_list.front());
+ EXPECT_EQ(crop_180.origin(), quad->uv_top_left);
+ EXPECT_EQ(crop_180.bottom_right(), quad->uv_bottom_right);
+ EXPECT_EQ(
+ ToPixel(gfx::Rect(0, 0, 52, 4)),
+ cc::MathUtil::MapEnclosingClippedRect(
+ quad->shared_quad_state->quad_to_target_transform, quad->rect));
+ }
+
+ surface->SetBufferTransform(Transform::ROTATE_270);
+ surface->Commit();
+
+ RunAllPendingInMessageLoop();
+
+ {
+ const cc::CompositorFrame& frame = GetFrameFromSurface(shell_surface.get());
+ ASSERT_EQ(1u, frame.render_pass_list.size());
+ const viz::QuadList& quad_list = frame.render_pass_list[0]->quad_list;
+ ASSERT_EQ(1u, quad_list.size());
+ const viz::TextureDrawQuad* quad =
+ viz::TextureDrawQuad::MaterialCast(quad_list.front());
+ EXPECT_EQ(crop_270.origin(), quad->uv_top_left);
+ EXPECT_EQ(crop_270.bottom_right(), quad->uv_bottom_right);
+ EXPECT_EQ(
+ target_with_no_viewport,
+ cc::MathUtil::MapEnclosingClippedRect(
+ quad->shared_quad_state->quad_to_target_transform, quad->rect));
+ }
+
+ surface->SetViewport(gfx::Size(128, 64));
+ surface->SetBufferTransform(Transform::NORMAL);
+ surface->Commit();
+
+ RunAllPendingInMessageLoop();
+
+ {
+ const cc::CompositorFrame& frame = GetFrameFromSurface(shell_surface.get());
+ ASSERT_EQ(1u, frame.render_pass_list.size());
+ const viz::QuadList& quad_list = frame.render_pass_list[0]->quad_list;
+ ASSERT_EQ(1u, quad_list.size());
+ const viz::TextureDrawQuad* quad =
+ viz::TextureDrawQuad::MaterialCast(quad_list.front());
+ EXPECT_EQ(crop_0.origin(), quad->uv_top_left);
+ EXPECT_EQ(crop_0.bottom_right(), quad->uv_bottom_right);
+ EXPECT_EQ(
+ target_with_viewport,
+ cc::MathUtil::MapEnclosingClippedRect(
+ quad->shared_quad_state->quad_to_target_transform, quad->rect));
+ }
+
+ surface->SetBufferTransform(Transform::ROTATE_90);
+ surface->Commit();
+
+ RunAllPendingInMessageLoop();
+
+ {
+ const cc::CompositorFrame& frame = GetFrameFromSurface(shell_surface.get());
+ ASSERT_EQ(1u, frame.render_pass_list.size());
+ const viz::QuadList& quad_list = frame.render_pass_list[0]->quad_list;
+ ASSERT_EQ(1u, quad_list.size());
+ const viz::TextureDrawQuad* quad =
+ viz::TextureDrawQuad::MaterialCast(quad_list.front());
+ EXPECT_EQ(crop_90.origin(), quad->uv_top_left);
+ EXPECT_EQ(crop_90.bottom_right(), quad->uv_bottom_right);
+ EXPECT_EQ(
+ target_with_viewport,
+ cc::MathUtil::MapEnclosingClippedRect(
+ quad->shared_quad_state->quad_to_target_transform, quad->rect));
+ }
+
+ surface->SetBufferTransform(Transform::ROTATE_180);
+ surface->Commit();
+
+ RunAllPendingInMessageLoop();
+
+ {
+ const cc::CompositorFrame& frame = GetFrameFromSurface(shell_surface.get());
+ ASSERT_EQ(1u, frame.render_pass_list.size());
+ const viz::QuadList& quad_list = frame.render_pass_list[0]->quad_list;
+ ASSERT_EQ(1u, quad_list.size());
+ const viz::TextureDrawQuad* quad =
+ viz::TextureDrawQuad::MaterialCast(quad_list.front());
+ EXPECT_EQ(crop_180.origin(), quad->uv_top_left);
+ EXPECT_EQ(crop_180.bottom_right(), quad->uv_bottom_right);
+ EXPECT_EQ(
+ target_with_viewport,
+ cc::MathUtil::MapEnclosingClippedRect(
+ quad->shared_quad_state->quad_to_target_transform, quad->rect));
+ }
+
+ surface->SetBufferTransform(Transform::ROTATE_270);
+ surface->Commit();
+
+ RunAllPendingInMessageLoop();
+
+ {
+ const cc::CompositorFrame& frame = GetFrameFromSurface(shell_surface.get());
+ ASSERT_EQ(1u, frame.render_pass_list.size());
+ const viz::QuadList& quad_list = frame.render_pass_list[0]->quad_list;
+ ASSERT_EQ(1u, quad_list.size());
+ const viz::TextureDrawQuad* quad =
+ viz::TextureDrawQuad::MaterialCast(quad_list.front());
+ EXPECT_EQ(crop_270.origin(), quad->uv_top_left);
+ EXPECT_EQ(crop_270.bottom_right(), quad->uv_bottom_right);
+ EXPECT_EQ(
+ target_with_viewport,
+ cc::MathUtil::MapEnclosingClippedRect(
+ quad->shared_quad_state->quad_to_target_transform, quad->rect));
+ }
+}
+
TEST_P(SurfaceTest, SetBlendMode) {
gfx::Size buffer_size(1, 1);
auto buffer = base::MakeUnique<Buffer>(
