Skip to main content
added 24 characters in body
Source Link
bcrist
  • 2.3k
  • 18
  • 16
  • Consider using glm::radians and glm::degrees instead of magic literal constants. This makes your intent much clearer, in addition to being less error prone and often more accurate. Note also that older versions of GLM's GTC_matrix_transform took angles in degrees by default, but newer versions expect radians by default throughout the entire library.

  • glm::lookAt isn't the most efficient way to generate a view matrix in situations like yours, where you're just generating the camera position from the target position (or vice-versa). It would be more efficient and clearer to just compose the view matrix directly. GLM's GTX_euler_angles provides convenient methods for this. For example:

     glm::mat4 view = glm::translate(glm::mat4(), glm::vec3(0, 0, -10.f))
             * glm::eulerAngleXY(pitch_in_radians, yaw_in_radians)
    
  • Try to use a consistent naming scheme. This is often more difficult in C++ than other popular languages where libraries have mostly standardized on a single style, but it's still good practice to be consistent in your own code. You have some parameters and variables in PascalCase, while others are in camelCase. Try to avoid naming functions, variables, and parameters with PascalCase. Outside of C# (and Pascal, of course) capitalized identifiers are usually reserved for types. Additionally InMatrix is a confusing name, considering this is what would be known as an out-parameter in C#. In C++ it's typical not to clutter up names with such metadata. Instead just assume that if a function takes a non-const reference-type parameter T& param, it plans to modify it.

  • Consider using glm::radians and glm::degrees instead of magic literal constants. This makes your intent much clearer, in addition to being less error prone and often more accurate. Note also that older versions of GLM's GTC_matrix_transform took angles in degrees by default, but newer versions expect radians by default throughout the entire library.

  • glm::lookAt isn't the most efficient way to generate a view matrix in situations like yours, where you're just generating the camera position from the target position (or vice-versa). It would be more efficient and clearer to just compose the view matrix directly. GLM's GTX_euler_angles provides convenient methods for this. For example:

     glm::mat4 view = glm::translate(glm::mat4(), glm::vec3(0, 0, -10.f))
             * glm::eulerAngleXY(pitch_in_radians, yaw_in_radians)
    
  • Try to use a consistent naming scheme. This is often more difficult in C++ than other popular languages where libraries have mostly standardized on a single style, but it's still good practice to be consistent in your own code. You have some parameters and variables in PascalCase, while others are in camelCase. Try to avoid naming functions, variables, and parameters with PascalCase. Outside of C# capitalized identifiers are usually reserved for types. Additionally InMatrix is a confusing name, considering this is what would be known as an out-parameter in C#. In C++ it's typical not to clutter up names with such metadata. Instead just assume that if a function takes a non-const reference-type parameter T& param, it plans to modify it.

  • Consider using glm::radians and glm::degrees instead of magic literal constants. This makes your intent much clearer, in addition to being less error prone and often more accurate. Note also that older versions of GLM's GTC_matrix_transform took angles in degrees by default, but newer versions expect radians by default throughout the entire library.

  • glm::lookAt isn't the most efficient way to generate a view matrix in situations like yours, where you're just generating the camera position from the target position (or vice-versa). It would be more efficient and clearer to just compose the view matrix directly. GLM's GTX_euler_angles provides convenient methods for this. For example:

     glm::mat4 view = glm::translate(glm::mat4(), glm::vec3(0, 0, -10.f))
             * glm::eulerAngleXY(pitch_in_radians, yaw_in_radians)
    
  • Try to use a consistent naming scheme. This is often more difficult in C++ than other popular languages where libraries have mostly standardized on a single style, but it's still good practice to be consistent in your own code. You have some parameters and variables in PascalCase, while others are in camelCase. Try to avoid naming functions, variables, and parameters with PascalCase. Outside of C# (and Pascal, of course) capitalized identifiers are usually reserved for types. Additionally InMatrix is a confusing name, considering this is what would be known as an out-parameter in C#. In C++ it's typical not to clutter up names with such metadata. Instead just assume that if a function takes a non-const reference-type parameter T& param, it plans to modify it.

edited body
Source Link
bcrist
  • 2.3k
  • 18
  • 16

After these changes your code will mostly work. Since you're using glm::lookAt with a fixed up vector, you're not going to be able to add roll to the camera, except by creating a separate rotation matrix for it and combining it with the result of lookAt by multiplication. It's not clear whether or not this is something that you wanted or not. Another consequence of using lookAt is that when the pitch causes the camera to cross the up vector "poles" the view will appear to flip.

After these changes your code will mostly work. Since you're using glm::lookAt with a fixed up vector, you're not going to be able to add roll to the camera. It's not clear whether or not this is something that you wanted or not.

After these changes your code will mostly work. Since you're using glm::lookAt with a fixed up vector, you're not going to be able to add roll to the camera, except by creating a separate rotation matrix for it and combining it with the result of lookAt by multiplication. It's not clear whether or not this is something that you wanted or not. Another consequence of using lookAt is that when the pitch causes the camera to cross the up vector "poles" the view will appear to flip.

edited body
Source Link
bcrist
  • 2.3k
  • 18
  • 16
  • Consider using glm::radians and glm::degrees instead of magic literal constants. This makes your intent much clearer, in addition to being less error prone and often more accurate. Note also that older versions of GLM's GTC_matrix_transform took angles in degrees by default, but newer versions expect radians by default throughout the entire library.

  • glm::lookAt isn't the most efficient way to generate a view matrix in situations like yours, where you're just generating the camera position from the target position (or vice-versa). It would be more efficient and clearer to just compose the view matrix directly. GLM's GTX_euler_angles provides convenient methods for this. For example:

     glm::mat4 view = glm::translate(glm::mat4(), glm::vec3(0, 0, -10.f))
             * glm::eulerAngleYXeulerAngleXY(yaw_in_radianspitch_in_radians, pitch_in_radiansyaw_in_radians)
    
  • Try to use a consistent naming scheme. This is often more difficult in C++ than other popular languages where libraries have mostly standardized on a single style, but it's still good practice to be consistent in your own code. You have some parameters and variables in PascalCase, while others are in camelCase. Try to avoid naming functions, variables, and parameters with PascalCase. Outside of C# capitalized identifiers are usually reserved for types. Additionally InMatrix is a confusing name, considering this is what would be known as an out-parameter in C#. In C++ it's typical not to clutter up names with such metadata. Instead just assume that if a function takes a non-const reference-type parameter T& param, it plans to modify it.

  • Consider using glm::radians and glm::degrees instead of magic literal constants. This makes your intent much clearer, in addition to being less error prone and often more accurate. Note also that older versions of GLM's GTC_matrix_transform took angles in degrees by default, but newer versions expect radians by default throughout the entire library.

  • glm::lookAt isn't the most efficient way to generate a view matrix in situations like yours, where you're just generating the camera position from the target position (or vice-versa). It would be more efficient and clearer to just compose the view matrix directly. GLM's GTX_euler_angles provides convenient methods for this. For example:

     glm::mat4 view = glm::translate(glm::mat4(), glm::vec3(0, 0, -10.f))
             * glm::eulerAngleYX(yaw_in_radians, pitch_in_radians)
    
  • Try to use a consistent naming scheme. This is often more difficult in C++ than other popular languages where libraries have mostly standardized on a single style, but it's still good practice to be consistent in your own code. You have some parameters and variables in PascalCase, while others are in camelCase. Try to avoid naming functions, variables, and parameters with PascalCase. Outside of C# capitalized identifiers are usually reserved for types. Additionally InMatrix is a confusing name, considering this is what would be known as an out-parameter in C#. In C++ it's typical not to clutter up names with such metadata. Instead just assume that if a function takes a non-const reference-type parameter T& param, it plans to modify it.

  • Consider using glm::radians and glm::degrees instead of magic literal constants. This makes your intent much clearer, in addition to being less error prone and often more accurate. Note also that older versions of GLM's GTC_matrix_transform took angles in degrees by default, but newer versions expect radians by default throughout the entire library.

  • glm::lookAt isn't the most efficient way to generate a view matrix in situations like yours, where you're just generating the camera position from the target position (or vice-versa). It would be more efficient and clearer to just compose the view matrix directly. GLM's GTX_euler_angles provides convenient methods for this. For example:

     glm::mat4 view = glm::translate(glm::mat4(), glm::vec3(0, 0, -10.f))
             * glm::eulerAngleXY(pitch_in_radians, yaw_in_radians)
    
  • Try to use a consistent naming scheme. This is often more difficult in C++ than other popular languages where libraries have mostly standardized on a single style, but it's still good practice to be consistent in your own code. You have some parameters and variables in PascalCase, while others are in camelCase. Try to avoid naming functions, variables, and parameters with PascalCase. Outside of C# capitalized identifiers are usually reserved for types. Additionally InMatrix is a confusing name, considering this is what would be known as an out-parameter in C#. In C++ it's typical not to clutter up names with such metadata. Instead just assume that if a function takes a non-const reference-type parameter T& param, it plans to modify it.

Source Link
bcrist
  • 2.3k
  • 18
  • 16
Loading