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

feat(OverviewCam): Add OverviewRotation option #714

Closed
wants to merge 1 commit into from

Conversation

badosu
Copy link
Collaborator

@badosu badosu commented Mar 23, 2023

Add "OverviewRotation" option, default false, so Overview camera controller is oriented in the closest direction to last camera controller rotation.

This avoids jarring transitions, especially useful for flipped overhead camera.

Add "OverviewRotation" option, default false, so Overview camera
controller is oriented in the closest direction to last camera
controller rotation.

This avoids jarring transitions, especially useful for flipped overhead
camera.
@@ -9,9 +9,18 @@ class COverviewController : public CCameraController
{
public:
COverviewController();
~COverviewController();

constexpr static float3 DIR_UP = float3(0.0f, -0.999999500000375f, -0.000999999500000375f); // -90 deg
Copy link
Collaborator

Choose a reason for hiding this comment

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

looks suspiciously close to (0, -1, 0), could use a comment on why the tiny difference

Copy link
Collaborator Author

@badosu badosu Mar 23, 2023

Choose a reason for hiding this comment

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

That's a good question! This is the explicit version of what was the previous dir float3(0.0f, -1.0f, -0.001f).ANormalize().

I have absolutely no idea! My intuition was that it is looking down (almost negative 1 on y direction) and a tiny x,-x,z,-z component for each of the directions (the comments are wrong).

Copy link
Collaborator

@sprunk sprunk Mar 23, 2023

Choose a reason for hiding this comment

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

Try (0, -1, 0) and see what happens.

I think it's just to specify a "heading" around the Y axis, i.e. otherwise it's unspecified where "north" is, for example for camera panning purposes - these two could both have (0, -1, 0) dir up:
image
But hopefully that's where the other constants (DIR_{LEFT,RIGHT}) come in to disambiguate.

A straight top-down angle also used to break shadows in the past AFAIK, could check if that is still the case.

@lhog lhog force-pushed the BAR105 branch 2 times, most recently from 3925acd to d32df95 Compare April 17, 2023 12:47
@lhog
Copy link
Collaborator

lhog commented Jun 8, 2023

What is the status here?
How does it relate to #774 ?

Copy link
Collaborator

@sprunk sprunk left a comment

Choose a reason for hiding this comment

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

My comments are a potential further improvement but they aren't a blocker, if this works it's a good feature already IMO. But yes this sounds like a different take at #774.

@saurtron
Copy link
Collaborator

saurtron commented Jan 9, 2025

Is this still wanted now that #774 is merged?

According to conversation here they're somehow overlapping. @badosu, what do you think?

@badosu
Copy link
Collaborator Author

badosu commented Jan 9, 2025

Don't know. Probably not needed

@badosu badosu closed this Jan 9, 2025
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.

4 participants