USE_POINT_Z_M default on #5456
USE_POINT_Z_M default on #5456
Conversation
Performance difference between on and off is minimal
|
If you change the code to "on" as default, you should change the comment too. |
|
Sorry, i forgot |
|
@DDanielH could you create this pull request to mapserver:master instead of branch-7-0 ? (because this changes the functionality of MapServer, it will have to be applied to the 7.2 release) |
|
Can anyone provide any information on why in CMakeLists there is the following comment?
Is this recommendation now out of date? What were the issues in using it? |
|
Assuming the cmake config was put together from the old configure build system it’s likely it’s out-of-date. There is at least one spot I know of that could leverage Z/M but doesn’t – that’s the templating code (shpxy tag). There’s a separate ticket for that to introduce an extended shpxy tag or more like add new ones. It’s possible other output drivers (e.g. GML, KML) aren’t complete in this area but if I remember correctly that’s why support was added in the first place, at least for GML. Another place that could be deficient is expression handling. We’d have to check…
I’m curious if the binary distros are built with this option or not.
…-Steve
From: Seth G [mailto:notifications@github.com]
Sent: Thursday, October 11, 2018 9:28 AM
To: mapserver/mapserver <mapserver@noreply.github.com>
Cc: Subscribed <subscribed@noreply.github.com>
Subject: Re: [mapserver/mapserver] USE_POINT_Z_M default on (#5456)
Can anyone provide any information on why in CMakeLists there is the following comment<https://github.com/mapserver/mapserver/blob/d5c22cd3113ba85f5b1f1c406fe4055f75d04ce3/CMakeLists.txt#L149>?
option(WITH_POINT_Z_M "include Z and M coordinates in point structure (advanced, not recommended)" OFF)
Is this recommendation now out of date? What were the issues in using it?
The comment was added during the migration from autotools to cmake 6 years ago.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub<#5456 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/ABhm-64ARWN-KZrIh-p1ixjOxc30Veohks5uj1VUgaJpZM4ON5_e>.
|
|
The output formats do support Z output but there are requirements on setting options on the output formats. See https://mapserver.org/output/ogr_output.html, specifically ows/wfs_geomtype.
Mike
From: Steve Lime <notifications@github.com>
Reply-To: mapserver/mapserver <reply@reply.github.com>
Date: Thursday, October 11, 2018 at 11:03 AM
To: mapserver/mapserver <mapserver@noreply.github.com>
Cc: Subscribed <subscribed@noreply.github.com>
Subject: Re: [mapserver/mapserver] USE_POINT_Z_M default on (#5456)
Assuming the cmake config was put together from the old configure build system it’s likely it’s out-of-date. There is at least one spot I know of that could leverage Z/M but doesn’t – that’s the templating code (shpxy tag). There’s a separate ticket for that to introduce an extended shpxy tag or more like add new ones. It’s possible other output drivers (e.g. GML, KML) aren’t complete in this area but if I remember correctly that’s why support was added in the first place, at least for GML. Another place that could be deficient is expression handling. We’d have to check…
I’m curious if the binary distros are built with this option or not.
…-Steve
From: Seth G [mailto:notifications@github.com]
Sent: Thursday, October 11, 2018 9:28 AM
To: mapserver/mapserver <mapserver@noreply.github.com>
Cc: Subscribed <subscribed@noreply.github.com>
Subject: Re: [mapserver/mapserver] USE_POINT_Z_M default on (#5456)
Can anyone provide any information on why in CMakeLists there is the following comment<https://github.com/mapserver/mapserver/blob/d5c22cd3113ba85f5b1f1c406fe4055f75d04ce3/CMakeLists.txt#L149>?
option(WITH_POINT_Z_M "include Z and M coordinates in point structure (advanced, not recommended)" OFF)
Is this recommendation now out of date? What were the issues in using it?
The comment was added during the migration from autotools to cmake 6 years ago.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub<#5456 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/ABhm-64ARWN-KZrIh-p1ixjOxc30Veohks5uj1VUgaJpZM4ON5_e>.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.
|
|
@sdlime - The Windows builds at GIS Internals don't have this option set - which then throws errors in various tests when using Python MapScript built with USE_POINT_Z_M ON (which is why it would be good to set this to ON by default if it is stable). |
|
Note that this switch was added a long time ago because having a point structure of 2 int instead of 4 int meant it was better handled in memory and made a "significant" performance improvement. See details here: https://trac.osgeo.org/mapserver/ticket/1244 It was a time of custom build and it was assumed that people were compiling a lot. This is not the case anymore and it is really possible that the performance "improvement" is not even true anymore. We could set this switch to ON by default or even remove the switch altogether if that is the case. |
|
@juliensam - thanks for the info. It sounds like at least the CMake warning can be removed, and then possibly set it to ON by default? This pull request however modifies nmake.opt - which as I understand it is redundant, and so a new pull request should be made and this one closed? |
|
I think this can be closed - nmake is no longer used since the switch to CMake. |
yes, feel free to submit a PR removing it. I'm supportive |
Performance difference between on and off is minimal