Draw timing

Support
81

Hi Chris,

As it turns out, it is beneficial to draw on every frame (see results). You can see that when your version idles for a while, it will then jitter a bit when re-starting to draw.

More details on the results:

As in previous test, the experiment consists of a random dot pattern that moves for a while. This time, however, it moves for approx. 30sec (refreshRate()/2) and then does nothing for 30 seconds in the first break, 60s in the second break, 90s in the third break, etc… It does this for 30min.
The test setup has a 120Hz Samsung Screen which is connected to one of two Radeon HD 5770. Apart from that it doesn’t differ from the previously tested setups (which I haven’t tested with this test so far).

The no-gl-fence version

no warnings about lost frames
6 refreshes are completely lost (the bitcode is never recognized by the diode array)
announcement-diode lag between 8.22 and 32.1 ms

The patched no-gl-fence version

no warnings about lost frames
all refreshes are recognized by the diode-array
announcement-diode lag between 14.61 and 17.51 ms

** PATCH **

I’ve made some changes to the StimulusDisplay to avoid the jittery behavior. What I’ve done there is to let the stimuli draw on every frame, but let them only log their redraw whenever it was a needed draw action. It is now the stimulus responsibility to make sure that in a redundant draw call there will be nothing new on the screen, so that we’re not drawing something that won’t be logged.
For this purpose I’ve added the getNeedDraw() function to the stimulus display. Stimuli can now poll the stimulus display for the type of draw. If getNeedDraw returns false, a draw action will not be logged! It must therefore be ensured that the draw is exactly the same as in the previous one.
The necessary changes in the stimuli are pretty straight forward and I’ve modified the PointStimulus, the BasicTransformStimulus, the ImageStimulus and the Background already. (see diffs)
Unfortunately, this has to be done for ALL of the stimuli, and some of them are cleaner coded than others (e.g. some seem to still poll the datum objects when they announce, which is very dangerous imho).

What do you think?
Philipp

82

Attachments:

83

Hi Philipp,

There is only one thing that’s left: Both versions produce a first displayUpdateEvent that is time-stamped ~900ms after the diodes update. it’s only the very first one, all the other updates are correctly timed. Maybe the DisplayLink messes up the very first outValue? I’ve double/triple-checked my analysis script and I can’t find the error there.

OK, I’ll try to figure out why this is happening.

As it turns out, it is beneficial to draw on every frame (see results).

Thanks for performing these tests. I believe there’s a simpler way to update the display every frame, which you’ve mentioned before: adding NSOpenGLPFABackingStore to the NSOpenGLPixelFormat array used to create the GL context.

I’ve added a compile-time option in the no_gl_fence branch that, when enabled, alters StimulusDisplay to update every frame unconditionally. (To enable it, you must edit OpenGLContextManager.h and set the value of M_DRAW_EVERY_FRAME to 1.) Because it uses NSOpenGLPFABackingStore, the stimulus drawing code is still called only when needed. On frames when no redraw is required, the existing back buffer is simply re-copied to the front buffer. In my testing, this copying has had negligible impact on MWServer’s performance.

I’ve created another no_gl_fence installer that has this option enabled. The installer package is named MWorks-NIGHTLY-no_gl_fence-M_DRAW_EVERY_FRAME.dmg. My hope is that this build will have the same performance characteristics as your patched no_gl_fence version, without requiring any changes to the stimulus classes. Do you think you can re-run your test using this build?

Thanks,
Chris

84

There is only one thing that’s left: Both versions produce a first displayUpdateEvent that is time-stamped ~900ms after the diodes update. it’s only the very first one, all the other updates are correctly timed. Maybe the DisplayLink messes up the very first outValue? I’ve double/triple-checked my analysis script and I can’t find the error there.

OK, I’ll try to figure out why this is happening.

I’ve solved this one today. The problem was within my IODevice Plugin, which didn’t initialize correctly and thus sometimes logged false timestamps for the first values. Sorry about that.

Do you think you can re-run your test using this build?

This is done, but unfortunately I don’t have good news. The new version keeps on spitting out warnings about lost frames (about once every 20 seconds) in idle times. Most of the logged bitcodes never get recorded by the diode array (as opposed to the patched nofence version which doesn’t have a single dropout). And the re-start glitches are still present, although not after all of the pauses.

My best guess is that the DisplayLink needs the flush command to come up with a solid draw-time estimation. Maybe there is a way to go through that interface to issue an update and still use the preserved back-buffer?

Btw: Congratulations!

Philipp

85

Hi Philipp,

I’ve solved this one today. The problem was within my IODevice Plugin, which didn’t initialize correctly and thus sometimes logged false timestamps for the first values.

OK, thanks for letting me know.

This is done, but unfortunately I don’t have good news … My best guess is that the DisplayLink needs the flush command to come up with a solid draw-time estimation. Maybe there is a way to go through that interface to issue an update and still use the preserved back-buffer?

That should be exactly what’s happening: When no stimuli need to be updated, StimulusDisplay doesn’t execute any drawing commands, but it does call flushBuffer on the GL context, which should copy the preserved back buffer to the front buffer. It’s possible that the graphics pipeline is being clever, e.g. it might recognize that the current front buffer would be unchanged after the flushBuffer and decide to drop the buffer copy as an optimization. If so, that’s too bad for us.

I’d still like to find an approach to this that doesn’t require modifying any stimulus code. One option is to render everything to an offscreen texture and then redraw that texture at every refresh, regardless of whether it’s changed. Maybe I’ll give that a try next.

Chris

86

I’d still like to find an approach to this that doesn’t require modifying any stimulus code. One option is to render everything to an offscreen texture and then redraw that texture at every refresh, regardless of whether it’s changed. Maybe I’ll give that a try next.

Very well, that could also work, although sounds complicated. I know that changing every single stimulus code sounds awful, but the necessary changes are really straight forward and some of the stimuli could need some reworking anyways. Moreover, compatibility for old stimuli will not break without the fix. It’s just a bit dangerous to let a stimulus run that could potentially alter the display output without being logged while doing so. But on the other hand, logging the contents of datum containers is also critical as they might change in between drawing and announcing …

Anyhow, I’m sure you will find the correct way to do this! If you need me to run any further tests, I will have time for that after mid next week.

Cheers,
Philipp

87

Hi Chris,

I have to report to my supervisor about the status of MWorks, so I wanted to see where we stand.
Unfortunately, the last commit to the master project seems to have taken place on September 09, 2011, the no-gl-fence branch was last updated November 30.

Is there anything new I may be able to tell my PI, maybe something that hasn’t been merged upstream, yet? How far from a stable release 0.5 are we anyways?

I imagine you have more interesting things to do right now, and I hope this is going well for you! I just know that the question about the status of this threads issue will come up in my evaluation and before I report that the development has been stalled for almost two months now, I wanted to check with you.

Best,
Philipp

88

Hi Philipp,

The main MWorks repositories moved a while back to GitHub - mworks/mworks: A framework for conducting realtime neurophysiology and psychophysics experiments . As you can see, development is continuing, if at a somewhat slower pace than usual (partly because I was out on paternity leave for a month last fall).

As for 0.5, my hope is that we’ll get it out this spring. In the past, most users have expressed a preference for yearly releases. 0.4.5 went out last April, so a spring release for 0.5 would keep us on that schedule.

Cheers,
Chris

89

I see, thank you.

Although I’m still failing to detect new commits on files relevant to this issue. From what I remember, we agreed on considering this a high-priority issue that affects not only Prof Treue’s lab but basically all people using MWorks and wanting to have correct draw time-stamps.

Issues like the format() argument are less important than this issue (I know because Ralf and I work together on this). If you could just tell me about your current status regarding your offscreen texture idea, I would be grateful!

Best,
Philipp

90

Hi Philipp,

You are correct. There have been no recent commits regarding the draw timing issue. I’ve spent the intervening time working on other projects, most of which have not yet resulted in any changes to the MWorks code.

My understanding is that you have resolved the issue to your satisfaction by modifying the stimulus display code to draw every frame, and that you are proceeding with your experiments using your modified version of MWorks. Given that, it’s now up to me to figure out how best to make this modification part of the main MWorks code base. Is that accurate? If not, I apologize for misunderstanding.

In any case, I assure you that I have not forgotten about this issue, and I appreciate you taking the time to remind me about it :). I believe the changes required for the offscreen texture idea are relatively straightforward. I will try to make a first pass at them this week and will send you an update when they’re available.

Cheers,
Chris

91

My understanding is that you have resolved the issue to your satisfaction by modifying the stimulus display code to draw every frame, and that you are proceeding with your experiments using your modified version of MWorks.

That is correct. It’s just that neither me nor my supervisor likes the idea of having a custom fix for a very general (and likewise severe) problem in the core code of MWorks. For training purposes, this fix is doing what we want and we feel comfortable using our version as it is. However, we are not far from starting to record, and when obtaining really important data it just doesn’t feel good to use core code that hasn’t been approved of. Having said that I want to point out once more that there might be many other people out there who currently use a version that does not do what it promises to do.

