-
Notifications
You must be signed in to change notification settings - Fork 141
Enable relative filenames in ImageElevationDatabase kwl map #290
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
base: dev
Are you sure you want to change the base?
Conversation
Instead of doing that lets change the code to work with relative or absolute file names in the map. Basically: if ( file.isRelative() ) file = ossimFilename(m_connectionString).dirCat( file ); We would need a flag to save the map file entry relative or absolute. Then if relative you just change the connection string if you move it in the ossim preferences file. |
So basically this: Would become: |
The loadState() and saveState() virtuals with two args (kwl, prefix) are declared WAY up the inheritance tree to ossimObject. Adding a special loadState() just for that class is not a good idea, given how the whole object factory system is set up. I agree with Burken on his potential solution. |
I agree that the change to loadState is ugly and a bad idea, but it's not clear to me how else we can correct the filename during FileEntry load/save. We could put a flag in the preferences file like this with a default to
But FileEntry can't see the preferences file or the connection string during load/save, so it can't tell when to save a relative path or where to cut the path to make it relative, loadState can't build an absolute path when given a relative one, and we don't want to interfere with inheritance by modifying their signature to accept more data. We could create a new FileEntry constructor that takes a reference to m_connectionString and a boolean flag to save relative paths and use it during loadMapFromKwl to fix loading and slip it into processFIle to fix saving. That's the cleanest way I can think of, but changing load/save behavior based on constructor values feels iffy. Would a new constructor be an inheritance issue? We could (if possible) rewrite the kwl pre- and post-save, but that feels like a string nightmare. I'm more than happy to do the legwork to make this happen, but I don't see a nice approach to doing it. |
Slept on it and I'm wondering what benefits absolute paths provide over relative paths? An absolute map duplicates the preference file connection string in a way that's somewhat hidden from the user and is very brittle because it can't survive the image directory being moved. A relative path has a small startup cost, but it's really just some string concatenation. I don't think the elevation kwl map has been in a release yet, maybe it would be best to switch to relative paths only? We just need to find a way to inject the connection string into FileEntry so it can save and load right. |
Just another comment on this. You can simply make the connection string a directory now provided you are OK mapping all the images in there. Like this: Test: |
The file map is a great addition, but it's not as flexible as it could be. I'm interested in hearing opinions on enabling relative paths in the file map. This allows for preexisting maps to be modified to work in environments where centralized elevation data may be mounted in different locations. Relative paths are a nice feature of VRTs, but I've been experiencing projection issues when doing elevation queries (#289 but I haven't had the opportunity to document the issue on the dev branch, and it's most likely an issue with the gdal plugin, not ossim core).
In order to sidestep potential issues with Windows, a file entry is only appended to connectionString if it starts with a period (instead of a non-slash).
The current patch works and does not change default behavior, but the addition of
connectionString
toossimImageElevationDatabase::ossimImageElevationFileEntry::loadState
may not be preferred. Doing it inloadMapFromKwl
feels better, but the way initialization is offloaded toloadState
prevents that.