openframeworks / openFrameworks

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

ofEasyCam setPosition doesn't work as ofCamera's does #532

Closed ofTheo closed 11 years ago

ofTheo commented 13 years ago

if ofEasyCam camera.setPosition(ofPoint(ofGetWidth()/2, ofGetHeight()/2, 700));

results in the bottom left corner of the normal OF view appearing at the center of the screen

if its ofCamera: camera.setPosition(ofPoint(ofGetWidth()/2, ofGetHeight()/2, 700));

results in the center of the normal OF View appearing at the center of the screen.

shouldn't these be the same @memo ?

bilderbuchi commented 12 years ago

@elliotwoods @roymacdonald is this bug still relevant? I have no idea sadly, but I remember you worked heavily on camera stuff.

roymacdonald commented 12 years ago

It shouldn't be relevant as theo's post is quite old compared to the new ofEasyCam implementation. Yet, I'll check it and let you know.

bilderbuchi commented 12 years ago

gracias!

roymacdonald commented 12 years ago

ok. Just by declaring an ofCamera and an ofEasyCam and just calling setPosition(ofPoint(ofGetWidth()/2, ofGetHeight()/2, 700)); in the setup will give different results for each camera. This is not a bug at all. What's going on is that ofEasyCam sets some extra stuff while initing. So, if the same things are setted manually for the ofCamera the results would be the same as for ofEasyCam. Although this is not a bug is probably not what one would expect. I think that changing either ofCamera or ofEasyCam so both init in the same way wont be much trouble. but which one should be modified?

check this code:

testApp.h

#pragma once

#include "ofMain.h"

class testApp : public ofBaseApp {
    public:

        void setup();
        void update();
        void draw();

    void drawCubes();

    ofEasyCam easycam;
    ofCamera ofcam;
};

testApp.cpp

#include "testApp.h"

//--------------------------------------------------------------
void testApp::setup(){
    ofSetVerticalSync(true);
    glEnable(GL_DEPTH_TEST);
        //*//this block of code is what is making ofCamera to init at the same place as ofEasyCam
    easycam.setPosition(ofPoint(ofGetWidth()/2, ofGetHeight()/2, 700));
    ofNode ofCamTarget = easycam.getTarget();
    ofcam.lookAt(ofCamTarget);

    ofcam.setPosition(ofPoint(ofGetWidth()/2, ofGetHeight()/2, 700));
    float distance = ofcam.getImagePlaneDistance(ofGetCurrentViewport());
    if (distance > 0.0f){
        ofcam.setPosition(ofCamTarget.getPosition() + (distance * ofcam.getZAxis()));
    }
        */ 
}

//--------------------------------------------------------------
void testApp::update(){

}

//--------------------------------------------------------------
void testApp::draw(){
    //*
    ofcam.begin();
    drawCubes();
    ofcam.end();
    //*/
    easycam.begin();
    drawCubes();
    easycam.end();

    drawCubes();
    //*/
}
//--------------------------------------------------------------
void testApp::drawCubes(){

    //ofRotateX(ofRadToDeg(.5));
    //ofRotateY(ofRadToDeg(-.5));

    //ofBackground(0);

    ofSetColor(255,0,0);
    ofFill();
    ofBox(30);
    ofNoFill();
    ofSetColor(0);
    ofBox(30);

    ofPushMatrix();
    ofTranslate(0,0,20);
    ofSetColor(0,0,255);
    ofFill();
    ofBox(5);
    ofNoFill();
    ofSetColor(0);
    ofBox(5);
    ofPopMatrix();

}
bilderbuchi commented 11 years ago

@ofTheo: do you have any input for @roymacdonald here?

ofTheo commented 11 years ago

well there is a bit of danger in changing default behaviors - but if we did I would be in favor of changing ofCamera to match ofEasyCam.

would be good to get other thoughts on this @arturoc @elliotwoods

but it might not be such an issue as people using ofCamera are expecting to do more manual setup anyway, so it might only affect a small % of ofCamera users.