But ok, I see that you are working on it as fast as possible, and I appreciate it very much!

Thank you!
Philipp

92

Hi Philipp,

I have a new version of the no_gl_fence branch available for testing. I’ve posted an installer that you can grab.

This version updates the display (i.e. performs a draw and GL buffer swap) during every refresh cycle. When a full redraw is required (e.g. a dynamic stimulus reports that it needs drawing, or the experiment executes a update_stimulus_display action), drawing and announcements occur as usual, with one additional step: After all stimuli are drawn, the StimulusDisplay copies the rendered frame to an offscreen framebuffer. During refreshes when a full redraw is not required, StimulusDisplay simply copies the most recently saved frame to the back buffer and flushes; no stimulus draw method is invoked, and no announcements are made. The main advantage of this approach is that per-stimulus drawing code is still invoked only as needed, so there’s no chance of something being drawn that isn’t logged, and stimulus classes require no modification.

The new behavior is enabled by default. However, you can disable it (thereby reverting to the previous draw-and-flush-only-as-needed scheme) in setup_variables.xml by adding to #mainScreenInfo the key “redraw_on_every_refresh” with value 0, i.e.

<variable_assignment variable="#mainScreenInfo">
    <dictionary>
        ...
        <dictionary_element>
            <key>redraw_on_every_refresh</key>
            <value type="integer">0</value>
        </dictionary_element>    
    </dictionary>
</variable_assignment>

I’d be very grateful if you could re-run your timing tests against this version. My hope is that it will perform as well as your patched build, but only your testing will tell.

Thanks,
Chris

93

Chris!

I am happy to report that at first glimpse, your version seems to run perfectly. I’ve encountered no problems in running the installation, performing the tests, and compiling our plugins against it.

I’ve attached the diode-results for you. The distribution of timestamps for updates that involve a high-low transition of one or more diodes doesn’t look so good, but I assume this is due to differences in adjustments of the diodes or light-leakage or similar. The most important result is that there is no odd behavior any more, no jittering and no wrong timestamps.

During the test with pauses it occurred twice that an update was delayed for more than two frames. I don’t know why this would happen with your solution but not with mine, but it is possible that it’s just a coincidence. I am rerunning the tests to have some more data.

For now, thank you very much! I’ve installed your version on one of our pilot setups so that it can be tested with “real” experiments. If I recognize anything strange, I will report this to you.

Cheers and good night!
Philipp

94

Attachment: nofence_result.png (10.6 KB)

95

Great! Please keep me updated as your testing progresses.

Thanks,
Chris

96

Hi Chris,

first things first: I’ve still not encountered any new errors, so I guess it’s about time to think about merging the changes into the main version of MWorks.

There is one thing left that is kind of unrelated to the issue of this thread imho:
When I query the display for ‘display->getCurrentContextIndex()’ in rapid succession (e.g. a couple of hundred times from within a loop during ‘draw()’ of my stimulus), the server crashes abruptly. The issue was easy to fix by saving the return value of ‘getCurrentContextIndex()’ to a local variable and then using this instead of asking the display every time. Do you see why this would happen?

Best,
Philipp

97

… huh… having said that, there is one (very old) setup that does not run with your version. It’s failing to load the experiment with:

