search

mxcube / mxcubecore

Backend used by MXCuBE
http://mxcube.github.io/mxcube/
GNU Lesser General Public License v3.0
13 stars 53 forks source link

Added a way for Shape to preserve user preferred state #1044

Closed mockoocy closed 3 weeks ago

mockoocy commented 1 month ago

The PR introduces user_state attribute that will be set by the web app. Previously, when the user set grid to be hidden or visible, it set state attribute of Grid class as either HIDDEN or SAVED. The same attribute was being changed as a result of diffractometer's motors having their state changed, like in the code below:

https://github.com/mxcube/mxcubecore/blob/d9668efbc0ea7c2d66536d9e6224beba212a4c40/mxcubecore/HardwareObjects/SampleView.py#L35C1-L56C35

The new user_state attribute is being intended to be able to be modified by the web app by listening to events such as phaseChanged or collectOscillationFinished in order to restore the state that user wants to see, after these operations are done.

marcus-oscarsson commented 1 month ago

Could you describe the use case and how this is a problem ?

marcus-oscarsson commented 1 month ago

The meshes are meant to be automatically hidden/shown when they are within a certain range of the angle where the mesh was created. The meshes are only valid for a certain projection. see https://github.com/mxcube/mxcubecore/blob/d9668efbc0ea7c2d66536d9e6224beba212a4c40/mxcubecore/HardwareObjects/SampleView.py#L556

This is perhaps not behaving as you wish ?

elmjag commented 1 month ago

Could you describe the use case and how this is a problem ?

This is an attempt to fix following bug:

After the data collection is finished, the mesh will be un-hidden.

The behavior we want is that meshes explicitly hidden by the user are never automatically un-hidden.

marcus-oscarsson commented 1 month ago

Ah, we should perhaps then add an option to disable the auto hide/show option ?

elmjag commented 1 month ago

Ah, we should perhaps then add an option to disable the auto hide/show option ?

I think the current behavior is fine. The users so far have not complained about meshes getting hidden during data collections, scans, etc.

It's only the part where all meshes become visible after the scan is finished that annoys users.

Also, here is the MXCuBE-web part of the bug fix: https://github.com/mxcube/mxcubeweb/pull/1460

marcus-oscarsson commented 1 month ago

It's only the part where all meshes become visible after the scan is finished that annoys users.

I see, is that because all the meshes where created at the same angle or for some other reason. Why is all meshes shown when the collection finishes ?

elmjag commented 1 month ago

It's only the part where all meshes become visible after the scan is finished that annoys users.

I see, is that because all the meshes where created at the same angle or for some other reason. Why is all meshes shown when the collection finishes ?

Yes, it is a common usage scenario at MicroMAX right now. Users use large chips to collect data. They draw a number of mesh grids at the same angle, as the chip don't rotate.

The reason all mesh grids are made visible, is because the code that auto-hides grids does something like this:

grid.status = "hidden"

Thus the information if a grids was hidden by the user is lost. When code that un-hides grids runs, it shows all grids with correct angle.

marcus-oscarsson commented 1 month ago

I see, I'm hesitant on adding a user_state. As far as I remember, and I had a quick look, the only place where we update the shape state to "HIDDEN" is in:

https://github.com/mxcube/mxcubecore/blob/d9668efbc0ea7c2d66536d9e6224beba212a4c40/mxcubecore/HardwareObjects/SampleView.py#L556

The drawing of the shapes on the canvas is not performed when the goniometer motors are moving but updated when the state changes to ready. Or did I perhaps leave something out ?

I guess in that case what is causing you this issue is that the state gets changed on rotation, maybe simply adding a option that disables the code above would be enough ?. You could perhaps try to comment out that section and see if it helps ?

mockoocy commented 1 month ago

I see, I'm hesitant on adding a user_state. As far as I remember, and I had a quick look, the only place where we update the shape state to "HIDDEN" is in:

You are right, the Shape.state is set only in the linked code.

I've talked with @elmjag and the desired functionality looks like this:

This would require to store the user's preference somewhere, e.g., in the user_state attribute.

marcus-oscarsson commented 1 month ago

The problem I see with this feature is that it clashes with the idea of auto hiding. The user would after some time not know which meshes are not being shown because they are auto hidden or explicitly hidden. I guess in that case it should be more evident in the grid list why a grid is hidden. I somehow thing that this might lead to some confusion, thats why I'm a bit hesitant.

It would perhaps be possible to always display a dashed outline of hidden grids or highlight them in the grid table, not sure what would be the best.

mockoocy commented 1 month ago

It would perhaps be possible to always display a dashed outline of hidden grids or highlight them in the grid table, not sure what would be the best.

I think you're right there; I've made some changes on the UI part so that the user-hidden grids appear with a darker background.

I've also pushed some changes to this repository. I've updated the functions that redraw the shapes, to be able to persist the state and user_state attributes.