openframeworks / openFrameworks

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

ofCamera is not aware of ofOrientation #931

Closed arturoc closed 11 years ago

ofTheo commented 11 years ago

For iOS this means that unless you are setup in the default portrait orientation ofCamera and ofEasyCam are basically useless as they don't take into account the viewport shape or the orientation of the screen.

We should look at adding in the orientation code from: ofGLRenderer::setupScreenPerspective Or unify the main window rendering code and ofCamera rendering code.

ofTheo commented 11 years ago

would be great to get some thoughts on this: @elliotwoods @roymacdonald @julapy

my feeling is that we shouldn't have duplicated code in ofGLRenderer::setupScreenPerspective and ofCamera::begin - we could move to a internal ofCamera or have ofCamera call a new ofSetupView function?

maybe the latter is better in this case as it would allow ofCameras to be rendered by other renders?

ie:

ofSetupView is called by ofSetupScreenPerspective ( simplified ) and ofSetupView is called by ofCamera.begin()

Internally ofSetupView passes off to render->setupView which in turn handles orientation and rendering commands.

This could be done in a way that is backwards compatible with our existing system. It would just mean no GL code in ofCamera ( which it prob shouldn't have anyway ).

arturoc commented 11 years ago

i think this will be solved with the gles2 branch, there we have an orientation matrix that is multiplied to the modelview everytime that one is uploaded. i want to refactor that code and some more matrices related code out of the gles2 renderer so it's used by both gl and gles2. that will help to solve also the problems with the vertical flip

bakercp commented 11 years ago

Also, for those who are interested, over in gles2/raspberrypi/e-linux land, we are updating examples as needed to get rid of all native matrix-related gl-calls in favor of the new matrix math future-gl-safe system metioned by @arturoc system. We are also replacing all openGL "immediate mode" calls (yes, there were still a few left :)) with oF-wrapped gles2/future-safe calls.

ofTheo commented 11 years ago

@arturoc will that work for non gles2 ? ie if someone uses ofSetOrientation(OF_ORIENTATION_90_RIGHT) with desktop?

just to confirm - the idea is to remove all gl calls from ofCamera?

arturoc commented 11 years ago

yes it will work with non gles2, and yes actually ofCamera doesn't have any gl calls anymore, well as soon as this PR: https://github.com/openframeworks/openFrameworks/pull/1666 is merged but that's included in the gles2 branch already so it can wait anyway

the idea is that when you set the orientation with ofSetOrientation there'll be an orientation matrix somewhere (right now in the renderer) then when you do any modelview transformation (translate,rotate...) or upload a new modelview that will get multiplied by the orientation matrix so the camera will work with that, of course as long as you use OF calls

ofTheo commented 11 years ago

sounds great. Not sure if that solves the movement of the camera though right? ie: dolly, truck, boom etc

that might get trickier as for positioning y becomes x etc

arturoc commented 11 years ago

not sure but i think it should: all the movements of the camera are store in a 4x4 matrix and uploaded to the graphics card in begin() using ofLoadMatrix which in turn multiplies it by the orientation matrix so it whould work

ofTheo commented 11 years ago

ahh okay great. going to test this with the gles2 branch.

ofTheo commented 11 years ago

just a quick test using the iOSES2ShaderExample - if I modify it to use ofEasyCam it behaves (mostly) as expected. but changing the orientation to horizontal the camera is no longer looking at (0,0,0)

it seems that the orientation is not quite correctly handled yet in #1668

to test try replacing this gist with testApp.mm in iOSES2ShaderExample and then uncomment line 13

see the screenshots here ( i had to rotate the horizontal one so you could see where the text was being drawn ):

The only difference between the two is: ofSetOrientation(OF_ORIENTATION_90_LEFT);

Screen shot 2012-12-11 at 10 54 35 AM

Screen shot 2012-12-11 at 10 55 34 AM

ofTheo commented 11 years ago

@arturoc @julapy any chance we could get a PR for just the camera / orientation fixes? would be great to get this in 0.7.4

arturoc commented 11 years ago

this is currently only working in gles2, in old opengl there's no way to avoid people from mixing ofPush/Pop/LoadMatrix with the gl versions which will mess things up, i think opengl1 should stay as it is and in the new renderer, since we have to manage the stacks ourselves anyway, we can intercept these calls and multiply by the orientation matrices