ERROR: Failed to create object.
Extended information:
reason: renderer does not support required OpenGL framebuffer extensions
object_type: experiment
ref_id: idp10208
component: New Experiment
parser_context: mw_create`

The setup is:
Model Name: Mac Pro Model Identifier: MacPro1,1 Processor Name: Dual-Core Intel Xeon Processor Speed: 2.66 GHz

with
NVIDIA GeForce 7300 GT: Chipset Model: NVIDIA GeForce 7300 GT Type: GPU Bus: PCIe Slot: Slot-1 PCIe Lane Width: x16 VRAM (Total): 256 MB

98

Hi Philipp,

I’ve still not encountered any new errors, so I guess it’s about time to think about merging the changes into the main version of MWorks.

OK, that’s good news. I’ll need to get Dave’s approval before moving these changes into the main branch; I’ll try to submit a pull request soon.

When I query the display for ‘display->getCurrentContextIndex()’ in rapid succession (e.g. a couple of hundred times from within a loop during ‘draw()’ of my stimulus), the server crashes abruptly. The issue was easy to fix by saving the return value of ‘getCurrentContextIndex()’ to a local variable and then using this instead of asking the display every time. Do you see why this would happen?

getCurrentContextIndex does nothing but return the value of an integer member of StimulusDisplay, so there isn’t much that could be going wrong there. That makes me think the issue lies elsewhere. If you want to share more of the crash-inducing code, I can take a look and see if anything looks problematic.

having said that, there is one (very old) setup that does not run with your version.

Looking at Apple’s OpenGL Capabilities Tables, I see that the GeForce 7300 doesn’t support the GL_EXT_framebuffer_blit extension, which is used by my redraw-on-every-refresh implementation. StimulusDisplay checks for that extension and throws an error is it’s missing, so the failure you’re seeing is by design.

Given that all relatively recent graphics cards do support the extension, my feeling is that it’s OK to rely on it, at the cost of not supporting the oldest cards. (Note that those cards can still be used with MWorks if you set the “redraw_on_every_refresh” setting to 0, as I’ve described previously.) Does that seem reasonable to you?

Chris

99

ok, this works flawlessly:

const double width_pixel = ViewportWidth[display->getCurrentContextIndex()];
double requested_size = (dot_size / display_width) * width_pixel;
GLfloat maximum_size;
glPointSize(requested_size);
glGetFloatv(GL_POINT_SIZE_MAX,&maximum_size);
if (requested_size > maximum_size)
    mwarning(M_DISPLAY_MESSAGE_DOMAIN,"RDP on Display %d: requested point size %f is larger than maximum point size of %f\n"
           ,display->getCurrentContextIndex(),requested_size,maximum_size);

if ( type.compare(0,1,"3D",0,1) != 0 ) {
    glVertexPointer(verticesPerDot, GL_DOUBLE, 0, &(dots[0]));
    glDrawArrays(GL_POINTS, 0, ndots);
}
else {
    const GLint velements = ndots*verticesPerDot;
    for (int i=0;i<velements;i+=verticesPerDot) {
        glPointSize(ddots_size[i] * width_pixel);
        glVertexPointer(verticesPerDot, GL_DOUBLE, 0, &(ddots[i]));
        glDrawArrays(GL_POINTS, 0, 1);
        glVertexPointer(verticesPerDot, GL_DOUBLE, 0, &(ddots[velements+i]));
        glDrawArrays(GL_POINTS, 0, 1);
    }
}

and this crashes after a couple of updates (it doesn’t happen immediately, but within 60 seconds or so)

double requested_size = (dot_size / display_width) * ViewportWidth[display->getCurrentContextIndex()];
GLfloat maximum_size;
glPointSize(requested_size);
glGetFloatv(GL_POINT_SIZE_MAX,&maximum_size);
if (requested_size > maximum_size)
    mwarning(M_DISPLAY_MESSAGE_DOMAIN,"RDP on Display %d: requested point size %f is larger than maximum point size of %f\n"
           ,display->getCurrentContextIndex(),requested_size,maximum_size);

if ( type.compare(0,1,"3D",0,1) != 0 ) {
    glVertexPointer(verticesPerDot, GL_DOUBLE, 0, &(dots[0]));
    glDrawArrays(GL_POINTS, 0, ndots);
}
else {
    const GLint velements = ndots*verticesPerDot;
    for (int i=0;i<velements;i+=verticesPerDot) {
        glPointSize(ddots_size[i] * ViewportWidth[display->getCurrentContextIndex()]);
        glVertexPointer(verticesPerDot, GL_DOUBLE, 0, &(ddots[i]));
        glDrawArrays(GL_POINTS, 0, 1);
        glVertexPointer(verticesPerDot, GL_DOUBLE, 0, &(ddots[velements+i]));
        glDrawArrays(GL_POINTS, 0, 1);
    }
}

It’s weird, I know. This is why I ask. The only thing I’ve changed is that I am not calling ´getCurrentContextIndex´ from within the loop.

Given that all relatively recent graphics cards do support the extension, my feeling is that it’s OK to rely on it, at the cost of not supporting the oldest cards. (Note that those cards can still be used with MWorks if you set the “redraw_on_every_refresh” setting to 0, as I’ve described previously.) Does that seem reasonable to you?

Perfect.

Cheers,
Philipp

100

Hi Philipp,

I’ve merged the no_gl_fence changes into master. They’ll be in the standard nightly build starting tonight and will be included in the upcoming 0.5 release.

Thank you for your patience and extensive help in resolving this issue!

Cheers,
Chris