Skip to content

Commit

Permalink
Revert "Reland "ozone/wayland: call submission callback only after On…
Browse files Browse the repository at this point in the history 
…Submission is received.""

This reverts commit 7d74066.

Reason for revert: suspect for pixel test flakiness - crbug.com/1410002

Original change's description:
> Reland "ozone/wayland: call submission callback only after OnSubmission is received."
>
> This is a reland of commit bc3ec39
>
> Original change's description:
> > ozone/wayland: call submission callback only after OnSubmission is received.
> >
> > Previously, a submission callback was called immediately, which sometimes
> > resulted in WaylandCanvasSurface running out of buffers as the
> > SoftwareOutputDevice was trying to get a new SkCanvas backed by
> > those buffers too early.
> >
> > The fix to the above problem is to store a submission callback
> > in the submitted buffer and call it upon OnSubmission.
> >
> > Bug: 1402208
> > Change-Id: Ie4edae7f2bcf49b9f0b0ad77b593fa4ecd10b900
> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4154635
> > Reviewed-by: Kyle Charbonneau <kylechar@chromium.org>
> > Commit-Queue: Maksim Sisov <msisov@igalia.com>
> > Reviewed-by: Kramer Ge <fangzhoug@chromium.org>
> > Cr-Commit-Position: refs/heads/main@{#1092964}
>
> Cq-Include-Trybots: luci.chromium.try:gpu-fyi-try-lacros-intel-rel
> Bug: 1402208, 1407698
> Change-Id: I49055b6b9ba2729e9522f39409686a76e65831cc
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4173527
> Reviewed-by: Kyle Charbonneau <kylechar@chromium.org>
> Commit-Queue: Maksim Sisov <msisov@igalia.com>
> Reviewed-by: Kramer Ge <fangzhoug@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1096055}

