Opened 9 years ago

Closed 9 years ago

#619 closed defect/bug (fixed)

gui_internal: "POIs" improvements

Reported by: flandry Owned by: number6
Priority: major Milestone:
Component: core Version: git master
Severity: Keywords: gui POI point of interest OSD
Cc: http://wiki.navit-project.org/index.php/user:nioui, http://wiki.navit-project.org/index.php/user:rdorsch, nick@…

Description

I'm using build 3479 by chollya with the gpainter graphics and internal gui.

When i select Actions->Vehicle Position->POIs, the processor load goes up and stays high forever, while navit never becomes responsive. I'm using a map from the navit map downloader.

Attachments (5)

POI_prepend.patch (696 bytes) - added by nioui 9 years ago.
Patch to build the POI list by prepending instead of appending
POI.diff (8.7 KB) - added by nioui 9 years ago.
Improvements to the POI menu entries
POI-101020.diff (13.7 KB) - added by tryagain 9 years ago.
Minor fix-up of my previous post.
POI-101022.diff (13.8 KB) - added by tryagain 9 years ago.
Yet another revision of POI.diff. New changes are small in size but significant for usability.
POI-101025.diff (6.7 KB) - added by tryagain 9 years ago.
Reincarnation of POI-101022 over current SVN with some improvements (see discussion).

Download all attachments as: .zip

Change History (17)

comment:1 Changed 9 years ago by nioui

  • Cc http://wiki.navit-project.org/index.php/user:nioui added

As far as I can tell, it just takes a very long time to list the POIs, but it will eventually succeed. Can you try on an area with very few POIs?

One part of the problem is that the list is build with g_list_append, which run in linear time, so that the full process takes quadratic time. I'm attaching a patch to use g_list_prepend instead, which takes constant time: this seems to significantly reduce the time needed to build the list of POIs. Since the list is sorted by distance at the end anyway, it shouldn't make any visible difference.

If there are other places where big list a being built with g_list_append, the code should be modified in a similar way...

A more complete solution to this issue would be to avoid building such a long list when there are too many POIs in the area.

Changed 9 years ago by nioui

Patch to build the POI list by prepending instead of appending

comment:2 Changed 9 years ago by rdorsch

  • Cc http://wiki.navit-project.org/index.php/user:rdorsch added

comment:3 Changed 9 years ago by nioui

  • Summary changed from "POIs" in gui menu never completes to gui_internal: "POIs" improvements

Here is a new patch with a better solving of this issue.

Instead of looking for all POI in a radius of 20 km, and building a huge list, I only keep the closest 10 POIs (using a Fibonacci heap). This small list is much faster to build. Note that the big list was useless anyway because it is not scrollable (http://trac.navit-project.org/ticket/352).

Moreover, I added a new entry titled "Next page" to access the next page of items (i.e. 10-20). Finally, if the area has been exhausted, there is now an option "More" to enlarge the search area. This allow to reduce the initial search area to 10km, which is better for urban areas, but the menu can now also be used in places where the first gas station is 50 km away (you just have to select "More" a few times).

The size of the page can be specified in the navit.xml file with <gui type="internal" enabled="yes" nb_lines="9">.

The POI menu is now definitely usable on my N900.

Changed 9 years ago by nioui

Improvements to the POI menu entries

comment:4 Changed 9 years ago by korrosa

This patch still applies cleanly on current SVN r3609.

I can confirm that this improves the POI interface massively. It is much more responsive, and enables pageing between POIs, as advertised.

However, pageing is implemented in a strange way. Strange as in -> doesn't conform to Navit's usual pageing mechanism (see the internal Roadbook for an idea of the norm). It has a "Next Page" button which appears at the end of the POI list. When clicked, it shows the next load of POIs, with another "Next Page" button at the end. However, there is no "Previous" button. Instead, it adds a "POI" header to the page, and adds another one each time the "Next Page" button is pressed, in a similar fashion to drilling down into a directory structure using Windows Explorer or Nautilus.

It should really have the Prev/Next? buttons, as with the Roadbook.

comment:5 Changed 9 years ago by number6

  • Cc nick@… added
  • Keywords gui POI point of interest OSD added

Previous and next buttons are vital, as is keeping the icons up the top when "next page" is selected.

Since top menu bar will change to "POI" >> "POI" >>"POI" >>... This really shouldn't happen, it gets very annoying and hard to "back track".

Good start though.

Changed 9 years ago by tryagain

Minor fix-up of my previous post.

comment:6 Changed 9 years ago by tryagain

Above POI-101020.diff is above POI.diff revision to address issues pointed in above posts by korrosa and number6.

I used a table widget for POI list rendering. So we have standard controls to move between pages.

Table widget for this use is improved and now can contain clickable elements in its rows. The latter can be used to give paging functionality to other lists in Navit like bookmarks and maps.

Also now POI menu contains an item to center the map on that POI position.

comment:7 Changed 9 years ago by korrosa

I can confirm all comments in above post by tryagain are working when applying diff to latest revision (currently 3610), and addresses all my concerns in my previous post. I consider this patch to fix the POI issue addressed in this ticket, and provides a variety of additional features which enhance the use of Navit greatly.

Thanks tryagain!

Changed 9 years ago by tryagain

Yet another revision of POI.diff. New changes are small in size but significant for usability.

comment:8 Changed 9 years ago by tryagain

Complete description of POI-101022.diff.

  1. There was a problem because fixed size 32 byte buffer was used to store POI label. Longer labels were truncated by multibyte-strings-unaware function. On Linux such strings were followed by short piece of garbage and on WinCE they were completely invisible. Very annoying on russian maps.
  1. Fix especially for weekends. Possible POI types for category "Others" were ending after ITEM(poi_selected_line) in item_def.h. Now POI types area end is just before ITEM2(0x80000000,line), so poi_nightclub and other far less significant POIs now should be included in POI category "Others".

comment:9 Changed 9 years ago by korrosa

  • Resolution set to fixed
  • Status changed from new to closed

POI-101022.diff included in r3611.

comment:10 Changed 9 years ago by tryagain

  • Resolution fixed deleted
  • Status changed from closed to reopened

It was POI-101020.diff included in r3611, not POI-101022.diff I'm posting a new patch over current SVN version to include improveents of POI-101022. I hope, this will fix problem "with prev-next buttons disappearing if the poi-names are too long" reported by noradtux.

Patch also includes a fix for POI category buttons being too small on mobile telephone devices reported by cp15. Commentaries were added for image_new_xs, image_new_s, image_new_l functions and icon_xs, icon_s, icon_l data members to state that _xs size images can be too small to click on some devices.

Changed 9 years ago by tryagain

Reincarnation of POI-101022 over current SVN with some improvements (see discussion).

comment:11 Changed 9 years ago by number6

  • Owner changed from KaZeR to number6
  • Status changed from reopened to new

Added POI-101022.diff in Revision 3621

comment:12 Changed 9 years ago by number6

  • Resolution set to fixed
  • Status changed from new to closed
Note: See TracTickets for help on using tickets.