Actively transform the scene when the view is dragged #4

javierggt · 2024-11-04T19:01:17Z

Description

Note: this PR requires sot/agasc/pull/193.

Changed the way graphic items are handled:

created several graphic item classes.
Now transformations are active transformations on the scene, instead of transformations on the view. This makes the code much more understandable, IMO. However, this would make the display slower when there are many stars, because when one rotates the view, all items positions are changed in the scene.
To mitigate the performance hit, items are hidden manually from the view, and there is an automatic maximum magnitude applied when there are many stars.
Stars are automatically added to the display as you pan/zoom around.

This change makes the code much easier to read. It also fixes a few issues:

Rotating the view meant that the graphic items in a catalog (notably the labels and the square boxes) were rotated in the star plot. With this change, the stars are moved instead, and the boxes keep their orientation.
Moving the view meant that the view origin might not be the scene origin, which meant there were two attitudes associated with the display (the center of the scene and the center of the view). That complicated the code. Now there is only one.
If the center of the view was not at the center of the camera, then shapes had to be deformed. Now that is not needed.
In master, changing the attitude in the text edit widgets does not change the star plot. This PR fixes that (made possible by the other changes)

Interface impacts

Testing

Note: this PR requires sot/agasc/pull/193.

Unit tests

No unit tests

Functional tests

I have the following JSON file

[{
    "obsid": 1,
    "target_name": "GRB 240825A",
    "chip_id": 7,
    "chipx": 193.74,
    "chipy": 520.0,
    "dec_targ": 1.026897,
    "detector": "ACIS-S",
    "dither_y": 4,
    "dither_z": 4,
    "focus_offset": 0,
    "man_angle": 5.0,
    "obs_date": "2024:312:00:00:00.000",
    "offset_y": 0.0,
    "offset_z": 0.0,
    "ra_targ": 344.571937,
    "roll_targ": 302.0,
    "sim_offset": 0
}]

and I ran

aperoll input.json

you can also run

python -m aperoll.widgets.star_plot

…tive transformations on the scene, instead of transformations on the view, created several graphic item classes.

- Hide frame when zoomed out. - Add automatic max-mag to speed up the view when there are many stars. - Added automatic item scaling when zooming out - Automatically update the stars when scaling, - prevent negative scaling, - limit scaling to a max size.

… and added corresponding properties. Removes update argument from StarPlot.set_attitude/set_time/set_catalog

Copilot reviewed 5 out of 5 changed files in this pull request and generated no suggestions.

aperoll/star_field_items.py

jeanconn · 2024-11-13T19:58:33Z

aperoll/star_field_items.py

+        # self.setPos(-star["yang"], -star["zang"])
+        self.setPos(star["row"], -star["col"])
+
+    def setPos(self, x, y):


And if everything basically gets plugged in as (row, -col) - would there be value in flipping the function signature and adding a docstring?

setPos is not written by me. It is the Qt function, and I don't want to start overriding all setPos functions for all possible items, and we do want to keep the appearance, so I do not see a way other than having minus signs all over.

There might be some kind of hack, but I prefer to keep it explicit.

I mean setPos for all other QGraphicsItems. This one is overridden but it is an exception.

jeanconn · 2024-11-21T12:52:50Z

aperoll/star_field_items.py

+    """
+    Utility class to keep together all graphics item for a star catalog.
+
+    Note that the position of the catalog is ALLWAYS (0,0) and the item positions need to be set


Typo in docstring.

jeanconn · 2024-11-21T12:53:35Z

aperoll/star_field_items.py

+        rect = QtC.QRectF(-s, -s, s * 2, s * 2)
+        super().__init__(rect, parent)
+        self.setPen(QtG.QPen(QtG.QColor("green"), w))
+        # self.setPos(-star["yang"], -star["zang"])


Can probably go through and remove commented out code in next PR.

jeanconn · 2024-11-21T12:54:35Z

aperoll/widgets/parameters.py

@@ -506,6 +506,7 @@ def proseco_args(self):
            "detector": self.values["instrument"],
            "sim_offset": 0,  # docs say this is optional, but it does not seem to be
            "focus_offset": 0,  # docs say this is optional, but it does not seem to be
+            "dyn_bgd_n_faint": 2,


I think this should not be needed now that we've changed the proseco default.

jeanconn

I note the tool is now more useable for things like scrolling around and finding this old agasc defect.

javierggt added 2 commits October 31, 2024 15:32

add star_plot.main for testing

f102728

Changed the way graphic items are handled: now transformations are ac…

ab2ad9f

…tive transformations on the scene, instead of transformations on the view, created several graphic item classes.

javierggt changed the title ~~Transform~~ WIP: Transform Nov 4, 2024

javierggt added 6 commits November 5, 2024 12:47

change scene coordinates back to pixel coordinates

e5db3bb

typo

9ad919b

Added StarField.set_time. Made time and attitude private in StarField…

5eddc41

… and added corresponding properties. Removes update argument from StarPlot.set_attitude/set_time/set_catalog

moved some stuff out of aperoll/widgets/star_plot.py for clarity

a4b14ba

change star plot attitude when parameters change in the dialog

1813394

javierggt changed the title ~~WIP: Transform~~ Transform Nov 8, 2024

ruff

d210ea3

javierggt requested a review from jeanconn November 8, 2024 19:24

jeanconn requested a review from Copilot November 13, 2024 19:33

Copilot AI reviewed Nov 13, 2024

View reviewed changes

jeanconn reviewed Nov 13, 2024

View reviewed changes

aperoll/star_field_items.py Show resolved Hide resolved

jeanconn reviewed Nov 13, 2024

View reviewed changes

use proseco.acq.get_acq_candidates_mask instead of implementing it

ad2a4dc

javierggt requested a review from jeanconn November 19, 2024 16:27

jeanconn reviewed Nov 21, 2024

View reviewed changes

jeanconn approved these changes Nov 21, 2024

View reviewed changes

javierggt changed the title ~~Transform~~ Actively transform the scene when the view is dragged Nov 21, 2024

javierggt merged commit 7cec510 into main Nov 21, 2024
2 checks passed

javierggt deleted the transform branch November 21, 2024 14:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Actively transform the scene when the view is dragged #4

Actively transform the scene when the view is dragged #4

javierggt commented Nov 4, 2024 •

edited by jeanconn

Loading

jeanconn Nov 13, 2024

javierggt Nov 13, 2024

javierggt Nov 13, 2024

jeanconn Nov 13, 2024

jeanconn Nov 21, 2024

jeanconn Nov 21, 2024

jeanconn Nov 21, 2024

jeanconn left a comment

Actively transform the scene when the view is dragged #4

Actively transform the scene when the view is dragged #4

Conversation

javierggt commented Nov 4, 2024 • edited by jeanconn Loading

Description

Interface impacts

Testing

Unit tests

Functional tests

Choose a reason for hiding this comment

jeanconn Nov 13, 2024

Choose a reason for hiding this comment

javierggt Nov 13, 2024

Choose a reason for hiding this comment

javierggt Nov 13, 2024

Choose a reason for hiding this comment

jeanconn Nov 13, 2024

Choose a reason for hiding this comment

jeanconn Nov 21, 2024

Choose a reason for hiding this comment

jeanconn Nov 21, 2024

Choose a reason for hiding this comment

jeanconn Nov 21, 2024

Choose a reason for hiding this comment

jeanconn left a comment

Choose a reason for hiding this comment

javierggt commented Nov 4, 2024 •

edited by jeanconn

Loading