From e2af0eeb9bf528ffd0c6c83d0a86634b31fa82fb Mon Sep 17 00:00:00 2001 From: Alan Date: Wed, 17 Aug 2022 15:27:50 +0100 Subject: [PATCH 1/5] Hide ANDROID_frame_boundary behind a flag / UI checkbox --- cmd/gapit/flags.go | 1 + cmd/gapit/trace.go | 1 + .../com/google/gapid/views/TracerDialog.java | 4 ++++ gapii/cc/call_observer.cpp | 4 ++++ gapii/cc/call_observer.h | 3 +++ gapii/cc/connection_header.h | 2 ++ gapii/cc/spy.cpp | 4 +++- gapii/cc/spy.h | 3 +++ gapii/cc/spy_base.h | 3 +++ gapii/client/capture.go | 3 +++ gapis/api/templates/api_spy.cpp.tmpl | 17 ++++++++++++++++- .../extensions/android_frame_boundary.api | 2 +- gapis/service/service.proto | 3 ++- gapis/trace/tracer/tracer.go | 3 +++ 14 files changed, 49 insertions(+), 4 deletions(-) diff --git a/cmd/gapit/flags.go b/cmd/gapit/flags.go index 1687db4ba..73fb7fa79 100644 --- a/cmd/gapit/flags.go +++ b/cmd/gapit/flags.go @@ -322,6 +322,7 @@ type ( Capture struct { Frames int `help:"only capture the given number of frames. 0 for all"` } + RespectAndroidFrameBoundary bool `help:"If true: respect the ANDROID_frame_boundary extension. If false: ignore."` No struct { Buffer bool `help:"Do not buffer the output, this helps if the application crashes"` } diff --git a/cmd/gapit/trace.go b/cmd/gapit/trace.go index 5639b0e0f..31f9e0980 100644 --- a/cmd/gapit/trace.go +++ b/cmd/gapit/trace.go @@ -237,6 +237,7 @@ func (verb *traceVerb) Run(ctx context.Context, flags flag.FlagSet) error { StartFrame: uint32(verb.Start.At.Frame), FramesToCapture: uint32(verb.Capture.Frames), DeferStart: verb.Start.Defer, + RespectAndroidFrameBoundary: verb.RespectAndroidFrameBoundary, NoBuffer: verb.No.Buffer, HideUnknownExtensions: verb.Disable.Unknown.Extensions, RecordTraceTimes: verb.Record.TraceTimes, diff --git a/gapic/src/main/com/google/gapid/views/TracerDialog.java b/gapic/src/main/com/google/gapid/views/TracerDialog.java index 7ac9b80f4..105c0be92 100644 --- a/gapic/src/main/com/google/gapid/views/TracerDialog.java +++ b/gapic/src/main/com/google/gapid/views/TracerDialog.java @@ -336,6 +336,7 @@ private static class TraceInput extends Composite { private final Spinner duration; private final Label durationUnit; private final Label startUnit; + private final Button respectAndroidFrameBoundary; private final Button withoutBuffering; private final Button includeUnsupportedExtensions; private final Button loadValidationLayer; @@ -489,6 +490,8 @@ protected String createAndShowDialog(String current) { duration.setVisible(DURATION_FRAMES_MAX > 1); durationUnit = createLabel(durGroup, DURATION_FRAMES_UNIT); durationUnit.setVisible(DURATION_FRAMES_MAX > 1); + respectAndroidFrameBoundary = createCheckbox(durGroup, "Respect ANDROID_frame_boundary", false); + respectAndroidFrameBoundary.setEnabled(true); Group optGroup = withLayoutData( createGroup(this, "Trace Options", new GridLayout(2, false)), @@ -1028,6 +1031,7 @@ public TraceRequest getTraceRequest(Settings settings) { .setUri(traceTarget.getText()) .setAdditionalCommandLineArgs(arguments.getText()) .setFramesToCapture(duration.getSelection()) + .setRespectAndroidFrameBoundary(respectAndroidFrameBoundary.getSelection()) .setNoBuffer(withoutBuffering.getSelection()) .setHideUnknownExtensions(!includeUnsupportedExtensions.getSelection()) .setServerLocalSavePath(output.getAbsolutePath()) diff --git a/gapii/cc/call_observer.cpp b/gapii/cc/call_observer.cpp index a3cda6bbb..8c0afaf59 100644 --- a/gapii/cc/call_observer.cpp +++ b/gapii/cc/call_observer.cpp @@ -132,6 +132,10 @@ void CallObserver::observeTimestamp() { encode_message(×tamp); } +bool CallObserver::respectAndroidFrameBoundary() { + return mSpy->respectAndroidFrameBoundary(); +} + void CallObserver::enter(const ::google::protobuf::Message* cmd) { endTraceIfRequested(); if (!mShouldTrace) { diff --git a/gapii/cc/call_observer.h b/gapii/cc/call_observer.h index 98272c40c..c6423780c 100644 --- a/gapii/cc/call_observer.h +++ b/gapii/cc/call_observer.h @@ -202,6 +202,9 @@ class CallObserver : gapil::Encoder { // observeTimestamp encodes a timestamp extra in the trace void observeTimestamp(); + // ALAN TODO + bool respectAndroidFrameBoundary(); + private: // shouldObserve returns true if the given slice is located in application // pool and we are supposed to observe application pool. diff --git a/gapii/cc/connection_header.h b/gapii/cc/connection_header.h index 532e438f7..a4c9a89e8 100644 --- a/gapii/cc/connection_header.h +++ b/gapii/cc/connection_header.h @@ -56,6 +56,8 @@ class ConnectionHeader { static const uint32_t FLAG_DISABLE_COHERENT_MEMORY_TRACKER = 0x00000100; // Waits for the debugger to attach (useful for debug) static const uint32_t FLAG_WAIT_FOR_DEBUGGER = 0x00000200; + // Set to enable use of ANDROID_frame_boundary extension + static const uint32_t FLAG_RESPECT_ANDROID_FRAME_BOUNDARY = 0x00001000; // read reads the ConnectionHeader from the provided stream, returning true // on success or false on error. diff --git a/gapii/cc/spy.cpp b/gapii/cc/spy.cpp index 4ab4d2df4..b2cfacebd 100644 --- a/gapii/cc/spy.cpp +++ b/gapii/cc/spy.cpp @@ -93,7 +93,8 @@ Spy::Spy() mSuspendCaptureFrames(0), mCaptureFrames(0), mObserveFrameFrequency(0), - mFrameNumber(0) { + mFrameNumber(0), + mRespectAndroidFrameBoundary(false) { // Start by checking whether to capture the current process: compare the // current process name with the "capture_proc_name" that we get from the // environment. An empty "capture_proc_name" means capture any process. This @@ -166,6 +167,7 @@ Spy::Spy() ? kSuspendIndefinitely : header.mStartFrame; mCaptureFrames = header.mNumFrames; + mRespectAndroidFrameBoundary = (header.mFlags & ConnectionHeader::FLAG_RESPECT_ANDROID_FRAME_BOUNDARY) != 0; set_valid_apis(header.mAPIs); GAPID_ERROR("APIS %08x", header.mAPIs); diff --git a/gapii/cc/spy.h b/gapii/cc/spy.h index cc5892511..41860f07c 100644 --- a/gapii/cc/spy.h +++ b/gapii/cc/spy.h @@ -85,6 +85,9 @@ class Spy : public VulkanSpy { int mObserveFrameFrequency; uint64_t mFrameNumber; + bool mRespectAndroidFrameBoundary; + bool respectAndroidFrameBoundary() override { return mRespectAndroidFrameBoundary; } + std::unique_ptr mMessageReceiverJob; friend struct spy_creator; diff --git a/gapii/cc/spy_base.h b/gapii/cc/spy_base.h index ed3862c6e..bbce08365 100644 --- a/gapii/cc/spy_base.h +++ b/gapii/cc/spy_base.h @@ -110,6 +110,9 @@ class SpyBase { void set_record_timestamps(bool record) { mRecordTimestamps = record; } bool should_record_timestamps() const { return mRecordTimestamps; } + // TODO ALAN + virtual bool respectAndroidFrameBoundary() { return false; } + // Ends the current trace if requested by client. virtual void endTraceIfRequested() {} diff --git a/gapii/client/capture.go b/gapii/client/capture.go index 4680c71d6..0d814b28e 100644 --- a/gapii/client/capture.go +++ b/gapii/client/capture.go @@ -52,6 +52,9 @@ const ( DisableCoherentMemoryTracker Flags = 0x000000100 // WaitForDebugger makes gapii wait for a debugger to connect WaitForDebugger Flags = 0x000000200 + // FLAG_RESPECT_ANDROID_FRAME_BOUNDARY + RespectAndroidFrameBoundary = 0x00001000 + // VulkanAPI is hard-coded bit mask for Vulkan API, it needs to be kept in sync // with the api_index in the vulkan.api file. diff --git a/gapis/api/templates/api_spy.cpp.tmpl b/gapis/api/templates/api_spy.cpp.tmpl index 18b26a569..d11f39fad 100644 --- a/gapis/api/templates/api_spy.cpp.tmpl +++ b/gapis/api/templates/api_spy.cpp.tmpl @@ -219,6 +219,12 @@ called = true; {{if GetAnnotation $ "frame_end"}} onPreEndOfFrame(observer, {{Global "ApiIndex"}}); + {{else}} + {{if GetAnnotation $ "frame_delimiter"}} + if(observer->respectAndroidFrameBoundary()) { + onPreEndOfFrame(observer, {{Global "ApiIndex"}}); + } + {{end}} {{end}} observer->observePending(); @@ -272,7 +278,16 @@ observer->observePending(); observer->exit(); - {{if GetAnnotation $ "frame_end"}}onPostEndOfFrame();{{end}} + {{if GetAnnotation $ "frame_end"}} + onPostEndOfFrame(); + {{else}} + {{if GetAnnotation $ "frame_delimiter"}} + if(observer->respectAndroidFrameBoundary()) { + onPostEndOfFrame(); + } + {{end}} + {{end}} + {{if not (IsVoid $.Return.Type)}}¶ return result; {{end}} diff --git a/gapis/api/vulkan/extensions/android_frame_boundary.api b/gapis/api/vulkan/extensions/android_frame_boundary.api index 6fde3a423..e2c8288d6 100644 --- a/gapis/api/vulkan/extensions/android_frame_boundary.api +++ b/gapis/api/vulkan/extensions/android_frame_boundary.api @@ -35,7 +35,7 @@ @extension("VK_ANDROID_frame_boundary") @indirect("VkDevice") @override -@frame_end +@frame_delimiter @no_replay cmd void vkFrameBoundaryANDROID( VkDevice device, diff --git a/gapis/service/service.proto b/gapis/service/service.proto index 4fdd32bba..a67481b88 100644 --- a/gapis/service/service.proto +++ b/gapis/service/service.proto @@ -1384,7 +1384,8 @@ message TraceOptions { uint32 start_frame = 12; // How many frames should we capture uint32 frames_to_capture = 13; - reserved 14; + // Set whether to respect the ANDROID_frame_boundary extension + bool respect_android_frame_boundary = 14; // Insert extra commands to record error state bool record_error_state = 15; // Wait for an event to start tracing diff --git a/gapis/trace/tracer/tracer.go b/gapis/trace/tracer/tracer.go index 98fe98616..56ccd5bde 100644 --- a/gapis/trace/tracer/tracer.go +++ b/gapis/trace/tracer/tracer.go @@ -127,6 +127,9 @@ func GapiiOptions(o *service.TraceOptions) gapii.Options { if o.WaitForDebugger { flags |= gapii.WaitForDebugger } + if o.RespectAndroidFrameBoundary { + flags |= gapii.RespectAndroidFrameBoundary + } return gapii.Options{ o.ObserveFrameFrequency, From 84d3bc3b5136340c494efba924d8d242855d1b98 Mon Sep 17 00:00:00 2001 From: Alan Date: Wed, 17 Aug 2022 16:00:03 +0100 Subject: [PATCH 2/5] Generalise the name of frame boundary delimiters --- cmd/gapit/trace.go | 40 +++++++++---------- .../com/google/gapid/views/TracerDialog.java | 2 +- gapii/cc/call_observer.cpp | 4 +- gapii/cc/call_observer.h | 4 +- gapii/cc/connection_header.h | 4 +- gapii/cc/spy.cpp | 4 +- gapii/cc/spy.h | 4 +- gapii/cc/spy_base.h | 4 +- gapii/client/capture.go | 2 +- gapis/api/templates/api_spy.cpp.tmpl | 4 +- gapis/service/service.proto | 2 +- gapis/trace/tracer/tracer.go | 4 +- 12 files changed, 39 insertions(+), 39 deletions(-) diff --git a/cmd/gapit/trace.go b/cmd/gapit/trace.go index 31f9e0980..437485503 100644 --- a/cmd/gapit/trace.go +++ b/cmd/gapit/trace.go @@ -228,26 +228,26 @@ func (verb *traceVerb) Run(ctx context.Context, flags flag.FlagSet) error { } options := &service.TraceOptions{ - Type: api.traceType, - AdditionalCommandLineArgs: verb.AdditionalArgs, - Cwd: verb.WorkingDir, - Environment: verb.Env, - Duration: float32(verb.For.Seconds()), - ObserveFrameFrequency: uint32(verb.Observe.Frames), - StartFrame: uint32(verb.Start.At.Frame), - FramesToCapture: uint32(verb.Capture.Frames), - DeferStart: verb.Start.Defer, - RespectAndroidFrameBoundary: verb.RespectAndroidFrameBoundary, - NoBuffer: verb.No.Buffer, - HideUnknownExtensions: verb.Disable.Unknown.Extensions, - RecordTraceTimes: verb.Record.TraceTimes, - ClearCache: verb.Clear.Cache, - ServerLocalSavePath: out, - PipeName: verb.PipeName, - DisableCoherentMemoryTracker: verb.Disable.CoherentMemoryTracker, - WaitForDebugger: verb.WaitForDebugger, - ProcessName: verb.ProcessName, - LoadValidationLayer: verb.LoadValidationLayer, + Type: api.traceType, + AdditionalCommandLineArgs: verb.AdditionalArgs, + Cwd: verb.WorkingDir, + Environment: verb.Env, + Duration: float32(verb.For.Seconds()), + ObserveFrameFrequency: uint32(verb.Observe.Frames), + StartFrame: uint32(verb.Start.At.Frame), + FramesToCapture: uint32(verb.Capture.Frames), + DeferStart: verb.Start.Defer, + RespectFrameBoundaryDelimiters: verb.RespectAndroidFrameBoundary, + NoBuffer: verb.No.Buffer, + HideUnknownExtensions: verb.Disable.Unknown.Extensions, + RecordTraceTimes: verb.Record.TraceTimes, + ClearCache: verb.Clear.Cache, + ServerLocalSavePath: out, + PipeName: verb.PipeName, + DisableCoherentMemoryTracker: verb.Disable.CoherentMemoryTracker, + WaitForDebugger: verb.WaitForDebugger, + ProcessName: verb.ProcessName, + LoadValidationLayer: verb.LoadValidationLayer, } target(options) diff --git a/gapic/src/main/com/google/gapid/views/TracerDialog.java b/gapic/src/main/com/google/gapid/views/TracerDialog.java index 105c0be92..1e3a7e165 100644 --- a/gapic/src/main/com/google/gapid/views/TracerDialog.java +++ b/gapic/src/main/com/google/gapid/views/TracerDialog.java @@ -1031,7 +1031,7 @@ public TraceRequest getTraceRequest(Settings settings) { .setUri(traceTarget.getText()) .setAdditionalCommandLineArgs(arguments.getText()) .setFramesToCapture(duration.getSelection()) - .setRespectAndroidFrameBoundary(respectAndroidFrameBoundary.getSelection()) + .setRespectFrameBoundaryDelimiters(respectAndroidFrameBoundary.getSelection()) .setNoBuffer(withoutBuffering.getSelection()) .setHideUnknownExtensions(!includeUnsupportedExtensions.getSelection()) .setServerLocalSavePath(output.getAbsolutePath()) diff --git a/gapii/cc/call_observer.cpp b/gapii/cc/call_observer.cpp index 8c0afaf59..dc47ec256 100644 --- a/gapii/cc/call_observer.cpp +++ b/gapii/cc/call_observer.cpp @@ -132,8 +132,8 @@ void CallObserver::observeTimestamp() { encode_message(×tamp); } -bool CallObserver::respectAndroidFrameBoundary() { - return mSpy->respectAndroidFrameBoundary(); +bool CallObserver::respectFrameBoundaryDelimiters() { + return mSpy->respectFrameBoundaryDelimiters(); } void CallObserver::enter(const ::google::protobuf::Message* cmd) { diff --git a/gapii/cc/call_observer.h b/gapii/cc/call_observer.h index c6423780c..7472472f7 100644 --- a/gapii/cc/call_observer.h +++ b/gapii/cc/call_observer.h @@ -202,8 +202,8 @@ class CallObserver : gapil::Encoder { // observeTimestamp encodes a timestamp extra in the trace void observeTimestamp(); - // ALAN TODO - bool respectAndroidFrameBoundary(); + // If true, respect frame delimiter hints (ie, ANDROID_frame_boundary) + bool respectFrameBoundaryDelimiters(); private: // shouldObserve returns true if the given slice is located in application diff --git a/gapii/cc/connection_header.h b/gapii/cc/connection_header.h index a4c9a89e8..98cdf5913 100644 --- a/gapii/cc/connection_header.h +++ b/gapii/cc/connection_header.h @@ -56,8 +56,8 @@ class ConnectionHeader { static const uint32_t FLAG_DISABLE_COHERENT_MEMORY_TRACKER = 0x00000100; // Waits for the debugger to attach (useful for debug) static const uint32_t FLAG_WAIT_FOR_DEBUGGER = 0x00000200; - // Set to enable use of ANDROID_frame_boundary extension - static const uint32_t FLAG_RESPECT_ANDROID_FRAME_BOUNDARY = 0x00001000; + // Set to enable use of frame delimiters, eg ANDROID_frame_boundary extension + static const uint32_t FLAG_RESPECT_FRAME_BOUNDARY_DELIMITERS = 0x00001000; // read reads the ConnectionHeader from the provided stream, returning true // on success or false on error. diff --git a/gapii/cc/spy.cpp b/gapii/cc/spy.cpp index b2cfacebd..349c4fbc6 100644 --- a/gapii/cc/spy.cpp +++ b/gapii/cc/spy.cpp @@ -94,7 +94,7 @@ Spy::Spy() mCaptureFrames(0), mObserveFrameFrequency(0), mFrameNumber(0), - mRespectAndroidFrameBoundary(false) { + mRespectFrameBoundaryDelimiters(false) { // Start by checking whether to capture the current process: compare the // current process name with the "capture_proc_name" that we get from the // environment. An empty "capture_proc_name" means capture any process. This @@ -167,7 +167,7 @@ Spy::Spy() ? kSuspendIndefinitely : header.mStartFrame; mCaptureFrames = header.mNumFrames; - mRespectAndroidFrameBoundary = (header.mFlags & ConnectionHeader::FLAG_RESPECT_ANDROID_FRAME_BOUNDARY) != 0; + mRespectFrameBoundaryDelimiters = (header.mFlags & ConnectionHeader::FLAG_RESPECT_FRAME_BOUNDARY_DELIMITERS) != 0; set_valid_apis(header.mAPIs); GAPID_ERROR("APIS %08x", header.mAPIs); diff --git a/gapii/cc/spy.h b/gapii/cc/spy.h index 41860f07c..43cc9cb36 100644 --- a/gapii/cc/spy.h +++ b/gapii/cc/spy.h @@ -85,8 +85,8 @@ class Spy : public VulkanSpy { int mObserveFrameFrequency; uint64_t mFrameNumber; - bool mRespectAndroidFrameBoundary; - bool respectAndroidFrameBoundary() override { return mRespectAndroidFrameBoundary; } + bool mRespectFrameBoundaryDelimiters; + bool respectFrameBoundaryDelimiters() override { return mRespectFrameBoundaryDelimiters; } std::unique_ptr mMessageReceiverJob; diff --git a/gapii/cc/spy_base.h b/gapii/cc/spy_base.h index bbce08365..de84fbe04 100644 --- a/gapii/cc/spy_base.h +++ b/gapii/cc/spy_base.h @@ -110,8 +110,8 @@ class SpyBase { void set_record_timestamps(bool record) { mRecordTimestamps = record; } bool should_record_timestamps() const { return mRecordTimestamps; } - // TODO ALAN - virtual bool respectAndroidFrameBoundary() { return false; } + // If true, respect frame delimiter extensions, eg ANDROID_frame_boundary + virtual bool respectFrameBoundaryDelimiters() { return false; } // Ends the current trace if requested by client. virtual void endTraceIfRequested() {} diff --git a/gapii/client/capture.go b/gapii/client/capture.go index 0d814b28e..8ff9ae3fd 100644 --- a/gapii/client/capture.go +++ b/gapii/client/capture.go @@ -53,7 +53,7 @@ const ( // WaitForDebugger makes gapii wait for a debugger to connect WaitForDebugger Flags = 0x000000200 // FLAG_RESPECT_ANDROID_FRAME_BOUNDARY - RespectAndroidFrameBoundary = 0x00001000 + RespectFrameBoundaryDelimiters = 0x00001000 // VulkanAPI is hard-coded bit mask for Vulkan API, it needs to be kept in sync diff --git a/gapis/api/templates/api_spy.cpp.tmpl b/gapis/api/templates/api_spy.cpp.tmpl index d11f39fad..8a41768e4 100644 --- a/gapis/api/templates/api_spy.cpp.tmpl +++ b/gapis/api/templates/api_spy.cpp.tmpl @@ -221,7 +221,7 @@ onPreEndOfFrame(observer, {{Global "ApiIndex"}}); {{else}} {{if GetAnnotation $ "frame_delimiter"}} - if(observer->respectAndroidFrameBoundary()) { + if(observer->respectFrameBoundaryDelimiters()) { onPreEndOfFrame(observer, {{Global "ApiIndex"}}); } {{end}} @@ -282,7 +282,7 @@ onPostEndOfFrame(); {{else}} {{if GetAnnotation $ "frame_delimiter"}} - if(observer->respectAndroidFrameBoundary()) { + if(observer->respectFrameBoundaryDelimiters()) { onPostEndOfFrame(); } {{end}} diff --git a/gapis/service/service.proto b/gapis/service/service.proto index a67481b88..4d29ad7dc 100644 --- a/gapis/service/service.proto +++ b/gapis/service/service.proto @@ -1385,7 +1385,7 @@ message TraceOptions { // How many frames should we capture uint32 frames_to_capture = 13; // Set whether to respect the ANDROID_frame_boundary extension - bool respect_android_frame_boundary = 14; + bool respect_frame_boundary_delimiters = 14; // Insert extra commands to record error state bool record_error_state = 15; // Wait for an event to start tracing diff --git a/gapis/trace/tracer/tracer.go b/gapis/trace/tracer/tracer.go index 56ccd5bde..761be9d78 100644 --- a/gapis/trace/tracer/tracer.go +++ b/gapis/trace/tracer/tracer.go @@ -127,8 +127,8 @@ func GapiiOptions(o *service.TraceOptions) gapii.Options { if o.WaitForDebugger { flags |= gapii.WaitForDebugger } - if o.RespectAndroidFrameBoundary { - flags |= gapii.RespectAndroidFrameBoundary + if o.RespectFrameBoundaryDelimiters { + flags |= gapii.RespectFrameBoundaryDelimiters } return gapii.Options{ From b36880c777d91e026aedef46f8c9da3ae1f76a18 Mon Sep 17 00:00:00 2001 From: Alan Date: Fri, 19 Aug 2022 12:19:36 +0100 Subject: [PATCH 3/5] Change respect to ignore --- cmd/gapit/flags.go | 2 +- cmd/gapit/trace.go | 2 +- gapic/src/main/com/google/gapid/views/TracerDialog.java | 8 ++++---- gapii/cc/call_observer.cpp | 4 ++-- gapii/cc/call_observer.h | 4 ++-- gapii/cc/connection_header.h | 2 +- gapii/cc/spy.cpp | 4 ++-- gapii/cc/spy.h | 4 ++-- gapii/cc/spy_base.h | 4 ++-- gapii/client/capture.go | 4 ++-- gapis/api/templates/api_spy.cpp.tmpl | 4 ++-- gapis/service/service.proto | 4 ++-- gapis/trace/tracer/tracer.go | 4 ++-- 13 files changed, 25 insertions(+), 25 deletions(-) diff --git a/cmd/gapit/flags.go b/cmd/gapit/flags.go index 73fb7fa79..0c83e660f 100644 --- a/cmd/gapit/flags.go +++ b/cmd/gapit/flags.go @@ -322,7 +322,7 @@ type ( Capture struct { Frames int `help:"only capture the given number of frames. 0 for all"` } - RespectAndroidFrameBoundary bool `help:"If true: respect the ANDROID_frame_boundary extension. If false: ignore."` + IgnoreAndroidFrameBoundary bool `help:"If true: ignore the ANDROID_frame_boundary extension. If false: respect it."` No struct { Buffer bool `help:"Do not buffer the output, this helps if the application crashes"` } diff --git a/cmd/gapit/trace.go b/cmd/gapit/trace.go index 437485503..3e8218ab6 100644 --- a/cmd/gapit/trace.go +++ b/cmd/gapit/trace.go @@ -237,7 +237,7 @@ func (verb *traceVerb) Run(ctx context.Context, flags flag.FlagSet) error { StartFrame: uint32(verb.Start.At.Frame), FramesToCapture: uint32(verb.Capture.Frames), DeferStart: verb.Start.Defer, - RespectFrameBoundaryDelimiters: verb.RespectAndroidFrameBoundary, + IgnoreFrameBoundaryDelimiters: verb.IgnoreAndroidFrameBoundary, NoBuffer: verb.No.Buffer, HideUnknownExtensions: verb.Disable.Unknown.Extensions, RecordTraceTimes: verb.Record.TraceTimes, diff --git a/gapic/src/main/com/google/gapid/views/TracerDialog.java b/gapic/src/main/com/google/gapid/views/TracerDialog.java index 1e3a7e165..fc7180090 100644 --- a/gapic/src/main/com/google/gapid/views/TracerDialog.java +++ b/gapic/src/main/com/google/gapid/views/TracerDialog.java @@ -336,7 +336,7 @@ private static class TraceInput extends Composite { private final Spinner duration; private final Label durationUnit; private final Label startUnit; - private final Button respectAndroidFrameBoundary; + private final Button ignoreAndroidFrameBoundary; private final Button withoutBuffering; private final Button includeUnsupportedExtensions; private final Button loadValidationLayer; @@ -490,8 +490,8 @@ protected String createAndShowDialog(String current) { duration.setVisible(DURATION_FRAMES_MAX > 1); durationUnit = createLabel(durGroup, DURATION_FRAMES_UNIT); durationUnit.setVisible(DURATION_FRAMES_MAX > 1); - respectAndroidFrameBoundary = createCheckbox(durGroup, "Respect ANDROID_frame_boundary", false); - respectAndroidFrameBoundary.setEnabled(true); + ignoreAndroidFrameBoundary = createCheckbox(durGroup, "Ignore ANDROID_frame_boundary extension", true); + ignoreAndroidFrameBoundary.setEnabled(true); Group optGroup = withLayoutData( createGroup(this, "Trace Options", new GridLayout(2, false)), @@ -1031,7 +1031,7 @@ public TraceRequest getTraceRequest(Settings settings) { .setUri(traceTarget.getText()) .setAdditionalCommandLineArgs(arguments.getText()) .setFramesToCapture(duration.getSelection()) - .setRespectFrameBoundaryDelimiters(respectAndroidFrameBoundary.getSelection()) + .setIgnoreFrameBoundaryDelimiters(ignoreAndroidFrameBoundary.getSelection()) .setNoBuffer(withoutBuffering.getSelection()) .setHideUnknownExtensions(!includeUnsupportedExtensions.getSelection()) .setServerLocalSavePath(output.getAbsolutePath()) diff --git a/gapii/cc/call_observer.cpp b/gapii/cc/call_observer.cpp index dc47ec256..78281cd6b 100644 --- a/gapii/cc/call_observer.cpp +++ b/gapii/cc/call_observer.cpp @@ -132,8 +132,8 @@ void CallObserver::observeTimestamp() { encode_message(×tamp); } -bool CallObserver::respectFrameBoundaryDelimiters() { - return mSpy->respectFrameBoundaryDelimiters(); +bool CallObserver::ignoreFrameBoundaryDelimiters() { + return mSpy->ignoreFrameBoundaryDelimiters(); } void CallObserver::enter(const ::google::protobuf::Message* cmd) { diff --git a/gapii/cc/call_observer.h b/gapii/cc/call_observer.h index 7472472f7..678c34959 100644 --- a/gapii/cc/call_observer.h +++ b/gapii/cc/call_observer.h @@ -202,8 +202,8 @@ class CallObserver : gapil::Encoder { // observeTimestamp encodes a timestamp extra in the trace void observeTimestamp(); - // If true, respect frame delimiter hints (ie, ANDROID_frame_boundary) - bool respectFrameBoundaryDelimiters(); + // If true, ignore frame delimiter hints (ie, ANDROID_frame_boundary) + bool ignoreFrameBoundaryDelimiters(); private: // shouldObserve returns true if the given slice is located in application diff --git a/gapii/cc/connection_header.h b/gapii/cc/connection_header.h index 98cdf5913..609f5ce28 100644 --- a/gapii/cc/connection_header.h +++ b/gapii/cc/connection_header.h @@ -57,7 +57,7 @@ class ConnectionHeader { // Waits for the debugger to attach (useful for debug) static const uint32_t FLAG_WAIT_FOR_DEBUGGER = 0x00000200; // Set to enable use of frame delimiters, eg ANDROID_frame_boundary extension - static const uint32_t FLAG_RESPECT_FRAME_BOUNDARY_DELIMITERS = 0x00001000; + static const uint32_t FLAG_IGNORE_FRAME_BOUNDARY_DELIMITERS = 0x00001000; // read reads the ConnectionHeader from the provided stream, returning true // on success or false on error. diff --git a/gapii/cc/spy.cpp b/gapii/cc/spy.cpp index 349c4fbc6..0db39a493 100644 --- a/gapii/cc/spy.cpp +++ b/gapii/cc/spy.cpp @@ -94,7 +94,7 @@ Spy::Spy() mCaptureFrames(0), mObserveFrameFrequency(0), mFrameNumber(0), - mRespectFrameBoundaryDelimiters(false) { + mIgnoreFrameBoundaryDelimiters(false) { // Start by checking whether to capture the current process: compare the // current process name with the "capture_proc_name" that we get from the // environment. An empty "capture_proc_name" means capture any process. This @@ -167,7 +167,7 @@ Spy::Spy() ? kSuspendIndefinitely : header.mStartFrame; mCaptureFrames = header.mNumFrames; - mRespectFrameBoundaryDelimiters = (header.mFlags & ConnectionHeader::FLAG_RESPECT_FRAME_BOUNDARY_DELIMITERS) != 0; + mIgnoreFrameBoundaryDelimiters = (header.mFlags & ConnectionHeader::FLAG_IGNORE_FRAME_BOUNDARY_DELIMITERS) != 0; set_valid_apis(header.mAPIs); GAPID_ERROR("APIS %08x", header.mAPIs); diff --git a/gapii/cc/spy.h b/gapii/cc/spy.h index 43cc9cb36..879a3c8b4 100644 --- a/gapii/cc/spy.h +++ b/gapii/cc/spy.h @@ -85,8 +85,8 @@ class Spy : public VulkanSpy { int mObserveFrameFrequency; uint64_t mFrameNumber; - bool mRespectFrameBoundaryDelimiters; - bool respectFrameBoundaryDelimiters() override { return mRespectFrameBoundaryDelimiters; } + bool mIgnoreFrameBoundaryDelimiters; + bool ignoreFrameBoundaryDelimiters() override { return mIgnoreFrameBoundaryDelimiters; } std::unique_ptr mMessageReceiverJob; diff --git a/gapii/cc/spy_base.h b/gapii/cc/spy_base.h index de84fbe04..4fcd470bc 100644 --- a/gapii/cc/spy_base.h +++ b/gapii/cc/spy_base.h @@ -110,8 +110,8 @@ class SpyBase { void set_record_timestamps(bool record) { mRecordTimestamps = record; } bool should_record_timestamps() const { return mRecordTimestamps; } - // If true, respect frame delimiter extensions, eg ANDROID_frame_boundary - virtual bool respectFrameBoundaryDelimiters() { return false; } + // If true, ignore frame delimiter extensions, eg ANDROID_frame_boundary + virtual bool ignoreFrameBoundaryDelimiters() { return true; } // Ends the current trace if requested by client. virtual void endTraceIfRequested() {} diff --git a/gapii/client/capture.go b/gapii/client/capture.go index 8ff9ae3fd..cec6b5425 100644 --- a/gapii/client/capture.go +++ b/gapii/client/capture.go @@ -52,8 +52,8 @@ const ( DisableCoherentMemoryTracker Flags = 0x000000100 // WaitForDebugger makes gapii wait for a debugger to connect WaitForDebugger Flags = 0x000000200 - // FLAG_RESPECT_ANDROID_FRAME_BOUNDARY - RespectFrameBoundaryDelimiters = 0x00001000 + // FLAG_IGNORE_ANDROID_FRAME_BOUNDARY + IgnoreFrameBoundaryDelimiters Flags = 0x00001000 // VulkanAPI is hard-coded bit mask for Vulkan API, it needs to be kept in sync diff --git a/gapis/api/templates/api_spy.cpp.tmpl b/gapis/api/templates/api_spy.cpp.tmpl index 8a41768e4..b6693a9fa 100644 --- a/gapis/api/templates/api_spy.cpp.tmpl +++ b/gapis/api/templates/api_spy.cpp.tmpl @@ -221,7 +221,7 @@ onPreEndOfFrame(observer, {{Global "ApiIndex"}}); {{else}} {{if GetAnnotation $ "frame_delimiter"}} - if(observer->respectFrameBoundaryDelimiters()) { + if(!observer->ignoreFrameBoundaryDelimiters()) { onPreEndOfFrame(observer, {{Global "ApiIndex"}}); } {{end}} @@ -282,7 +282,7 @@ onPostEndOfFrame(); {{else}} {{if GetAnnotation $ "frame_delimiter"}} - if(observer->respectFrameBoundaryDelimiters()) { + if(!observer->ignoreFrameBoundaryDelimiters()) { onPostEndOfFrame(); } {{end}} diff --git a/gapis/service/service.proto b/gapis/service/service.proto index 4d29ad7dc..82229766e 100644 --- a/gapis/service/service.proto +++ b/gapis/service/service.proto @@ -1384,8 +1384,8 @@ message TraceOptions { uint32 start_frame = 12; // How many frames should we capture uint32 frames_to_capture = 13; - // Set whether to respect the ANDROID_frame_boundary extension - bool respect_frame_boundary_delimiters = 14; + // Set whether to ignore the ANDROID_frame_boundary extension + bool ignore_frame_boundary_delimiters = 14; // Insert extra commands to record error state bool record_error_state = 15; // Wait for an event to start tracing diff --git a/gapis/trace/tracer/tracer.go b/gapis/trace/tracer/tracer.go index 761be9d78..ff9ac65dc 100644 --- a/gapis/trace/tracer/tracer.go +++ b/gapis/trace/tracer/tracer.go @@ -127,8 +127,8 @@ func GapiiOptions(o *service.TraceOptions) gapii.Options { if o.WaitForDebugger { flags |= gapii.WaitForDebugger } - if o.RespectFrameBoundaryDelimiters { - flags |= gapii.RespectFrameBoundaryDelimiters + if o.IgnoreFrameBoundaryDelimiters { + flags |= gapii.IgnoreFrameBoundaryDelimiters } return gapii.Options{ From cf79bdbc53f04c97d3ca3f52f8c4b5e2f09d919e Mon Sep 17 00:00:00 2001 From: Alan Date: Fri, 19 Aug 2022 13:09:39 +0100 Subject: [PATCH 4/5] Change flag meaning to respect vkQueuePresent xor ANDROID_frame_boundary --- gapis/api/templates/api_spy.cpp.tmpl | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/gapis/api/templates/api_spy.cpp.tmpl b/gapis/api/templates/api_spy.cpp.tmpl index b6693a9fa..570f86cb8 100644 --- a/gapis/api/templates/api_spy.cpp.tmpl +++ b/gapis/api/templates/api_spy.cpp.tmpl @@ -217,14 +217,17 @@ bool called = false; auto call = [{{Macro "CallCapture" $}}] { called = true; + {{if GetAnnotation $ "frame_end"}} - onPreEndOfFrame(observer, {{Global "ApiIndex"}}); - {{else}} - {{if GetAnnotation $ "frame_delimiter"}} - if(!observer->ignoreFrameBoundaryDelimiters()) { - onPreEndOfFrame(observer, {{Global "ApiIndex"}}); - } - {{end}} + if(observer->ignoreFrameBoundaryDelimiters()) { + onPreEndOfFrame(observer, {{Global "ApiIndex"}}); + } + {{end}} + + {{if GetAnnotation $ "frame_delimiter"}} + if(!observer->ignoreFrameBoundaryDelimiters()) { + onPreEndOfFrame(observer, {{Global "ApiIndex"}}); + } {{end}} observer->observePending(); @@ -279,13 +282,15 @@ observer->exit(); {{if GetAnnotation $ "frame_end"}} - onPostEndOfFrame(); - {{else}} - {{if GetAnnotation $ "frame_delimiter"}} + if(observer->ignoreFrameBoundaryDelimiters()) { + onPostEndOfFrame(); + } + {{end}} + + {{if GetAnnotation $ "frame_delimiter"}} if(!observer->ignoreFrameBoundaryDelimiters()) { onPostEndOfFrame(); } - {{end}} {{end}} {{if not (IsVoid $.Return.Type)}}¶ From 0f22c076330edf4e0a723447a10e25990a752fed Mon Sep 17 00:00:00 2001 From: Alan Date: Fri, 19 Aug 2022 16:40:16 +0100 Subject: [PATCH 5/5] fix presubmit --- cmd/gapit/flags.go | 2 +- cmd/gapit/trace.go | 40 ++++++++++++++++++------------------ gapii/cc/connection_header.h | 2 +- gapii/cc/spy.cpp | 4 +++- gapii/cc/spy.h | 4 +++- gapii/client/capture.go | 1 - 6 files changed, 28 insertions(+), 25 deletions(-) diff --git a/cmd/gapit/flags.go b/cmd/gapit/flags.go index 0c83e660f..ee50b267a 100644 --- a/cmd/gapit/flags.go +++ b/cmd/gapit/flags.go @@ -323,7 +323,7 @@ type ( Frames int `help:"only capture the given number of frames. 0 for all"` } IgnoreAndroidFrameBoundary bool `help:"If true: ignore the ANDROID_frame_boundary extension. If false: respect it."` - No struct { + No struct { Buffer bool `help:"Do not buffer the output, this helps if the application crashes"` } API string `help:"only capture the given API valid options are vulkan, angle and perfetto"` diff --git a/cmd/gapit/trace.go b/cmd/gapit/trace.go index 3e8218ab6..6b461459a 100644 --- a/cmd/gapit/trace.go +++ b/cmd/gapit/trace.go @@ -228,26 +228,26 @@ func (verb *traceVerb) Run(ctx context.Context, flags flag.FlagSet) error { } options := &service.TraceOptions{ - Type: api.traceType, - AdditionalCommandLineArgs: verb.AdditionalArgs, - Cwd: verb.WorkingDir, - Environment: verb.Env, - Duration: float32(verb.For.Seconds()), - ObserveFrameFrequency: uint32(verb.Observe.Frames), - StartFrame: uint32(verb.Start.At.Frame), - FramesToCapture: uint32(verb.Capture.Frames), - DeferStart: verb.Start.Defer, - IgnoreFrameBoundaryDelimiters: verb.IgnoreAndroidFrameBoundary, - NoBuffer: verb.No.Buffer, - HideUnknownExtensions: verb.Disable.Unknown.Extensions, - RecordTraceTimes: verb.Record.TraceTimes, - ClearCache: verb.Clear.Cache, - ServerLocalSavePath: out, - PipeName: verb.PipeName, - DisableCoherentMemoryTracker: verb.Disable.CoherentMemoryTracker, - WaitForDebugger: verb.WaitForDebugger, - ProcessName: verb.ProcessName, - LoadValidationLayer: verb.LoadValidationLayer, + Type: api.traceType, + AdditionalCommandLineArgs: verb.AdditionalArgs, + Cwd: verb.WorkingDir, + Environment: verb.Env, + Duration: float32(verb.For.Seconds()), + ObserveFrameFrequency: uint32(verb.Observe.Frames), + StartFrame: uint32(verb.Start.At.Frame), + FramesToCapture: uint32(verb.Capture.Frames), + DeferStart: verb.Start.Defer, + IgnoreFrameBoundaryDelimiters: verb.IgnoreAndroidFrameBoundary, + NoBuffer: verb.No.Buffer, + HideUnknownExtensions: verb.Disable.Unknown.Extensions, + RecordTraceTimes: verb.Record.TraceTimes, + ClearCache: verb.Clear.Cache, + ServerLocalSavePath: out, + PipeName: verb.PipeName, + DisableCoherentMemoryTracker: verb.Disable.CoherentMemoryTracker, + WaitForDebugger: verb.WaitForDebugger, + ProcessName: verb.ProcessName, + LoadValidationLayer: verb.LoadValidationLayer, } target(options) diff --git a/gapii/cc/connection_header.h b/gapii/cc/connection_header.h index 609f5ce28..7958ff1d1 100644 --- a/gapii/cc/connection_header.h +++ b/gapii/cc/connection_header.h @@ -56,7 +56,7 @@ class ConnectionHeader { static const uint32_t FLAG_DISABLE_COHERENT_MEMORY_TRACKER = 0x00000100; // Waits for the debugger to attach (useful for debug) static const uint32_t FLAG_WAIT_FOR_DEBUGGER = 0x00000200; - // Set to enable use of frame delimiters, eg ANDROID_frame_boundary extension + // Set to enable use of frame delimiters, eg ANDROID_frame_boundary extension static const uint32_t FLAG_IGNORE_FRAME_BOUNDARY_DELIMITERS = 0x00001000; // read reads the ConnectionHeader from the provided stream, returning true diff --git a/gapii/cc/spy.cpp b/gapii/cc/spy.cpp index 0db39a493..ff1724c71 100644 --- a/gapii/cc/spy.cpp +++ b/gapii/cc/spy.cpp @@ -167,7 +167,9 @@ Spy::Spy() ? kSuspendIndefinitely : header.mStartFrame; mCaptureFrames = header.mNumFrames; - mIgnoreFrameBoundaryDelimiters = (header.mFlags & ConnectionHeader::FLAG_IGNORE_FRAME_BOUNDARY_DELIMITERS) != 0; + mIgnoreFrameBoundaryDelimiters = + (header.mFlags & + ConnectionHeader::FLAG_IGNORE_FRAME_BOUNDARY_DELIMITERS) != 0; set_valid_apis(header.mAPIs); GAPID_ERROR("APIS %08x", header.mAPIs); diff --git a/gapii/cc/spy.h b/gapii/cc/spy.h index 879a3c8b4..6c8709e5a 100644 --- a/gapii/cc/spy.h +++ b/gapii/cc/spy.h @@ -86,7 +86,9 @@ class Spy : public VulkanSpy { uint64_t mFrameNumber; bool mIgnoreFrameBoundaryDelimiters; - bool ignoreFrameBoundaryDelimiters() override { return mIgnoreFrameBoundaryDelimiters; } + bool ignoreFrameBoundaryDelimiters() override { + return mIgnoreFrameBoundaryDelimiters; + } std::unique_ptr mMessageReceiverJob; diff --git a/gapii/client/capture.go b/gapii/client/capture.go index cec6b5425..f54e47331 100644 --- a/gapii/client/capture.go +++ b/gapii/client/capture.go @@ -55,7 +55,6 @@ const ( // FLAG_IGNORE_ANDROID_FRAME_BOUNDARY IgnoreFrameBoundaryDelimiters Flags = 0x00001000 - // VulkanAPI is hard-coded bit mask for Vulkan API, it needs to be kept in sync // with the api_index in the vulkan.api file. VulkanAPI = uint32(1 << 2)