Skip to content
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

Multi export #314

Merged
merged 110 commits into from
Jul 2, 2019
Merged

Multi export #314

merged 110 commits into from
Jul 2, 2019

Conversation

sashadev-sky
Copy link
Member

@sashadev-sky sashadev-sky commented Jun 29, 2019

Fixes #270 (<=== Add issue number here)
/ PR at #310

Also:

Make sure these boxes are checked before your pull request (PR) is ready to be reviewed and merged. Thanks!

  • PR is descriptively titled 📑 and links the original issue above 🔗
  • tests pass -- look for a green checkbox ✔️ a few minutes after opening your PR -- or run tests locally with grunt
  • code is in uniquely-named feature branch and has no merge conflicts 📁
  • screenshots/GIFs are attached 📎 in case of UI updates
  • @mention the original creator of the issue in a comment below for help or for a review

We're happy to help you get this ready -- don't be afraid to ask for help, and don't be discouraged if your tests fail at first!

If tests do fail, click on the red X to learn why by reading the logs.

Please be sure you've reviewed our contribution guidelines at https://publiclab.org/contributing-to-public-lab-software

Thanks!

=================================

Changes:

  • Removed: unused: "strict" from JSHINT gruntfile
  • L.Map.BoxSelector is fixed (Multi-select with shift-drag)
  • src/edit/tools folder with DistortableImage.ControlBar.js and DistortableImage.PopupBar.js, each corresponding to the new class names for the 2 types of toolbars and EditAction.js the classname for the class we extend to create new actions.
  • leaflet-toolbar upgraded to "0.4.0-alpha.2" and added as a dependency in package.json
  • Two multi-image interface tools: Exports, Deletes
  • Toolbar API: removeTool, addTool, hasTool for the DistortableCollection and DistortableImage.Edit class - allows changing the toolbar actions in runtime
  • DistortableCollection and DistortableImageOverlay instantiation updates: new action option allows overwriting the default toolbar actions for respective toolbars initially if don't want to use the runtime API above
  • DistortableCollection instantiation updates: images are now only added to the collection class, not the map directly.
  • _updateCorners and _updateCorner() are now setCorners and setCorner() but not ready to be documented yet - need overlay.fire("update") in them to work sep. but I don't want to change where we currently fire it in our code bc of the bugs with rotation, scaling ,etc.
  • Deletion of unnecessary committed leaflet.distortableimage.js.map file two months ago
  • Testing updates to match changes to DistortableCollection class

========================

Pending:

  • loader UI: considering for now just adding animated SVG loader icon to replace Export icon during load
  • Multiple Exports still not working
  • make a DistortableCollection.Edit class so the Tool API will work the same on it as on DistortableImage.Edit - then update docs

@jywarren
Copy link
Member

jywarren commented Jul 2, 2019

I think we need to re-order the corners we get from this section:

this._corners = [
map.containerPointToLatLng(center.subtract(offset)),
map.containerPointToLatLng(
center.add(L.point(offset.x, -offset.y))
),
map.containerPointToLatLng(
center.add(L.point(-offset.x, offset.y))
),
map.containerPointToLatLng(center.add(offset))
];

@jywarren
Copy link
Member

jywarren commented Jul 2, 2019

Yes, i'm going to bump version number so it'll get picked up by MapKnitter, then we can come back to UI. Thanks!

@jywarren
Copy link
Member

jywarren commented Jul 2, 2019

OK, i'm adding a note to the README that this library does NW, NE, SW, SE corners!

@jywarren
Copy link
Member

jywarren commented Jul 2, 2019

Done and pushing to my gh-pages to test: http://jywarren.github.io/Leaflet.DistortableImage/examples/select

@jywarren
Copy link
Member

jywarren commented Jul 2, 2019

Darn! I'm still getting convert-im6.q16: invalid argument for option Perspective : 'Unsolvable Matrix' @ error/distort.c/GenerateCoefficients/837. -- why? let me double-check the order...

@jywarren
Copy link
Member

jywarren commented Jul 2, 2019

