search

kleinerm / Psychtoolbox-3

This is kleinerm's git repository for development of Psychtoolbox-3. Regular end users should stay away from it, unless instructed by him otherwise, and use the official Psychtoolbox-3 GitHub page or distribution system for production releases.
104 stars 307 forks source link

OffsetRect and single row-indexed (4,1) rect input #199

Closed iandol closed 3 years ago

iandol commented 3 years ago

If you try to run ProceduralGarboriumDemo(1) you get a crash in OffsetRect.m:

Error using OffsetRect (line 33)
Wrong size rect argument. A single rect must be a 1-row 4 element vector!

Error in CenterRectOnPointd (line 26)
newRect = OffsetRect(rect,x-cX,y-cY);

Error in ProceduralGarboriumDemo (line 236)
    dstRects = CenterRectOnPointd(inrect .* repmat(scale,4,1), x, y);

ProceduralGarboriumDemo(2) works (even though it also passes row-indexed rect) and this is due to the following check starting line 30 of OffsetRect.m:

if PsychNumel(oldRect) == 4
    % Single rect case:
    if size(oldRect,2)~=4
        error('Wrong size rect argument. A single rect must be a 1-row 4 element vector!');
    end

Other PsychRect functions are quite robust about swapping row/column order, and OffsetRect works fine if the error condition is removed. I'm not sure if you prefer to keep this strict check in place or relax it?

kleinerm commented 3 years ago

I commited a fix to my master, doable for this case. Ofc. it now still fails for nrects=4, because of the ambiguity in some functions if a 4x4 matrix is 4 rects in 4 rows, or 4 rects in 4 columns. Ever allowing the level of flexibility in the PsychRects functions we have now, and making them "guess" from the shape of input args what they should do was a bad api design idea.

iandol commented 3 years ago

Indeed I think general flexibility always has a price, as is clear for the edge case of 4x4 inputs. Changing the behaviour to be consistent will break backwards compatibility, so your approach is the only viable one…