Skip to content

Commit

Permalink
BF: enable viewScale and viewPos
Browse files Browse the repository at this point in the history 
- allow them to work simultaneously (moving glMatrixMode(GL.GL_MODEL_VIEW) higher does this)
- allow whole screen mirror-image via viewScale parameter, by using abs(scale)

Limitations:
- not tested what happens with orientation
- does not fix psychopy#89
  • Loading branch information
@jeremygray
jeremygray committed Nov 17, 2015
1 parent 136873c commit ff76914
Showing 1 changed file with 6 additions and 8 deletions.
14 changes: 6 additions & 8 deletions psychopy/visual/window.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -614,17 +614,15 @@ def flip(self, clearBuffer=True):
GL.glLoadIdentity()
GL.glOrtho(-1, 1, -1, 1, -1, 1)
GL.glScalef(self.viewScale[0], self.viewScale[1], 1)
scale = self.viewScale
else:
GL.glLoadIdentity() # still worth loading identity
GL.glMatrixMode(GL.GL_MODELVIEW)
GL.glLoadIdentity()
scale = [1, 1]

if self.viewPos is not None:
GL.glMatrixMode(GL.GL_MODELVIEW)
if not self.viewScale:
scale = [1, 1]
else:
scale = self.viewScale
norm_rf_pos_x = self.viewPos[0]/scale[0]
norm_rf_pos_y = self.viewPos[1]/scale[1]
norm_rf_pos_x = self.viewPos[0]/abs(scale[0])
norm_rf_pos_y = self.viewPos[1]/abs(scale[1])
GL.glTranslatef(norm_rf_pos_x, norm_rf_pos_y, 0.0)

if self.viewOri is not None:
Expand Down

7 comments on commit ff76914

@jeremygray
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@peircej: does this look about right? It works in the test scenarios I threw at it. tests pass, but that's not surprising since if there had been tests they would have been failing before.

If so, adding a scale field to the Exp settings -> screen would allow people to enter -1 to flip the entire display horizontally or vertically. So you could develop an experiment offline in normal view, and then flip for viewing in fMRI by changing a single parameter in the exp settings dialog. TextStim still are unaffected (i.e., psychopy#89 remains).

@peircej
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, this looks wrong to me in two ways:

  1. by including the abs() call you actually prevent the flipping being possible, whereas previously it could be done, because any negative scale value gets made positive. No?
  2. I don't see what change you've made here fixed the original issue that the scale combined with position change broke things for the user. To be fair I have gone and tried to replicate his problem, but I think we should do that first (with a test that fails) and then fix it (so the test passes).

For the flipping we can also now do this by a different method that will be immune to the fact that text and pixel-by-pixel drawing (SimpleImageStim?) render with a different coordinates system. When the user has FBO turned on we can use the _renderFBO() method to control flipping and rescaling there instead. A few users have found slower performance with FBO turned on (I think for a very fast graphics card it won't be noticeable though) so we might turn it off by default (currently on by default i think). But with FBO on and a quick change to that _renderFBO function I'm certain that proper frame mirroring and rescaling can be achieved.

@jeremygray
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ha, ok: 1) The abs() part is essential to make the mirror image part actually work. Otherwise you get a double negative for scale values < 0. The first negative is from the GL.glScalef(self.viewScale[0], self.viewScale[1], 1), then followed by the norm_rf_pos[x,y]. So negatives were canceling out, but no longer do with abs() in one of the places. And it actually flips now in practice and did not before :-)

  1. The change that fixed the combined viewScale and viewPos was to avoid setting the glMatrixMode twice. If doing both viewScale and viewPos, it was getting set once here
if self.viewScale is not None:
        GL.glMatrixMode(GL.GL_PROJECTION)

and then again a second time here:

GL.glMatrixMode(GL.GL_MODELVIEW)

With the code in this commit, you always get one or the other but not both. It was the both case that was problematic.

The sample script that Alexander posted could be the basis of a test. The 'both' case would fail, and the code here would make that pass.

BUT: I'm unsure about the differences between GL.GL_MODELVIEW and GL.GL_PROJECTION, and possible carry over effects of using one versus the other.

@peircej
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I see. But the second scale use is only applied to the positioning. I guess the question is whether the mirror should be applied to the shift or not. I think if you used a photograph you would see that the mirror is occurring (the abs func is only affecting a translate function, right?). Choosing whether or not to apply the sign of the scale during translate should decide whether the shift follows the mirror or not.

For GL_MODELVIEW and GL_PROJECTION I've also forgotten the important aspects of these (haven't touched them in a while). Hopefully it won't affect anything because each draw() function sets its matrix mode, but if I've missed that for anything then that object might get affected by a change here. Or we set the matrix mode back to some default at the end of the command (or push/pop matrix mode?)

Maybe @toddrjen can comment on the change, since he wrote the original code here

@jeremygray
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From googling around, it looks like CL_PROJECTION is probably not the thing to use here, based on https://www.opengl.org/discussion_boards/showthread.php/133880-Diference-between-GL_MODELVIEW-and-GL_PROJECTION and also http://sjbaker.org/steve/omniv/projection_abuse.html "in most cases you would want to define the projection matrix just once and use the modelview matrix all other times."

Using only GL_MODELVIEW fixed another issue I was having. So I now think lines ~611+ should be like this

    #rescale/reposition view of the window
    GL.glMatrixMode(GL.GL_MODELVIEW)
    GL.glLoadIdentity()
    if self.viewScale is not None:
        GL.glOrtho(-1, 1, -1, 1, -1, 1)
        GL.glScalef(self.viewScale[0], self.viewScale[1], 1)
        scale = self.viewScale
    else:
        scale = [1, 1]

@jeremygray
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've convinced myself that the proper mirror-reversals are happening when viewScale values are either positive or negative, and these are properly interacting with viewPos values (a non-zero position is the critical test). viewOri is not interacting correctly yet.

Here's a test script that could become a test (ignoring viewOri):

# test window.viewScale and .viewPos simultaneous and negative-going values
# the green square/rect should move clockwise around the text

from psychopy import visual, event
win = visual.Window(size=(400,400), units='pix')

v = [(1,1),(1,-1),(-1,-1),(-1,1)]  # vertices to use = square
n = 30  # size of the square
pimg= (n,n)  # position for the image
pgrn = (-n,-n)  # position for green square
for offset in [(0,0), (-.4,0)]:
    win.viewPos = offset
    for scale in [[1,1],  # normal: green at lower left
                  [1,-1],  # mirror vert only: green appears to move up, text mirrored
                  [-1,-1],  # mirror horiz & vert: green appears to move right, text normal but upside down
                  [-1,1],  # mirror horiz only: green appears to move down, text mirrored
            [2,2],[2,-2],[-2,-2],[-2,2]]:  # same, but both larger
        win.viewScale = scale
        grn = visual.ShapeStim(win, vertices=v, pos=pgrn, size=n, fillColor='darkgreen')
        img = visual.ImageStim(win, image='s.png', pos=pimg)
        grn.draw()
        img.draw()
        win.flip()
        event.waitKeys()

with the attached file s.png (image containing text) that looks like "fill fill", here:
s

@peircej
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds great. Pending any comment from toddrjen I think we should include the change

Please sign in to comment.