Skip to content

Commit

Permalink
gpu: Remove gpu's dependency on ui/latency_info.
Browse files Browse the repository at this point in the history
Add a gfx::SwapResponse that contains the swap result and the timing
information for every swap.

Let the client add those timestamps to cached LatencyInfos rather
than passing LatencyInfo to the GPU service to modify and send back.

Behaviorally, this patch is a no-op, but the intent is to:
1) Simplify later patches that will add timestamps to the SwapResponse.
2) Step towards removing PassThroughImageTransportSurface.
3) Remove gpu's dependency on ui/latency_info, which was a layering smell.

Bug: 769415
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: I9e1ac97d32600d13ee05467d70fad5674e1355f0
Reviewed-on: https://chromium-review.googlesource.com/767572
Commit-Queue: Brian Anderson <brianderson@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Reviewed-by: danakj <danakj@chromium.org>
Reviewed-by: Kenneth Russell <kbr@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#517614}
  • Loading branch information
briandersn authored and Commit Bot committed Nov 17, 2017
1 parent c3f2c90 commit feaa47c
Show file tree
Hide file tree
Showing 52 changed files with 374 additions and 322 deletions.
3 changes: 1 addition & 2 deletions cc/test/test_context_support.cc
Original file line number Diff line number Diff line change
Expand Up @@ -94,8 +94,7 @@ uint64_t TestContextSupport::ShareGroupTracingGUID() const {
void TestContextSupport::SetErrorMessageCallback(
const base::Callback<void(const char*, int32_t)>& callback) {}

void TestContextSupport::AddLatencyInfo(
const std::vector<ui::LatencyInfo>& latency_info) {}
void TestContextSupport::SetSnapshotRequested() {}

bool TestContextSupport::ThreadSafeShallowLockDiscardableTexture(
uint32_t texture_id) {
Expand Down
3 changes: 1 addition & 2 deletions cc/test/test_context_support.h
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,7 @@ class TestContextSupport : public gpu::ContextSupport {
uint64_t ShareGroupTracingGUID() const override;
void SetErrorMessageCallback(
const base::Callback<void(const char*, int32_t)>& callback) override;
void AddLatencyInfo(
const std::vector<ui::LatencyInfo>& latency_info) override;
void SetSnapshotRequested() override;
bool ThreadSafeShallowLockDiscardableTexture(uint32_t texture_id) override;
void CompleteLockDiscardableTexureOnContextThread(
uint32_t texture_id) override;
Expand Down
61 changes: 61 additions & 0 deletions components/viz/service/display/output_surface.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include "gpu/GLES2/gl2extchromium.h"
#include "gpu/command_buffer/client/context_support.h"
#include "gpu/command_buffer/client/gles2_interface.h"
#include "ui/gfx/swap_result.h"

namespace viz {

Expand All @@ -38,4 +39,64 @@ OutputSurface::OutputSurface(

OutputSurface::~OutputSurface() = default;

OutputSurface::LatencyInfoCache::SwapInfo::SwapInfo(
uint64_t id,
std::vector<ui::LatencyInfo> info)
: swap_id(id), latency_info(std::move(info)) {}

OutputSurface::LatencyInfoCache::SwapInfo::SwapInfo(SwapInfo&& src) = default;

OutputSurface::LatencyInfoCache::SwapInfo&
OutputSurface::LatencyInfoCache::SwapInfo::operator=(SwapInfo&& src) = default;

OutputSurface::LatencyInfoCache::SwapInfo::~SwapInfo() = default;

OutputSurface::LatencyInfoCache::LatencyInfoCache(Client* client)
: client_(client) {
DCHECK(client);
}

OutputSurface::LatencyInfoCache::~LatencyInfoCache() = default;

bool OutputSurface::LatencyInfoCache::WillSwap(
std::vector<ui::LatencyInfo> latency_info) {
bool snapshot_requested = false;
for (const auto& latency : latency_info) {
if (latency.FindLatency(ui::BROWSER_SNAPSHOT_FRAME_NUMBER_COMPONENT,
nullptr)) {
snapshot_requested = true;
break;
}
}

// Don't grow unbounded in case of error.
while (swap_infos_.size() >= kCacheCountMax) {
client_->LatencyInfoCompleted(swap_infos_.front().latency_info);
swap_infos_.pop_front();
}
swap_infos_.emplace_back(swap_id_++, std::move(latency_info));

return snapshot_requested;
}

void OutputSurface::LatencyInfoCache::OnSwapBuffersCompleted(
const gfx::SwapResponse& response) {
auto it = std::find_if(
swap_infos_.begin(), swap_infos_.end(),
[&](const SwapInfo& si) { return si.swap_id == response.swap_id; });

if (it != swap_infos_.end()) {
for (auto& latency : it->latency_info) {
latency.AddLatencyNumberWithTimestamp(
ui::INPUT_EVENT_GPU_SWAP_BUFFER_COMPONENT, 0, 0, response.swap_start,
1);
latency.AddLatencyNumberWithTimestamp(
ui::INPUT_EVENT_LATENCY_TERMINATED_FRAME_SWAP_COMPONENT, 0, 0,
response.swap_end, 1);
}
client_->LatencyInfoCompleted(it->latency_info);
swap_infos_.erase(it);
}
}

} // namespace viz
47 changes: 47 additions & 0 deletions components/viz/service/display/output_surface.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

#include <memory>

#include "base/containers/circular_deque.h"
#include "base/macros.h"
#include "base/memory/ref_counted.h"
#include "base/threading/thread_checker.h"
Expand All @@ -18,10 +19,12 @@
#include "components/viz/service/viz_service_export.h"
#include "gpu/command_buffer/common/texture_in_use_response.h"
#include "ui/gfx/color_space.h"
#include "ui/latency/latency_info.h"

namespace gfx {
class ColorSpace;
class Size;
struct SwapResponse;
} // namespace gfx

namespace viz {
Expand Down Expand Up @@ -119,6 +122,50 @@ class VIZ_SERVICE_EXPORT OutputSurface {
// after returning from this method in order to unblock the next frame.
virtual void SwapBuffers(OutputSurfaceFrame frame) = 0;

// A helper class for implementations of OutputSurface that want to cache
// LatencyInfos that can be updated when we get the corresponding
// gfx::SwapResponse.
class VIZ_SERVICE_EXPORT LatencyInfoCache {
public:
class Client {
public:
virtual ~Client() = default;
virtual void LatencyInfoCompleted(
const std::vector<ui::LatencyInfo>& latency_info) = 0;
};

explicit LatencyInfoCache(Client* client);
~LatencyInfoCache();

// Returns true if there's a snapshot request.
bool WillSwap(std::vector<ui::LatencyInfo> latency_info);
void OnSwapBuffersCompleted(const gfx::SwapResponse& response);

private:
struct SwapInfo {
SwapInfo(uint64_t id, std::vector<ui::LatencyInfo> info);
SwapInfo(SwapInfo&& src);
SwapInfo& operator=(SwapInfo&& src);
~SwapInfo();
uint64_t swap_id;
std::vector<ui::LatencyInfo> latency_info;
DISALLOW_COPY_AND_ASSIGN(SwapInfo);
};

Client* client_ = nullptr;

// Incremented in sync with the ImageTransportSurface's swap_id_.
uint64_t swap_id_ = 0;
base::circular_deque<SwapInfo> swap_infos_;

// We only expect a couple swap acks outstanding, but there are cases where
// we will get timestamps for swaps from several frames ago when using
// platform extensions like eglGetFrameTimestampsANDROID.
static constexpr size_t kCacheCountMax = 10;

DISALLOW_COPY_AND_ASSIGN(LatencyInfoCache);
};

protected:
struct OutputSurface::Capabilities capabilities_;
scoped_refptr<ContextProvider> context_provider_;
Expand Down
18 changes: 11 additions & 7 deletions components/viz/service/display_embedder/display_output_surface.cc
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ DisplayOutputSurface::DisplayOutputSurface(
: OutputSurface(context_provider),
synthetic_begin_frame_source_(synthetic_begin_frame_source),
latency_tracker_(true),
latency_info_cache_(this),
weak_ptr_factory_(this) {
capabilities_.flipped_output_surface =
context_provider->ContextCapabilities().flips_vertically;
Expand Down Expand Up @@ -83,8 +84,8 @@ void DisplayOutputSurface::Reshape(const gfx::Size& size,
void DisplayOutputSurface::SwapBuffers(OutputSurfaceFrame frame) {
DCHECK(context_provider_);

if (frame.latency_info.size() > 0)
context_provider_->ContextSupport()->AddLatencyInfo(frame.latency_info);
if (latency_info_cache_.WillSwap(std::move(frame.latency_info)))
context_provider_->ContextSupport()->SetSnapshotRequested();

set_draw_rectangle_for_frame_ = false;
if (frame.sub_buffer_rect) {
Expand Down Expand Up @@ -134,14 +135,17 @@ void DisplayOutputSurface::DidReceiveSwapBuffersAck(gfx::SwapResult result) {
}

void DisplayOutputSurface::OnGpuSwapBuffersCompleted(
const std::vector<ui::LatencyInfo>& latency_info,
gfx::SwapResult result,
const gfx::SwapResponse& response,
const gpu::GpuProcessHostedCALayerTreeParamsMac* params_mac) {
DidReceiveSwapBuffersAck(response.result);
latency_info_cache_.OnSwapBuffersCompleted(response);
}

void DisplayOutputSurface::LatencyInfoCompleted(
const std::vector<ui::LatencyInfo>& latency_info) {
for (const auto& latency : latency_info) {
if (latency.latency_components().size() > 0)
latency_tracker_.OnGpuSwapBuffersCompleted(latency);
latency_tracker_.OnGpuSwapBuffersCompleted(latency);
}
DidReceiveSwapBuffersAck(result);
}

void DisplayOutputSurface::OnVSyncParametersUpdated(base::TimeTicks timebase,
Expand Down
11 changes: 8 additions & 3 deletions components/viz/service/display_embedder/display_output_surface.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ class SyntheticBeginFrameSource;

// An OutputSurface implementation that directly draws and
// swaps to an actual GL surface.
class DisplayOutputSurface : public OutputSurface {
class DisplayOutputSurface : public OutputSurface,
public OutputSurface::LatencyInfoCache::Client {
public:
DisplayOutputSurface(scoped_refptr<InProcessContextProvider> context_provider,
SyntheticBeginFrameSource* synthetic_begin_frame_source);
Expand All @@ -44,6 +45,10 @@ class DisplayOutputSurface : public OutputSurface {
bool HasExternalStencilTest() const override;
void ApplyExternalStencil() override;

// OutputSurface::LatencyInfoCache::Client implementation.
void LatencyInfoCompleted(
const std::vector<ui::LatencyInfo>& latency_info) override;

protected:
OutputSurfaceClient* client() const { return client_; }

Expand All @@ -53,8 +58,7 @@ class DisplayOutputSurface : public OutputSurface {
private:
// Called when a swap completion is signaled from ImageTransportSurface.
void OnGpuSwapBuffersCompleted(
const std::vector<ui::LatencyInfo>& latency_info,
gfx::SwapResult result,
const gfx::SwapResponse& response,
const gpu::GpuProcessHostedCALayerTreeParamsMac* params_mac);
void OnVSyncParametersUpdated(base::TimeTicks timebase,
base::TimeDelta interval);
Expand All @@ -67,6 +71,7 @@ class DisplayOutputSurface : public OutputSurface {
// True if the draw rectangle has been set at all since the last resize.
bool has_set_draw_rectangle_since_last_resize_ = false;
gfx::Size size_;
LatencyInfoCache latency_info_cache_;

base::WeakPtrFactory<DisplayOutputSurface> weak_ptr_factory_;
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ GpuBrowserCompositorOutputSurface::GpuBrowserCompositorOutputSurface(
overlay_candidate_validator)
: BrowserCompositorOutputSurface(std::move(context),
update_vsync_parameters_callback,
std::move(overlay_candidate_validator)) {
std::move(overlay_candidate_validator)),
latency_info_cache_(this) {
if (capabilities_.uses_default_gl_framebuffer) {
capabilities_.flipped_output_surface =
context_provider()->ContextCapabilities().flips_vertically;
Expand All @@ -55,11 +56,15 @@ void GpuBrowserCompositorOutputSurface::SetNeedsVSync(bool needs_vsync) {
}

void GpuBrowserCompositorOutputSurface::OnGpuSwapBuffersCompleted(
const std::vector<ui::LatencyInfo>& latency_info,
gfx::SwapResult result,
const gfx::SwapResponse& response,
const gpu::GpuProcessHostedCALayerTreeParamsMac* params_mac) {
RenderWidgetHostImpl::OnGpuSwapBuffersCompleted(latency_info);
client_->DidReceiveSwapBuffersAck();
latency_info_cache_.OnSwapBuffersCompleted(response);
}

void GpuBrowserCompositorOutputSurface::LatencyInfoCompleted(
const std::vector<ui::LatencyInfo>& latency_info) {
RenderWidgetHostImpl::OnGpuSwapBuffersCompleted(latency_info);
}

void GpuBrowserCompositorOutputSurface::OnReflectorChanged() {
Expand Down Expand Up @@ -113,7 +118,8 @@ void GpuBrowserCompositorOutputSurface::Reshape(

void GpuBrowserCompositorOutputSurface::SwapBuffers(
viz::OutputSurfaceFrame frame) {
GetCommandBufferProxy()->AddLatencyInfo(frame.latency_info);
if (latency_info_cache_.WillSwap(std::move(frame.latency_info)))
GetCommandBufferProxy()->SetSnapshotRequested();

gfx::Size surface_size = frame.size;
if (reflector_) {
Expand Down
15 changes: 10 additions & 5 deletions content/browser/compositor/gpu_browser_compositor_output_surface.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ struct GpuProcessHostedCALayerTreeParamsMac;

namespace ui {
class ContextProviderCommandBuffer;
class LatencyInfo;
}

namespace content {
Expand All @@ -33,8 +32,10 @@ class ReflectorTexture;
// Adapts a WebGraphicsContext3DCommandBufferImpl into a
// viz::OutputSurface that also handles vsync parameter updates
// arriving from the GPU process.
class GpuBrowserCompositorOutputSurface : public BrowserCompositorOutputSurface,
public GpuVSyncControl {
class GpuBrowserCompositorOutputSurface
: public BrowserCompositorOutputSurface,
public GpuVSyncControl,
public viz::OutputSurface::LatencyInfoCache::Client {
public:
GpuBrowserCompositorOutputSurface(
scoped_refptr<ui::ContextProviderCommandBuffer> context,
Expand All @@ -50,8 +51,7 @@ class GpuBrowserCompositorOutputSurface : public BrowserCompositorOutputSurface,
// TODO(ccameron): Remove |params_mac| when the CALayer tree is hosted in the
// browser process.
virtual void OnGpuSwapBuffersCompleted(
const std::vector<ui::LatencyInfo>& latency_info,
gfx::SwapResult result,
const gfx::SwapResponse& response,
const gpu::GpuProcessHostedCALayerTreeParamsMac* params_mac);

// BrowserCompositorOutputSurface implementation.
Expand Down Expand Up @@ -82,6 +82,10 @@ class GpuBrowserCompositorOutputSurface : public BrowserCompositorOutputSurface,
// GpuVSyncControl implementation.
void SetNeedsVSync(bool needs_vsync) override;

// OutputSurface::LatencyInfoCache::Client implementation.
void LatencyInfoCompleted(
const std::vector<ui::LatencyInfo>& latency_info) override;

protected:
void OnVSyncParametersUpdated(base::TimeTicks timebase,
base::TimeDelta interval);
Expand All @@ -94,6 +98,7 @@ class GpuBrowserCompositorOutputSurface : public BrowserCompositorOutputSurface,
// True if the draw rectangle has been set at all since the last resize.
bool has_set_draw_rectangle_since_last_resize_ = false;
gfx::Size size_;
LatencyInfoCache latency_info_cache_;

private:
DISALLOW_COPY_AND_ASSIGN(GpuBrowserCompositorOutputSurface);
Expand Down
3 changes: 1 addition & 2 deletions content/browser/compositor/gpu_output_surface_mac.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,7 @@ class GpuOutputSurfaceMac

// BrowserCompositorOutputSurface implementation.
void OnGpuSwapBuffersCompleted(
const std::vector<ui::LatencyInfo>& latency_info,
gfx::SwapResult result,
const gfx::SwapResponse& response,
const gpu::GpuProcessHostedCALayerTreeParamsMac* params_mac) override;
void SetSurfaceSuspendedForRecycle(bool suspended) override;

Expand Down
5 changes: 2 additions & 3 deletions content/browser/compositor/gpu_output_surface_mac.mm
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,7 @@ void UpdateLayers(CAContextID content_ca_context_id,
}

void GpuOutputSurfaceMac::OnGpuSwapBuffersCompleted(
const std::vector<ui::LatencyInfo>& latency_info,
gfx::SwapResult result,
const gfx::SwapResponse& response,
const gpu::GpuProcessHostedCALayerTreeParamsMac* params_mac) {
remote_layers_->UpdateLayers(params_mac->ca_context_id,
params_mac->fullscreen_low_power_ca_context_id);
Expand All @@ -106,7 +105,7 @@ void UpdateLayers(CAContextID content_ca_context_id,
}
client_->DidReceiveTextureInUseResponses(params_mac->responses);
GpuSurfacelessBrowserCompositorOutputSurface::OnGpuSwapBuffersCompleted(
latency_info, result, params_mac);
response, params_mac);
}

void GpuOutputSurfaceMac::SetSurfaceSuspendedForRecycle(bool suspended) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,20 +111,20 @@ void GpuSurfacelessBrowserCompositorOutputSurface::Reshape(
}

void GpuSurfacelessBrowserCompositorOutputSurface::OnGpuSwapBuffersCompleted(
const std::vector<ui::LatencyInfo>& latency_info,
gfx::SwapResult result,
const gfx::SwapResponse& response,
const gpu::GpuProcessHostedCALayerTreeParamsMac* params_mac) {
gfx::SwapResponse modified_response(response);
bool force_swap = false;
if (result == gfx::SwapResult::SWAP_NAK_RECREATE_BUFFERS) {
if (response.result == gfx::SwapResult::SWAP_NAK_RECREATE_BUFFERS) {
// Even through the swap failed, this is a fixable error so we can pretend
// it succeeded to the rest of the system.
result = gfx::SwapResult::SWAP_ACK;
modified_response.result = gfx::SwapResult::SWAP_ACK;
buffer_queue_->RecreateBuffers();
force_swap = true;
}
buffer_queue_->PageFlipComplete();
GpuBrowserCompositorOutputSurface::OnGpuSwapBuffersCompleted(
latency_info, result, params_mac);
modified_response, params_mac);
if (force_swap)
client_->SetNeedsRedrawRect(gfx::Rect(swap_size_));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,7 @@ class GpuSurfacelessBrowserCompositorOutputSurface

// BrowserCompositorOutputSurface implementation.
void OnGpuSwapBuffersCompleted(
const std::vector<ui::LatencyInfo>& latency_info,
gfx::SwapResult result,
const gfx::SwapResponse& response,
const gpu::GpuProcessHostedCALayerTreeParamsMac* params_mac) override;

private:
Expand Down
Loading

0 comments on commit feaa47c

Please sign in to comment.