i've just merged this https://github.com/openframeworks/openFrameworks/pull/1846 with that we could make ofLoadMatrix multiply the matrix by the orientation in old gl too so at least cameras will be aware of orientation

roymacdonald commented 11 years ago

cant we have an event sent each time the orientation changes, either by manually calling ofSetOrientation or when the device reports an orientation change? this way ofCamera can listen to the orientation changes and call ofSetupPerspective. mouse interactions (in the case of ofEasyCam) should be fine as the mouse is already rotated in the glut mouse callbacks. I suspect that this shouldn't interfere with the opengl version being used. I'll try to implement it. any thoughts about this idea?

ofTheo commented 11 years ago

I think this would be good to get in pre ES 2.0 Its really quite a lot of work using ofCamera / ofEasyCam on iOS in non portrait mode.

I think it shouldn't be too hard to fix!

arturoc commented 11 years ago

i don't think it's necessary to have an event, and it'll complicate things even more, we just need to check the orientation when setting up the matrix, in the gles2 version there's actually a matrix for the orientation and all the projectionmodelviews are multiplied by that, it's only a matter of adding that at a global level but only for the case of ofLoadMatrix or have the camera ask for the orientation or the orientation matrix when setting up the matrix

arturoc commented 11 years ago

btw, won't using ofxiPhoneSetOrientation solve that? in android this works cause the orientation is managed by the device can't we make ofSetOrientation go through ofxiPhoneSetOrientation the way it's done in android

roymacdonald commented 11 years ago

@arturoc I was thinking about something like that when i mentioned the event thing, but rethinking it with out the event what you say seems quite plausible. I'll check that and let you know.

ofTheo commented 11 years ago

as far as I am aware ofxiPhoneSetOrientation doesn't work because all that code is in the core setupScreenPerspective and ofCamera has its own screen setup code.

arturoc commented 11 years ago

not sure what that function does but doesn't seem to be related to setupPerspective, this is the code:

switch (orientation) {
    case OF_ORIENTATION_DEFAULT:
        [[UIApplication sharedApplication] setStatusBarOrientation: UIInterfaceOrientationPortrait];
        break;
    case OF_ORIENTATION_180:
        [[UIApplication sharedApplication] setStatusBarOrientation: UIInterfaceOrientationPortraitUpsideDown];
        break;
    case OF_ORIENTATION_90_RIGHT:
        [[UIApplication sharedApplication] setStatusBarOrientation: UIInterfaceOrientationLandscapeLeft];
        break;
    case OF_ORIENTATION_90_LEFT:
        [[UIApplication sharedApplication] setStatusBarOrientation: UIInterfaceOrientationLandscapeRight];          
         break;
}

which seems to be setting the application orientation through iOS native calls, perhaps @julapy can clarify it but i remember he mentioning that this call is now prefered to ofSetOrientation.

there's a flag, ofDoesHWOrientation() which is read by ofSetOrientation in order to know if it needs to alter the matrices or the orientation is done in hardware (as it is in android) so perhaps is just a matter of implementing that.

arturoc commented 11 years ago

that said it would be useful to implement the plain orientation method so the camera is aware of it but it's kind of a complex change if we do it for every renderer not only GL and would be a good occasion for refactoring the matrix management in general (ie: the vflip...) so i would prefer to not do a quick patch that will make the code more complex/messy than it is right now

ofTheo commented 11 years ago

this is the code I was talking about:

https://github.com/openframeworks/openFrameworks/blob/master/libs/openFrameworks/gl/ofGLRenderer.cpp#L379

we used to be doing this separately in ofxiPhone but it was consolidated with desktop so that you can easily run screens in vertical orientation with just one line of code.

I believe this: [[UIApplication sharedApplication] setStatusBarOrientation: UIInterfaceOrientationLandscapeRight];

just changes the status bar orientation - you are still give a vertical openGL world even if the orientation is set to horizontal.

I'm definitely for adding a way to set orientation for ofCamera / ofEasyCam. The rest of OF has it and right now for both desktop and iphone you can run almost anything in OF in different orientations except ofCamera.

