Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Frameboundary2 #1179

Merged
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions cmd/gapit/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -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."`
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we rename this to IgnoreFrameBoundaryExtension and change the default value, hint text and behaviour accordingly?

No struct {
Buffer bool `help:"Do not buffer the output, this helps if the application crashes"`
}
Expand Down
39 changes: 20 additions & 19 deletions cmd/gapit/trace.go
Original file line number Diff line number Diff line change
Expand Up @@ -228,25 +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,
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)

Expand Down
4 changes: 4 additions & 0 deletions gapic/src/main/com/google/gapid/views/TracerDialog.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)),
Expand Down Expand Up @@ -1028,6 +1031,7 @@ public TraceRequest getTraceRequest(Settings settings) {
.setUri(traceTarget.getText())
.setAdditionalCommandLineArgs(arguments.getText())
.setFramesToCapture(duration.getSelection())
.setRespectFrameBoundaryDelimiters(respectAndroidFrameBoundary.getSelection())
.setNoBuffer(withoutBuffering.getSelection())
.setHideUnknownExtensions(!includeUnsupportedExtensions.getSelection())
.setServerLocalSavePath(output.getAbsolutePath())
Expand Down
4 changes: 4 additions & 0 deletions gapii/cc/call_observer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,10 @@ void CallObserver::observeTimestamp() {
encode_message(&timestamp);
}

bool CallObserver::respectFrameBoundaryDelimiters() {
return mSpy->respectFrameBoundaryDelimiters();
}

void CallObserver::enter(const ::google::protobuf::Message* cmd) {
endTraceIfRequested();
if (!mShouldTrace) {
Expand Down
3 changes: 3 additions & 0 deletions gapii/cc/call_observer.h
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,9 @@ 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();

private:
// shouldObserve returns true if the given slice is located in application
// pool and we are supposed to observe application pool.
Expand Down
2 changes: 2 additions & 0 deletions gapii/cc/connection_header.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 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.
Expand Down
4 changes: 3 additions & 1 deletion gapii/cc/spy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,8 @@ Spy::Spy()
mSuspendCaptureFrames(0),
mCaptureFrames(0),
mObserveFrameFrequency(0),
mFrameNumber(0) {
mFrameNumber(0),
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
Expand Down Expand Up @@ -166,6 +167,7 @@ Spy::Spy()
? kSuspendIndefinitely
: header.mStartFrame;
mCaptureFrames = header.mNumFrames;
mRespectFrameBoundaryDelimiters = (header.mFlags & ConnectionHeader::FLAG_RESPECT_FRAME_BOUNDARY_DELIMITERS) != 0;

set_valid_apis(header.mAPIs);
GAPID_ERROR("APIS %08x", header.mAPIs);
Expand Down
3 changes: 3 additions & 0 deletions gapii/cc/spy.h
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,9 @@ class Spy : public VulkanSpy {
int mObserveFrameFrequency;
uint64_t mFrameNumber;

bool mRespectFrameBoundaryDelimiters;
bool respectFrameBoundaryDelimiters() override { return mRespectFrameBoundaryDelimiters; }

std::unique_ptr<core::AsyncJob> mMessageReceiverJob;

friend struct spy_creator;
Expand Down
3 changes: 3 additions & 0 deletions gapii/cc/spy_base.h
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,9 @@ 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; }

// Ends the current trace if requested by client.
virtual void endTraceIfRequested() {}

Expand Down
3 changes: 3 additions & 0 deletions gapii/client/capture.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
RespectFrameBoundaryDelimiters = 0x00001000
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need 'Flags' suffix here ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apparently not, but good catch. Added



// 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.
Expand Down
17 changes: 16 additions & 1 deletion gapis/api/templates/api_spy.cpp.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,12 @@
called = true;
{{if GetAnnotation $ "frame_end"}}
onPreEndOfFrame(observer, {{Global "ApiIndex"}});
{{else}}
{{if GetAnnotation $ "frame_delimiter"}}
if(observer->respectFrameBoundaryDelimiters()) {
onPreEndOfFrame(observer, {{Global "ApiIndex"}});
}
{{end}}
{{end}}

observer->observePending();
Expand Down Expand Up @@ -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->respectFrameBoundaryDelimiters()) {
onPostEndOfFrame();
}
{{end}}
{{end}}

{{if not (IsVoid $.Return.Type)}}¶
return result;
{{end}}
Expand Down
2 changes: 1 addition & 1 deletion gapis/api/vulkan/extensions/android_frame_boundary.api
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
@extension("VK_ANDROID_frame_boundary")
@indirect("VkDevice")
@override
@frame_end
@frame_delimiter
@no_replay
cmd void vkFrameBoundaryANDROID(
VkDevice device,
Expand Down
3 changes: 2 additions & 1 deletion gapis/service/service.proto
Original file line number Diff line number Diff line change
Expand Up @@ -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_frame_boundary_delimiters = 14;
// Insert extra commands to record error state
bool record_error_state = 15;
// Wait for an event to start tracing
Expand Down
3 changes: 3 additions & 0 deletions gapis/trace/tracer/tracer.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,9 @@ func GapiiOptions(o *service.TraceOptions) gapii.Options {
if o.WaitForDebugger {
flags |= gapii.WaitForDebugger
}
if o.RespectFrameBoundaryDelimiters {
flags |= gapii.RespectFrameBoundaryDelimiters
}

return gapii.Options{
o.ObserveFrameFrequency,
Expand Down