[{"lat":51.51300227287522,"lng":-0.12977341306395831},
{"lat":51.53297533262061,"lng":-0.11981705320067705},
{"lat":51.53297533262061,"lng":-0.07981995237059891},
{"lat":51.50869919147977,"lng":-0.07673346321098508}

That's: SW NW NE SE -- that should be correct... in the submitted order... let me check where they're coming out on the cloud logs...

@jywarren
Copy link
Member

jywarren commented Jul 2, 2019

0,0 0,3573     NW to SW
521,0 0,0      NE to NW
521,348 0,0      SE to NW
0,348 0,4343      SW to SW

...roughly. Main problem is that points 2 and 3 of 4 are both being sent to 0,0!

@jywarren
Copy link
Member

jywarren commented Jul 2, 2019

Oh gosh, you're right! The exporter service uses lon -- ok, changing!

@sashadev-sky
Copy link
Member Author

@jywarren Ok great! I had on my todo list to add to the README a blurb about coordinate order because at one point we had them backwards!

@jywarren
Copy link
Member

jywarren commented Jul 2, 2019

Hooray! There we go!

https://mapknitter-exports-warps.storage.googleapis.com/1562027947/1562027947.jpg

{"status_url":"https://mapknitter-exports-warps.storage.googleapis.com/1562027947/status.json","status":"complete","tms":"public/tms/1562027947/","geotiff":"public/warps/1562027947/1562027947.tif","zip":"public/tms/1562027947.zip","jpg":"public/warps/1562027947/1562027947.jpg","export_id":1562027947,"user_id":null,"size":"78.9256MB","width":"7388","height":"2670","start_time":"2019-07-02 00:39:06 +0000","run_time":null,"gem_version":"1.0.9","cm_per_pixel":100.0}

There are some timing issues... or something... for example I'm still getting responses like this as well:

{"status_url":null,"status":"starting","tms":false,"geotiff":false,"zip":false,"jpg":false,"export_id":1562027947,"user_id":null,"size":null,"width":null,"height":null,"start_time":"2019-07-02 00:39:06 +0000","run_time":null,"gem_version":"1.0.9","cm_per_pixel":null}

@sashadev-sky
Copy link
Member Author

@jywarren It is working! But I am stuck on warping 1/1 still. Is this because I'm not on gh-pages?

@jywarren
Copy link
Member

jywarren commented Jul 2, 2019

It's a big export... just give it a few more minutes?

@jywarren
Copy link
Member

jywarren commented Jul 2, 2019

Mine has some caching issues. If i call:

$.ajax('http://export.mapknitter.org/id/1562027947/status.json').done(function(r){console.log(r)})

I get status: 'starting' even though I've /also/ received a complete response earlier. I keep getting started and so it won't kill our interval timer...

Can't seem to bust that cached response with this either:

$.ajax('http://export.mapknitter.org/id/1562027947/status.json?_=12421', {cache: false}).done(function(r){console.log(r)})

OK. We basically got there. Let's see if we can fix this cache issue and tomorrow we can merge this and publish it.

@jywarren
Copy link
Member

jywarren commented Jul 2, 2019

Ah! OK! So -- here it is -- if we try to bust the cache by appending a timestamp, like:

$.ajax('http://export.mapknitter.org/id/1562027947/status.json?123').done(function(r){console.log(r)})

It will strip the ?123 when it redirects to https://mapknitter-exports-warps.storage.googleapis.com/1562027947/status.json

But if we directly call the final URL with a ?123 appended to bust /that/ cache, it works!

$.ajax('https://mapknitter-exports-warps.storage.googleapis.com/1562027947/status.json?123').done(function(r){console.log(r)})

So, we really do have to get this segment of code working:

// this may be overridden to update the UI to show export progress or completion
function _defaultUpdater(data) {
// optimization: fetch status directly from google storage:
if (data.hasOwnProperty('status_url') && statusUrl !== data.status_url && data.status_url.match('.json')) { statusUrl = data.status_url; } if (data.status === "complete") {
clearInterval(updateInterval);
alert("Export complete. " + data.jpg);
}

Where it can detect the "final" address.

@jywarren
Copy link
Member

jywarren commented Jul 2, 2019

Actually I think I can fix it here too:

https://github.com/publiclab/mapknitter-exporter-sinatra/blob/4620dd9d28edae612fe59ee30a1ed303f8e47289/app.rb#L51-L59

OK - i did this in that project. I'm going to merge now! Let's do all this follow-up too!

@jywarren jywarren merged commit d934714 into publiclab:main Jul 2, 2019
@jywarren
Copy link
Member

jywarren commented Jul 2, 2019

So, prompt, spinner, and then MK integration!

@jywarren
Copy link
Member

jywarren commented Jul 2, 2019

Exciting!

@jywarren
Copy link
Member

jywarren commented Jul 2, 2019

and -- we should rejigger the section of code above that tries to match "complete" and the new JSON address. I think if we fix it, things will run a lot smoother!

@jywarren
Copy link
Member

jywarren commented Jul 2, 2019

I published this to NPM!

@sashadev-sky
Copy link
Member Author

@jywarren should I try to connect it in MK first?

@sashadev-sky
Copy link
Member Author

Ill open a new issue for the next 3 parts u just listed!

@jywarren
Copy link
Member

jywarren commented Jul 2, 2019 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

generateExportJson follow-ups
2 participants