Anyway I'm happy to help - we could push this back to 0.7.5 - but would love to get this resolved soon.

arturoc commented 11 years ago

mmh ok, then the best way to do it i think would be to add a check in ofLoadMatrix (in the glrenderer by now) where if the matrix mode is model view then it modifies the uploaded matrix accordingly to apply the orientation. i can then generalize that for all the renderers in the gles2 branch

arturoc commented 11 years ago

btw, i was looking at this sometime ago and i don't see why do we need to pass the orientation in the setupPerspecive method. Won't it be better to just use ofSetOrientation and then use ofGetOrientation from setupPerspective? what happens if you set a perspective with ofSetOrientation and then use a different one in setupPerspective?

ofTheo commented 11 years ago

"what happens if you set a perspective with ofSetOrientation and then use a different one in setupPerspective?"

ha - no idea. it might allow for some interesting perspectives :) but you're right - could be ofGetOrientation instead.

ofTheo commented 11 years ago

ps - I moved this back to 0.7.5 - so we have a bit more time to get a good working solution.

arturoc commented 11 years ago

:) ok perfect.

roymacdonald commented 11 years ago

Hey guys, by modifying ofCamera I got it to work correctly. probably this isn't the definitive fix but at least works as a proof of concept. check it here https://github.com/roymacdonald/openFrameworks/commit/ef9bccc0df916d281fb2db5f937c6178e05bdcca As the orientation relative transform is handled by the camera there's no need to adapt it for each renderer, although I don't know how are the rest orientation relative transforms handled, but as theo says, ofCamera is the only thing missing the orientation relative transforms, just implementing this would make everything work, although not the most correct thing to do, I guess. BTW, the orientationMatrices array that I added to ofCamera should be probably a static global variable. Maybe it could be even better to have a function like ofGetOrientationMatrix() that implements what i've implemented here in ofCamera but at a "global" level, so I'ts also useful to other classes and for calling it directly.

arturoc commented 11 years ago

this is simple enough to merge it even if we do something else, only thing can you check ofDoesHWOrientation and not multiply by the orientation in that case. didn't remember that cameras don't have vflip so it is way simpler than i expected. @ofTheo if you think this is ok and want to merge it, go for it. i'll move it to somewhere else more general before merging gles2

ofTheo commented 11 years ago

great - thanks @roymacdonald - I'll test this today and confirm it works. looks pretty clean / straightforward.

btw - does it make sense to apply the rotation when the orientation is no orientation? right now it seems to be also doing orientationMatrices[0] which is no rotation.

roymacdonald commented 11 years ago

Ok. so I added ofDoesHWOrientation check. also made static ofCamera::orientationMatrices. @ofTheo It was just my laziness. I had it already working when I realized that orientationMatrices[0] wasn't needed. I removed it btw. https://github.com/roymacdonald/openFrameworks/commit/7ec240d52c04b16491a313681f5dd2edd5710029

@ofTheo @arturoc If you're alright with it just tell me and I can do a PR.

kylemcdonald commented 11 years ago

hey @ofTheo did you get the chance to test this and confirm it works?

if so let's merge this asap since it's marked "critical" :)

i remember at eyeo you also said you have a "custom" branch of OF that you've been using recently, maybe this is one of the things you've fixed...? :)

arturoc commented 11 years ago

this is fixed in the https://github.com/arturoc/openFrameworks/tree/feature-elinuxProgrammableGL

bilderbuchi commented 11 years ago

@arturoc will you PR that branch sometime soon, so we can mark/link all the issues which are fixed by it?

ofTheo commented 11 years ago

I did test @roymacdonald's branch. It didn't completely solve the problem though.

The camera orientation does work but there our some other unaddressed issues.

Outstanding issues:

@arturoc what is the timeline for merging in feature-elinuxProgrammableGL ? Just wondering if we can wait on fixing this, till thats in. Also @arturoc do you know if your branch fixes the other issues I listed?

For iOS at least the better approach will be to do hardware orientation and not have to deal with the craziness of everything being rotated. But having this fixed will be needed for people who need to make a desktop project run in portrait mode when using cameras.

arturoc commented 11 years ago

