Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix structure viewer rendering on macOS and add basic GUI tests based on screenshot comparison #29

Merged
merged 20 commits into from
May 27, 2024

Conversation

AndrewSazonov
Copy link
Member

This PR fixes an issue with QtQuick3D failing to render on macOS. We used os.environ['QSG_RHI_BACKEND'] = 'opengl' to force OpenGL to be the RHI backend on all platforms to be able to use the useOpenGL property for QtCharts XYSeries. This leads to errors like:

  • Particles not supported due to missing RGBA32F and RGBA16F texture format support... and
  • No GLSL shader code found (versions tried: QList(120) ) in the baked shader....

RHI is now automatically selected depending on the platform. OpenGL acceleration has been temporarily disabled.

Another addition to this PR is the GUI basic tests, which are based on comparing screenshots for each page of the application to the desired ones. Although we do not use platform-specific GUI elements, screenshots vary slightly from platform to platform. Therefore, comparisons are always made with images created for the same platform. These tests are called twice during the CI process - before creating the application installer and after installing the application on a fresh machine.

@AndrewSazonov AndrewSazonov added the bug Something isn't working label Apr 30, 2024
Copy link

@andped10 andped10 left a comment

Choose a reason for hiding this comment

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

Looks good.

I would prefer to have all dependencies in the pyproject.toml file.

I did not do any testing of the behavior on OSX.

I tried to do the GUI tests on my windows setup. However, I had problems when the screendump for the model page should be generated since the data to be loaded where not at the right place or something. I also noticed that my black theme seemed would cause problems. So I assume the tests are exclusive for usage on pipelines.

@@ -223,6 +236,7 @@ jobs:
- name: Install Python dependences
run: |
python -m pip install requests
python -m pip install pytest pytest-image-diff

Choose a reason for hiding this comment

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

Why are these not defined in the project.toml file.

Copy link
Member Author

Choose a reason for hiding this comment

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

Moved to pyproject.toml. Although another flit package installation step is added to be able to install EasyDiffractionApp dependencies, but not the EasyDiffractionApp package itself. This feature does not seem to be implemented in pip.

@@ -66,15 +66,13 @@ jobs:
python -m pip install numpy lmfit numdifftools

Choose a reason for hiding this comment

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

Why are these here rather than an optional dependency in the project.toml file?

Copy link
Member Author

Choose a reason for hiding this comment

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

Moved to pyproject.toml. Although another flit package installation step is added to be able to install EasyDiffractionApp dependencies, but not the EasyDiffractionApp package itself. This feature does not seem to be implemented in pip.

@@ -20,13 +20,13 @@
EnvironmentVariables.set()
console.debug('Environment variables defined')

# Magically fixes the following issues in QtQuick3D on macOS for local run
# Magically fixes the following issues in QtQuick3D on macOS when using pyside6==6.5.3

Choose a reason for hiding this comment

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

Suggest to delete this if it is no longer needed

Copy link
Member Author

Choose a reason for hiding this comment

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

Old irrelevant comments have been removed. Although the QtWebEngine initialisation step is commented out, it is temporarily preserved.

@AndrewSazonov AndrewSazonov merged commit b5dfa3b into master May 27, 2024
22 checks passed
@AndrewSazonov AndrewSazonov deleted the gui-tests branch May 30, 2024 13:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants