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

Heading Tilt #1131

Merged
merged 9 commits into from Sep 11, 2013
Merged

Heading Tilt #1131

merged 9 commits into from Sep 11, 2013

Conversation

bagnell
Copy link
Contributor

@bagnell bagnell commented Sep 10, 2013

Added getHeading, setHeading, getTilt and setTilt to CameraController for #977 and #1111.

Roll was unneeded. It can be done with:
camera.controller.look(camera.direction, angle);

var tilt = this.getTilt();
angle = angle - tilt;

/*
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove commented out code.

@pjcozzi
Copy link
Contributor

pjcozzi commented Sep 10, 2013

@bagnell this is good with me, but do you want to move the entire camera to get/set properties instead of functions since these are not performance critical? Or perhaps I should ask: how much of the camera is called more than once per frame? Perhaps some getters for automatic uniforms? Maybe we could provide fast accessors for those until browsers get their act together.

@emackey
Copy link
Contributor

emackey commented Sep 10, 2013

@pjcozzi we need this for GSoC (/cc @macoda), so can we do the getters/setters in a separate pull after this is merged?

@pjcozzi
Copy link
Contributor

pjcozzi commented Sep 10, 2013

@emackey just branch off this in the meantime. Why bring something into master that we might change in a day or two?

@bagnell
Copy link
Contributor Author

bagnell commented Sep 10, 2013

@pjcozzi This is ready for another review. I changed the camera to use get/set properties.

@pjcozzi
Copy link
Contributor

pjcozzi commented Sep 11, 2013

Looks good. Update the camera tutorial if need be.

pjcozzi added a commit that referenced this pull request Sep 11, 2013
@pjcozzi pjcozzi merged commit 0831633 into master Sep 11, 2013
@pjcozzi pjcozzi deleted the headingTiltRoll branch September 11, 2013 15:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants