[SDEV-1871] investigate gui software lag find proper fix #2752
Conversation
nandishjpatel
changed the title
nandishjpatel
force-pushed
the
SDEV-1871-investigate-gui-software-lag-find-proper-fix
branch
from
44fb4d8
to
e2f4d50
Compare
nandishjpatel
force-pushed
the
SDEV-1871-investigate-gui-software-lag-find-proper-fix
branch
3 times, most recently
from
1acd9b6
to
a36d41f
Compare
nandishjpatel
force-pushed
the
SDEV-1871-investigate-gui-software-lag-find-proper-fix
branch
2 times, most recently
from
6664b7b
to
25a03b4
Compare
…s represented in view coordinates
…s edges, rotation point and shape's inside. Given this the selected VA can be found using is_point_in_shape.
…optimize _get_shape method by making use of cKDTree. The cKDTree has data about the shape's center. When a on_left_down event occurs find 4 shapes whose centers are closest to the event position. Out of the 4 shapes check if the event position happened inside the shape and if that shape was selected by the user. By doing so avoid iteration over all the shapes. The shapes were drawn 2 times, this has been fixed by only allowing ShapesOverlay draw the shapes. A newly created shape is not added to the world overlay.
nandishjpatel
force-pushed
the
SDEV-1871-investigate-gui-software-lag-find-proper-fix
branch
from
25a03b4
to
c4a486d
Compare
| if self.selected.value: | ||
| self.points.value = self._points | ||
| WorldOverlay.on_left_up(self, evt) | ||
| # If the Diagonal points are not the same means the rectangle has been created |
There was a problem hiding this comment.
This code is for ellipses, is "rectangle" a copy paste error? Otherwise I don't think I understand the comment :)
Same for rectangle_points on line 127 and 128
There was a problem hiding this comment.
Well, this is correct. We get the rectangle points 1, 2, 3, 4 first. Then the ellipse points are drawn inside the rectangle in the draw() method. You see this happening in line 140 where an ellipse circumference points are drawn based on rectangles points 1, 2, 3, 4 and finally the point VA value is assigned
There was a problem hiding this comment.
Although the code uses internally a diagonal and a rectangle, here the comment mentions "the rectangle" as the ellipse shape. So it should be "rectangle" -> "ellipse".
I think the variable names are fine... though maybe it'd be less confusing if you pick a word different from the name of the other shape. Maybe something like "extrema_points" or "skeleton_points". (not "bounding box", because typically bounding box is aligned with the axes, while here, the "rectangle" can be rotated, right?)
nandishjpatel
force-pushed
the
SDEV-1871-investigate-gui-software-lag-find-proper-fix
branch
from
c4a486d
to
764d4b4
Compare
| if self.selected.value: | ||
| self.points.value = self._points | ||
| WorldOverlay.on_left_up(self, evt) | ||
| # If the Diagonal points are not the same means the rectangle has been created |
There was a problem hiding this comment.
Although the code uses internally a diagonal and a rectangle, here the comment mentions "the rectangle" as the ellipse shape. So it should be "rectangle" -> "ellipse".
I think the variable names are fine... though maybe it'd be less confusing if you pick a word different from the name of the other shape. Maybe something like "extrema_points" or "skeleton_points". (not "bounding box", because typically bounding box is aligned with the axes, while here, the "rectangle" can be rotated, right?)
nandishjpatel
force-pushed
the
SDEV-1871-investigate-gui-software-lag-find-proper-fix
branch
from
764d4b4
to
82d6858
Compare
|
Can we merge this PR? I wanna re-base my vEM GUI branch |
It seems you've made the changes but pushed them as an amended commit. If you do extra changes to fix new comments, it's better to put them in a new commit, instead of amending the latest commit and rewriting the history. This helps to see only the most recent changes, and also I get notification in Github. Also, please mark the comments as "resolved". That also hints me that something has changed! |
NOTE: #2726 must be reviewed first