From eb8b9153c50aaf8f547bf879681a9e3c46ec983f Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Wed, 18 Oct 2023 15:56:08 -0700 Subject: [PATCH 01/10] ++ --- impeller/renderer/backend/gles/BUILD.gn | 3 + .../renderer/backend/gles/context_gles.cc | 2 + impeller/renderer/backend/gles/context_gles.h | 6 ++ .../renderer/backend/gles/gpu_tracer_gles.cc | 85 +++++++++++++++++++ .../renderer/backend/gles/gpu_tracer_gles.h | 34 ++++++++ .../renderer/backend/gles/proc_table_gles.h | 11 ++- .../renderer/backend/gles/render_pass_gles.cc | 16 +++- .../renderer/backend/metal/gpu_tracer_mtl.h | 11 +-- 8 files changed, 157 insertions(+), 11 deletions(-) create mode 100644 impeller/renderer/backend/gles/gpu_tracer_gles.cc create mode 100644 impeller/renderer/backend/gles/gpu_tracer_gles.h diff --git a/impeller/renderer/backend/gles/BUILD.gn b/impeller/renderer/backend/gles/BUILD.gn index 1d783c0f8c841..e99902f7d48d8 100644 --- a/impeller/renderer/backend/gles/BUILD.gn +++ b/impeller/renderer/backend/gles/BUILD.gn @@ -75,6 +75,9 @@ impeller_component("gles") { "surface_gles.h", "texture_gles.cc", "texture_gles.h", + "context_gles.h", + "gpu_tracer_gles.cc", + "gpu_tracer_gles.h", ] if (!is_android && !is_fuchsia) { diff --git a/impeller/renderer/backend/gles/context_gles.cc b/impeller/renderer/backend/gles/context_gles.cc index 6896d6609777e..37dde418c5a92 100644 --- a/impeller/renderer/backend/gles/context_gles.cc +++ b/impeller/renderer/backend/gles/context_gles.cc @@ -7,6 +7,7 @@ #include "impeller/base/config.h" #include "impeller/base/validation.h" #include "impeller/renderer/backend/gles/command_buffer_gles.h" +#include "impeller/renderer/backend/gles/gpu_tracer_gles.h" namespace impeller { @@ -62,6 +63,7 @@ ContextGLES::ContextGLES(std::unique_ptr gl, device_capabilities_->SupportsDecalSamplerAddressMode())); } + gpu_tracer_ = std::make_shared(reactor_->GetProcTable()); is_valid_ = true; } diff --git a/impeller/renderer/backend/gles/context_gles.h b/impeller/renderer/backend/gles/context_gles.h index 56da6c55a8092..fa8b89962cadd 100644 --- a/impeller/renderer/backend/gles/context_gles.h +++ b/impeller/renderer/backend/gles/context_gles.h @@ -8,6 +8,7 @@ #include "impeller/base/backend_cast.h" #include "impeller/renderer/backend/gles/allocator_gles.h" #include "impeller/renderer/backend/gles/capabilities_gles.h" +#include "impeller/renderer/backend/gles/gpu_tracer_gles.h" #include "impeller/renderer/backend/gles/pipeline_library_gles.h" #include "impeller/renderer/backend/gles/reactor_gles.h" #include "impeller/renderer/backend/gles/sampler_library_gles.h" @@ -38,12 +39,17 @@ class ContextGLES final : public Context, bool RemoveReactorWorker(ReactorGLES::WorkerID id); + std::shared_ptr GetGPUTracer() const { + return gpu_tracer_; + } + private: ReactorGLES::Ref reactor_; std::shared_ptr shader_library_; std::shared_ptr pipeline_library_; std::shared_ptr sampler_library_; std::shared_ptr resource_allocator_; + std::shared_ptr gpu_tracer_; // Note: This is stored separately from the ProcTableGLES CapabilitiesGLES // in order to satisfy the Context::GetCapabilities signature which returns // a reference. diff --git a/impeller/renderer/backend/gles/gpu_tracer_gles.cc b/impeller/renderer/backend/gles/gpu_tracer_gles.cc new file mode 100644 index 0000000000000..cfb2e4b1a203c --- /dev/null +++ b/impeller/renderer/backend/gles/gpu_tracer_gles.cc @@ -0,0 +1,85 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "impeller/renderer/backend/gles/gpu_tracer_gles.h" +#include "GLES2/gl2ext.h" +#include "impeller/renderer/backend/gles/context_gles.h" + +namespace impeller { + +static const constexpr char* kAngleTimerQuery = "GL_ANGLE_timer_query"; +static const constexpr char* kExtTimerQuery = "GL_EXT_timer_query"; +static const constexpr char* kExtDisjointTimerQuery = + "GL_EXT_disjoint_timer_query"; + +GPUTracerGLES::GPUTracerGLES(const ProcTableGLES& gl) { +#ifdef IMPELLER_DEBUG + auto desc = gl.GetDescription(); + enabled_ = desc->HasExtension(kAngleTimerQuery) || + desc->HasExtension(kExtTimerQuery) || + desc->HasExtension(kExtDisjointTimerQuery); +#endif // IMPELLER_DEBUG +} + +void GPUTracerGLES::MarkFrameStart(const ProcTableGLES& gl) { + if (!enabled_) { + return; + } + has_started_frame_ = true; + if (queries_.empty()) { + queries_ = std::vector(32); + gl.GenQueriesEXT(32, queries_.data()); + } + + // At the beginning of a frame, check the status of all pending + // previous queries. + if (!pending_traces_.empty()) { + std::vector pending_traces = pending_traces_; + pending_traces_.clear(); + + for (auto query : pending_traces) { + // First check if the query is complete without blocking + // on the result. Incomplete results are left in the pending + // trace vector and will not be checked again for another + // frame. + uint available = 0; + FML_LOG(ERROR) << "Checking: " << query; + gl.GetQueryObjectuivEXT(query, GL_QUERY_RESULT_AVAILABLE_EXT, &available); + + if (available) { + // Return the timer resolution in nanoseconds. + uint64_t duration = 0; + gl.GetQueryObjectui64vEXT(query, GL_QUERY_RESULT, &duration); + auto gpu_ms = duration / 1000000.0; + FML_LOG(ERROR) << "gpu_ms: " << gpu_ms; + } else { + pending_traces_.push_back(query); + } + } + } + + // Allocate a single query object for the start and end of the frame. + FML_CHECK(!active_frame_.has_value()); + uint32_t query = queries_.back(); + queries_.pop_back(); + FML_CHECK(query != 0); + active_frame_ = query; + FML_LOG(ERROR) << "query: " << query; + gl.BeginQueryEXT(GL_TIME_ELAPSED_EXT, query); +} + +void GPUTracerGLES::MarkFrameEnd(const ProcTableGLES& gl) { + if (!enabled_ || !active_frame_.has_value()) { + return; + } + auto query = active_frame_.value(); + FML_LOG(ERROR) << "END " << query; + gl.EndQueryEXT(GL_TIME_ELAPSED_EXT); + + pending_traces_.push_back(query); + active_frame_ = std::nullopt; + has_started_frame_ = false; +} + +} // namespace impeller diff --git a/impeller/renderer/backend/gles/gpu_tracer_gles.h b/impeller/renderer/backend/gles/gpu_tracer_gles.h new file mode 100644 index 0000000000000..b3019c0672650 --- /dev/null +++ b/impeller/renderer/backend/gles/gpu_tracer_gles.h @@ -0,0 +1,34 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#pragma once + +#include "impeller/renderer/backend/gles/proc_table_gles.h" + +namespace impeller { + +class GPUTracerGLES { + public: + explicit GPUTracerGLES(const ProcTableGLES& gl); + + ~GPUTracerGLES() = default; + + void MarkFrameStart(const ProcTableGLES& gl); + + void MarkFrameEnd(const ProcTableGLES& gl); + + bool HasStartedFrame() const { + return has_started_frame_; + } + + private: + std::vector queries_; + std::vector pending_traces_; + std::optional active_frame_ = std::nullopt; + bool has_started_frame_ = false; + + bool enabled_ = false; +}; + +} // namespace impeller diff --git a/impeller/renderer/backend/gles/proc_table_gles.h b/impeller/renderer/backend/gles/proc_table_gles.h index bbf94a23ea985..26191f03d6e6f 100644 --- a/impeller/renderer/backend/gles/proc_table_gles.h +++ b/impeller/renderer/backend/gles/proc_table_gles.h @@ -196,7 +196,16 @@ struct GLProc { PROC(PushDebugGroupKHR); \ PROC(PopDebugGroupKHR); \ PROC(ObjectLabelKHR); \ - PROC(RenderbufferStorageMultisampleEXT); + PROC(RenderbufferStorageMultisampleEXT); \ + PROC(GenQueriesEXT); \ + PROC(DeleteQueriesEXT); \ + PROC(IsQueryEXT); \ + PROC(QueryCounterEXT); \ + PROC(GetQueryObjectui64vEXT); \ + PROC(BeginQueryEXT); \ + PROC(EndQueryEXT); \ + PROC(GetQueryObjectuivEXT); \ + PROC(GetQueryObjectivEXT); enum class DebugResourceType { kTexture, diff --git a/impeller/renderer/backend/gles/render_pass_gles.cc b/impeller/renderer/backend/gles/render_pass_gles.cc index 4816a2973b0da..531871f5fc4fc 100644 --- a/impeller/renderer/backend/gles/render_pass_gles.cc +++ b/impeller/renderer/backend/gles/render_pass_gles.cc @@ -7,8 +7,10 @@ #include "flutter/fml/trace_event.h" #include "fml/closure.h" #include "impeller/base/validation.h" +#include "impeller/renderer/backend/gles/context_gles.h" #include "impeller/renderer/backend/gles/device_buffer_gles.h" #include "impeller/renderer/backend/gles/formats_gles.h" +#include "impeller/renderer/backend/gles/gpu_tracer_gles.h" #include "impeller/renderer/backend/gles/pipeline_gles.h" #include "impeller/renderer/backend/gles/texture_gles.h" @@ -141,7 +143,8 @@ struct RenderPassData { const RenderPassData& pass_data, const std::shared_ptr& transients_allocator, const ReactorGLES& reactor, - const std::vector& commands) { + const std::vector& commands, + const std::shared_ptr& tracer) { TRACE_EVENT0("impeller", "RenderPassGLES::EncodeCommandsInReactor"); if (commands.empty()) { @@ -149,6 +152,9 @@ struct RenderPassData { } const auto& gl = reactor.GetProcTable(); + if (!tracer->HasStartedFrame()) { + tracer->MarkFrameStart(gl); + } fml::ScopedCleanupClosure pop_pass_debug_marker( [&gl]() { gl.PopDebugGroup(); }); @@ -492,6 +498,9 @@ struct RenderPassData { attachments.data() // size ); } + if (is_default_fbo) { + tracer->MarkFrameEnd(gl); + } return true; } @@ -549,12 +558,13 @@ bool RenderPassGLES::OnEncodeCommands(const Context& context) const { } std::shared_ptr shared_this = shared_from_this(); + auto tracer = ContextGLES::Cast(context).GetGPUTracer(); return reactor_->AddOperation([pass_data, allocator = context.GetResourceAllocator(), - render_pass = std::move(shared_this)]( + render_pass = std::move(shared_this), tracer]( const auto& reactor) { auto result = EncodeCommandsInReactor(*pass_data, allocator, reactor, - render_pass->commands_); + render_pass->commands_, tracer); FML_CHECK(result) << "Must be able to encode GL commands without error."; }); } diff --git a/impeller/renderer/backend/metal/gpu_tracer_mtl.h b/impeller/renderer/backend/metal/gpu_tracer_mtl.h index bfa227ed10e81..ac2c3916d0207 100644 --- a/impeller/renderer/backend/metal/gpu_tracer_mtl.h +++ b/impeller/renderer/backend/metal/gpu_tracer_mtl.h @@ -17,9 +17,8 @@ namespace impeller { class ContextMTL; /// @brief Approximate the GPU frame time by computing a difference between the -/// smallest -/// GPUStartTime and largest GPUEndTime for all cmd buffers submitted in -/// a frame workload. +/// smallest GPUStartTime and largest GPUEndTime for all command buffers +/// submitted in a frame workload. class GPUTracerMTL : public std::enable_shared_from_this { public: GPUTracerMTL() = default; @@ -27,13 +26,11 @@ class GPUTracerMTL : public std::enable_shared_from_this { ~GPUTracerMTL() = default; /// @brief Record that the current frame has ended. Any additional cmd buffers - /// will be - /// attributed to the "next" frame. + /// will be attributed to the "next" frame. void MarkFrameEnd(); /// @brief Record the current cmd buffer GPU execution timestamps into an - /// aggregate - /// frame workload metric. + /// aggregate frame workload metric. void RecordCmdBuffer(id buffer); private: From c46d3eb8e3ec2bc37a3f7276d730704c4c9fcff2 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Wed, 18 Oct 2023 16:08:31 -0700 Subject: [PATCH 02/10] ++ --- impeller/renderer/backend/gles/BUILD.gn | 6 +++--- impeller/renderer/backend/gles/context_gles.h | 4 +--- impeller/renderer/backend/gles/gpu_tracer_gles.h | 4 +--- impeller/renderer/backend/gles/render_pass_gles.cc | 4 ++-- 4 files changed, 7 insertions(+), 11 deletions(-) diff --git a/impeller/renderer/backend/gles/BUILD.gn b/impeller/renderer/backend/gles/BUILD.gn index e99902f7d48d8..b7cad3acc50db 100644 --- a/impeller/renderer/backend/gles/BUILD.gn +++ b/impeller/renderer/backend/gles/BUILD.gn @@ -44,6 +44,7 @@ impeller_component("gles") { "command_buffer_gles.h", "context_gles.cc", "context_gles.h", + "context_gles.h", "description_gles.cc", "description_gles.h", "device_buffer_gles.cc", @@ -51,6 +52,8 @@ impeller_component("gles") { "formats_gles.cc", "formats_gles.h", "gles.h", + "gpu_tracer_gles.cc", + "gpu_tracer_gles.h", "handle_gles.cc", "handle_gles.h", "pipeline_gles.cc", @@ -75,9 +78,6 @@ impeller_component("gles") { "surface_gles.h", "texture_gles.cc", "texture_gles.h", - "context_gles.h", - "gpu_tracer_gles.cc", - "gpu_tracer_gles.h", ] if (!is_android && !is_fuchsia) { diff --git a/impeller/renderer/backend/gles/context_gles.h b/impeller/renderer/backend/gles/context_gles.h index fa8b89962cadd..40c63d92b1aa0 100644 --- a/impeller/renderer/backend/gles/context_gles.h +++ b/impeller/renderer/backend/gles/context_gles.h @@ -39,9 +39,7 @@ class ContextGLES final : public Context, bool RemoveReactorWorker(ReactorGLES::WorkerID id); - std::shared_ptr GetGPUTracer() const { - return gpu_tracer_; - } + std::shared_ptr GetGPUTracer() const { return gpu_tracer_; } private: ReactorGLES::Ref reactor_; diff --git a/impeller/renderer/backend/gles/gpu_tracer_gles.h b/impeller/renderer/backend/gles/gpu_tracer_gles.h index b3019c0672650..f6ef995124186 100644 --- a/impeller/renderer/backend/gles/gpu_tracer_gles.h +++ b/impeller/renderer/backend/gles/gpu_tracer_gles.h @@ -18,9 +18,7 @@ class GPUTracerGLES { void MarkFrameEnd(const ProcTableGLES& gl); - bool HasStartedFrame() const { - return has_started_frame_; - } + bool HasStartedFrame() const { return has_started_frame_; } private: std::vector queries_; diff --git a/impeller/renderer/backend/gles/render_pass_gles.cc b/impeller/renderer/backend/gles/render_pass_gles.cc index 531871f5fc4fc..a4e811a9f6084 100644 --- a/impeller/renderer/backend/gles/render_pass_gles.cc +++ b/impeller/renderer/backend/gles/render_pass_gles.cc @@ -561,8 +561,8 @@ bool RenderPassGLES::OnEncodeCommands(const Context& context) const { auto tracer = ContextGLES::Cast(context).GetGPUTracer(); return reactor_->AddOperation([pass_data, allocator = context.GetResourceAllocator(), - render_pass = std::move(shared_this), tracer]( - const auto& reactor) { + render_pass = std::move(shared_this), + tracer](const auto& reactor) { auto result = EncodeCommandsInReactor(*pass_data, allocator, reactor, render_pass->commands_, tracer); FML_CHECK(result) << "Must be able to encode GL commands without error."; From 6034da3154d4fa4c23761d9b68f36436f90264f8 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Wed, 18 Oct 2023 21:14:26 -0700 Subject: [PATCH 03/10] ++ --- .../renderer/backend/gles/context_gles.cc | 1 - impeller/renderer/backend/gles/context_gles.h | 17 ++++++- .../renderer/backend/gles/gpu_tracer_gles.cc | 49 +++++++++++++------ .../renderer/backend/gles/gpu_tracer_gles.h | 9 ++-- 4 files changed, 55 insertions(+), 21 deletions(-) diff --git a/impeller/renderer/backend/gles/context_gles.cc b/impeller/renderer/backend/gles/context_gles.cc index 37dde418c5a92..ae162711b4ce7 100644 --- a/impeller/renderer/backend/gles/context_gles.cc +++ b/impeller/renderer/backend/gles/context_gles.cc @@ -63,7 +63,6 @@ ContextGLES::ContextGLES(std::unique_ptr gl, device_capabilities_->SupportsDecalSamplerAddressMode())); } - gpu_tracer_ = std::make_shared(reactor_->GetProcTable()); is_valid_ = true; } diff --git a/impeller/renderer/backend/gles/context_gles.h b/impeller/renderer/backend/gles/context_gles.h index 40c63d92b1aa0..99678c3120810 100644 --- a/impeller/renderer/backend/gles/context_gles.h +++ b/impeller/renderer/backend/gles/context_gles.h @@ -4,6 +4,8 @@ #pragma once +#include +#include #include "flutter/fml/macros.h" #include "impeller/base/backend_cast.h" #include "impeller/renderer/backend/gles/allocator_gles.h" @@ -39,7 +41,16 @@ class ContextGLES final : public Context, bool RemoveReactorWorker(ReactorGLES::WorkerID id); - std::shared_ptr GetGPUTracer() const { return gpu_tracer_; } + std::shared_ptr GetGPUTracer() const { + FML_LOG(ERROR) << "Size: " << gpu_tracers_.size(); + auto result = gpu_tracers_.find(std::this_thread::get_id()); + if (result == gpu_tracers_.end()) { + return gpu_tracers_[std::this_thread::get_id()] = + std::make_shared(GetReactor()->GetProcTable()); + } else { + return result->second; + } + } private: ReactorGLES::Ref reactor_; @@ -47,7 +58,9 @@ class ContextGLES final : public Context, std::shared_ptr pipeline_library_; std::shared_ptr sampler_library_; std::shared_ptr resource_allocator_; - std::shared_ptr gpu_tracer_; + mutable std::unordered_map> + gpu_tracers_; + // Note: This is stored separately from the ProcTableGLES CapabilitiesGLES // in order to satisfy the Context::GetCapabilities signature which returns // a reference. diff --git a/impeller/renderer/backend/gles/gpu_tracer_gles.cc b/impeller/renderer/backend/gles/gpu_tracer_gles.cc index cfb2e4b1a203c..6f2c7631d98f6 100644 --- a/impeller/renderer/backend/gles/gpu_tracer_gles.cc +++ b/impeller/renderer/backend/gles/gpu_tracer_gles.cc @@ -4,7 +4,7 @@ #include "impeller/renderer/backend/gles/gpu_tracer_gles.h" #include "GLES2/gl2ext.h" -#include "impeller/renderer/backend/gles/context_gles.h" +#include "GLES3/gl3.h" namespace impeller { @@ -23,37 +23,53 @@ GPUTracerGLES::GPUTracerGLES(const ProcTableGLES& gl) { } void GPUTracerGLES::MarkFrameStart(const ProcTableGLES& gl) { - if (!enabled_) { + if (!enabled_ || has_started_frame_) { return; } - has_started_frame_ = true; - if (queries_.empty()) { - queries_ = std::vector(32); - gl.GenQueriesEXT(32, queries_.data()); + std::stringstream ss; + ss << std::this_thread::get_id(); + uint64_t id = std::stoull(ss.str()); + + FML_LOG(ERROR) << "Start: " << id; + + uint32_t query = 0; + gl.GenQueriesEXT(1, &query); + if (query == 0) { + return; } + has_started_frame_ = true; + // At the beginning of a frame, check the status of all pending // previous queries. if (!pending_traces_.empty()) { std::vector pending_traces = pending_traces_; pending_traces_.clear(); + bool done = false; for (auto query : pending_traces) { + if (done) { + pending_traces_.push_back(query); + continue; + } // First check if the query is complete without blocking // on the result. Incomplete results are left in the pending // trace vector and will not be checked again for another // frame. - uint available = 0; - FML_LOG(ERROR) << "Checking: " << query; + GLuint available = GL_FALSE; gl.GetQueryObjectuivEXT(query, GL_QUERY_RESULT_AVAILABLE_EXT, &available); - if (available) { + if (available == GL_TRUE) { // Return the timer resolution in nanoseconds. uint64_t duration = 0; - gl.GetQueryObjectui64vEXT(query, GL_QUERY_RESULT, &duration); + gl.GetQueryObjectui64vEXT(query, GL_QUERY_RESULT_EXT, &duration); auto gpu_ms = duration / 1000000.0; FML_LOG(ERROR) << "gpu_ms: " << gpu_ms; + gl.DeleteQueriesEXT(1, &query); + // Only ever check for one query a frame... + done = true; } else { + done = true; pending_traces_.push_back(query); } } @@ -61,20 +77,23 @@ void GPUTracerGLES::MarkFrameStart(const ProcTableGLES& gl) { // Allocate a single query object for the start and end of the frame. FML_CHECK(!active_frame_.has_value()); - uint32_t query = queries_.back(); - queries_.pop_back(); FML_CHECK(query != 0); active_frame_ = query; - FML_LOG(ERROR) << "query: " << query; gl.BeginQueryEXT(GL_TIME_ELAPSED_EXT, query); } void GPUTracerGLES::MarkFrameEnd(const ProcTableGLES& gl) { - if (!enabled_ || !active_frame_.has_value()) { + if (!enabled_ || !active_frame_.has_value() || !has_started_frame_){ return; } + + std::stringstream ss; + ss << std::this_thread::get_id(); + uint64_t id = std::stoull(ss.str()); + + FML_LOG(ERROR) << "End: " << id; + auto query = active_frame_.value(); - FML_LOG(ERROR) << "END " << query; gl.EndQueryEXT(GL_TIME_ELAPSED_EXT); pending_traces_.push_back(query); diff --git a/impeller/renderer/backend/gles/gpu_tracer_gles.h b/impeller/renderer/backend/gles/gpu_tracer_gles.h index f6ef995124186..b7b5bf8e3a242 100644 --- a/impeller/renderer/backend/gles/gpu_tracer_gles.h +++ b/impeller/renderer/backend/gles/gpu_tracer_gles.h @@ -4,6 +4,7 @@ #pragma once +#include #include "impeller/renderer/backend/gles/proc_table_gles.h" namespace impeller { @@ -14,16 +15,18 @@ class GPUTracerGLES { ~GPUTracerGLES() = default; + bool HasStartedFrame() const { + return has_started_frame_; + } + void MarkFrameStart(const ProcTableGLES& gl); void MarkFrameEnd(const ProcTableGLES& gl); - bool HasStartedFrame() const { return has_started_frame_; } - private: - std::vector queries_; std::vector pending_traces_; std::optional active_frame_ = std::nullopt; + std::thread::id raster_thread_; bool has_started_frame_ = false; bool enabled_ = false; From c014eba26e0278a9438e08750fc46f6c71f4f536 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Wed, 18 Oct 2023 22:13:52 -0700 Subject: [PATCH 04/10] ++ --- .../renderer/backend/gles/context_gles.cc | 3 +- impeller/renderer/backend/gles/context_gles.h | 12 +- .../renderer/backend/gles/gpu_tracer_gles.cc | 106 ++++++++---------- .../renderer/backend/gles/gpu_tracer_gles.h | 14 ++- .../renderer/backend/gles/proc_table_gles.h | 5 +- .../renderer/backend/gles/render_pass_gles.cc | 8 +- .../renderer/backend/gles/surface_gles.cc | 4 + 7 files changed, 71 insertions(+), 81 deletions(-) diff --git a/impeller/renderer/backend/gles/context_gles.cc b/impeller/renderer/backend/gles/context_gles.cc index ae162711b4ce7..f142b9d79a5e4 100644 --- a/impeller/renderer/backend/gles/context_gles.cc +++ b/impeller/renderer/backend/gles/context_gles.cc @@ -3,6 +3,7 @@ // found in the LICENSE file. #include "impeller/renderer/backend/gles/context_gles.h" +#include #include "impeller/base/config.h" #include "impeller/base/validation.h" @@ -62,7 +63,7 @@ ContextGLES::ContextGLES(std::unique_ptr gl, std::shared_ptr(new SamplerLibraryGLES( device_capabilities_->SupportsDecalSamplerAddressMode())); } - + gpu_tracer_ = std::make_shared(GetReactor()->GetProcTable()); is_valid_ = true; } diff --git a/impeller/renderer/backend/gles/context_gles.h b/impeller/renderer/backend/gles/context_gles.h index 99678c3120810..896de2de506d1 100644 --- a/impeller/renderer/backend/gles/context_gles.h +++ b/impeller/renderer/backend/gles/context_gles.h @@ -42,14 +42,7 @@ class ContextGLES final : public Context, bool RemoveReactorWorker(ReactorGLES::WorkerID id); std::shared_ptr GetGPUTracer() const { - FML_LOG(ERROR) << "Size: " << gpu_tracers_.size(); - auto result = gpu_tracers_.find(std::this_thread::get_id()); - if (result == gpu_tracers_.end()) { - return gpu_tracers_[std::this_thread::get_id()] = - std::make_shared(GetReactor()->GetProcTable()); - } else { - return result->second; - } + return gpu_tracer_; } private: @@ -58,8 +51,7 @@ class ContextGLES final : public Context, std::shared_ptr pipeline_library_; std::shared_ptr sampler_library_; std::shared_ptr resource_allocator_; - mutable std::unordered_map> - gpu_tracers_; + std::shared_ptr gpu_tracer_; // Note: This is stored separately from the ProcTableGLES CapabilitiesGLES // in order to satisfy the Context::GetCapabilities signature which returns diff --git a/impeller/renderer/backend/gles/gpu_tracer_gles.cc b/impeller/renderer/backend/gles/gpu_tracer_gles.cc index 6f2c7631d98f6..098d2417bd6b2 100644 --- a/impeller/renderer/backend/gles/gpu_tracer_gles.cc +++ b/impeller/renderer/backend/gles/gpu_tracer_gles.cc @@ -3,34 +3,27 @@ // found in the LICENSE file. #include "impeller/renderer/backend/gles/gpu_tracer_gles.h" -#include "GLES2/gl2ext.h" -#include "GLES3/gl3.h" +#include +#include "fml/trace_event.h" namespace impeller { -static const constexpr char* kAngleTimerQuery = "GL_ANGLE_timer_query"; -static const constexpr char* kExtTimerQuery = "GL_EXT_timer_query"; -static const constexpr char* kExtDisjointTimerQuery = - "GL_EXT_disjoint_timer_query"; - GPUTracerGLES::GPUTracerGLES(const ProcTableGLES& gl) { #ifdef IMPELLER_DEBUG auto desc = gl.GetDescription(); - enabled_ = desc->HasExtension(kAngleTimerQuery) || - desc->HasExtension(kExtTimerQuery) || - desc->HasExtension(kExtDisjointTimerQuery); + enabled_ = desc->HasExtension("GL_EXT_disjoint_timer_query"); #endif // IMPELLER_DEBUG } void GPUTracerGLES::MarkFrameStart(const ProcTableGLES& gl) { - if (!enabled_ || has_started_frame_) { + if (!enabled_ || has_started_frame_ || + std::this_thread::get_id() != raster_thread_) { return; } - std::stringstream ss; - ss << std::this_thread::get_id(); - uint64_t id = std::stoull(ss.str()); - FML_LOG(ERROR) << "Start: " << id; + // At the beginning of a frame, check the status of all pending + // previous queries. + ProcessQueries(gl); uint32_t query = 0; gl.GenQueriesEXT(1, &query); @@ -40,58 +33,53 @@ void GPUTracerGLES::MarkFrameStart(const ProcTableGLES& gl) { has_started_frame_ = true; - // At the beginning of a frame, check the status of all pending - // previous queries. - if (!pending_traces_.empty()) { - std::vector pending_traces = pending_traces_; - pending_traces_.clear(); - - bool done = false; - for (auto query : pending_traces) { - if (done) { - pending_traces_.push_back(query); - continue; - } - // First check if the query is complete without blocking - // on the result. Incomplete results are left in the pending - // trace vector and will not be checked again for another - // frame. - GLuint available = GL_FALSE; - gl.GetQueryObjectuivEXT(query, GL_QUERY_RESULT_AVAILABLE_EXT, &available); - - if (available == GL_TRUE) { - // Return the timer resolution in nanoseconds. - uint64_t duration = 0; - gl.GetQueryObjectui64vEXT(query, GL_QUERY_RESULT_EXT, &duration); - auto gpu_ms = duration / 1000000.0; - FML_LOG(ERROR) << "gpu_ms: " << gpu_ms; - gl.DeleteQueriesEXT(1, &query); - // Only ever check for one query a frame... - done = true; - } else { - done = true; - pending_traces_.push_back(query); - } - } - } - - // Allocate a single query object for the start and end of the frame. - FML_CHECK(!active_frame_.has_value()); - FML_CHECK(query != 0); + FML_DCHECK(!active_frame_.has_value()); + FML_DCHECK(query != 0); active_frame_ = query; gl.BeginQueryEXT(GL_TIME_ELAPSED_EXT, query); } -void GPUTracerGLES::MarkFrameEnd(const ProcTableGLES& gl) { - if (!enabled_ || !active_frame_.has_value() || !has_started_frame_){ +void GPUTracerGLES::RecordRasterThread() { + raster_thread_ = std::this_thread::get_id(); +} + +void GPUTracerGLES::ProcessQueries(const ProcTableGLES& gl) { + if (pending_traces_.empty()) { return; } - std::stringstream ss; - ss << std::this_thread::get_id(); - uint64_t id = std::stoull(ss.str()); + // For reasons unknown to me, querying the state of more than + // on query object per frame causes crashes on a Pixel 6 pro. + auto latest_query = pending_traces_.front(); + + // First check if the query is complete without blocking + // on the result. Incomplete results are left in the pending + // trace vector and will not be checked again for another + // frame. + GLuint available = GL_FALSE; + gl.GetQueryObjectuivEXT(latest_query, GL_QUERY_RESULT_AVAILABLE_EXT, + &available); + + if (available == GL_TRUE) { + // Return the timer resolution in nanoseconds. + uint64_t duration = 0; + gl.GetQueryObjectui64vEXT(latest_query, GL_QUERY_RESULT_EXT, &duration); + auto gpu_ms = duration / 1000000.0; + + FML_TRACE_COUNTER("flutter", "GPUTracer", + reinterpret_cast(this), // Trace Counter ID + "FrameTimeMS", gpu_ms); + + gl.DeleteQueriesEXT(1, &latest_query); + pending_traces_.pop_front(); + } +} - FML_LOG(ERROR) << "End: " << id; +void GPUTracerGLES::MarkFrameEnd(const ProcTableGLES& gl) { + if (!enabled_ || std::this_thread::get_id() != raster_thread_ || + !active_frame_.has_value() || !has_started_frame_) { + return; + } auto query = active_frame_.value(); gl.EndQueryEXT(GL_TIME_ELAPSED_EXT); diff --git a/impeller/renderer/backend/gles/gpu_tracer_gles.h b/impeller/renderer/backend/gles/gpu_tracer_gles.h index b7b5bf8e3a242..bd5cc960ad32f 100644 --- a/impeller/renderer/backend/gles/gpu_tracer_gles.h +++ b/impeller/renderer/backend/gles/gpu_tracer_gles.h @@ -4,7 +4,9 @@ #pragma once +#include #include + #include "impeller/renderer/backend/gles/proc_table_gles.h" namespace impeller { @@ -15,16 +17,20 @@ class GPUTracerGLES { ~GPUTracerGLES() = default; - bool HasStartedFrame() const { - return has_started_frame_; - } + /// @brief Record the thread id of the raster thread. + void RecordRasterThread(); + /// @brief Record the start of a frame workload, if one hasn't already been + /// started. void MarkFrameStart(const ProcTableGLES& gl); + /// @brief Record the end of a frame workload. void MarkFrameEnd(const ProcTableGLES& gl); private: - std::vector pending_traces_; + void ProcessQueries(const ProcTableGLES& gl); + + std::deque pending_traces_; std::optional active_frame_ = std::nullopt; std::thread::id raster_thread_; bool has_started_frame_ = false; diff --git a/impeller/renderer/backend/gles/proc_table_gles.h b/impeller/renderer/backend/gles/proc_table_gles.h index 26191f03d6e6f..643ddd413f3f9 100644 --- a/impeller/renderer/backend/gles/proc_table_gles.h +++ b/impeller/renderer/backend/gles/proc_table_gles.h @@ -199,13 +199,10 @@ struct GLProc { PROC(RenderbufferStorageMultisampleEXT); \ PROC(GenQueriesEXT); \ PROC(DeleteQueriesEXT); \ - PROC(IsQueryEXT); \ - PROC(QueryCounterEXT); \ PROC(GetQueryObjectui64vEXT); \ PROC(BeginQueryEXT); \ PROC(EndQueryEXT); \ - PROC(GetQueryObjectuivEXT); \ - PROC(GetQueryObjectivEXT); + PROC(GetQueryObjectuivEXT); enum class DebugResourceType { kTexture, diff --git a/impeller/renderer/backend/gles/render_pass_gles.cc b/impeller/renderer/backend/gles/render_pass_gles.cc index a4e811a9f6084..b3b22ffbdad22 100644 --- a/impeller/renderer/backend/gles/render_pass_gles.cc +++ b/impeller/renderer/backend/gles/render_pass_gles.cc @@ -152,9 +152,9 @@ struct RenderPassData { } const auto& gl = reactor.GetProcTable(); - if (!tracer->HasStartedFrame()) { - tracer->MarkFrameStart(gl); - } +#ifdef IMPELLER_DEBUG + tracer->MarkFrameStart(gl); +#endif // IMPELLER_DEBUG fml::ScopedCleanupClosure pop_pass_debug_marker( [&gl]() { gl.PopDebugGroup(); }); @@ -498,9 +498,11 @@ struct RenderPassData { attachments.data() // size ); } +#ifdef IMPELLER_DEBUG if (is_default_fbo) { tracer->MarkFrameEnd(gl); } +#endif // IMPELLER_DEBUG return true; } diff --git a/impeller/renderer/backend/gles/surface_gles.cc b/impeller/renderer/backend/gles/surface_gles.cc index a179b3798f3d9..9c817e6e382a7 100644 --- a/impeller/renderer/backend/gles/surface_gles.cc +++ b/impeller/renderer/backend/gles/surface_gles.cc @@ -60,6 +60,10 @@ std::unique_ptr SurfaceGLES::WrapFBO( render_target_desc.SetColorAttachment(color0, 0u); render_target_desc.SetStencilAttachment(stencil0); +#ifdef IMPELLER_DEBUG + gl_context.GetGPUTracer()->RecordRasterThread(); +#endif // IMPELLER_DEBUG + return std::unique_ptr( new SurfaceGLES(std::move(swap_callback), render_target_desc)); } From bf6f77b6c8900fc3d76f52921896ba06b601f9cc Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Wed, 18 Oct 2023 22:14:20 -0700 Subject: [PATCH 05/10] ++ --- impeller/renderer/backend/gles/context_gles.h | 4 +--- impeller/renderer/backend/gles/render_pass_gles.cc | 4 ++-- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/impeller/renderer/backend/gles/context_gles.h b/impeller/renderer/backend/gles/context_gles.h index 896de2de506d1..7bce5905569aa 100644 --- a/impeller/renderer/backend/gles/context_gles.h +++ b/impeller/renderer/backend/gles/context_gles.h @@ -41,9 +41,7 @@ class ContextGLES final : public Context, bool RemoveReactorWorker(ReactorGLES::WorkerID id); - std::shared_ptr GetGPUTracer() const { - return gpu_tracer_; - } + std::shared_ptr GetGPUTracer() const { return gpu_tracer_; } private: ReactorGLES::Ref reactor_; diff --git a/impeller/renderer/backend/gles/render_pass_gles.cc b/impeller/renderer/backend/gles/render_pass_gles.cc index b3b22ffbdad22..37f9e91994561 100644 --- a/impeller/renderer/backend/gles/render_pass_gles.cc +++ b/impeller/renderer/backend/gles/render_pass_gles.cc @@ -154,7 +154,7 @@ struct RenderPassData { const auto& gl = reactor.GetProcTable(); #ifdef IMPELLER_DEBUG tracer->MarkFrameStart(gl); -#endif // IMPELLER_DEBUG +#endif // IMPELLER_DEBUG fml::ScopedCleanupClosure pop_pass_debug_marker( [&gl]() { gl.PopDebugGroup(); }); @@ -502,7 +502,7 @@ struct RenderPassData { if (is_default_fbo) { tracer->MarkFrameEnd(gl); } -#endif // IMPELLER_DEBUG +#endif // IMPELLER_DEBUG return true; } From 3eb2edddae976706c1dcac7744afb55657365386 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Thu, 19 Oct 2023 10:29:21 -0700 Subject: [PATCH 06/10] add 'test' --- impeller/renderer/backend/gles/BUILD.gn | 2 +- .../gles/test/gpu_tracer_gles_unittests.cc | 49 +++++++++++++++ .../renderer/backend/gles/test/mock_gles.cc | 59 +++++++++++++++++++ 3 files changed, 109 insertions(+), 1 deletion(-) create mode 100644 impeller/renderer/backend/gles/test/gpu_tracer_gles_unittests.cc diff --git a/impeller/renderer/backend/gles/BUILD.gn b/impeller/renderer/backend/gles/BUILD.gn index b7cad3acc50db..4188d25478250 100644 --- a/impeller/renderer/backend/gles/BUILD.gn +++ b/impeller/renderer/backend/gles/BUILD.gn @@ -16,6 +16,7 @@ impeller_component("gles_unittests") { sources = [ "test/capabilities_unittests.cc", "test/formats_gles_unittests.cc", + "test/gpu_tracer_gles_unittests.cc", "test/mock_gles.cc", "test/mock_gles.h", "test/mock_gles_unittests.cc", @@ -44,7 +45,6 @@ impeller_component("gles") { "command_buffer_gles.h", "context_gles.cc", "context_gles.h", - "context_gles.h", "description_gles.cc", "description_gles.h", "device_buffer_gles.cc", diff --git a/impeller/renderer/backend/gles/test/gpu_tracer_gles_unittests.cc b/impeller/renderer/backend/gles/test/gpu_tracer_gles_unittests.cc new file mode 100644 index 0000000000000..d800cb3477586 --- /dev/null +++ b/impeller/renderer/backend/gles/test/gpu_tracer_gles_unittests.cc @@ -0,0 +1,49 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "flutter/testing/testing.h" // IWYU pragma: keep +#include "gtest/gtest.h" +#include "impeller/renderer/backend/gles/gpu_tracer_gles.h" +#include "impeller/renderer/backend/gles/test/mock_gles.h" + +namespace impeller { +namespace testing { + +#ifdef IMPELLER_DEBUG +TEST(GPUTracerGLES, CanFormatFramebufferErrorMessage) { + auto const extensions = std::vector{ + reinterpret_cast("GL_KHR_debug"), // + reinterpret_cast("GL_EXT_disjoint_timer_query"), // + }; + auto mock_gles = MockGLES::Init(extensions); + auto tracer = std::make_shared(mock_gles->GetProcTable()); + tracer->RecordRasterThread(); + tracer->MarkFrameStart(mock_gles->GetProcTable()); + tracer->MarkFrameEnd(mock_gles->GetProcTable()); + + auto calls = mock_gles->GetCapturedCalls(); + + std::vector expected = {"glGenQueriesEXT", "glBeginQueryEXT", + "glEndQueryEXT"}; + for (auto i = 0; i < 3; i++) { + EXPECT_EQ(calls[i], expected[i]); + } + + // Begin second frame, which prompts the tracer to query the result + // from the previous frame. + tracer->MarkFrameStart(mock_gles->GetProcTable()); + + calls = mock_gles->GetCapturedCalls(); + std::vector expected_b = {"glGetQueryObjectuivEXT", + "glGetQueryObjectui64vEXT", + "glDeleteQueriesEXT"}; + for (auto i = 0; i < 3; i++) { + EXPECT_EQ(calls[i], expected_b[i]); + } +} + +#endif // IMPELLER_DEBUG + +} // namespace testing +} // namespace impeller diff --git a/impeller/renderer/backend/gles/test/mock_gles.cc b/impeller/renderer/backend/gles/test/mock_gles.cc index 1abf7e8721d46..30ea1ae3eefc7 100644 --- a/impeller/renderer/backend/gles/test/mock_gles.cc +++ b/impeller/renderer/backend/gles/test/mock_gles.cc @@ -112,6 +112,53 @@ void mockPushDebugGroupKHR(GLenum source, static_assert(CheckSameSignature::value); +void mockGenQueriesEXT(GLsizei n, GLuint* ids) { + RecordGLCall("glGenQueriesEXT"); + for (auto i = 0; i < n; i++) { + ids[i] = i + 1; + } +} + +static_assert(CheckSameSignature::value); + +void mockBeginQueryEXT(GLenum target, GLuint id) { + RecordGLCall("glBeginQueryEXT"); +} + +static_assert(CheckSameSignature::value); + +void mockEndQueryEXT(GLuint id) { + RecordGLCall("glEndQueryEXT"); +} + +static_assert(CheckSameSignature::value); + +void mockGetQueryObjectuivEXT(GLuint id, GLenum target, GLuint* result) { + RecordGLCall("glGetQueryObjectuivEXT"); + *result = GL_TRUE; +} + +static_assert(CheckSameSignature::value); + +void mockGetQueryObjectui64vEXT(GLuint id, GLenum target, GLuint64* result) { + RecordGLCall("glGetQueryObjectui64vEXT"); + *result = 1000u; +} + +static_assert(CheckSameSignature::value); + +void mockDeleteQueriesEXT(GLsizei size, const GLuint* queries) { + RecordGLCall("glDeleteQueriesEXT"); +} + +static_assert(CheckSameSignature::value); + std::shared_ptr MockGLES::Init( const std::optional>& extensions) { // If we cannot obtain a lock, MockGLES is already being used elsewhere. @@ -136,6 +183,18 @@ const ProcTableGLES::Resolver kMockResolver = [](const char* name) { return reinterpret_cast(&mockGetIntegerv); } else if (strcmp(name, "glGetError") == 0) { return reinterpret_cast(&mockGetError); + } else if (strcmp(name, "glGenQueriesEXT") == 0) { + return reinterpret_cast(&mockGenQueriesEXT); + } else if (strcmp(name, "glBeginQueryEXT") == 0) { + return reinterpret_cast(&mockBeginQueryEXT); + } else if (strcmp(name, "glEndQueryEXT") == 0) { + return reinterpret_cast(&mockEndQueryEXT); + } else if (strcmp(name, "glDeleteQueriesEXT") == 0) { + return reinterpret_cast(&mockDeleteQueriesEXT); + } else if (strcmp(name, "glGetQueryObjectui64vEXT") == 0) { + return reinterpret_cast(mockGetQueryObjectui64vEXT); + } else if (strcmp(name, "glGetQueryObjectuivEXT") == 0) { + return reinterpret_cast(mockGetQueryObjectuivEXT); } else { return reinterpret_cast(&doNothing); } From 12f86c3f61688ae73d64576128e07396702851e3 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Thu, 19 Oct 2023 10:50:07 -0700 Subject: [PATCH 07/10] licenses and cleanup. --- ci/licenses_golden/licenses_flutter | 4 ++++ .../renderer/backend/gles/gpu_tracer_gles.cc | 24 ++++++++++--------- 2 files changed, 17 insertions(+), 11 deletions(-) diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index 71e1e4ae28ba6..83f1244159edb 100644 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -2086,6 +2086,8 @@ ORIGIN: ../../../flutter/impeller/renderer/backend/gles/device_buffer_gles.h + . ORIGIN: ../../../flutter/impeller/renderer/backend/gles/formats_gles.cc + ../../../flutter/LICENSE ORIGIN: ../../../flutter/impeller/renderer/backend/gles/formats_gles.h + ../../../flutter/LICENSE ORIGIN: ../../../flutter/impeller/renderer/backend/gles/gles.h + ../../../flutter/LICENSE +ORIGIN: ../../../flutter/impeller/renderer/backend/gles/gpu_tracer_gles.cc + ../../../flutter/LICENSE +ORIGIN: ../../../flutter/impeller/renderer/backend/gles/gpu_tracer_gles.h + ../../../flutter/LICENSE ORIGIN: ../../../flutter/impeller/renderer/backend/gles/handle_gles.cc + ../../../flutter/LICENSE ORIGIN: ../../../flutter/impeller/renderer/backend/gles/handle_gles.h + ../../../flutter/LICENSE ORIGIN: ../../../flutter/impeller/renderer/backend/gles/pipeline_gles.cc + ../../../flutter/LICENSE @@ -4854,6 +4856,8 @@ FILE: ../../../flutter/impeller/renderer/backend/gles/device_buffer_gles.h FILE: ../../../flutter/impeller/renderer/backend/gles/formats_gles.cc FILE: ../../../flutter/impeller/renderer/backend/gles/formats_gles.h FILE: ../../../flutter/impeller/renderer/backend/gles/gles.h +FILE: ../../../flutter/impeller/renderer/backend/gles/gpu_tracer_gles.cc +FILE: ../../../flutter/impeller/renderer/backend/gles/gpu_tracer_gles.h FILE: ../../../flutter/impeller/renderer/backend/gles/handle_gles.cc FILE: ../../../flutter/impeller/renderer/backend/gles/handle_gles.h FILE: ../../../flutter/impeller/renderer/backend/gles/pipeline_gles.cc diff --git a/impeller/renderer/backend/gles/gpu_tracer_gles.cc b/impeller/renderer/backend/gles/gpu_tracer_gles.cc index 098d2417bd6b2..8bbc65e4eb487 100644 --- a/impeller/renderer/backend/gles/gpu_tracer_gles.cc +++ b/impeller/renderer/backend/gles/gpu_tracer_gles.cc @@ -50,6 +50,7 @@ void GPUTracerGLES::ProcessQueries(const ProcTableGLES& gl) { // For reasons unknown to me, querying the state of more than // on query object per frame causes crashes on a Pixel 6 pro. + // It does not crash on an S10. auto latest_query = pending_traces_.front(); // First check if the query is complete without blocking @@ -60,19 +61,20 @@ void GPUTracerGLES::ProcessQueries(const ProcTableGLES& gl) { gl.GetQueryObjectuivEXT(latest_query, GL_QUERY_RESULT_AVAILABLE_EXT, &available); - if (available == GL_TRUE) { - // Return the timer resolution in nanoseconds. - uint64_t duration = 0; - gl.GetQueryObjectui64vEXT(latest_query, GL_QUERY_RESULT_EXT, &duration); - auto gpu_ms = duration / 1000000.0; + if (available != GL_TRUE) { + return; + } + // Return the timer resolution in nanoseconds. + uint64_t duration = 0; + gl.GetQueryObjectui64vEXT(latest_query, GL_QUERY_RESULT_EXT, &duration); + auto gpu_ms = duration / 1000000.0; - FML_TRACE_COUNTER("flutter", "GPUTracer", - reinterpret_cast(this), // Trace Counter ID - "FrameTimeMS", gpu_ms); + FML_TRACE_COUNTER("flutter", "GPUTracer", + reinterpret_cast(this), // Trace Counter ID + "FrameTimeMS", gpu_ms); - gl.DeleteQueriesEXT(1, &latest_query); - pending_traces_.pop_front(); - } + gl.DeleteQueriesEXT(1, &latest_query); + pending_traces_.pop_front(); } void GPUTracerGLES::MarkFrameEnd(const ProcTableGLES& gl) { From 51ad782ca039cfe7a57e8cf83704afe15915c59f Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Thu, 19 Oct 2023 18:57:48 -0700 Subject: [PATCH 08/10] Add option to enable GPU tracing for OpenGL from Android Manifest --- common/settings.h | 4 ++ .../backend/gles/playground_impl_gles.cc | 4 +- .../renderer/backend/gles/context_gles.cc | 15 ++-- impeller/renderer/backend/gles/context_gles.h | 6 +- .../renderer/backend/gles/gpu_tracer_gles.cc | 68 +++++++++---------- .../renderer/backend/gles/gpu_tracer_gles.h | 9 ++- .../gles/test/gpu_tracer_gles_unittests.cc | 3 +- shell/common/switches.cc | 2 + shell/common/switches.h | 4 ++ .../android/android_context_gl_impeller.cc | 28 ++++---- .../android/android_context_gl_impeller.h | 4 +- .../android_context_gl_impeller_unittests.cc | 6 +- .../android/android_context_vulkan_impeller.h | 2 +- .../engine/loader/FlutterLoader.java | 5 ++ .../platform/android/platform_view_android.cc | 13 ++-- 15 files changed, 103 insertions(+), 70 deletions(-) diff --git a/common/settings.h b/common/settings.h index af002c93006c4..00dc5aeaf8d04 100644 --- a/common/settings.h +++ b/common/settings.h @@ -228,6 +228,10 @@ struct Settings { // must be available to the application. bool enable_vulkan_validation = false; + // Enable GPU tracing in GLES backends. + // Some devices claim to support the required APIs but crash on their usage. + bool enable_opengl_gpu_tracing = false; + // Data set by platform-specific embedders for use in font initialization. uint32_t font_initialization_data = 0; diff --git a/impeller/playground/backend/gles/playground_impl_gles.cc b/impeller/playground/backend/gles/playground_impl_gles.cc index 6e5c3f11ab198..fbf64c44ba35c 100644 --- a/impeller/playground/backend/gles/playground_impl_gles.cc +++ b/impeller/playground/backend/gles/playground_impl_gles.cc @@ -112,8 +112,8 @@ std::shared_ptr PlaygroundImplGLES::GetContext() const { return nullptr; } - auto context = - ContextGLES::Create(std::move(gl), ShaderLibraryMappingsForPlayground()); + auto context = ContextGLES::Create( + std::move(gl), ShaderLibraryMappingsForPlayground(), true); if (!context) { FML_LOG(ERROR) << "Could not create context."; return nullptr; diff --git a/impeller/renderer/backend/gles/context_gles.cc b/impeller/renderer/backend/gles/context_gles.cc index f142b9d79a5e4..779255dffa6c4 100644 --- a/impeller/renderer/backend/gles/context_gles.cc +++ b/impeller/renderer/backend/gles/context_gles.cc @@ -14,14 +14,16 @@ namespace impeller { std::shared_ptr ContextGLES::Create( std::unique_ptr gl, - const std::vector>& shader_libraries) { + const std::vector>& shader_libraries, + bool enable_gpu_tracing) { return std::shared_ptr( - new ContextGLES(std::move(gl), shader_libraries)); + new ContextGLES(std::move(gl), shader_libraries, enable_gpu_tracing)); } -ContextGLES::ContextGLES(std::unique_ptr gl, - const std::vector>& - shader_libraries_mappings) { +ContextGLES::ContextGLES( + std::unique_ptr gl, + const std::vector>& shader_libraries_mappings, + bool enable_gpu_tracing) { reactor_ = std::make_shared(std::move(gl)); if (!reactor_->IsValid()) { VALIDATION_LOG << "Could not create valid reactor."; @@ -63,7 +65,8 @@ ContextGLES::ContextGLES(std::unique_ptr gl, std::shared_ptr(new SamplerLibraryGLES( device_capabilities_->SupportsDecalSamplerAddressMode())); } - gpu_tracer_ = std::make_shared(GetReactor()->GetProcTable()); + gpu_tracer_ = std::make_shared(GetReactor()->GetProcTable(), + enable_gpu_tracing); is_valid_ = true; } diff --git a/impeller/renderer/backend/gles/context_gles.h b/impeller/renderer/backend/gles/context_gles.h index 7bce5905569aa..0d097c3f18a87 100644 --- a/impeller/renderer/backend/gles/context_gles.h +++ b/impeller/renderer/backend/gles/context_gles.h @@ -26,7 +26,8 @@ class ContextGLES final : public Context, public: static std::shared_ptr Create( std::unique_ptr gl, - const std::vector>& shader_libraries); + const std::vector>& shader_libraries, + bool enable_gpu_tracing); // |Context| ~ContextGLES() override; @@ -59,7 +60,8 @@ class ContextGLES final : public Context, ContextGLES( std::unique_ptr gl, - const std::vector>& shader_libraries); + const std::vector>& shader_libraries, + bool enable_gpu_tracing); // |Context| std::string DescribeGpuModel() const override; diff --git a/impeller/renderer/backend/gles/gpu_tracer_gles.cc b/impeller/renderer/backend/gles/gpu_tracer_gles.cc index 8bbc65e4eb487..c92a8e88aca9f 100644 --- a/impeller/renderer/backend/gles/gpu_tracer_gles.cc +++ b/impeller/renderer/backend/gles/gpu_tracer_gles.cc @@ -8,15 +8,16 @@ namespace impeller { -GPUTracerGLES::GPUTracerGLES(const ProcTableGLES& gl) { +GPUTracerGLES::GPUTracerGLES(const ProcTableGLES& gl, bool enable_tracing) { #ifdef IMPELLER_DEBUG auto desc = gl.GetDescription(); - enabled_ = desc->HasExtension("GL_EXT_disjoint_timer_query"); + enabled_ = + enable_tracing && desc->HasExtension("GL_EXT_disjoint_timer_query"); #endif // IMPELLER_DEBUG } void GPUTracerGLES::MarkFrameStart(const ProcTableGLES& gl) { - if (!enabled_ || has_started_frame_ || + if (!enabled_ || active_frame_.has_value() || std::this_thread::get_id() != raster_thread_) { return; } @@ -31,10 +32,7 @@ void GPUTracerGLES::MarkFrameStart(const ProcTableGLES& gl) { return; } - has_started_frame_ = true; - FML_DCHECK(!active_frame_.has_value()); - FML_DCHECK(query != 0); active_frame_ = query; gl.BeginQueryEXT(GL_TIME_ELAPSED_EXT, query); } @@ -44,42 +42,41 @@ void GPUTracerGLES::RecordRasterThread() { } void GPUTracerGLES::ProcessQueries(const ProcTableGLES& gl) { - if (pending_traces_.empty()) { - return; - } - // For reasons unknown to me, querying the state of more than - // on query object per frame causes crashes on a Pixel 6 pro. + // one query object per frame causes crashes on a Pixel 6 pro. // It does not crash on an S10. - auto latest_query = pending_traces_.front(); - - // First check if the query is complete without blocking - // on the result. Incomplete results are left in the pending - // trace vector and will not be checked again for another - // frame. - GLuint available = GL_FALSE; - gl.GetQueryObjectuivEXT(latest_query, GL_QUERY_RESULT_AVAILABLE_EXT, - &available); - - if (available != GL_TRUE) { - return; + while (!pending_traces_.empty()) { + auto query = pending_traces_.front(); + + // First check if the query is complete without blocking + // on the result. Incomplete results are left in the pending + // trace vector and will not be checked again for another + // frame. + GLuint available = GL_FALSE; + gl.GetQueryObjectuivEXT(query, GL_QUERY_RESULT_AVAILABLE_EXT, &available); + + if (available != GL_TRUE) { + // If a query is not available, then all subsequent queries will be + // unavailable. + return; + } + // Return the timer resolution in nanoseconds. + uint64_t duration = 0; + gl.GetQueryObjectui64vEXT(query, GL_QUERY_RESULT_EXT, &duration); + auto gpu_ms = duration / 1000000.0; + + FML_TRACE_COUNTER("flutter", "GPUTracer", + reinterpret_cast(this), // Trace Counter ID + "FrameTimeMS", gpu_ms); + FML_LOG(ERROR) << gpu_ms; + gl.DeleteQueriesEXT(1, &query); + pending_traces_.pop_front(); } - // Return the timer resolution in nanoseconds. - uint64_t duration = 0; - gl.GetQueryObjectui64vEXT(latest_query, GL_QUERY_RESULT_EXT, &duration); - auto gpu_ms = duration / 1000000.0; - - FML_TRACE_COUNTER("flutter", "GPUTracer", - reinterpret_cast(this), // Trace Counter ID - "FrameTimeMS", gpu_ms); - - gl.DeleteQueriesEXT(1, &latest_query); - pending_traces_.pop_front(); } void GPUTracerGLES::MarkFrameEnd(const ProcTableGLES& gl) { if (!enabled_ || std::this_thread::get_id() != raster_thread_ || - !active_frame_.has_value() || !has_started_frame_) { + !active_frame_.has_value() || !active_frame_.has_value()) { return; } @@ -88,7 +85,6 @@ void GPUTracerGLES::MarkFrameEnd(const ProcTableGLES& gl) { pending_traces_.push_back(query); active_frame_ = std::nullopt; - has_started_frame_ = false; } } // namespace impeller diff --git a/impeller/renderer/backend/gles/gpu_tracer_gles.h b/impeller/renderer/backend/gles/gpu_tracer_gles.h index bd5cc960ad32f..45af352705480 100644 --- a/impeller/renderer/backend/gles/gpu_tracer_gles.h +++ b/impeller/renderer/backend/gles/gpu_tracer_gles.h @@ -11,9 +11,15 @@ namespace impeller { +/// @brief Trace GPU execution times using GL_EXT_disjoint_timer_query on GLES. +/// +/// Note: there are a substantial number of GPUs where usage of the this API is +/// known to cause crashes. As a result, this functionality is disabled by +/// default and can only be enabled in debug/profile mode via a specific opt-in +/// flag that is exposed in the Android manifest. class GPUTracerGLES { public: - explicit GPUTracerGLES(const ProcTableGLES& gl); + GPUTracerGLES(const ProcTableGLES& gl, bool enable_tracing); ~GPUTracerGLES() = default; @@ -33,7 +39,6 @@ class GPUTracerGLES { std::deque pending_traces_; std::optional active_frame_ = std::nullopt; std::thread::id raster_thread_; - bool has_started_frame_ = false; bool enabled_ = false; }; diff --git a/impeller/renderer/backend/gles/test/gpu_tracer_gles_unittests.cc b/impeller/renderer/backend/gles/test/gpu_tracer_gles_unittests.cc index d800cb3477586..d0579a2091b23 100644 --- a/impeller/renderer/backend/gles/test/gpu_tracer_gles_unittests.cc +++ b/impeller/renderer/backend/gles/test/gpu_tracer_gles_unittests.cc @@ -17,7 +17,8 @@ TEST(GPUTracerGLES, CanFormatFramebufferErrorMessage) { reinterpret_cast("GL_EXT_disjoint_timer_query"), // }; auto mock_gles = MockGLES::Init(extensions); - auto tracer = std::make_shared(mock_gles->GetProcTable()); + auto tracer = + std::make_shared(mock_gles->GetProcTable(), true); tracer->RecordRasterThread(); tracer->MarkFrameStart(mock_gles->GetProcTable()); tracer->MarkFrameEnd(mock_gles->GetProcTable()); diff --git a/shell/common/switches.cc b/shell/common/switches.cc index e7c5666b147e2..8ed53d09ff13b 100644 --- a/shell/common/switches.cc +++ b/shell/common/switches.cc @@ -477,6 +477,8 @@ Settings SettingsFromCommandLine(const fml::CommandLine& command_line) { settings.enable_vulkan_validation = command_line.HasOption(FlagForSwitch(Switch::EnableVulkanValidation)); + settings.enable_opengl_gpu_tracing = + command_line.HasOption(FlagForSwitch(Switch::EnableOpenGLGPUTracing)); settings.enable_embedder_api = command_line.HasOption(FlagForSwitch(Switch::EnableEmbedderAPI)); diff --git a/shell/common/switches.h b/shell/common/switches.h index c6048705f3121..ddc4b3c8827aa 100644 --- a/shell/common/switches.h +++ b/shell/common/switches.h @@ -275,6 +275,10 @@ DEF_SWITCH(EnableVulkanValidation, "Enable loading Vulkan validation layers. The layers must be " "available to the application and loadable. On non-Vulkan backends, " "this flag does nothing.") +DEF_SWITCH(EnableOpenGLGPUTracing, + "enable-opengl-gpu-tracing", + "Enable tracing of GPU execution time when using the Impeller " + "OpenGLES backend.") DEF_SWITCH(LeakVM, "leak-vm", "When the last shell shuts down, the shared VM is leaked by default " diff --git a/shell/platform/android/android_context_gl_impeller.cc b/shell/platform/android/android_context_gl_impeller.cc index 83c1bb4fedd91..8c9b80645d7ab 100644 --- a/shell/platform/android/android_context_gl_impeller.cc +++ b/shell/platform/android/android_context_gl_impeller.cc @@ -49,7 +49,8 @@ class AndroidContextGLImpeller::ReactorWorker final }; static std::shared_ptr CreateImpellerContext( - const std::shared_ptr& worker) { + const std::shared_ptr& worker, + bool enable_gpu_tracing) { auto proc_table = std::make_unique( impeller::egl::CreateProcAddressResolver()); @@ -59,19 +60,20 @@ static std::shared_ptr CreateImpellerContext( } std::vector> shader_mappings = { - std::make_shared(impeller_entity_shaders_gles_data, - impeller_entity_shaders_gles_length), - std::make_shared( - impeller_framebuffer_blend_shaders_gles_data, - impeller_framebuffer_blend_shaders_gles_length), + std::make_shared( + impeller_entity_shaders_gles_data, + impeller_entity_shaders_gles_length), + std::make_shared( + impeller_framebuffer_blend_shaders_gles_data, + impeller_framebuffer_blend_shaders_gles_length), #if IMPELLER_ENABLE_3D - std::make_shared(impeller_scene_shaders_gles_data, - impeller_scene_shaders_gles_length), + std::make_shared( + impeller_scene_shaders_gles_data, impeller_scene_shaders_gles_length), #endif // IMPELLER_ENABLE_3D }; - auto context = - impeller::ContextGLES::Create(std::move(proc_table), shader_mappings); + auto context = impeller::ContextGLES::Create( + std::move(proc_table), shader_mappings, enable_gpu_tracing); if (!context) { FML_LOG(ERROR) << "Could not create OpenGLES Impeller Context."; return nullptr; @@ -86,7 +88,8 @@ static std::shared_ptr CreateImpellerContext( } AndroidContextGLImpeller::AndroidContextGLImpeller( - std::unique_ptr display) + std::unique_ptr display, + bool enable_gpu_tracing) : AndroidContext(AndroidRenderingAPI::kOpenGLES), reactor_worker_(std::shared_ptr(new ReactorWorker())), display_(std::move(display)) { @@ -147,7 +150,8 @@ AndroidContextGLImpeller::AndroidContextGLImpeller( return; } - auto impeller_context = CreateImpellerContext(reactor_worker_); + auto impeller_context = + CreateImpellerContext(reactor_worker_, enable_gpu_tracing); if (!impeller_context) { FML_DLOG(ERROR) << "Could not create Impeller context."; diff --git a/shell/platform/android/android_context_gl_impeller.h b/shell/platform/android/android_context_gl_impeller.h index 90466db683a8a..234ff58d2c91e 100644 --- a/shell/platform/android/android_context_gl_impeller.h +++ b/shell/platform/android/android_context_gl_impeller.h @@ -13,8 +13,8 @@ namespace flutter { class AndroidContextGLImpeller : public AndroidContext { public: - explicit AndroidContextGLImpeller( - std::unique_ptr display); + AndroidContextGLImpeller(std::unique_ptr display, + bool enable_gpu_tracing); ~AndroidContextGLImpeller(); diff --git a/shell/platform/android/android_context_gl_impeller_unittests.cc b/shell/platform/android/android_context_gl_impeller_unittests.cc index 452c55fc76261..e8f52832d8f94 100644 --- a/shell/platform/android/android_context_gl_impeller_unittests.cc +++ b/shell/platform/android/android_context_gl_impeller_unittests.cc @@ -45,7 +45,8 @@ TEST(AndroidContextGLImpeller, MSAAFirstAttempt) { .WillOnce(Return(ByMove(std::move(second_result)))); ON_CALL(*display, ChooseConfig(_)) .WillByDefault(Return(ByMove(std::unique_ptr()))); - auto context = std::make_unique(std::move(display)); + auto context = + std::make_unique(std::move(display), true); ASSERT_TRUE(context); } @@ -76,7 +77,8 @@ TEST(AndroidContextGLImpeller, FallbackForEmulator) { .WillOnce(Return(ByMove(std::move(third_result)))); ON_CALL(*display, ChooseConfig(_)) .WillByDefault(Return(ByMove(std::unique_ptr()))); - auto context = std::make_unique(std::move(display)); + auto context = + std::make_unique(std::move(display), true); ASSERT_TRUE(context); } } // namespace testing diff --git a/shell/platform/android/android_context_vulkan_impeller.h b/shell/platform/android/android_context_vulkan_impeller.h index 02e9a305cd039..1800fe00bccc1 100644 --- a/shell/platform/android/android_context_vulkan_impeller.h +++ b/shell/platform/android/android_context_vulkan_impeller.h @@ -14,7 +14,7 @@ namespace flutter { class AndroidContextVulkanImpeller : public AndroidContext { public: - AndroidContextVulkanImpeller(bool enable_validation); + explicit AndroidContextVulkanImpeller(bool enable_validation); ~AndroidContextVulkanImpeller(); diff --git a/shell/platform/android/io/flutter/embedding/engine/loader/FlutterLoader.java b/shell/platform/android/io/flutter/embedding/engine/loader/FlutterLoader.java index da114ae3978d7..1bbeefe9c0ae9 100644 --- a/shell/platform/android/io/flutter/embedding/engine/loader/FlutterLoader.java +++ b/shell/platform/android/io/flutter/embedding/engine/loader/FlutterLoader.java @@ -45,6 +45,8 @@ public class FlutterLoader { "io.flutter.embedding.android.EnableVulkanValidation"; private static final String IMPELLER_BACKEND_META_DATA_KEY = "io.flutter.embedding.android.ImpellerBackend"; + private static final String IMPELLER_OPENGL_GPU_TRACING_DATA_KEY = + "io.flutter.embedding.android.EnableOpenGLGPUTracing"; private static final String DISABLE_IMAGE_READER_PLATFORM_VIEWS_KEY = "io.flutter.embedding.android.DisableImageReaderPlatformViews"; @@ -340,6 +342,9 @@ public void ensureInitializationComplete( ENABLE_VULKAN_VALIDATION_META_DATA_KEY, areValidationLayersOnByDefault())) { shellArgs.add("--enable-vulkan-validation"); } + if (metaData.getBoolean(IMPELLER_OPENGL_GPU_TRACING_DATA_KEY, false)) { + shellArgs.add("--enable-opengl-gpu-tracing"); + } String backend = metaData.getString(IMPELLER_BACKEND_META_DATA_KEY); if (backend != null) { shellArgs.add("--impeller-backend=" + backend); diff --git a/shell/platform/android/platform_view_android.cc b/shell/platform/android/platform_view_android.cc index 485f00eb189e8..c4617c4b99cea 100644 --- a/shell/platform/android/platform_view_android.cc +++ b/shell/platform/android/platform_view_android.cc @@ -70,7 +70,8 @@ static std::shared_ptr CreateAndroidContext( uint8_t msaa_samples, bool enable_impeller, const std::optional& impeller_backend, - bool enable_vulkan_validation) { + bool enable_vulkan_validation, + bool enable_opengl_gpu_tracing) { if (use_software_rendering) { return std::make_shared(AndroidRenderingAPI::kSoftware); } @@ -90,7 +91,8 @@ static std::shared_ptr CreateAndroidContext( switch (backend) { case AndroidRenderingAPI::kOpenGLES: return std::make_unique( - std::make_unique()); + std::make_unique(), + enable_opengl_gpu_tracing); case AndroidRenderingAPI::kVulkan: return std::make_unique( enable_vulkan_validation); @@ -99,7 +101,8 @@ static std::shared_ptr CreateAndroidContext( enable_vulkan_validation); if (!vulkan_backend->IsValid()) { return std::make_unique( - std::make_unique()); + std::make_unique(), + enable_opengl_gpu_tracing); } return vulkan_backend; } @@ -131,7 +134,9 @@ PlatformViewAndroid::PlatformViewAndroid( msaa_samples, delegate.OnPlatformViewGetSettings().enable_impeller, delegate.OnPlatformViewGetSettings().impeller_backend, - delegate.OnPlatformViewGetSettings().enable_vulkan_validation)) {} + delegate.OnPlatformViewGetSettings().enable_vulkan_validation, + delegate.OnPlatformViewGetSettings().enable_opengl_gpu_tracing)) { +} PlatformViewAndroid::PlatformViewAndroid( PlatformView::Delegate& delegate, From cd0c46debf605a4ca303fb85a6a3ba8d4d4bfe23 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Thu, 19 Oct 2023 19:07:47 -0700 Subject: [PATCH 09/10] ++ --- impeller/renderer/backend/gles/gpu_tracer_gles.cc | 1 - impeller/renderer/backend/gles/gpu_tracer_gles.h | 5 +++++ .../embedder/embedder_surface_gl_impeller.cc | 13 +++++++------ 3 files changed, 12 insertions(+), 7 deletions(-) diff --git a/impeller/renderer/backend/gles/gpu_tracer_gles.cc b/impeller/renderer/backend/gles/gpu_tracer_gles.cc index c92a8e88aca9f..0d0f95ae658ed 100644 --- a/impeller/renderer/backend/gles/gpu_tracer_gles.cc +++ b/impeller/renderer/backend/gles/gpu_tracer_gles.cc @@ -68,7 +68,6 @@ void GPUTracerGLES::ProcessQueries(const ProcTableGLES& gl) { FML_TRACE_COUNTER("flutter", "GPUTracer", reinterpret_cast(this), // Trace Counter ID "FrameTimeMS", gpu_ms); - FML_LOG(ERROR) << gpu_ms; gl.DeleteQueriesEXT(1, &query); pending_traces_.pop_front(); } diff --git a/impeller/renderer/backend/gles/gpu_tracer_gles.h b/impeller/renderer/backend/gles/gpu_tracer_gles.h index 45af352705480..8de6963fc6759 100644 --- a/impeller/renderer/backend/gles/gpu_tracer_gles.h +++ b/impeller/renderer/backend/gles/gpu_tracer_gles.h @@ -17,6 +17,11 @@ namespace impeller { /// known to cause crashes. As a result, this functionality is disabled by /// default and can only be enabled in debug/profile mode via a specific opt-in /// flag that is exposed in the Android manifest. +/// +/// To enable, add the following metadata to the application's Android manifest: +/// class GPUTracerGLES { public: GPUTracerGLES(const ProcTableGLES& gl, bool enable_tracing); diff --git a/shell/platform/embedder/embedder_surface_gl_impeller.cc b/shell/platform/embedder/embedder_surface_gl_impeller.cc index 340725d2b9d3b..c061c091a912b 100644 --- a/shell/platform/embedder/embedder_surface_gl_impeller.cc +++ b/shell/platform/embedder/embedder_surface_gl_impeller.cc @@ -65,11 +65,12 @@ EmbedderSurfaceGLImpeller::EmbedderSurfaceGLImpeller( gl_dispatch_table_.gl_make_current_callback(); std::vector> shader_mappings = { - std::make_shared(impeller_entity_shaders_gles_data, - impeller_entity_shaders_gles_length), + std::make_shared( + impeller_entity_shaders_gles_data, + impeller_entity_shaders_gles_length), #if IMPELLER_ENABLE_3D - std::make_shared(impeller_scene_shaders_gles_data, - impeller_scene_shaders_gles_length), + std::make_shared( + impeller_scene_shaders_gles_data, impeller_scene_shaders_gles_length), #endif // IMPELLER_ENABLE_3D }; auto gl = std::make_unique( @@ -78,8 +79,8 @@ EmbedderSurfaceGLImpeller::EmbedderSurfaceGLImpeller( return; } - impeller_context_ = - impeller::ContextGLES::Create(std::move(gl), shader_mappings); + impeller_context_ = impeller::ContextGLES::Create( + std::move(gl), shader_mappings, /*enable_gpu_tracing=*/false); if (!impeller_context_) { FML_LOG(ERROR) << "Could not create Impeller context."; From 6a93525a0f11956b6ee06b5e986e0d17937cc500 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Fri, 20 Oct 2023 09:49:14 -0700 Subject: [PATCH 10/10] ++ --- impeller/renderer/backend/gles/gpu_tracer_gles.cc | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/impeller/renderer/backend/gles/gpu_tracer_gles.cc b/impeller/renderer/backend/gles/gpu_tracer_gles.cc index 0d0f95ae658ed..6e1e167877b3d 100644 --- a/impeller/renderer/backend/gles/gpu_tracer_gles.cc +++ b/impeller/renderer/backend/gles/gpu_tracer_gles.cc @@ -32,7 +32,6 @@ void GPUTracerGLES::MarkFrameStart(const ProcTableGLES& gl) { return; } - FML_DCHECK(!active_frame_.has_value()); active_frame_ = query; gl.BeginQueryEXT(GL_TIME_ELAPSED_EXT, query); } @@ -75,7 +74,7 @@ void GPUTracerGLES::ProcessQueries(const ProcTableGLES& gl) { void GPUTracerGLES::MarkFrameEnd(const ProcTableGLES& gl) { if (!enabled_ || std::this_thread::get_id() != raster_thread_ || - !active_frame_.has_value() || !active_frame_.has_value()) { + !active_frame_.has_value()) { return; }