openframeworks / openFrameworks

openFrameworks is a community-developed cross platform toolkit for creative coding in C++.
http://openframeworks.cc
Other
9.83k stars 2.56k forks source link

ofGetCurrentMatrix(OF_MATRIX_PROJECTION) returns wrong matrix when camera is enclosed in FBO #4349

Open ghost opened 8 years ago

ghost commented 8 years ago

Without fbo:

#include "ofMain.h"

class ofApp : public ofBaseApp
{
  public:
    void setup();
    void draw();

    ofCamera cam;
};

int main()
{
  ofGLFWWindowSettings settings;

  auto window = ofCreateWindow(settings);
  auto app = make_shared<ofApp>();
  ofRunApp(window, app);

  return ofRunMainLoop();
}

void ofApp::setup()
{
}

void ofApp::draw()
{
  cam.begin();
    cout << ofGetCurrentMatrix(OF_MATRIX_PROJECTION) << endl << endl;
  cam.end();
}

With fbo:

#include "ofMain.h"

class ofApp : public ofBaseApp
{
  public:
    void setup();
    void draw();

    ofFbo fbo;
    ofCamera cam;
};

int main()
{
  ofGLFWWindowSettings settings;

  auto window = ofCreateWindow(settings);
  auto app = make_shared<ofApp>();
  ofRunApp(window, app);

  return ofRunMainLoop();
}

void ofApp::setup()
{
  fbo.allocate(ofGetWidth(), ofGetHeight());
}

void ofApp::draw()
{
  fbo.begin();
    cam.begin();
      cout << ofGetCurrentMatrix(OF_MATRIX_PROJECTION) << endl << endl;
    cam.end();
  fbo.end();
  fbo.draw(0,0);
}

Projection matrix without fbo (correct):

 1.29904,        0,        0,        0
       0,  1.73205,        0,        0
       0,        0,   -1.002,       -1
       0,        0, -13.3155,        0

Projection matrix with fbo (wrong):

 1.29904,       -0,        0,        0
       0, -1.73205,        0,        0
       0,        0,   -1.002,       -1
       0,        0, -13.3155,        0

Projection matrix with fbo taken both from cam and ofGetCurrentMatrix(OF_MATRIX_PROJECTION), side by side:

void ofApp::draw()
{
  fbo.begin();
    cam.begin();

      cout << "Matrix from cam: " << endl
           << cam.getProjectionMatrix() << endl;

      cout << "Matrix from ofGetCurrentMatrix: " << endl
           << ofGetCurrentMatrix(OF_MATRIX_PROJECTION) << endl << endl;

    cam.end();
  fbo.end();
  fbo.draw(0,0);
}

Should be the same, but they're not:

Matrix from cam: 
 1.29904,        0,        0,        0
       0,  1.73205,        0,        0
       0,        0,   -1.002,       -1
       0,        0, -13.3155,        0
Matrix from ofGetCurrentMatrix: 
 1.29904,       -0,        0,        0
       0, -1.73205,        0,        0
       0,        0,   -1.002,       -1
       0,        0, -13.3155,        0

Currently addons which rely on ofGetCurrentMatrix(OF_MATRIX_PROJECTION) command are all broken because it returns incorrect matrix if one wants record an FBO. For workaround you can pass camera directly as a parameter to custom draw() function.

Compiled against latest commit.

tgfrerer commented 8 years ago

Uh, that's probably related to the orientation matrix being applied into the projection matrix - in any case I agree, it seems counterintuitive that we're not returning the same matrix.

ghost commented 8 years ago

it seems counterintuitive that we're not returning the same matrix

And cause bugs for stuff like raycasting if you feed these values :)

arturoc commented 8 years ago

This fixes a lot other problems we had for more basic uses, it mostly has to do with the flip of the y coordinate. Mostly this allows to draw into an fbo and then when drawing that fbo it doesn't appear flipped.

This can be fixed to by flipping the texture coordinates when drawing the fbo instead of flipping the projection matrix but then shaders accessing the fbo as frag coordinates get the texture flipped, also when getting the pixels and saving for example you would also get the pixels flipped in y.

It's really hard to fix this problem for every possible case and the current solution has been working well for most people.

One possibility would be to add one more uniform with the non-flipped projection matrix, something like unorientedProjectionMatrix so shaders that need that can use it and perhaps a global getter for that matrix.

If you are using normal landscape orientation you can query something like:

if(ofIsVFlipped() && !fbo_binded){
    projection_matrix.scale(1,-1,1);
}

or rotate 180 in z, will give you the projection matrix without the orientation.

tgfrerer commented 8 years ago

Yeah, tricky.

On second thought, it makes sense: the camera will always return it's proper projection matrix, whilst ofGetCurrentMatrix queries the renderer's state (the matrix which will ultimately be fed through to any shaders using the default uniforms).

For ray casting I remember having used the camera's matrix, and not the renderer state, which is what I would suggest to do in your case, @procedural.

But I wonder whether the oriented or the "pure" projection matrix should be returned by default for the renderer query method, as the method returns not exactly what its name promises.

Maybe the default shaders could use the oriented projection matrix more explicitly ... definitely something I'd suggest thinking more about in terms of upcoming releases / strategy, unless we needed to fix this earlier if the current way introduced too many regression issues.

ghost commented 8 years ago

For ray casting I remember having used the camera's matrix, and not the renderer state, which is what I would suggest to do in your case, @procedural.

Yea, that's my current workaround which I mentioned above, I just feel uneasy that there's no way to get current bound camera matrix between cam.begin() and cam.end(), for addons which require projection especially, people are forced to pass camera around explicitly...

ghost commented 8 years ago

One possibility would be to add one more uniform with the non-flipped projection matrix, something like unorientedProjectionMatrix so shaders that need that can use it and perhaps a global getter for that matrix.

This would be ideal way since it won't break anything, I like it.

ghost commented 8 years ago

Also, unorientedProjectionMatrix uniform and global getter will indicate for people that addon uses 0.9.0

ghost commented 8 years ago

The problem is worse than I expected... I have 2 addons in works, both of which require camera matrix for interactions, this is how my code starts to look now:

void ofApp::draw()
{
  cam.begin();
    addonObj1.draw(cam);
    ...
    addonObj2.draw(cam);
    addonObj3.draw(cam);
  cam.end();
}
ghost commented 8 years ago

By the way,

This fixes a lot other problems we had for more basic uses, it mostly has to do with the flip of the y coordinate. Mostly this allows to draw into an fbo and then when drawing that fbo it doesn't appear flipped.

can GLSL's origin_upper_left qualifier help with that without doing any flips?