Skip to content

Conversation

@thw26
Copy link
Collaborator

@thw26 thw26 commented Mar 22, 2025

Modified this to use a subprocess call rather than using the OpenGL Python libraries. This bypasses the GTK issues described below.

Fixes #366. Adds ability to get OpenGL version, and we compare the retrieved version to the minimum version.

Main test case is a PineTab2, which has a graphics limit of OpenGL 3.1. I'm not actually sure if OpenGL 3.2 is a minimum; it is just a guess based on my current knowledge of the lower limit.

We run this before install in order to prevent wasting the user's time.

We run this whenever we launch Logos to ensure that we can launch, e.g., if the user moves an install to a new computer.

Does some minor work in moving imports around in order to make the program run and to clean up ordering.


TODO

  • Suppress error messages in TUI
    • The current implementation is making calls to Gtk which can cause Gtk and other system errors to be reported to terminal, and thus break the TUI. All my attempted implementations of blocking these have failed.

@thw26 thw26 marked this pull request as draft March 22, 2025 14:37
@thw26 thw26 marked this pull request as ready for review April 10, 2025 01:40
@thw26 thw26 linked an issue Apr 10, 2025 that may be closed by this pull request
@thw26 thw26 requested a review from ctrlaltf24 April 10, 2025 01:40

def get_opengl_version(required_version="3.2"):
try:
result = subprocess.run(['glxinfo'], capture_output=True, text=True, check=True)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is not currently using our system.run_command

Copy link
Contributor

Choose a reason for hiding this comment

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

At very least this needs the LD_LIBRARY_PATH fix. See the invitation of subprocess in wine.py for how to use

Copy link
Collaborator Author

@thw26 thw26 Jul 30, 2025

Choose a reason for hiding this comment

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

Submitting a commit soon that I believe implements this. Will leave open for verification.


def get_opengl_version(required_version="3.2"):
try:
result = subprocess.run(['glxinfo'], capture_output=True, text=True, check=True)
Copy link
Contributor

Choose a reason for hiding this comment

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

At very least this needs the LD_LIBRARY_PATH fix. See the invitation of subprocess in wine.py for how to use

@ctrlaltf24
Copy link
Contributor

Two main concerns:

  1. Not sure if I love blocking the install process on this - seems more likely to do harm than good. Should be a warning instead? A logging.warning and app.status plus a sleep of say 5 seconds? So even if the output of glxinfo changes or what have you it still functions
  2. mesa-utils wouldn't be installed for existing installs, nor do I see it in the system.py dependencies. We can't be relying on something we don't check to make sure it's installed. Plus OpenGL check is happing BEFORE we even install system dependencies. We'd need to move it to later

@thw26
Copy link
Collaborator Author

thw26 commented Jul 30, 2025

Two main concerns:

1. Not sure if I love blocking the install process on this - seems more likely to do harm than good. Should be a warning instead? A logging.warning and app.status plus a sleep of say 5 seconds? So even if the output of glxinfo changes or what have you it still functions

My suggestion would be let's add a continue question?

2. mesa-utils wouldn't be installed for existing installs, nor do I see it in the system.py dependencies. We can't be relying on something we don't check to make sure it's installed. Plus OpenGL check is happing BEFORE we even install system dependencies. We'd need to move it to later

This I think is the main issue, even with the continue question. So, given this, the OpenGL check should happen right after package install, no?

@thw26
Copy link
Collaborator Author

thw26 commented Sep 12, 2025

I added a new function for all system checks. I think this is cleanest and helps specify these aren't ensure steps as it were. They may be better located under check system dependencies itself.

@thw26 thw26 merged commit 62e3067 into main Sep 13, 2025
1 check passed
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.

Add GPU Detection

3 participants