roymacdonald commented 11 years ago

Hi. I'll check as soon as I get to my computer. At a barbecue now :)

Roy Macdonald 8248-8478

On 02-12-2012, at 17:14, Christoph Buchner notifications@github.com wrote:

@ofTheo https://github.com/ofTheo: do you have any input for @roymacdonaldhttps://github.com/roymacdonaldhere?

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

elliotwoods commented 11 years ago

so i presume the issue is that they have different default fov. i would propose that ofEasyCam is set to match ofCamera, (unless that makes ofEasyCam is unusable), as ofEasyCam is generally set for interactive environments where dollying the camera out isn't an issue, whereas ofCamera is generally used in static situations, and is more likely to break projects.

ofTheo commented 11 years ago

hey @elliotwoods def see your point. it would be good though to check the example above as its not really a fov issue - but a camera center / look-at issue.

changing ofEasyCam to match ofCamera would result in a pretty strange experience afaict.

roymacdonald commented 11 years ago

I completely agree with @elliotwoods. so I went through it and found several things. In the example that I posted before in the setup both cameras are forced to match each other by making them look at the same point, the target, and be at the same distance from it, hence position and orientation are the same in both cameras.

By default after each camera's constructor is run their properties are the same: this are ofCam pos : 0, 0, 0 rot : 0, 0, 0, 1 scale: 1, 1, 1 fov : 60 easyCam pos : 0, 0, 0 rot : 0, 0, 0, 1 scale: 1, 1, 1 fov : 60

Yet, ofEasyCam checks if the distance to the target has been set in it's first update: if(!bDistanceSet){ setDistance(getImagePlaneDistance(viewport), true); } making it's z position not 0.

so if this check is avoided ofEasyCam would init with the same properties as ofCamera, but by doing so easycam inits positioned at the origin which in many cases could be a bit annoying as it would be over or "inside" the rendered geometries.

Besides this, by avoiding the distance set check a bug in ofEasyCam that I just found becomes completely evident. I posted an issue for this bug. (this bug can be easily solved though). https://github.com/openframeworks/openFrameworks/issues/1718

So, I guess that having the option to enable/disable this distance check would be the way to go. I'd leave it by default as it is right now and have a method like disableDistanceCheck() so it inits in the same way as ofCam.

What do you think?

arturoc commented 11 years ago

+1 for making ofEasyCam behave like ofCamera or not modifying them at all, ofCamera it should be as raw as possible while ofEasyCamera is slightly smarter so it does this checks that ofCamera shouldn't be doing or you'll get unexpected behaviors. not sure about disableDistanceCheck() the name is kind of obscure but yes if we can find a better naming that's probably the best solution

ofTheo commented 11 years ago

okay - in that case I would be in favor of not modifying ofEasyCam. I think the point of ofCamera being bare bones, make sense. But modifying ofEasyCam so it sets up the same as ofCamera (even if it is used more interactively), could still change/break a lot of projects.

roymacdonald commented 11 years ago

@arturoc sure, a better name for such method is needed. @ofTheo, adding this method to disable the distance check would make the easycam set up with the same properties as of cam but it wouldn't be the default so easycam would still behave in the same way it does now, so nothing would be broken/changed. I guess we should wait for Zack and others opinion to decide?

ofTheo commented 11 years ago

oh - sorry, totally fine with that. as long as the default view doesn't change.

T

roymacdonald commented 11 years ago

:) so, any idea for a good name for the mentioned method? initLikeOfCamera()?

or a adding an argument to the constructor?

ofTheo commented 11 years ago

setAutoDistance(bool bAutoDistance); ?

roymacdonald commented 11 years ago

its fine for me. anyone else?

roymacdonald commented 11 years ago

So I added the setAutoDistance feature. Here it is. https://github.com/roymacdonald/openFrameworks/tree/fix-ofEasyCam Should I PR?

bilderbuchi commented 11 years ago

sure :-)

bilderbuchi commented 11 years ago

Closed by #1724