Moin, I have taken a look at the add-pois-to-areas function: The function is implemented in MAPMaker.java (makeAreaPOIs), which means, it is active, AFTER the processing of the osm-data. So without a greater change this function can not be modified to be controlled by action rules during the style controlled osm-data processing. So I tried a limited modification, its not perfekt but for me it is an improvement over the actual implementation. This does not allow to create POIs for areas without the creation of a polygon (as a workaround a transparent polygon can be created). But the modification allows to control the POI generation for each polygon type on its own, i.e. via the style file single polygon types can be selected for the POI generation. My modification disables the add-pois-to-areas command line option, instead the POI generation is now triggered by a new command "add_poi" in the element type definition in the polygon style, e.g.: amenity=bank [0x2f06 level 3 add_poi] Since I am not really familiar with my java environment, I can not provide my modification as a patch, instead you will find the modified files attached to this mail. I have marked my changes with "//TTT-start" and "//TTT-end". Gruss Torsten
Great "patch". Twould be nice to have it commited. Is there interest for a patch file? I could offer one (I'ld just have to take out some of my own patches beforehand). On 19.02.2011 18:30, Torsten Leistikow wrote:
Moin,
I have taken a look at the add-pois-to-areas function: The function is implemented in MAPMaker.java (makeAreaPOIs), which means, it is active, AFTER the processing of the osm-data. So without a greater change this function can not be modified to be controlled by action rules during the style controlled osm-data processing.
So I tried a limited modification, its not perfekt but for me it is an improvement over the actual implementation. This does not allow to create POIs for areas without the creation of a polygon (as a workaround a transparent polygon can be created). But the modification allows to control the POI generation for each polygon type on its own, i.e. via the style file single polygon types can be selected for the POI generation.
My modification disables the add-pois-to-areas command line option, instead the POI generation is now triggered by a new command "add_poi" in the element type definition in the polygon style, e.g.: amenity=bank [0x2f06 level 3 add_poi]
Since I am not really familiar with my java environment, I can not provide my modification as a patch, instead you will find the modified files attached to this mail. I have marked my changes with "//TTT-start" and "//TTT-end".
Gruss Torsten
_______________________________________________ mkgmap-dev mailing list mkgmap-dev@lists.mkgmap.org.uk http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
BTW here are the changes as patch applied against 1871. I think it would be worthwhile adding it to the trunk. On 19.02.2011 18:30, Torsten Leistikow wrote:
Moin,
I have taken a look at the add-pois-to-areas function: The function is implemented in MAPMaker.java (makeAreaPOIs), which means, it is active, AFTER the processing of the osm-data. So without a greater change this function can not be modified to be controlled by action rules during the style controlled osm-data processing.
So I tried a limited modification, its not perfekt but for me it is an improvement over the actual implementation. This does not allow to create POIs for areas without the creation of a polygon (as a workaround a transparent polygon can be created). But the modification allows to control the POI generation for each polygon type on its own, i.e. via the style file single polygon types can be selected for the POI generation.
My modification disables the add-pois-to-areas command line option, instead the POI generation is now triggered by a new command "add_poi" in the element type definition in the polygon style, e.g.: amenity=bank [0x2f06 level 3 add_poi]
Since I am not really familiar with my java environment, I can not provide my modification as a patch, instead you will find the modified files attached to this mail. I have marked my changes with "//TTT-start" and "//TTT-end".
Gruss Torsten
_______________________________________________ mkgmap-dev mailing list mkgmap-dev@lists.mkgmap.org.uk http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
Felix Hartmann schrieb am 28.02.2011 22:55:
BTW here are the changes as patch applied against 1871. I think it would be worthwhile adding it to the trunk.
I removed my change-markings from the files and attached the cleaned source files. (I still haven't figured out how to provide a proper patch) Certainly I am in favour of commiting this change, but be aware, that the change is not backward compatible: The add-pois-to-areas argument is not supported any more. Gruss Torsten
On 01.03.2011 18:32, Torsten Leistikow wrote:
Felix Hartmann schrieb am 28.02.2011 22:55:
BTW here are the changes as patch applied against 1871. I think it would be worthwhile adding it to the trunk. I removed my change-markings from the files and attached the cleaned source files. (I still haven't figured out how to provide a proper patch) I use TortoiseSVN mainly (as my dev machine is a windows server). There is a button where you can click on "create patch" - then select the files you want to have for the patch, and voila. Actually simpler than applying a patch (where you might have conflicts if patch is outdated). Certainly I am in favour of commiting this change, but be aware, that the change is not backward compatible: The add-pois-to-areas argument is not supported any more.
Well I could easily offer a sensible patch for the polygons file, to include sensible POI (like all amenity but no landuse as there are no responding categories in the search fields anyhow).
Gruss Torsten
_______________________________________________ mkgmap-dev mailing list mkgmap-dev@lists.mkgmap.org.uk http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
On Tue, Mar 01, 2011 at 06:32:23PM +0100, Torsten Leistikow wrote:
I removed my change-markings from the files and attached the cleaned source files. (I still haven't figured out how to provide a proper patch)
If you have checked out the source tree with 'svn checkout', then you can just do 'svn diff' to create a patch. The output of that command will also show the svn revision against which the patch was prepared (but sadly not the full path to the repository). See also http://svnbook.red-bean.com/. Marko
Moin, my mkgmap build process is very shaky, but finally I managed to create a patch of my changes. Perhaps it is even working? Gruss Torsten
On Thu, Mar 03, 2011 at 04:59:37PM +0100, Torsten Leistikow wrote:
my mkgmap build process is very shaky, but finally I managed to create a patch of my changes. Perhaps it is even working?
Could you elaborate what your patch does? Does it remove the option add-pois-to-areas altogether? Even though an add_poi action would replace add-pois-to-areas, I think that we should support both forms for some time. You may ignore the rest of this message. I did a too quick read of the patch and thought that you had changed accidentally white space in some lines. You did that on purpose, because you removed one level of indentation when removing the check for the add-pois-to-areas parameter. Usually, I would do something like this: svn diff > add_poi.patch emacs add_poi.patch and then fix any mistakes in the patch, either manually or by reverting unintended changes with C-u C-c C-a (diff-apply-hunk with a prefix argument). When reviewing a patch, you could save some work by telling svn diff to ignore the white space changes: svn diff x -wpu > add_poi-w.patch Old versions of "svn" do not accept the -x option, unless you also specify --diff-cmd: svn diff --diff-cmd diff -x -wpu > add_poi-w.patch On a related note, the patch -l option is useful when there are white space differences between the patch and the source file, such as when TABs have been converted to spaces in the patch or in the source file. Marko
Marko Mäkelä schrieb am 03.03.2011 22:54:
Could you elaborate what your patch does? Does it remove the option add-pois-to-areas altogether? Even though an add_poi action would replace add-pois-to-areas, I think that we should support both forms for some time.
Sadly this patch disables the add-pois-to-areas option completely. I also would like to have both capabilities for backward compatibiliy, but in the style processing the command line arguments are not visible at the moment. So there is no easy way for testing either whether the addpoi command is set in the style or whether the command line argument is set. The trunk version generates in the style processing the basis for the POI generation for ALL objects regardless the command line argument. During the map generation the command line argument is test, for whether the available POIs are generated or not. In my patch during the style creation only the basis for the POI generation of the objects with the addpoi command is created. And during the map generation a POI is ALWAYS created, if for an object the corresponding basis is found.
You may ignore the rest of this message. I did a too quick read of the patch and thought that you had changed accidentally white space in some lines. You did that on purpose, because you removed one level of indentation when removing the check for the add-pois-to-areas parameter.
I have read the rest of your message, but probably only understood half of it. (I am neither familiar with eclipse nor with svn, so I hardly now what I am doing when building my own mkgmap.) What is the desired approach on the indentation when a patch has such a structure change like removing an if-bracketing? In my change I also "corrected" the indentation of otherwise unchanged lines. Shouldn't such changes be included in the patch? If they are not included, how is the indentation then "corrected"? Gruss Torsten
On Fri, Mar 4, 2011 at 8:48 AM, Torsten Leistikow <de_muur@gmx.de> wrote:
What is the desired approach on the indentation when a patch has such a structure change like removing an if-bracketing?
Changing indentation when removing a nesting level is fine, but it should be confined to the area where the nesting was removed.
In my change I also "corrected" the indentation of otherwise unchanged lines. Shouldn't such changes be included in the patch? If they are not included, how is the indentation then "corrected"?
Whitespace-only changes should be included in a separate patch that does not change any functionality. That makes it easier to review changes. -- Jeff Ollie
Jeffrey Ollie schrieb am 04.03.2011 16:09:
Whitespace-only changes should be included in a separate patch that does not change any functionality. That makes it easier to review changes.
Thanks for the clarification. Gruss Torsten
Well it's about 30 seconds to get the current behavior. You simply go into the polygons file, and exchange all "]" with "command ]"
participants (4)
-
Felix Hartmann -
Jeffrey Ollie -
Marko Mäkelä -
Torsten Leistikow