Skip to content

Refactor: PointLightShadowRenderer reduce object allocations #2514

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

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

Conversation

capdevon
Copy link
Contributor

@capdevon capdevon commented Jun 23, 2025

This PR refactors the PointLightShadowRenderer class to improve readability and maintainability. Key changes include:

  • Replaces repeated inline vector negations (e.g., Vector3f.UNIT_X.mult(-1f)) with clearly named constants (X_NEG, Y_NEG, Z_NEG).
  • Updates camera axis assignments in updateShadowCams() to use these new constants, reducing code duplication and potential errors.
  • Minor improvements to JavaDoc for clarity and consistency.

These changes make the codebase cleaner and easier to understand without altering any functionality.

*
* @author Rémy Bouquet aka Nehon
* @author Nehon
Copy link
Member

Choose a reason for hiding this comment

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

Why the author change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The original author's alias 'Nehon' is already the standard signature for most of their contributions, consistent with their GitHub nickname. To maintain uniformity with the other 500 classes in the jme3-core module and to align with this established practice, I've adjusted the signatures in these four Shadow classes. My intention is never to remove an original author's alias from any jME class, only to ensure consistency where a clear alias already exists and is widely used. This change also implicitly supports privacy by standardizing the alias used.

@richardTingle
Copy link
Member

richardTingle commented Jun 26, 2025

(Apart from the author change) looks good. Makes sense to avoid pointless negative vector allocations

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.

2 participants