-
Notifications
You must be signed in to change notification settings - Fork 856
Enable FrameTimingManager tests in standalone builds #6019
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
Enable FrameTimingManager tests in standalone builds #6019
Conversation
Hi! This comment will help you figure out which jobs to run before merging your PR. The suggestions are dynamic based on what files you have changed. SRP Core Depending on the scope of your PR, you may need to run more jobs than what has been suggested. Please speak to your lead or a Graphics SDET (#devs-graphics-automation) if you are unsure. |
778aac6
to
1d61dd4
Compare
1d61dd4
to
19749cc
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It doesn't work as intended. Even the FrameTimingStats checkbox was disabled in the Player Settings, I was still able to see the stats and no warning appeared.
Tested with:
VLNQA00292, iPad Air3, SoC: A12, iOS: 13.2.3
2022.1.0a15.2209
Revision: trunk 49120632d05b
Built: Tue, 09 Nov 2021 06:50:39 GMT
@ernestasKupciunas thanks for checking. I've investigated and it appears I've misunderstood, and there's no need for this warning after all, iOS works the same as other platform which is great. I'll remove the UI change from this, and repurpose this PR to just be about improving the unit test, which I still want to do. |
19749cc
to
da47afa
Compare
- This allows runtime profiler tests to run in Player builds, now that there's api to query it.
da47afa
to
0c6226c
Compare
- WaitForEndOfFrame is not supported in batchmode.
0c6226c
to
27b2a88
Compare
This PR is now ready, in the end I'm just modifying the runtimeprofiler tests:
Checked that the test is now skipped on Linux and passes on other platforms. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks, good. We should create a task to reenable the test for Linux Platform?
"Restoring test on Linux" tracked here: https://jira.unity3d.com/browse/XPIPELINE-451 |
Purpose of this PR
EDIT: I had misunderstood the code, the UI change is not necessary because iOS works the same as other platforms. I've repurposed this PR to just be about improving the tests.
Add actionable warning message that informs user why they are seeing only 0 in each value. This is because FrameTimingManager is not available.Note: This message is be rarely visible because for most platforms, FrameTimingManager is always enabled in development builds (and Rendering Debugger is only accessible in development builds). However, the message should appear on iOS development builds when Frame Timing Stats is disabled in Player Settings. What it should look like:Enable RT profiler unit test in player builds, now that there is player-visible API for checking whether FrameTimingManager is enabled. Before this API was added, these tests had to be disabled in player builds because it was not possible to query if frame timings are available.
Testing status
EDIT: UI changes are removed, so manual testing is not necessary here, I will focus on ensuring Yamato is fine with the test changes.
Please verify the following things using an iOS development build:1) The warning message is shown and the frame stats are 0, when Frame Timing Stats is disabled in Player Settings.2) The warning message is not shown and the frame stats show valid numbers, when Frame Timing Stats is enabled in Player Settings.
NOTE: Testing requires building latest version of trunk (feature relies on new trunk API)