Skip to content

Opengl setup cleanups#34

Closed
Kicer86 wants to merge 2 commits into
sharjith:masterfrom
Kicer86:opengl_setup_cleanups
Closed

Opengl setup cleanups#34
Kicer86 wants to merge 2 commits into
sharjith:masterfrom
Kicer86:opengl_setup_cleanups

Conversation

@Kicer86

@Kicer86 Kicer86 commented May 29, 2026

Copy link
Copy Markdown
Contributor

Pull Request

Thanks for contributing to ModelViewer! Please fill in the details below to help us understand and review your changes.

Description

What changes do you make? Provide a clear summary of your modifications.

I got some crashes on linux with wayland. these changes reorganize opengl calls to prevent these problems

Type of Change

What kind of change is this?

  • 🐛 Bug fix (fixes an existing issue)
  • ✨ New feature (adds new functionality)
  • 📚 Documentation (updates docs, README, comments)
  • 🎨 Code style (formatting, cleanup, no logic changes)
  • ♻️ Refactoring (code reorganization, no feature/bug changes)
  • ⚡ Performance improvement (optimization)
  • 🧪 Test (adding or updating tests)
  • 🔧 Build/dependencies (CMake, package updates, etc.)

Related Issues

Does this PR fix or relate to any GitHub issues?

Motivation & Context

Why are these changes needed? What problem do they solve?

How Has This Been Tested?

How did you test these changes? Please be specific.

Test Cases:

  • Tested with glTF 2.1 models
  • Tested with STEP files with assemblies
  • Tested on Linux
  • Tested on Windows
  • Tested on macOS
  • Tested with large models (100+ MB)
  • Other: ________________

Steps to Reproduce/Verify:

Screenshots / Video (if applicable)

Before & After (for visual changes):

Before:
After:

Checklist

Please verify your PR meets these requirements:

  • My code follows the project's style guidelines
  • I have performed a self-review of my code
  • I have commented my code, especially complex sections
  • My changes generate no new compiler warnings
  • I have added/updated tests for my changes (if applicable)
  • New dependencies are documented in README.md
  • I have updated relevant documentation
  • I have tested on multiple platforms (if applicable)
  • I have verified no performance regressions

Breaking Changes?

Does this PR introduce breaking changes?

  • No, this is a backwards-compatible change
  • Yes, this breaks existing functionality:

Additional Notes / Comments

Any other information reviewers should know?

  • Known limitations or TODOs
  • Future improvements planned
  • Dependencies on other PRs
  • Performance considerations

Related Documentation / References

Links to relevant documentation, specifications, or references:

Thank you for your contribution! 🙌

Once submitted, your PR will be:

  1. Reviewed for code quality and functionality
  2. Tested on multiple platforms
  3. Discussed if changes are needed
  4. Merged when approved!

If you have any questions, feel free to ask in the comments. We're here to help! 💬

sharjith added a commit that referenced this pull request May 30, 2026
@sharjith @claude
Fix OpenGL initialisation crashes on Linux/Wayland and add context gu…
032a710 
…ards

Extract configureOpenGLAttributes() to set AA_UseDesktopOpenGL and
AA_ShareOpenGLContexts before QApplication construction — the required
placement for Wayland. Both attributes are gated to non-Windows builds
to avoid interfering with driver auto-selection (ANGLE vs native) on
Windows, where MSAA continues to be read from QSettings safely inside
the constructor.

Add _openGLInitialized flag to GLWidget: initializeGL() now validates
the current context and checks the return value of
initializeOpenGLFunctions() before proceeding, setting the flag only
on success. resizeGL() and paintGL() early-return if the flag is not
set, preventing crashes when initialisation fails silently.

Guard the glGetString / graphics-info block in main.cpp against a null
current context, which can occur on Wayland and headless systems.

Incorporates the fix proposed in PR #34 (Kicer86/ModelViewer-Qt)
adapted for the camera-revamp branch with Windows MSAA fencing.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@sharjith

sharjith commented May 30, 2026

Copy link
Copy Markdown
Owner

Thank you for this contribution, @Kicer86 — the Wayland crash fix is correct and the approach is sound!

The active development branch for this project is experimental/camera-revamp, which is currently 240 commits ahead of master. Rather than merging into the stale master, I've manually integrated your changes into camera-revamp in commit 032a710, with the following adaptations:

configureOpenGLAttributes() is gated with #if !defined(Q_OS_WIN) so AA_UseDesktopOpenGL and AA_ShareOpenGLContexts are only set on Linux/non-Windows builds, avoiding interference with ANGLE/driver auto-selection on Windows.
MSAA continues to be read from QSettings inside the ModelViewerApplication constructor (where QApplication already exists), keeping the Windows MSAA path safe.
The _openGLInitialized guard, initializeOpenGLFunctions() return-value check, and main.cpp null-context guard are all applied as proposed.
Closing this PR as the fix is now part of the active branch. Thanks again!

@sharjith sharjith closed this May 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@Kicer86 @sharjith