GNOME Bugzilla – Bug 704645
Add support for OpenWeatherMap
Last modified: 2013-08-21 14:38:24 UTC
OpenWeatherMap (http://openweathermap.org) provides with very nice weather maps for the whole world, and it would be cool if gnome-weather could just use libchamplain to show them.
Created attachment 249752 [details] [review] Add support for OpenWeatherMap OpenWeatherMaps provides free (CC-BY-SA 2.0) maps for various kind of weather data, such as clouds, wind, temperature and precipitation. This is not fully functional because OWM maps are just layers, intended to be placed above OpenStreetMaps.
Created attachment 249753 [details] [review] ChamplainView: add support for secondary sources OWM are layers to be rendered on top of an existing map, and the simplest way to achieve that is to have ChamplainView render maps from a list of sources.
Hey Giovanni, that's a really cool feature, thanks for the patch! I had a look at the patches and have a few comments: 1. I have changed the tile loading code a bit to improve responsiveness (it's in master now) and the patch will have to be modified slightly - it should be quite straightforward though. (There might be a slight problem with the tile_map code and we might need a tile map for each map source but this can be done later.) 2. I would prefer a slightly different naming of the primary/secondary sources. I would just keep map_source as it is and call the secondary sources "overlay sources" (and change the variable names and methods accordingly). 3. I get crashes when quickly zooming in with the mouse wheel with the message (lt-launcher-gtk:28846): Cogl-ERROR **: Failed to create an OpenGL framebuffer object This typically happens when there's not enough of texture memory. I have played with the code a bit and noticed that when opacity is set to 255, this doesn't happen. To workaround the crash I think only the map tiles should be used for the zoom actor and not the overlay tiles. There could be some flag in ChamplainTile indicating whether it's an overlay tile and we could check this flag when creating the zooming actor. 4. I'd prefer to have the demo as a separate patch. 5. In the demo you use the cached source for the overlay tiles. This is not a good idea for weather because the cached source caches the tiles for 7 days so the weather tiles aren't up-to-date. Also if the weather tile is not available for some reason, this will show an error tile over the map. It would be better to use a map source consisting only of the memory cache and the network source. Maybe adding a new factory method for this by modifying champlain_map_source_factory_create_cached_source() would be useful too. 6. Question: what's the reason behind re-setting the overlay sources in the set_map_source() method? Could you update the patches for (1), (2), (4)? We can do the rest later.
Created attachment 251942 [details] [review] Add support for OpenWeatherMap OpenWeatherMaps provides free (CC-BY-SA 2.0) maps for various kind of weather data, such as clouds, wind, temperature and precipitation. This is not fully functional because OWM maps are just layers, intended to be placed above OpenStreetMaps.
Created attachment 251943 [details] [review] ChamplainView: add support for overlay sources OWM are layers to be rendered on top of an existing map, and the simplest way to achieve that is to have ChamplainView render maps from a list of sources. set_map_source() clears the overlay sources to keep the existing semantic that changing the primary source changes the entire map. Overlay sources are somehow attached to the primary source, and don't make sense on their own. (In practice, overlay sources are only useful for openweathermap, and only above a terrain map, or at most a geopolitical one, so it's not like the user will change the map interactively)
Created attachment 251944 [details] [review] Add demo of the new overlay sources Allow to add overlay sources in the demo app.
Thanks, I've just applied your patches to master. I've made a few changes/additions: 1. You removed the check for map source in fill_tile_cb() which I added back. The reason for the check is that since the loading is done in the idle functions now, it can theoretically happen that map source is changed after the idle functions are added to the run loop but before the idle functions are actually started in which case the tiles shouldn't be loaded. Since the overlay sources are kind of attached to the main map source, it is sufficient to check for the main map source change. 2. Just made some formatting changes to match the libchamplain style and added documentation too. 3. The license actor needs to display the licenses of both the map source and the overlay sources now. For this I added an additional function to retrieve all map sources from ChamplainView, read the licenses in the license view and display all of them (unless there's a duplicate text in the layer licenses). In order to detect addition/removal of the overlay license, the add/remove functions now emit the "map-source" signal.