Skip to content

Add 'fit to scene' feature to examples/viewer#425

Open
Copilot wants to merge 2 commits intoreleasefrom
copilot/add-fit-to-scene-feature
Open

Add 'fit to scene' feature to examples/viewer#425
Copilot wants to merge 2 commits intoreleasefrom
copilot/add-fit-to-scene-feature

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Mar 27, 2026

Press F to reset camera to view the entire mesh. Initial camera is now properly positioned on load.

  • Center objects at origin: Enabled the centering translation that was previously #if 0'd out. Without this, off-origin models render outside the viewport.
  • FitToScene() function: Resets trackball quaternion, turntable angles, and camera to default pose (0, 0, 3) looking at origin. Works because the render loop already normalizes geometry to ±1 via glScalef(1/maxExtent).
  • Global scene bounds: Moved bmin/bmax/maxExtent to file scope so keyboardFunc can trigger FitToScene() and the render loop can reference them after the centering fix.
  • F key binding in keyboardFunc, called once after LoadObjAndConvert for initial fit.

🔒 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.

- Press 'F' key to reset camera to view entire model
- Initial camera position set to fit the scene on load
- Enable centering translation to center objects at origin
- Store bounding box data in globals for FitToScene() access
- Update help text to document 'F' key

Agent-Logs-Url: https://github.com/tinyobjloader/tinyobjloader/sessions/3b05e7bf-a2b5-4b2d-b308-718537dfc75a

Co-authored-by: syoyo <18676+syoyo@users.noreply.github.com>
Copilot AI changed the title [WIP] Add fit to scene feature to examples/viewer Add 'fit to scene' feature to examples/viewer Mar 27, 2026
Copilot AI requested a review from syoyo March 27, 2026 19:51
@syoyo syoyo marked this pull request as ready for review March 27, 2026 20:03
Copilot AI review requested due to automatic review settings March 27, 2026 20:03
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

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:

  • Introduces FitToScene() and binds it to the F key; also calls it after loading to set a sensible initial view.
  • Persists scene bounds (scene_bmin/scene_bmax) and scene_maxExtent at file scope for use during rendering and fit/reset.
  • Enables the previously-disabled centering translation and uses the persisted bounds/extent in the render loop.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +1118 to +1136
// 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;
}
Copy link

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

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

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.

Copilot uses AI. Check for mistakes.
Comment on lines +1189 to +1190
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];
Copy link

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

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

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.

Suggested change
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];

Copilot uses AI. Check for mistakes.
Comment on lines +1228 to +1229
glScalef(1.0f / scene_maxExtent, 1.0f / scene_maxExtent,
1.0f / scene_maxExtent);
Copy link

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

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

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.

Suggested change
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);

Copilot uses AI. Check for mistakes.
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.

3 participants