Bug: 1402208, 1407698
Change-Id: I1906477ba70836d1dd950894d74b18718378a6a3
Cq-Include-Trybots: luci.chromium.try:gpu-fyi-try-lacros-intel-rel
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4192630
Commit-Queue: Sunny Sachanandani <sunnyps@chromium.org>
Owners-Override: Sunny Sachanandani <sunnyps@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/main@{#1096472}
  • Loading branch information
@sunnyps
sunnyps authored and Chromium LUCI CQ committed Jan 24, 2023
1 parent c0b17a2 commit 8069e95
Show file tree
Hide file tree
Showing 3 changed files with 71 additions and 406 deletions.
155 changes: 64 additions & 91 deletions ui/ozone/platform/wayland/gpu/wayland_canvas_surface.cc
View file
Expand Up @@ -27,7 +27,7 @@ namespace ui {
namespace {

// The maximum number of buffers we allow to be created.
constexpr size_t kMaxNumberOfBuffers = 3;
constexpr uint32_t kMaxNumbferOfBuffers = 3;

} // namespace

Expand All @@ -39,8 +39,10 @@ size_t CalculateStride(int width) {

class WaylandCanvasSurface::SharedMemoryBuffer {
public:
explicit SharedMemoryBuffer(WaylandBufferManagerGpu* buffer_manager)
SharedMemoryBuffer(gfx::AcceleratedWidget widget,
WaylandBufferManagerGpu* buffer_manager)
: buffer_id_(buffer_manager->AllocateBufferID()),
widget_(widget),
buffer_manager_(buffer_manager) {
DCHECK(buffer_manager_);
}
Expand Down Expand Up @@ -97,6 +99,12 @@ class WaylandCanvasSurface::SharedMemoryBuffer {
dirty_region_.setRect(gfx::RectToSkIRect(gfx::Rect(size)));
}

void CommitBuffer(const gfx::Rect& damage, float buffer_scale) {
buffer_manager_->CommitBuffer(widget_, buffer_id_, /*frame_id*/ buffer_id_,
frame_data_, gfx::Rect(size_),
gfx::RoundedCornersF(), buffer_scale, damage);
}

void OnUse() {
DCHECK(!used_);
used_ = true;
Expand Down Expand Up @@ -135,6 +143,8 @@ class WaylandCanvasSurface::SharedMemoryBuffer {
return pending_damage_region_;
}

void set_frame_data(const gfx::FrameData& data) { frame_data_ = data; }

private:
// The size of the buffer.
gfx::Size size_;
Expand All @@ -145,6 +155,9 @@ class WaylandCanvasSurface::SharedMemoryBuffer {
// Whether this buffer is currently being used.
bool used_ = false;

// The widget this buffer is created for.
const gfx::AcceleratedWidget widget_;

// Non-owned pointer to the buffer manager on the gpu process/thread side.
const raw_ptr<WaylandBufferManagerGpu> buffer_manager_;

Expand All @@ -159,6 +172,9 @@ class WaylandCanvasSurface::SharedMemoryBuffer {

// Pending damage region if the buffer is pending to be submitted.
gfx::Rect pending_damage_region_;

// Frame data.
gfx::FrameData frame_data_;
};

class WaylandCanvasSurface::VSyncProvider : public gfx::VSyncProvider {
Expand Down Expand Up @@ -196,28 +212,6 @@ class WaylandCanvasSurface::VSyncProvider : public gfx::VSyncProvider {
base::WeakPtr<WaylandCanvasSurface> surface_;
};

WaylandCanvasSurface::PendingFrame::PendingFrame(
uint32_t frame_id,
const gfx::Size& surface_size,
SwapBuffersCallback callback,
gfx::FrameData frame_data,
SharedMemoryBuffer* frame_buffer)
: frame_id(frame_id),
surface_size(surface_size),
swap_ack_callback(std::move(callback)),
data(std::move(frame_data)),
frame_buffer(frame_buffer) {
// The frame might be invalid if there is no frame buffer. However, if there
// is a buffer, it must be marked as used.
DCHECK(!frame_buffer || frame_buffer->used());
}

WaylandCanvasSurface::PendingFrame::~PendingFrame() {
if (swap_ack_callback) {
std::move(swap_ack_callback).Run(surface_size);
}
}

WaylandCanvasSurface::WaylandCanvasSurface(
WaylandBufferManagerGpu* buffer_manager,
gfx::AcceleratedWidget widget)
Expand All @@ -241,9 +235,11 @@ SkCanvas* WaylandCanvasSurface::GetCanvas() {
}

if (!pending_buffer_) {
// It must be impossible that the maximum number of buffers that can be
// created is achieved.
DCHECK_LE(buffers_.size(), kMaxNumberOfBuffers);
if (buffers_.size() >= kMaxNumbferOfBuffers) {
// We have achieved the maximum number of buffers we can create. Wait for
// a free buffer.
return nullptr;
}
auto buffer = CreateSharedMemoryBuffer();
pending_buffer_ = buffer.get();
buffers_.push_back(std::move(buffer));
Expand All @@ -263,20 +259,10 @@ void WaylandCanvasSurface::ResizeCanvas(const gfx::Size& viewport_size,
// the new size still fits (but still reallocate if the new size is much
// smaller than the old size).
buffers_.clear();
current_buffer_ = nullptr;
previous_buffer_ = nullptr;
pending_buffer_ = nullptr;

// First clear submitted frame, which will execute the pending swap ack
// callback and only then clear unsubmitted ones. This helps to preserve order
// of swap ack callbacks.
submitted_frame_.reset();

// We must preserve FIFO. Thus, manually destroy pending frames.
for (auto& frame : unsubmitted_frames_) {
frame.reset();
}
unsubmitted_frames_.clear();

unsubmitted_buffers_.clear();
size_ = viewport_size;
viewport_scale_ = scale;
}
Expand All @@ -294,12 +280,16 @@ bool WaylandCanvasSurface::SupportsAsyncBufferSwap() const {

void WaylandCanvasSurface::OnSwapBuffers(SwapBuffersCallback swap_ack_callback,
gfx::FrameData data) {
unsubmitted_frames_.push_back(std::make_unique<PendingFrame>(
next_frame_id(), size_, std::move(swap_ack_callback), std::move(data),
pending_buffer_));
pending_buffer_ = nullptr;
if (pending_buffer_) {
pending_buffer_->set_frame_data(data);
unsubmitted_buffers_.push_back(pending_buffer_);
pending_buffer_ = nullptr;
}

MaybeProcessUnsubmittedFrames();
if (!unsubmitted_buffers_.empty())
ProcessUnsubmittedBuffers();
base::SingleThreadTaskRunner::GetCurrentDefault()->PostTask(
FROM_HERE, base::BindOnce(std::move(swap_ack_callback), size_));
}

std::unique_ptr<gfx::VSyncProvider>
Expand All @@ -312,82 +302,64 @@ bool WaylandCanvasSurface::SupportsOverridePlatformSize() const {
return true;
}

void WaylandCanvasSurface::MaybeProcessUnsubmittedFrames() {
// Don't submit a new frame if there's one already submitted being
void WaylandCanvasSurface::ProcessUnsubmittedBuffers() {
DCHECK(!unsubmitted_buffers_.empty() && unsubmitted_buffers_.front()->used());

// Don't submit a new buffer if there's one already submitted being
// processed.
if (submitted_frame_ || unsubmitted_frames_.empty()) {
if (current_buffer_)
return;
}

auto frame = std::move(unsubmitted_frames_.front());
unsubmitted_frames_.erase(unsubmitted_frames_.begin());
current_buffer_ = std::move(unsubmitted_buffers_.front());
unsubmitted_buffers_.erase(unsubmitted_buffers_.begin());

auto* frame_buffer = frame->frame_buffer.get();
// There was no frame buffer associated with this frame. Skip that and process
// the next frame.
if (!frame_buffer) {
// Reset the frame so that it executes the swap ack callback.
frame.reset();
MaybeProcessUnsubmittedFrames();
return;
}

gfx::Rect damage = frame_buffer->pending_damage_region();
gfx::Rect damage = current_buffer_->pending_damage_region();

// The buffer has been updated. Thus, the |damage| can be subtracted
// from its dirty region.
frame_buffer->UpdateDirtyRegion(damage, SkRegion::kDifference_Op);
current_buffer_->UpdateDirtyRegion(damage, SkRegion::kDifference_Op);

// Make sure the buffer is up-to-date by copying the outdated region from
// the previous buffer.
if (previous_buffer_ && previous_buffer_ != frame_buffer) {
frame_buffer->CopyDirtyRegionFrom(previous_buffer_);
}
if (previous_buffer_ && previous_buffer_ != current_buffer_)
current_buffer_->CopyDirtyRegionFrom(previous_buffer_);

// As long as the current frame's buffer has been updated, add dirty region to
// As long as the |current_buffer_| has been updated, add dirty region to
// other buffers to make sure their regions will be updated with up-to-date
// content.
for (auto& buffer : buffers_) {
if (buffer.get() != frame_buffer) {
if (buffer.get() != current_buffer_)
buffer->UpdateDirtyRegion(damage, SkRegion::kUnion_Op);
}
}

buffer_manager_->CommitBuffer(
widget_, frame->frame_id, frame_buffer->buffer_id(),
std::move(frame->data), gfx::Rect(size_), gfx::RoundedCornersF(),
viewport_scale_, damage);

submitted_frame_ = std::move((frame));
current_buffer_->CommitBuffer(damage, viewport_scale_);
}

void WaylandCanvasSurface::OnSubmission(uint32_t frame_id,
const gfx::SwapResult& swap_result,
gfx::GpuFenceHandle release_fence) {
DCHECK(release_fence.is_null());
// We may get an OnSubmission callback for a frame that was submitted
// before a ResizeCanvas call, which clears all our submitted frame. Check to
// see if we still know about this frame. If we know about this frame
// it must have the same |frame_id| because we only submit new frames when
// |submitted_frame_| is nullptr, and it is only set to nullptr in
// |OnSubmission| and |ResizeCanvas|. In |ResizeCanvas|, |submitted_frame_|
// is cleared so we will not know about |frame_id|. In addition to that, if
// the frame_id is stale, the gpu process may have just recovered from a crash
// so this frame_id can also be ignored.
if (!submitted_frame_ || submitted_frame_->frame_id != frame_id) {
// We may get an OnSubmission callback for a buffer that was submitted
// before a ResizeCanvas call, which clears all our buffers. Check to
// see if we still know about this buffer. If we know about this buffer
// it must be |current_buffer_| because we only submit new buffers when
// |current_buffer_| is nullptr, and it is only set to nullptr in
// |OnSubmission| and |ResizeCanvas|. In |ResizeCanvas|, |buffers_| is cleared
// so we will not know about |frame_id|.
if (!base::Contains(buffers_, frame_id, &SharedMemoryBuffer::buffer_id))
return;
}

DCHECK_EQ(size_, submitted_frame_->surface_size);
DCHECK(current_buffer_);
DCHECK_EQ(current_buffer_->buffer_id(), frame_id);

if (previous_buffer_)
previous_buffer_->OnRelease();

previous_buffer_ = submitted_frame_->frame_buffer;
// The frame will automatically execute the swap ack callback on destruction.
submitted_frame_.reset();
previous_buffer_ = current_buffer_;
current_buffer_ = nullptr;

MaybeProcessUnsubmittedFrames();
if (!unsubmitted_buffers_.empty())
ProcessUnsubmittedBuffers();
}

void WaylandCanvasSurface::OnPresentation(
Expand All @@ -402,7 +374,8 @@ std::unique_ptr<WaylandCanvasSurface::SharedMemoryBuffer>
WaylandCanvasSurface::CreateSharedMemoryBuffer() {
DCHECK(!size_.IsEmpty());

auto canvas_buffer = std::make_unique<SharedMemoryBuffer>(buffer_manager_);
auto canvas_buffer =
std::make_unique<SharedMemoryBuffer>(widget_, buffer_manager_);
canvas_buffer->Initialize(size_);
return canvas_buffer;
}
Expand Down
38 changes: 7 additions & 31 deletions ui/ozone/platform/wayland/gpu/wayland_canvas_surface.h
View file
Expand Up @@ -75,31 +75,7 @@ class WaylandCanvasSurface : public SurfaceOzoneCanvas,
// Internal implementation of gfx::VSyncProvider.
class VSyncProvider;

struct PendingFrame {
PendingFrame(uint32_t frame_id,
const gfx::Size& surface_size,
SwapBuffersCallback callback,
gfx::FrameData frame_data,
SharedMemoryBuffer* frame_buffer);
~PendingFrame();

// Unique identifier of the frame within this AcceleratedWidget.
const uint32_t frame_id;

// Current surface size. This is required for the |swap_ack_callback|.
const gfx::Size surface_size;

SwapBuffersCallback swap_ack_callback;
gfx::FrameData data;

// Buffer associated with this frame. If null, the frame is invalid and
// requires execution of the |swap_ack_callback| as viz may still request
// to swap buffers without calling GetCanvas first. In that case, it skips
// drawing a root render pass and there is nothing to present.
const raw_ptr<SharedMemoryBuffer, DanglingUntriaged> frame_buffer;
};

void MaybeProcessUnsubmittedFrames();
void ProcessUnsubmittedBuffers();

// WaylandSurfaceGpu overrides:
void OnSubmission(uint32_t frame_id,
Expand All @@ -118,17 +94,17 @@ class WaylandCanvasSurface : public SurfaceOzoneCanvas,
float viewport_scale_ = 1.f;
std::vector<std::unique_ptr<SharedMemoryBuffer>> buffers_;

// Contains pending to be submitted frames. The vector is processed as FIFO.
std::vector<std::unique_ptr<PendingFrame>> unsubmitted_frames_;

// Currently submitted frame that waits OnSubmission. Set on OnSwapBuffers and
// release on OnSubmission() call.
std::unique_ptr<PendingFrame> submitted_frame_;
// Contains pending to be submitted buffers. The vector is processed as FIFO.
std::vector<SharedMemoryBuffer*> unsubmitted_buffers_;

// Pending buffer that is to be placed into the |unsubmitted_buffers_| to be
// processed.
raw_ptr<SharedMemoryBuffer, DanglingUntriaged> pending_buffer_ = nullptr;

// Currently used buffer. Set on PresentCanvas() and released on
// OnSubmission() call.
raw_ptr<SharedMemoryBuffer, DanglingUntriaged> current_buffer_ = nullptr;

// Previously used buffer. Set on OnSubmission().
raw_ptr<SharedMemoryBuffer, DanglingUntriaged> previous_buffer_ = nullptr;

Expand Down

0 comments on commit 8069e95

Please sign in to comment.