we should merge it today. i'm not completely sure if it fixes everything for iOS but i've done tons of tests on desktop and seems to work for every case

ofTheo commented 11 years ago

great! will be good to have it in so we can fix any remaining issues!

roymacdonald commented 11 years ago

Hi, I was trying to fix this, but as I saw that arturo's programmableGL branch fixed this issue and several others I didn't continue trying to fix this. In fact arturo's branch has several other really nice things. +100 for merging it :)

ofTheo commented 11 years ago

I've noticed a few issues in the programmableGL branch in terms of orientation and cameras - so lets keep this issue open till all the issues are patched. it is much better though than how things were.

roymacdonald commented 11 years ago

@ofTheo what are those issues you've noticed? so far I've tested this with my computer, iPhone and iPad. with the computer it seems to behave correctly. This is, that when the device is rotated the camera rotates accordingly around its z axis, so when I rotate the device the things are drawn ok; If I draw something "center, up", I'll always see it that way regardless of the orientation of the device. This also means that the drawing coordinates never change from my point of view, the screen origin is always in the upper left corner. worldToScreen, screenToWorld, worldToCamera and cameraToWorld all seem to behave correctly. According to you, Is this the expected behavior?

on iOS devices it doesn't work. so far I'm guessing that it has to do with the ofGetCurrentViewport function behaving incorrectly. Thus cameras behave incorrectly. It always returns the same value, regardless of the orientation. edit: it totally has to do with the viewport. forcing the camera to use the expected viewport depending on orientation gives the expected camera behavior. @arturo, is the programmableGL renderer affecting the renderers for iOS? there might be the fix? I'm still digging in the iOS rendering workflow in order to understand what's going on. Any clues?

There is only one "odd" thing that happens with the camera position. As it is calculated using the viewport, and the fov, the fov remains constant so the camera moves over it's z axis depending on the orientation. This behavior is correct but I'm not sure if it is what one would expect. From a user point of view, I'd expect that the camera stays in the same place and that I'm able to see more to the sides or up/down when I rotate the device. Should we do something with this? I think I know how to implement this.

All the best!

arturoc commented 11 years ago

i think the problem with iOS is that there's different methods to handle the orientation and probably need to be changed to mirror how ofSetOrientation behaves right now. the renderers are the same for every platform, ofGLRenderer by default but you can set programmable in main before setupGL. Surely @julapy has a clue of what can be going on

julapy commented 11 years ago

will look into this a bit later today.

On Mon, Jun 17, 2013 at 7:14 PM, arturo notifications@github.com wrote:

i think the problem with iOS is that there's different methods to handle the orientation and probably need to be changed to mirror how ofSetOrientation behaves right now. the renderers are the same for every platform, ofGLRenderer by default but you can set programmable in main before setupGL. Surely @julapy https://github.com/julapy has a clue of what can be going on

— Reply to this email directly or view it on GitHubhttps://github.com/openframeworks/openFrameworks/issues/931#issuecomment-19533915 .

Lukasz Karluk | Interaction Design www.julapy.com - 0415179079

ofTheo commented 11 years ago

@roymacdonald - what I've seen since the new branch is merged in is:

Actually most likely because the ofSetOrientation is not being applied to the app as a whole. I bet if the first issue gets fixed it will fix the second too.

I just checked on desktop and this issue doesn't seem to be present.

screen shot 2013-06-17 at 7 48 52 am

note in the image that the star and shapes are stretched in the x axis.

julapy commented 11 years ago

ive been looking into the orientation issue on iOS and i think the problem may be inside ofMatrixStack::setOrientation

the orientation matrix is now only made up of rotation, but it doesn't translate the screen after rotating it like it did previously. to see the difference, look inside ofMatrixStack::setOrientation and compare it to the way orientation was previously handled in the code below.

if(ofDoesHWOrientation()){
    if(vFlip){
        glScalef(1, -1, 1);
        glTranslatef(0, -height, 0);
    }
}else{
    if( orientation == OF_ORIENTATION_UNKNOWN ) orientation = ofGetOrientation();
    switch(orientation) {
        case OF_ORIENTATION_180:
            glRotatef(-180, 0, 0, 1);
            if(vFlip){
                glScalef(1, -1, 1);
                glTranslatef(-width, 0, 0);
            }else{
                glTranslatef(-width, -height, 0);
            }

            break;

        case OF_ORIENTATION_90_RIGHT:
            glRotatef(-90, 0, 0, 1);
            if(vFlip){
                glScalef(-1, 1, 1);
            }else{
                glScalef(-1, -1, 1);
                glTranslatef(0, -height, 0);
            }
            break;

        case OF_ORIENTATION_90_LEFT:
            glRotatef(90, 0, 0, 1);
            if(vFlip){
                glScalef(-1, 1, 1);
                glTranslatef(-width, -height, 0);
            }else{
                glScalef(-1, -1, 1);
                glTranslatef(-width, 0, 0);
            }
            break;

        case OF_ORIENTATION_DEFAULT:
        default:
            if(vFlip){
                glScalef(1, -1, 1);
                glTranslatef(0, -height, 0);
            }
            break;
    }
}
arturoc commented 11 years ago

this is because the orientation now is done in the projection matrix not in the model view so the code is way simpler but works the same, this works on desktop so it must be something else

roymacdonald commented 11 years ago

Hey, I found that the problem has to do with the swapping of the width and height. I modified the getCurrentViewport function within matrixStack.cpp so it doesn't swap withd and height for the iphone and the cameras now work correctly but the 2D drawing is strerched sideways.

ofRectangle ofMatrixStack::getCurrentViewport(){
    ofRectangle currentViewport = this->currentViewport;
    if (isVFlipped()){
        currentViewport.y = getRenderSurfaceHeight() - (currentViewport.y + currentViewport.height);
    }
    if(ofGetTargetPlatform() !=OF_TARGET_IPHONE){
    if(!doesHWOrientation() && (orientation==OF_ORIENTATION_90_LEFT || orientation==OF_ORIENTATION_90_RIGHT)){
        swap(currentViewport.width,currentViewport.height);
        swap(currentViewport.x,currentViewport.y);
    }
    }
    return currentViewport;
}

I think that the stretching issue has to do with the width and height not being updated according to the orientation within the iPhone specific code. within ofxiOSEAGLView.mm I changed the following inside drawView function ofViewport(ofRectangle(0, 0, windowSize->x, windowSize->y)); for ofViewport(ofRectangle(0, 0, ofGetWidth(), ofGetHeight()));

as windowSize is not updated according to the orientation It still doesn't work but it is better that in the begining. screen-shot-2013-06-17-at-2 04 23-pm

I'm quite confident that the sideways stretching has to do with something within the iOS rendering workflow that is using a non updated var for the width instead of using ofGetWidth, I'll let you know if I find anything.

roymacdonald commented 11 years ago

OK. Found the problem. Just by changing within ofxiOSEAGLView.mm in drawView function ofViewport(ofRectangle(0, 0, windowSize->x, windowSize->y)); for ofViewport(ofRectangle(0, 0, ofGetWidth(), ofGetHeight())); forget what I said about matrixStack fixes all the problems. 2D drawing as well as cameras. haven't tried all the possible scenarios but so far it seems to be the fix. Should I PR?

ofTheo commented 11 years ago

perfect! yes please :)

roymacdonald commented 11 years ago

great. As I was testing the examples I was copying and pasting ofSetOrientation(ofOrientation(newOrientation)); into void testApp::deviceOrientationChanged(int newOrientation){} I think that it would be nice to add this to all the examples as orientation now works correctly. Should I do it? what do yo think

julapy commented 11 years ago

yeah now that orientation in managed by the new renderer, think we should deprecate ofxiPhoneSetOrientation() and ofxiPhoneGetOrientation()

there is also a bunch of defines, OFXIPHONE_ORIENTATION_LANDSCAPE_LEFT etc which also need to go.

but yeah a good place to start is by going through the examples and make sure they are only using, ofSetOrientation(), ofGetOrientation() and ofOrientation

roymacdonald commented 11 years ago

@julapy I already tried ofSetOrientation(), ofGetOrientation() and ofOrientation()with the examples and these work correctly. It sounds reasonable to delete those iPhone specific orientation stuff. You wrote the OF iPhone implementation? if yes you might have a better idea about what to delete.