NVlabs / FPSci

Aim Training Experiments
Other
70 stars 23 forks source link

Support for programmable variable frame rate #319

Closed jspjutNV closed 3 years ago

jspjutNV commented 3 years ago

This PR adds support for specifying frame rate variability through one of the following approaches. This PR adds two render parameters, frameTimeArray and randomFrameTime, both of which are ignored if frameTimeArray is empty, which is the default.

jspjutNV commented 3 years ago

This PR touches some of the inner workings of our frame rate control, and in particular, I haven't yet tested how it interacts with frame delay settings. It's possible that we'll need to do some fixing to make them work together, though my cursory look appeared to work fine.

jspjutNV commented 3 years ago

I'm also not committed to the variable naming I chose, nor the function placement necessarily. Given that these parameters belong to the RenderConfig but the FPSciApp is requesting the target frame time from the Session the functionality could really go anywhere in that stack. Happy to hear other opinions on variable naming.

jspjutNV commented 3 years ago

As a test case, I used the following settings with randomFrameTime switched true and false. This gives you half a second of 10 fps, then half a second of 30 fps, which are noticeable enough to see the change. With randomFrameTime = true it gives the 10 Hz stutter randomly, which is noticeably different than the half-second on/off.

            frameTimeArray = ( 0.1, 0.1, 0.1, 0.1, 0.1, 0.033, 0.033, 0.034, 0.033, 0.033, 0.034, 0.033, 0.033, 0.034, 0.033, 0.033, 0.034, 0.033, 0.033, 0.034, );
            randomFrameTime = false;
bboudaoud-nv commented 3 years ago

I spent some time looking at SDTs reported in the results files vs original patterns provided in the config. Overall things look fairly accurate/true to what is specified. The discontinuities/phase offsets below likely occur due to new trials starting and could be sorted out w/ more sophisticated management of the frame time index.

image

Based on these results, I think I'm going to suggest that we want to (in some cases) reset the frame time indexing variable at the start of each trial. This could be done conditional on another flag.

jspjutNV commented 3 years ago

I just tried building and running this, and I get a memory fault at the end of creating the results file function that I don't understand. I'm not sure when this regression showed up, but it seems similar to what I've been experiencing with #314. It would be good to know if this is only happening on my machine, or if others also see this bug.

jspjutNV commented 3 years ago

I believe I've gotten it to build by changing line 182 in Logger.cpp to make the first "'" literal into a G3D::String with the following.

        String("'") + sessConfig->id + "'",

Since there's a bunch more of the same implicit conversion to G3D strings to get the G3D + operators to work, it seems like we should probably be explicitly making those strings into G3D::String types. I cannot explain why this only breaks on a couple branches, some of which were working for me before, but doesn't break on the master branch.

jspjutNV commented 3 years ago

Even more baffling, after removing the String() type cast once it worked once, the previously failing branch works again without any changes. There's possibly a bug with some heuristic race condition in the way VS compiles that exposes a missing (or buggy) operator overload in G3D. I think we should probably add the explicit type cast as a preventative measure against this same type of problem showing up in the future.

bboudaoud-nv commented 3 years ago

@jspjutNV can you confirm that the sessConfig->id was valid when the crash occurred? We use the "[literal] + G3D::String" combo throughout the logger and have never seen any issues from this.

jspjutNV commented 3 years ago

@jspjutNV can you confirm that the sessConfig->id was valid when the crash occurred? We use the "[literal] + G3D::String" combo throughout the logger and have never seen any issues from this.

I cannot because it isn't crashing any more so I can't inspect to see what's wrong.

jspjutNV commented 3 years ago

I ran some tests with a variety of sequences, and the results match my expectations. Sharing the plots of those tests here.

image image image