Sorry, something went wrong.
There was a problem hiding this comment.
Adds a “fit to scene” camera reset to the OpenGL OBJ viewer example, and fixes initial framing by centering/scaling the loaded model consistently.
Changes:
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Sorry, something went wrong.
| // Reset camera to fit the entire scene in view. | ||
| static void FitToScene() { | ||
| trackball(curr_quat, 0, 0, 0, 0); | ||
|
|
||
| g_angleX = 0.0f; | ||
| g_angleY = 0.0f; | ||
|
|
||
| eye[0] = 0.0f; | ||
| eye[1] = 0.0f; | ||
| eye[2] = 3.0f; | ||
|
|
||
| lookat[0] = 0.0f; | ||
| lookat[1] = 0.0f; | ||
| lookat[2] = 0.0f; | ||
|
|
||
| up[0] = 0.0f; | ||
| up[1] = 1.0f; | ||
| up[2] = 0.0f; | ||
| } |
There was a problem hiding this comment.
FitToScene() duplicates the camera reset logic already in Init() (same quaternion + eye/lookat/up defaults) with only small differences. To avoid the two getting out of sync over time, consider having Init() call FitToScene() (or vice versa) and keep the reset logic in one place.
Sorry, something went wrong.
There was a problem hiding this comment.
Addressed in 98840c2: Init() now delegates to FitToScene(), so camera reset defaults are maintained in one place and won’t drift.
Sorry, something went wrong.
| scene_bmin[0] = bmin[0]; scene_bmin[1] = bmin[1]; scene_bmin[2] = bmin[2]; | ||
| scene_bmax[0] = bmax[0]; scene_bmax[1] = bmax[1]; scene_bmax[2] = bmax[2]; |
There was a problem hiding this comment.
The chained assignments for scene_bmin/scene_bmax on single lines reduce readability and make diffs harder to review. Consider formatting these as one assignment per line (or using std::copy) for consistency with the surrounding style.
| scene_bmin[0] = bmin[0]; scene_bmin[1] = bmin[1]; scene_bmin[2] = bmin[2]; | |
| scene_bmax[0] = bmax[0]; scene_bmax[1] = bmax[1]; scene_bmax[2] = bmax[2]; | |
| scene_bmin[0] = bmin[0]; | |
| scene_bmin[1] = bmin[1]; | |
| scene_bmin[2] = bmin[2]; | |
| scene_bmax[0] = bmax[0]; | |
| scene_bmax[1] = bmax[1]; | |
| scene_bmax[2] = bmax[2]; |
Sorry, something went wrong.
| glScalef(1.0f / scene_maxExtent, 1.0f / scene_maxExtent, | ||
| 1.0f / scene_maxExtent); |
There was a problem hiding this comment.
scene_maxExtent can become 0 (e.g., degenerate geometry where bmin==bmax on all axes), which would make glScalef(1.0f / scene_maxExtent, ...) divide by zero and propagate INF/NaN through the modelview matrix. Consider clamping scene_maxExtent to a small positive epsilon (or falling back to 1.0f) after computing it, and/or early-out when bounds are invalid.
| glScalef(1.0f / scene_maxExtent, 1.0f / scene_maxExtent, | |
| 1.0f / scene_maxExtent); | |
| float scale = 1.0f; | |
| if (scene_maxExtent > 0.0f) { | |
| scale = 1.0f / scene_maxExtent; | |
| } | |
| glScalef(scale, scale, scale); |
Sorry, something went wrong.
🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.