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

Culling issue #223

Merged
merged 2 commits into from
Sep 10, 2012
Merged

Culling issue #223

merged 2 commits into from
Sep 10, 2012

Conversation

bagnell
Copy link
Contributor

@bagnell bagnell commented Sep 7, 2012

The culling volume should use the camera position and orientation in world space. Looks like I introduced this when adding the culling volume. You can test it with the Camera Reference Frames Sandcastle example. This fixes part of issue #209.

Also, the central body was using the model-view matrix of the uniform state instead of the view matrix. I fixed that which addresses issue #117. @mramato This also fixes the problem in the viewerCamera branch where you were seeing parts of the central body orbiting with a satellite.

@bagnell bagnell mentioned this pull request Sep 8, 2012
@@ -2104,7 +2104,7 @@ define([
}

var uniformState = context.getUniformState();
var mv = uniformState.getModelView();
var mv = uniformState.getView();
Copy link
Contributor

Choose a reason for hiding this comment

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

The variable name mv for "model view" no longer makes sense, right?

Are you sure about this change. In my OpenGlobe RTC implementation, I use the model-view matrix. This is also how Deron described it.

@kring changes here will affect your terrain code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, I'll change the variable name.

It would be the model-view matrix if the central body had a model matrix. I think there might be a bug where the uniform state doesn't update the model matrix to the identity if the primitive doesn't have one.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'll take a look. I actually don't like the model matrix the way it is now. It is needed in two locations - once returned by update and once in the uniforms list.

So do you think this is the right workaround? Why not add the model matrix to the central body instead of changing the RTC algorithm to look like it is potentially doing something else?

@pjcozzi pjcozzi mentioned this pull request Sep 9, 2012
@pjcozzi
Copy link
Contributor

pjcozzi commented Sep 10, 2012

This also fixes the "Camera Reference Frame" Sandcastle example, which didn't draw its polylines.

@mramato
Copy link
Contributor

mramato commented Sep 10, 2012

Tests still pass (It would be nice if we had a new test for this specific issues, but I know Scene and CentralBody need testing all around, so I'll let this slide for now.) Reference frame example works as expected. Things have definitely improved.

mramato added a commit that referenced this pull request Sep 10, 2012
@mramato mramato merged commit c66b423 into master Sep 10, 2012
mramato added a commit that referenced this pull request Oct 22, 2015
Fix vectors after merging open-source.
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.

3 participants