Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ofCamera is not aware of ofOrientation #931

Closed
arturoc opened this issue Feb 23, 2012 · 60 comments
Closed

ofCamera is not aware of ofOrientation #931

arturoc opened this issue Feb 23, 2012 · 60 comments

Comments

@arturoc
Copy link
Member

arturoc commented Feb 23, 2012

No description provided.

@ofTheo
Copy link
Member

ofTheo commented Dec 10, 2012

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
Copy link
Member

ofTheo commented Dec 10, 2012

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
Copy link
Member Author

arturoc commented Dec 10, 2012

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
Copy link
Member

bakercp commented Dec 10, 2012

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
Copy link
Member

ofTheo commented Dec 10, 2012

@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
Copy link
Member Author

arturoc commented Dec 11, 2012

yes it will work with non gles2, and yes actually ofCamera doesn't have any gl calls anymore, well as soon as this PR: #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
Copy link
Member

ofTheo commented Dec 11, 2012

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
Copy link
Member Author

arturoc commented Dec 11, 2012

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
Copy link
Member

ofTheo commented Dec 11, 2012

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

@ofTheo
Copy link
Member

ofTheo commented Dec 11, 2012

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
Copy link
Member

ofTheo commented Feb 2, 2013

@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
Copy link
Member Author

arturoc commented Feb 2, 2013

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 #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
Copy link
Member

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
Copy link
Member

ofTheo commented Feb 3, 2013

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
Copy link
Member Author

arturoc commented Feb 3, 2013

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
Copy link
Member Author

arturoc commented Feb 3, 2013

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
Copy link
Member

@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
Copy link
Member

ofTheo commented Feb 3, 2013

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
Copy link
Member Author

arturoc commented Feb 3, 2013

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
Copy link
Member Author

arturoc commented Feb 3, 2013

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
Copy link
Member

ofTheo commented Feb 3, 2013

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
Copy link
Member Author

arturoc commented Feb 3, 2013

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
Copy link
Member Author

arturoc commented Feb 3, 2013

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
Copy link
Member

ofTheo commented Feb 3, 2013

"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
Copy link
Member

ofTheo commented Feb 3, 2013

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

@arturoc
Copy link
Member Author

arturoc commented Feb 3, 2013

:) ok perfect.

@roymacdonald
Copy link
Member

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
roymacdonald@ef9bccc
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
Copy link
Member Author

arturoc commented Feb 4, 2013

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
Copy link
Member

ofTheo commented Feb 4, 2013

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
Copy link
Member

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
Copy link
Member

ofTheo commented Jun 15, 2013

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
Copy link
Member

@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
Copy link
Member Author

arturoc commented Jun 17, 2013

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
Copy link
Member

julapy commented Jun 17, 2013

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//issues/931#issuecomment-19533915
.

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

@ofTheo
Copy link
Member

ofTheo commented Jun 17, 2013

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

  • ofSetOrientation now fails to work for iOS apps when no camera is used. ( so horizontal apps are being shown vertically ). check the ios/polygonExample to see this.
  • if I use an ofEasyCam ( say with the polygon example ) and I have the ofSetOrientation set for horizontal ( which is in the example by default ). The orientation is correctly applied to the camera, but the aspect ratio stretches the image, I think this is to do with the viewport potentially not being swapped?

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
Copy link
Member

julapy commented Jun 17, 2013

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
Copy link
Member Author

arturoc commented Jun 17, 2013

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
Copy link
Member

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
Copy link
Member

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
Copy link
Member

ofTheo commented Jun 17, 2013

perfect!
yes please :)

@roymacdonald
Copy link
Member

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
Copy link
Member

julapy commented Jun 18, 2013

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
Copy link
Member

@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.

@julapy
Copy link
Member

julapy commented Jun 18, 2013

im more like 3rd or 4th generation ofxiPhone dev :) its passed through a lot of hands...
ofxiPhoneSetOrientation() has been there from the very start... and now it has to go.

i can go through and deprecate all ofxiPhoneSetOrientation related functions.
if you're going through the iOS examples, its just a matter of switching ofxiPhoneSetOrientation for ofSetOrientation

@roymacdonald
Copy link
Member

@julapy OK. I'll check the examples tomorrow.

@roymacdonald
Copy link
Member

Hey,
there are no examples using either ofxiPhoneSetOrientation() or ofxiPhoneGetOrientation.
close issue?

@julapy
Copy link
Member

julapy commented Jun 20, 2013

hey @roymacdonald,
think all the landscape ios examples currently use ofxiPhoneSetOrientation in setup
just had a quick look inside advancedGraphics example and it still calls ofxiPhoneSetOrientation, unless its already been changed in a PR somewhere...

@ofTheo
Copy link
Member

ofTheo commented Jun 20, 2013

yeah, was going to say. its totally fine to change this in the examples to the newer system.
just definitely don't completely remove the old way ( as it can be considered essentially the same thing ).

@roymacdonald
Copy link
Member

ohh sorry! I've been a bit feverish all day long and only making mistakes. I did a cli find and grep that thew nothing but I misspelled something. I'll fix all those and PR.

@bilderbuchi
Copy link
Member

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

@julapy this is not fixed yet, right? Otherwise I think we can close this now.

@julapy
Copy link
Member

julapy commented Jun 20, 2013

i was thinking of removing those when ofxiPhoneSetOrientation and ofxiPhoneGetOrientation have been removed, but yeah i think you're right... since nothing is using those defines any more, its probably better to remove them.
will make this change now.

@ofTheo
Copy link
Member

ofTheo commented Jun 20, 2013

please don't remove them! ( the OFXIPHONE_ORIENTATION_LANDSCAPE_LEFT etc )
no need to break old apps that are making those calls.

I think marking ofxiPhoneSetOrientation as deprecated is enough.

@bilderbuchi
Copy link
Member

OK, closing this then since all other issues in here seem fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants