-
Notifications
You must be signed in to change notification settings - Fork 3
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
Add demo for basic image warping via gdal.js + loam #30
Conversation
This commit adds a new mini app, but does not complete the implementation. Rather, it bootstraps the basic app skeleton with: - gdal.js + loam correctly installed, no errors - User can upload a file from their machine - The selected image will be loaded and displayed on a leaflet map using the distortable image plugin - Stub methods for the start of the calculations necessary to calculate GCPs and call GDAL translate + warp See map-warpr/README.md for some basic notes.
Translate + warp throws: loam.min.js:1 Uncaught (in promise) Error: integer result unrepresentable
Passing to gdal warp as ints causes the warp command to read them as byte strings rather than the values we're interested in.
MapWarpr: Implement the warp operation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good; I found one typo and made a couple usage suggestions that may make the experience a bit smoother for users.
var map = L.map('map', { | ||
center: [39.5, -98.35], | ||
zoom: 5 | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can use loam.initialize()
to start pulling down the WebAssembly asynchronously right after the initial page load. That should address the long latency for the initial warp. All the other loam operations (are supposed to) rely on the loam.initialize()
promise before they do their thing, so you don't (shouldn't) have to pay attention to the return value of that function.
Another possibility is to initialize()
once someone clicks the file-picker so that we don't use that bandwidth until we know they're serious, but in this case that's basically the only thing they can do, so I think it makes more sense to start loading the wasm immediately.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Second part of your comment is a great optimization and it wouldn't hurt to just put it there right away. I'll make the change.
map-warpr/main.js
Outdated
layerGroup.addLayer(L.marker(point)); | ||
}); | ||
|
||
var translateArgs = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this needs to be removed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
whoa. oops.
map-warpr/main.js
Outdated
}); | ||
|
||
var translateArgs = | ||
loam.open(file).then(function (ds) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This and the warp call are going to leak memory unless you keep track of the intermediate datasets and close them. It's not great, but the best approach is probably something like this:
var imgDataset, gcpDataset;
loam.open(file).then(function (ds) {
imgDataset = ds;
...
}).then(function (ds) {
gcpDataset = ds;
...
}).then(...).then(...)
.finally(function () {
if (imgDataset) {
imgDataset.close();
}
if (gcpDataset) {
gcpDataset.close();
}
});
I've got some ideas for refactoring Loam so that this isn't necessary because it's super annoying; I'd be interested in talking those over at some point now that you've used it a bit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, not ideal but I'll make this change. It's reasonable, even with the demo, to expect someone to attempt to warp multiple images.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd be happy to talk about ways to improve loam in this regard, feel free to toss something on the schedule, or we can chat over lunch sometime.
Overview
This PR adds a relatively simple demo for image warping via gdal.js + loam.
The basic algorithm used to warp the image is:
-tps
argumentDemo
Notes
A discussion with @designmatty resulted in this super basic, no minification, no scss project structure. We figured that we can clean this up and add any appropriate build tooling once this is a workable demo.
Thanks to @ddohler for assistance in some debugging of gdal.js, including instruction on how to do custom builds, navigating the emscripten build options and some debugging help with loam+gdal.js.
There are quite a few directions we could go with this from here:
Testing
See the "Developing" section of the new README for instructions on running locally. Ensure you're able to upload an image file and have it display on the map. Then ensure that after clicking the "engage" button, that it is disabled while the warping operation takes place, and a usable warped tiff is downloaded. Some debugging info will be printed to the console as the operation commences.
The app is testing to work on Chrome/FF. Theoretically, it should also work on Edge. I had trouble running on Safari, where I got an error in the webworker while performing the warping operations. I didn't investigate this any further ATM: