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

1763 implement the orientation viewer in 5x and GL Subsystem #2394

Merged
merged 101 commits into from
Feb 2, 2023

Conversation

lucas-wilkins
Copy link
Contributor

@lucas-wilkins lucas-wilkins commented Nov 25, 2022

This PR re-implements the orientation viewer, as demonstrated at the code camp. At the time there was some issues with pyqtgraph.

Fixes #1763

GL Subsystem

As pyqtgraph was terrible, I've implemented an OpenGL subsystem, with the hope it can be used elsewhere. I intend to replace all instances of matplotlib 3D with it - the next on the list is the representations of macromolecules.

The GL system is currently fairly basic, but improvements can be made as required. Developers documentation beyond that in the code will have to wait, I've asked about where it should go, but explaining the GL stuff is best done through a diagram @smk78. Maybe migrate some of this PR.

The QtWidget that does GL is called Scene and it uses a standard scene graph type model.
Scene contains all the mouse feedback and motion.
Scene can contain multiple Renderable objects, that may support solid or wireframe rendering or both.
SceneGraphNodes and subclasses all have children, and can be used to build up a tree representing a scene.

Here's a class diagram

graph TD;
    Renderable-->ModelBase-->SolidModel-->SolidVertexModel-->FullModel;
    ModelBase-->WireModel-->FullModel;
    FullModel-->Cube;
    FullModel-->Cylinder;
    FullModel-->Cone;
    FullModel-->Icosahedron;
    FullModel-->Sphere;
    FullModel-->Surface;
    Renderable-->SceneGraphNode-->Translation;
    SceneGraphNode-->Rotation;
    SceneGraphNode-->Scaling;
    SceneGraphNode-->Transform;

Loading

There are then a couple of colour related things in color

Orientation viewer

The orientation viewer is made of 2 classes, the controller, which is QtDesigner component and the viewer itself (which contains a scene with all the objects in it)

How Has This Been Tested?

Hard to test GL stuff, there are some files in qtgui/GL/visual_checks that allow one to check rendering of a good proportion of the functionality.

Review Checklist (please remove items if they don't apply):

  • Is the representation of polydispersity correct? @pkienzle @smk78
  • Does the GL stuff work across all platforms correctly? It seems to work on windows - @wpotrzebowski @llimeht
  • Code has been reviewed
  • Functionality has been tested
  • Windows installer (GH artifact) has been tested (installed and worked)
  • MacOSX installer (GH artifact) has been tested (installed and worked)
  • User documentation is available and complete (if required)
  • The introduced changes comply with SasView license (BSD 3-Clause)

@wpotrzebowski wpotrzebowski added the Discuss At The Call Issues to be discussed at the fortnightly call label Jan 31, 2023
@wpotrzebowski
Copy link
Contributor

@lucas-wilkins: Eh, it still doesn't start on my machine... Looking at the log it still complains about Model.2:
2023-01-31T10:58:36.8259460Z /Users/runner/work/sasview/sasview/installers/dist/SasView5.app: bundle format unrecognized, invalid, or unsuitable 2023-01-31T10:58:36.8260210Z In subcomponent: /Users/runner/work/sasview/sasview/installers/dist/SasView5.app/Contents/MacOS/PyQt5/Qt5/qml/QtQml/Models.2
And I cannot see this file in "Resources" either, which I believe should have been created by build_tools/fix_qt_folder_names_for_codesign.py?

@wpotrzebowski
Copy link
Contributor

I've checked the latest version of installer and it is failing to start. It seems that fix_qt_folder_names_for_codesign.py didn't work as expected? I will try to take a look at it later.

@lucas-wilkins
Copy link
Contributor Author

@wpotrzebowski I've also been looking at it, I agree that its not working, though the signing script seems to complete now.

@lucas-wilkins
Copy link
Contributor Author

image

????

@wpotrzebowski
Copy link
Contributor

@lucas-wilkins I think I know what's goining on. We publish installer file before signing.

@wpotrzebowski
Copy link
Contributor

@wpotrzebowski: I've changed the order of tasks in the ci.yml file. Let's see if this fixes the problem.

@lucas-wilkins
Copy link
Contributor Author

image

@lucas-wilkins
Copy link
Contributor Author

@wpotrzebowski Is this error because its not notarized?

@wpotrzebowski
Copy link
Contributor

@lucas-wilkins yes, that's what you normally see for signed app... Does it work for you though? In my case, it doesn't come with a damaged disk error as before but crashes immediately after the start. After some debugging it complains about: ModuleNotFoundError: No module named 'sas.qtgui.Utilities.OrientationViewer.UI' Do you have any clue why is that? Could it be related to setup clean-up PR we merged few days ago?

@lucas-wilkins
Copy link
Contributor Author

@wpotrzebowski I'm told it runs fine for my colleague, as far as he can tell.

@lucas-wilkins
Copy link
Contributor Author

ModuleNotFoundError: No module named 'sas.qtgui.Utilities.OrientationViewer.UI'

That kind of error shouldn't be happening with the setup.py changes, as it was exactly this kind of error those changes were supposed to fix.

Are you sure you got the latest artefact?

@wpotrzebowski
Copy link
Contributor

@lucas-wilkins that's really really wired! I've checked now 4 times that I am using the right installer (I am using this one: https://github.com/SasView/sasview/suites/10714249084/artifacts/538369808) and it still crashes immediately after I start it. I will try on another computer later. Have you checked if Orientation Viewer works?

@lucas-wilkins
Copy link
Contributor Author

I will try on another computer later. Have you checked if Orientation Viewer works?

Actually, no. I will do.

@lucas-wilkins
Copy link
Contributor Author

We checked .../4053202588. So there is a difference there.

@lucas-wilkins
Copy link
Contributor Author

We're getting the same error as you now, which is good, in a way.

@wpotrzebowski
Copy link
Contributor

Hurray!
Screenshot 2023-02-02 at 14 26 42

@lucas-wilkins
Copy link
Contributor Author

Wohooo!

@lucas-wilkins
Copy link
Contributor Author

The scaling looks very strange, but this is an ongoing issue to be resolved elsewhere I think!

@wpotrzebowski
Copy link
Contributor

The scaling looks very strange, but this is an ongoing issue to be resolved elsewhere I think!

I am fine with resolving the scaling issue separately.

If we are ok with accepting the fact that installers will become 20-40MB bigger after merging this PR, I would vote for merging it.

@lucas-wilkins
Copy link
Contributor Author

I'm fine with the size increase, although I wonder how much is this PR, and how much is the changes in setup.py.

@wpotrzebowski
Copy link
Contributor

Good point. @butlerpd, @smk78 are ok with merging this PR?

@lucas-wilkins lucas-wilkins merged commit 655ca05 into main Feb 2, 2023
@smk78
Copy link
Contributor

smk78 commented Feb 2, 2023

Mantid 6.5.0 was 758MB. SasView 5.0.5 was 168MB. Nuff said!

@krzywon krzywon deleted the 1763-implement-the-orientation-viewer-in-5x branch February 2, 2023 17:39
@butlerpd butlerpd removed the Discuss At The Call Issues to be discussed at the fortnightly call label Aug 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Blocker Prevents a different issue from being resolved Enhancement Feature requests and/or general improvements For Feature Parity Issues to give 5x the same functionality as 4x
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement the Orientation Viewer in 5x
7 participants