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

Remove some unused variables, less verbose animation logic #1100

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

jherico
Copy link
Contributor

@jherico jherico commented Jan 25, 2024

This adds a Random utility class that provides more functionality than the existing rnd() function used in the multithreaded example.

It does some minor cleanup on the animation logic.

  • glm::vec3 rotation replaced by float rotationAxis; because the X and Z values are never read or written.
  • glm::mat4 model removed as it's only used as an intermediate value to write to the push constant structure
  • The push constant structure is absorbed by ObjectData because there's no reason for separate storage, they're always 1:1
  • Logic to initialize ObjectData, update it per-frame and extract the mat4 are moved into the structure itself, reducing the verbosity and amount of indirection required to access all the variables
  • Where possible convert values to radians at initialization time rather than each update
  • In a couple places where the logic was something like x = a + rnd(b) I've updated it to be random.real(a, a+b) which provides the same range of values but feels easier to read
  • Eliminated unused variables stateT, posX and posZ


glm::mat4 getModel() {
auto model = glm::translate(glm::mat4(1.0f), pos);
model = glm::rotate(model, -sinf(deltaT * M_TAU) * 0.25f, glm::vec3(rotationDir, 0.0f, 0.0f));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've reproduced the original logic here, but I'm confused. glm::rotate expects an angle here, in radiants, but we're passing the output of a sin function. Two lines below we pass in deltaT * M_TAU without wrapping it in a sin. I'm not sure what the intent is.

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

1 participant