Nice!
A few suggestions though. Your scripts now use …
PsychImaging('AddTask', 'General', 'FloatingPoint32Bit');
… but I don’t see quite why? This is useful if you need more than 8 bit per color channel (8 bpc) color precision, or in fact you need maximum precision. E.g., HDR displays, fine contrast control, > 16 Million colors, super precise alpha-blending, or various image post-processing operations by the imaging pipeline.
These demos seems to not require any of that. There is a cost associated with using these high-precision buffers, so one shouldn’t use them by default for no reason:
- 32 bit float framebuffers consume twice the amount of video memory, and also memory/processing bandwidth, so they slow down drawing/post-processing/flipping. Depending on display resolution, desired framerate and performance of the used graphics-card this can be significant.
- They are not supported on old graphics cards (before ~year 2007) or on low end systems like RaspberryPi 1/2/3.
- On older, or new but cheap/lower end systems, alpha blending may not be supported at all, or only at very slow performance, for such float 32 framebuffers. E.g., the RaspberryPi 4/400 won’t alpha-blend and only slowly filter textures of that precision.
For this reason, there is PsychImaging('AddTask', 'General', 'FloatingPoint32BitIfPossible'); to allow PTB to downgrade precision to 16 bit floating point if 32 bit is not possible or not possible with good performance. 16 bpc float still gives enough precision for ~11 bpc SDR framebuffers or supposedly for typical HDR rendering.
But in the end, high precision framebuffers should not be used needlessly, because there is always a performance hit.
The other thing is
PsychImaging('OpenWindow', screenid, black, [], 32, 2, [], multiSample);
That 32, 2 sequence I’d recommend replacing with [], [], because there is literally no other setting there that makes sense in any use case. Those Screen parameters only exist for backwards compatibility with old scripts written against Psychtoolbox-2 from almost 20 years ago. E.g., single buffering 1 is theoretically possible, but useless, untested since many years, probably broken, and ignored with any PsychImaging functionality. Technically there are other magic values for pixelSize than 32, but only PsychImaging('OpenWindow',...) knows how to map certain requirements and task specifications to suitable Screen('Openwindow', ... pixelSize, ...) values that would make any sense.
Iow. 32, 2 is useless/meaningless at best, so leaving it out / at [],[] default is better.
The snippet
% Fill the screen black
Screen('FillRect', window, black);
Screen('Flip', window);
is redundant if it follows almost directly after ‘OpenWindow’, because OpenWindow will already have done that. The reason some PTB demos still contain that is because it is a left-over from very early versions of PTB-3 from before the year 2005, which had a bug preventing the clear color in ‘OpenWindow’ from taking effect.
% Set the black and white index
black = BlackIndex(screenid);
Nothing wrong with that in principle, but if you use PsychDefaultSetup(2); in modern code, which requests color representation in normalized 0.0 - 1.0 range for 0% to 100% intensity, by definition BlackIndex() will always be 0.0, WhiteIndex() will always be 1.0 and GrayIndex() will always be 0.5 for 50% intensity. So the statements are basically redundant. With old style code that has 0-255 color range or has set a custom color range, e.g., 0-1023 for 10 bit framebuffers, BlackIndex() will always be 0 and WhiteIndex() will be 255, 1023, … etc. But the point of the modern normalized color range mode is that one doesn’t have to think about this anymore and always can use 0 - 1 range for black to white. The exception are HDR display modes where the color is usually represented in Nits or other “HDR units”, but then those Black/Gray/WhiteIndex() functions become meaningless anyway.
% Unify the keyboard names for mac and windows computers
KbName('UnifyKeyNames');
applies also to Linux, or any other operating system on which PTB could theoretically be ported to. But this function call is redundant, because this is implied in PsychDefaultSetup(n) for any n > 0, iow. already done in your PsychDefaultSetup(2) statement at the beginning of demo scripts.
PsychDefaultSetup is meant as catch-all / replacement for typical boilerplate code. Currently it makes AssertOpenGL, KbName('UnifyKeynames') and color range setup redundant.
Some (many?) of our PsychDemos/ will demonstrate similar anachronisms / redundant calls, etc., because nobody ever found the time or energy to update/rewrite them for the latest best practices. It is good though to stop using them in new sample code.
-mario
