-
Notifications
You must be signed in to change notification settings - Fork 150
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
Landmarks really observed #185
base: master
Are you sure you want to change the base?
Conversation
…marks seen by the current camera.
This idea behind this branch is really useful, but I think it needs to be implemented differently. This is something @mwoehlke-kitware and I discussed a while back but never got around to implementing. I do feel that the UI needs to be unified the Landmarks menu. Having two top-level menus with the same icon is confusing, especially since some platforms only show the icon with no text. More importantly this feature needs to do more than just redraw landmarks seen from the current frame in a different color method. It needs to also not draw landmarks seen in the current frame with the other coloring method. Otherwise you have multiple points at the same point in space with different color fighting to be displayed and you don't know which one you will see, unless you can somehow guarantee which points are drawn on top. I recommend a checkbox under Landmarks menu to color visible landmarks and a color selector to pick which solid color to use and possibly another size slider. We don't need the color by data or true color options for visible landmarks, the point is to just to highlight which landmarks are currently seen and a fixed color or larger size is best for that. When the box is checked, you'll need to draw the visible landmarks in the selected color and draw only the non-visible landmarks by the primary coloring parameters. Also, finding which landmarks are visible on each frame is very inefficient and greatly slows down playback. The culprit here is MAP-Tk's current track data structure which is inefficient at the type of query you are trying to do. Until we fix that, I recommend building a lookup table for all frames that indicates which landmarks are visible on each frame and consulting that data structure when the frame index changes instead of scanning all the tracks again. |
…rks_really_observed
I agree with this, but it may raise another issue. Right now, the Landmarks menu is accessible even when the Landmarks are not shown (same thing for the other menus). Thus, with a checkbox in this menu to display the visible landmarks, it may be possible to display the visible landmarks with the rest of the landmarks hidden. According to me, it's a good thing to be able to display the one without the other, but it might be confusing to have this option in a menu under a unchecked button. |
@mwoehlke-kitware is the UI guy and maybe he has some thoughts on this. I agree that there may be value in showing only the visible landmarks and hiding the others. My first thought would be to add another UI element to enable this under the landmarks menu. Maybe it's another checkbox or maybe it's radio buttons or an option box, I don't know. But the choices are
and of course all landmarks can be hidden by the top-level visibility toggle. |
…ndmarks as application start
…rks_really_observed
I tried two solutions for this issue. First, I tried to load every visible landmarks for each cameras at launch. It dramatically increases launch time, but there's no more loading to do once the application is started. You'll find this solution in commit cd3f2f7. I then tested to load the visible landmarks for each active camera. When a camera get active and has no visible landmarks loaded, we load them. If we come back later to this camera, the visible landmarks are already loaded and they're directly displayed. This solution can be found in commit 8ece367. The two solutions save the visible landmarks to a map. The only difference is when the map is populated (at launch or when a camera become active for the first time). If you have a better solution to suggest, I'm all for it. I tried to optimize those ones but it's overall still slow, especially for the first one. |
The third option, which @mwoehlke-kitware often points out to me, is to run this pre-caching in the background in a separate thread. I don't know how difficult that is to do. Maybe it is better left for a later branch. For the first approach, you can make that more efficient by moving the inner loop over frame_id to inside the inner conditional ( For the second approach, you can make it more efficient by using I haven't tested either approach, but I suspect I would still prefer to compute this map once upfront. That is probably best done in a separate thread, but to keep it simple for now it could be done at load time, or maybe when the user first enables showing visible landmarks. |
…isplaying visible landmarks
I'll use this for now. I think using a thread for caching the visible landmarks is in fact a better solution, but I'd like to have some advices from @mwoehlke-kitware on the best way to do it. My intuition is to use |
…rks_really_observed
I've now tested the latest changes to this this branch. I like the new UI better, though for me I'm seeing some extra whitespace in the pop-up menu. Some items are aligned to the left and others to the right. So it looks a little odd. Functionally the branch also seems to do the right thing except that it is really slow for larger data sets the first time you navigate through the frames (>1 second per frame). It's fine once the cache is computed. I think we really need to compute that cache in advance, even if it is a little slower at load time and not in a separate thread yet. It occurred to me that we must already be computing these visible landmark ids elsewhere in the GUI. The residuals feature that @mwoehlke-kitware added draws lines between corresponding features and the visible landmarks on each frame. So we should already know which landmarks are visible, but maybe that cached information is not exposed. I think it would be best if we could reuse information between these two features. Maybe @mwoehlke-kitware can elaborate further on this. Also, I am seeing an occasional segfault when playing the video on this branch. I have not debugged yet to track down what is going on. |
Added options to the World and Camera views to visualize the landmarks seen by the current camera.
In each view, the display is triggered by a button. It uses the same options as the Landmarks menu.
It could also be interesting to fuse it with the Landmarks menu and transform it as a simple checkbox, but having a separate menu give more freedom on the display options (have size/colors or coloring method differ than the ones set for the landmarks).