Commit 9b44fadc authored by Maksim Sisov's avatar Maksim Sisov Committed by Commit Bot

ozone/x11: Fix broken software compositing.

We used to be requesting a SkCanvas from the bitmap presenter and
creating own SkSurface for that canvas despite the fact that
X11BitmapSoftwarePresenter has already stored a SkSurface for
that canvas. This resulted in a weird behaviour - frames
were not presented.

So, instead of doing that roundtrip and looking for solutions for
creating a surface storage for that SkCanvas, change the
SurfaceOzoneCanvas to return SkCanvas instead of SkSurface.

This does not imply any behavioural changes.

Bug: 1045347
Change-Id: I09442a6019b1c073f4b1b3407c529792cf74c984
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2017432
Commit-Queue: Maksim Sisov <msisov@igalia.com>
Reviewed-by: default avatarRobert Kroeger <rjkroege@chromium.org>
Cr-Commit-Position: refs/heads/master@{#741369}
parent a54d53db
...@@ -36,10 +36,10 @@ void SoftwareOutputDeviceOzone::Resize(const gfx::Size& viewport_pixel_size, ...@@ -36,10 +36,10 @@ void SoftwareOutputDeviceOzone::Resize(const gfx::Size& viewport_pixel_size,
SkCanvas* SoftwareOutputDeviceOzone::BeginPaint(const gfx::Rect& damage_rect) { SkCanvas* SoftwareOutputDeviceOzone::BeginPaint(const gfx::Rect& damage_rect) {
DCHECK(gfx::Rect(viewport_pixel_size_).Contains(damage_rect)); DCHECK(gfx::Rect(viewport_pixel_size_).Contains(damage_rect));
// Get canvas for next frame. damage_rect_ = damage_rect;
surface_ = surface_ozone_->GetSurface();
return SoftwareOutputDevice::BeginPaint(damage_rect); // Get canvas for next frame.
return surface_ozone_->GetCanvas();
} }
void SoftwareOutputDeviceOzone::EndPaint() { void SoftwareOutputDeviceOzone::EndPaint() {
......
...@@ -57,12 +57,12 @@ void SoftwareRenderer::RenderFrame() { ...@@ -57,12 +57,12 @@ void SoftwareRenderer::RenderFrame() {
float fraction = NextFraction(); float fraction = NextFraction();
sk_sp<SkSurface> surface = software_surface_->GetSurface(); SkCanvas* canvas = software_surface_->GetCanvas();
SkColor color = SkColor color =
SkColorSetARGB(0xff, 0, 0xff * fraction, 0xff * (1 - fraction)); SkColorSetARGB(0xff, 0, 0xff * fraction, 0xff * (1 - fraction));
surface->getCanvas()->clear(color); canvas->clear(color);
software_surface_->PresentCanvas(gfx::Rect(size_)); software_surface_->PresentCanvas(gfx::Rect(size_));
......
...@@ -26,7 +26,7 @@ class DummySurface : public SurfaceOzoneCanvas { ...@@ -26,7 +26,7 @@ class DummySurface : public SurfaceOzoneCanvas {
~DummySurface() override {} ~DummySurface() override {}
// SurfaceOzoneCanvas implementation: // SurfaceOzoneCanvas implementation:
sk_sp<SkSurface> GetSurface() override { return surface_; } SkCanvas* GetCanvas() override { return surface_->getCanvas(); }
void ResizeCanvas(const gfx::Size& viewport_size) override { void ResizeCanvas(const gfx::Size& viewport_size) override {
surface_ = surface_ =
......
...@@ -75,7 +75,7 @@ class FileSurface : public SurfaceOzoneCanvas { ...@@ -75,7 +75,7 @@ class FileSurface : public SurfaceOzoneCanvas {
surface_ = SkSurface::MakeRaster(SkImageInfo::MakeN32Premul( surface_ = SkSurface::MakeRaster(SkImageInfo::MakeN32Premul(
viewport_size.width(), viewport_size.height())); viewport_size.width(), viewport_size.height()));
} }
sk_sp<SkSurface> GetSurface() override { return surface_; } SkCanvas* GetCanvas() override { return surface_->getCanvas(); }
void PresentCanvas(const gfx::Rect& damage) override { void PresentCanvas(const gfx::Rect& damage) override {
if (base_path_.empty()) if (base_path_.empty())
return; return;
......
...@@ -83,7 +83,7 @@ void ScenicWindowCanvas::ResizeCanvas(const gfx::Size& viewport_size) { ...@@ -83,7 +83,7 @@ void ScenicWindowCanvas::ResizeCanvas(const gfx::Size& viewport_size) {
} }
} }
sk_sp<SkSurface> ScenicWindowCanvas::GetSurface() { SkCanvas* ScenicWindowCanvas::GetCanvas() {
if (viewport_size_.IsEmpty() || frames_[current_frame_].is_empty()) if (viewport_size_.IsEmpty() || frames_[current_frame_].is_empty())
return nullptr; return nullptr;
...@@ -108,7 +108,7 @@ sk_sp<SkSurface> ScenicWindowCanvas::GetSurface() { ...@@ -108,7 +108,7 @@ sk_sp<SkSurface> ScenicWindowCanvas::GetSurface() {
} }
} }
return frames_[current_frame_].surface; return frames_[current_frame_].surface->getCanvas();
} }
void ScenicWindowCanvas::PresentCanvas(const gfx::Rect& damage) { void ScenicWindowCanvas::PresentCanvas(const gfx::Rect& damage) {
......
...@@ -17,6 +17,8 @@ ...@@ -17,6 +17,8 @@
#include "ui/ozone/platform/scenic/scenic_surface_factory.h" #include "ui/ozone/platform/scenic/scenic_surface_factory.h"
#include "ui/ozone/public/surface_ozone_canvas.h" #include "ui/ozone/public/surface_ozone_canvas.h"
class SkSurface;
namespace scenic { namespace scenic {
class Session; class Session;
} // namespace scenic } // namespace scenic
...@@ -36,7 +38,7 @@ class ScenicWindowCanvas : public SurfaceOzoneCanvas { ...@@ -36,7 +38,7 @@ class ScenicWindowCanvas : public SurfaceOzoneCanvas {
// SurfaceOzoneCanvas implementation. // SurfaceOzoneCanvas implementation.
void ResizeCanvas(const gfx::Size& viewport_size) override; void ResizeCanvas(const gfx::Size& viewport_size) override;
sk_sp<SkSurface> GetSurface() override; SkCanvas* GetCanvas() override;
void PresentCanvas(const gfx::Rect& damage) override; void PresentCanvas(const gfx::Rect& damage) override;
std::unique_ptr<gfx::VSyncProvider> CreateVSyncProvider() override; std::unique_ptr<gfx::VSyncProvider> CreateVSyncProvider() override;
......
...@@ -170,7 +170,7 @@ WaylandCanvasSurface::~WaylandCanvasSurface() { ...@@ -170,7 +170,7 @@ WaylandCanvasSurface::~WaylandCanvasSurface() {
buffer_manager_->UnregisterSurface(widget_); buffer_manager_->UnregisterSurface(widget_);
} }
sk_sp<SkSurface> WaylandCanvasSurface::GetSurface() { SkCanvas* WaylandCanvasSurface::GetCanvas() {
DCHECK(!pending_buffer_) DCHECK(!pending_buffer_)
<< "The previous pending buffer has not been presented yet"; << "The previous pending buffer has not been presented yet";
...@@ -197,7 +197,7 @@ sk_sp<SkSurface> WaylandCanvasSurface::GetSurface() { ...@@ -197,7 +197,7 @@ sk_sp<SkSurface> WaylandCanvasSurface::GetSurface() {
DCHECK(pending_buffer_); DCHECK(pending_buffer_);
pending_buffer_->OnUse(); pending_buffer_->OnUse();
return pending_buffer_->sk_surface(); return pending_buffer_->sk_surface()->getCanvas();
} }
void WaylandCanvasSurface::ResizeCanvas(const gfx::Size& viewport_size) { void WaylandCanvasSurface::ResizeCanvas(const gfx::Size& viewport_size) {
......
...@@ -15,6 +15,8 @@ ...@@ -15,6 +15,8 @@
#include "ui/ozone/platform/wayland/gpu/wayland_surface_gpu.h" #include "ui/ozone/platform/wayland/gpu/wayland_surface_gpu.h"
#include "ui/ozone/public/surface_ozone_canvas.h" #include "ui/ozone/public/surface_ozone_canvas.h"
class SkCanvas;
namespace ui { namespace ui {
class WaylandBufferManagerGpu; class WaylandBufferManagerGpu;
...@@ -31,7 +33,7 @@ class WaylandCanvasSurface : public SurfaceOzoneCanvas, ...@@ -31,7 +33,7 @@ class WaylandCanvasSurface : public SurfaceOzoneCanvas,
~WaylandCanvasSurface() override; ~WaylandCanvasSurface() override;
// SurfaceOzoneCanvas // SurfaceOzoneCanvas
sk_sp<SkSurface> GetSurface() override; SkCanvas* GetCanvas() override;
void ResizeCanvas(const gfx::Size& viewport_size) override; void ResizeCanvas(const gfx::Size& viewport_size) override;
void PresentCanvas(const gfx::Rect& damage) override; void PresentCanvas(const gfx::Rect& damage) override;
std::unique_ptr<gfx::VSyncProvider> CreateVSyncProvider() override; std::unique_ptr<gfx::VSyncProvider> CreateVSyncProvider() override;
......
...@@ -132,7 +132,8 @@ TEST_P(WaylandSurfaceFactoryTest, Canvas) { ...@@ -132,7 +132,8 @@ TEST_P(WaylandSurfaceFactoryTest, Canvas) {
ASSERT_TRUE(canvas); ASSERT_TRUE(canvas);
canvas->ResizeCanvas(window_->GetBounds().size()); canvas->ResizeCanvas(window_->GetBounds().size());
canvas->GetSurface(); auto* sk_canvas = canvas->GetCanvas();
DCHECK(sk_canvas);
canvas->PresentCanvas(gfx::Rect(5, 10, 20, 15)); canvas->PresentCanvas(gfx::Rect(5, 10, 20, 15));
// Wait until the mojo calls are done. // Wait until the mojo calls are done.
...@@ -161,9 +162,11 @@ TEST_P(WaylandSurfaceFactoryTest, CanvasResize) { ...@@ -161,9 +162,11 @@ TEST_P(WaylandSurfaceFactoryTest, CanvasResize) {
ASSERT_TRUE(canvas); ASSERT_TRUE(canvas);
canvas->ResizeCanvas(window_->GetBounds().size()); canvas->ResizeCanvas(window_->GetBounds().size());
canvas->GetSurface(); auto* sk_canvas = canvas->GetCanvas();
DCHECK(sk_canvas);
canvas->ResizeCanvas(gfx::Size(100, 50)); canvas->ResizeCanvas(gfx::Size(100, 50));
canvas->GetSurface(); sk_canvas = canvas->GetCanvas();
DCHECK(sk_canvas);
canvas->PresentCanvas(gfx::Rect(0, 0, 100, 50)); canvas->PresentCanvas(gfx::Rect(0, 0, 100, 50));
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
......
...@@ -21,16 +21,12 @@ X11CanvasSurface::X11CanvasSurface( ...@@ -21,16 +21,12 @@ X11CanvasSurface::X11CanvasSurface(
X11CanvasSurface::~X11CanvasSurface() = default; X11CanvasSurface::~X11CanvasSurface() = default;
sk_sp<SkSurface> X11CanvasSurface::GetSurface() { SkCanvas* X11CanvasSurface::GetCanvas() {
DCHECK(surface_); return x11_software_bitmap_presenter_.GetSkCanvas();
return surface_;
} }
void X11CanvasSurface::ResizeCanvas(const gfx::Size& viewport_size) { void X11CanvasSurface::ResizeCanvas(const gfx::Size& viewport_size) {
x11_software_bitmap_presenter_.Resize(viewport_size); x11_software_bitmap_presenter_.Resize(viewport_size);
SkImageInfo info = SkImageInfo::MakeN32(
viewport_size.width(), viewport_size.height(), kOpaque_SkAlphaType);
surface_ = x11_software_bitmap_presenter_.GetSkCanvas()->makeSurface(info);
} }
void X11CanvasSurface::PresentCanvas(const gfx::Rect& damage) { void X11CanvasSurface::PresentCanvas(const gfx::Rect& damage) {
......
...@@ -18,6 +18,8 @@ ...@@ -18,6 +18,8 @@
#include "ui/gfx/native_widget_types.h" #include "ui/gfx/native_widget_types.h"
#include "ui/ozone/public/surface_ozone_canvas.h" #include "ui/ozone/public/surface_ozone_canvas.h"
class SkSurface;
namespace ui { namespace ui {
// The platform-specific part of an software output. The class is intended // The platform-specific part of an software output. The class is intended
...@@ -31,7 +33,7 @@ class X11CanvasSurface : public SurfaceOzoneCanvas { ...@@ -31,7 +33,7 @@ class X11CanvasSurface : public SurfaceOzoneCanvas {
~X11CanvasSurface() override; ~X11CanvasSurface() override;
// SurfaceOzoneCanvas overrides: // SurfaceOzoneCanvas overrides:
sk_sp<SkSurface> GetSurface() override; SkCanvas* GetCanvas() override;
void ResizeCanvas(const gfx::Size& viewport_size) override; void ResizeCanvas(const gfx::Size& viewport_size) override;
void PresentCanvas(const gfx::Rect& damage) override; void PresentCanvas(const gfx::Rect& damage) override;
std::unique_ptr<gfx::VSyncProvider> CreateVSyncProvider() override; std::unique_ptr<gfx::VSyncProvider> CreateVSyncProvider() override;
......
...@@ -11,7 +11,7 @@ ...@@ -11,7 +11,7 @@
#include "base/component_export.h" #include "base/component_export.h"
#include "third_party/skia/include/core/SkRefCnt.h" #include "third_party/skia/include/core/SkRefCnt.h"
class SkSurface; class SkCanvas;
namespace gfx { namespace gfx {
class Rect; class Rect;
...@@ -29,8 +29,11 @@ class COMPONENT_EXPORT(OZONE_BASE) SurfaceOzoneCanvas { ...@@ -29,8 +29,11 @@ class COMPONENT_EXPORT(OZONE_BASE) SurfaceOzoneCanvas {
public: public:
virtual ~SurfaceOzoneCanvas(); virtual ~SurfaceOzoneCanvas();
// Returns an SkSurface for drawing on the window. // Returns an SkCanvas for drawing on the window. The SurfaceOzoneCanvas keeps
virtual sk_sp<SkSurface> GetSurface() = 0; // the SkCanvas alive until the client finishes writing contents and calls
// PresentCanvas. Additionally, the SkCanvas becomes invalid after
// ResizeCanvas is called. See comment at ResizeCanvas.
virtual SkCanvas* GetCanvas() = 0;
// Attempts to resize the canvas to match the viewport size. After // Attempts to resize the canvas to match the viewport size. After
// resizing, the compositor must call GetSurface() to get the next // resizing, the compositor must call GetSurface() to get the next
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment