WIP/Fix: correct parallel projection frustum plane normals for left/right#2640
WIP/Fix: correct parallel projection frustum plane normals for left/right#2640zzuegg wants to merge 1 commit intojMonkeyEngine:masterfrom
Conversation
getLeft() returns up×direction but fromFrame() uses direction×up as the view matrix X axis. These are negatives of each other. The hardcoded parallel projection coefficients (1 for left, -1 for right) built plane normals from getLeft(), causing them to point opposite to the view matrix. This made frustum culling reject visible geometry for any camera with a non-standard orientation (e.g. looking along +Z) or asymmetric frustum values in parallel projection mode. Fix: negate the left/right coefficients so plane normals align with the view matrix's X axis. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
🖼️ Screenshot tests have failed. The purpose of these tests is to ensure that changes introduced in this PR don't break visual features. They are visual unit tests. 📄 Where to find the report:
✅ If you did mean to change things: ✨ If you are creating entirely new tests: Note; it is very important that the committed reference images are created on the build pipeline, locally created images are not reliable. Similarly tests will fail locally but you can look at the report to check they are "visually similar". See https://github.com/jMonkeyEngine/jmonkeyengine/blob/master/jme3-screenshot-tests/README.md for more information Contact @richardTingle (aka richtea) for guidance if required |
|
i actually do not know why the screenshot tests are failing. at least i did not expect these to fail |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a critical bug in the camera's parallel projection frustum calculation. By negating specific coefficients for the left and right frustum planes, it ensures that the plane normals correctly align with the view matrix's X-axis, thereby preventing visible geometry from being erroneously culled, especially for cameras with unusual orientations. Highlights
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request addresses a bug in the frustum culling for parallel projections. The change corrects the coefficients for the left and right frustum planes to align their normals with the view matrix's coordinate system. My analysis confirms that Camera.getLeft() and the view matrix's X-axis were indeed opposed, and this change correctly inverts the coefficients to fix the issue. The added comment clearly explains the rationale behind the fix. The change appears correct and should resolve the described culling problems.
looks like they are passing now.. there is a bug or something that causes them to fail sometimes, maybe it depends on the action worker? |
|
from the screenshots it looks like a very small timing issue? i have not checked how the system works exactly. it should run on constant tpf's |
2 participants
getLeft() returns up×direction but fromFrame() uses direction×up as the view matrix X axis. These are negatives of each other. The hardcoded parallel projection coefficients (1 for left, -1 for right) built plane normals from getLeft(), causing them to point opposite to the view matrix.
This made frustum culling reject visible geometry for any camera with a non-standard orientation (e.g. looking along +Z) or asymmetric frustum values in parallel projection mode.
Fix: negate the left/right coefficients so plane normals align with the view matrix's X axis.
Note: this bug was found and fixed by claude. all jme tests pass, my local project passes too, but i am not sure of the consequences. I create this PR now to run all the remote testing.