Drag and Drop JSON File to Tile Layer to Reconstruct Saved Map

segun-codes commented 1 year ago

Fixes #1344

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

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

Hi @jywarren, the feature to drag and drop .json files to tile layer for the reconstruction of map to its previous saved state is now up and running. Please, review and advise with your thoughts. See Illustration 1 below for some insights. Many thanks!

ILLUSTRATION 1

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!

segun-codes commented 1 year ago

This is really good. What do you think of integrating this into archive.js?

Oh! okay... sounds good too. I'll sure do.

segun-codes commented 1 year ago

Hi @jywarren, drag and drop feature (for image and JSON file) now integrated into archive.js. Kindly review. Illustrations 2, 3 and 4 below are for your reference.

ILLUSTRATION 2 MapKnitter-Lite-2

ILLUSTRATION 3 MapKnitter-Lite-3

ILLUSTRATION 4 MapKnitter-Lite-4

jywarren commented 1 year ago

This is looking good! Is local.js still necessary to change?

Also, I wasn't sure I followed, is it recovering the corner latitude longitude pairs and placing images where they had been?

segun-codes commented 1 year ago

This is looking good! Is local.js still necessary to change?

Also, I wasn't sure I followed, is it recovering the corner latitude longitude pairs and placing images where they had been?

Is local.js still necessary to change? No, the new functionality integrated into archive.js is completely independent of local.js

Is it recovering the corner latitude longitude pairs and placing images where they had been? Yes. This means, archive.js does two additional things: 1. supports dragging and dropping of images to the tile 2. fetches, places images (on the tile) in coordinates specified in a JSON file that must be dragged and dropped on the tile.

segun-codes commented 1 year ago

Hi @jywarren, this is for your review.

The code has been refactored to support drag and drop for local.html, archive.html as well as reconstructing images (from URL) using the agreed JSON format. Also, the save icon on sidebar now writes to disk JSON file consistent with the agreed JSON format. The below is for your reference.

JSON files downloaded from MK Lite, they now use the new JSON format agreed on:

URLs used for testing URL-based image reconstruction:

Note: As for files[0].type === 'application/json' on line 346 inside archive.js, the property type seems to be introduced by the event object passed to handleDrop().

jywarren commented 1 year ago

Exciting!! So, I tried making a map, downloading it, then refreshing the page and dragging it back on after dismissing the welcome modal. But I got an error! Can you try this workflow? I can record the errors too.

On Tue, Feb 7, 2023, 6:58 PM Segun @.***> wrote:

Hi @jywarren https://github.com/jywarren, this is for your review.

The code has been refactored to support drag and drop for local.html, archive.html as well as reconstructing images (from URL) using the agreed JSON format. Also, the save icon on sidebar now writes to disk JSON file consistent with the agreed JSON format. The below is for your reference.

JSON files downloaded from MK Lite, they now use the new JSON format agreed on:

https://ia804700.us.archive.org/5/items/mkl-1/mkl-1.json

https://ia804700.us.archive.org/5/items/mkl-3/mkl-3.json

URLs used for testing URL-based image reconstruction:

1. http://localhost:8081/examples/archive.html?json=https://archive.org/download/mkl-1/mkl-1.json 2. http://localhost:8081/examples/archive.html?json=https://archive.org/download/mkl-3/mkl-3.json

Note: As for files[0].type === 'application/json' on line 346 inside archive.js, the property type seems to be introduced by the event object passed to handleDrop().

— Reply to this email directly, view it on GitHub https://github.com/publiclab/Leaflet.DistortableImage/pull/1345#issuecomment-1421666898, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAF6J6URJFV55XQ2BJ6BDLWWLORPANCNFSM6AAAAAAUPSQWS4 . You are receiving this because you were mentioned.Message ID: @.***>

jywarren commented 1 year ago

Here's the error I got:

Uncaught TypeError: Cannot read properties of undefined (reading '0')
    at i.getCorner (DistortableImageOverlay.js:519:25)
    at i._calculateProjectiveTransform (DistortableImageOverlay.js:535:43)
    at i._animateZoom (DistortableImageOverlay.js:501:34)
    at i.fire (Events.js:203:11)
    at i._animateZoom (Map.js:1699:8)
    at i.<anonymous> (Map.js:1677:9)

And here's the .json:

{"avg_cm_per_pixel":6.763221163059949,
"collection":[{"id":134,"src":"https://archive.org/download/texas-barnraising//IMG_0137.JPG","width":3648,"height":2736,"tooltipText":"IMG_0137","image_file_name":"IMG_0137.JPG",
"nodes":[
{"lat":51.49046894696402,"lon":-0.06231307983398438},
{"lat":51.48565891027461,"lon":-0.03896713256835938},
{"lat":51.46427482966439,"lon":-0.03896713256835938},
{"lat":51.46427482966439,"lon":-0.08462905883789064}
],"cm_per_pixel":3.926851070243194},
{"id":132,"src":"https://archive.org/download/texas-barnraising//IMG_0128.JPG","width":3648,"height":2736,"tooltipText":"IMG_0128","image_file_name":"IMG_0128.JPG",
"nodes":[
{"lat":51.53629915545823,"lon":-0.15552520751953128},
{"lat":51.53629915545823,"lon":-0.10986328125000001},
{"lat":51.5027589576403,"lon":-0.12445449829101564},
{"lat":51.51493882813492,"lon":-0.15552520751953128}
],"cm_per_pixel":7.291666666666667},
{"id":120,"src":"https://archive.org/download/texas-barnraising//IMG_0114.JPG","width":3648,"height":2736,"tooltipText":"IMG_0114","image_file_name":"IMG_0114.JPG",
"nodes":[
{"lat":51.499766912405946,"lon":-0.1694297790527344},
{"lat":51.486834743880806,"lon":-0.11655807495117189},
{"lat":51.47838944760882,"lon":-0.12376785278320314},
{"lat":51.47838944760882,"lon":-0.1694297790527344}
],"cm_per_pixel":9.071145752269986}]}

The JSON looks OK, but I think somehow it's not correctly reading the coordinates?

segun-codes commented 1 year ago

Somehow, every effort to reproduce the error failed as the browser console didn't return any error message. Everything works each time I followed the flow you suggested. For instance, see illustration 8 below.

ILLUSTRATION 8

However, I observed a different issue which I will present separately in the following exchanges.

segun-codes commented 1 year ago

One more thing, I noticed reconstructed images are not placed exactly on same coordinates as they were before they were saved into JSON file. Yet I confirmed, it's exactly same sets of coordinates in the right order that were passed into the distortableImageOverlay constructor during image reconstruction. I'm still trying to process what could be wrong but it seems to be something related to the library itself, wondering if you have any ideas.

ILLUSTRATION 9 - Initial state of map. Map was saved in this state test2-map-initialState

ILLUSTRATION 10 - The said saved map was reconstructed into this. Notice the difference between Illustration 9 & 10 test2-map-reconstructedState(cornersTurnedOn imageNotMovedAround)

ILLUSTRATION 11 To confirm the same sets of coordinates (and in correct order) are used to instantiate distortableImageOverlay objects during map reconstruction (either by DnD or URL), I printed the corners used during the map reconstruction as shown in Section B, Section C has the coordinates generated when the maps were initially saved to json file from MK lite. comparison

jywarren commented 1 year ago

Wow, that is strange. Can you try manually creating the image from the JS console, using:

var image = L.distortableImageOverlay(
      imageURL,
      {
        corners: ARRAY
      }
    ).addTo(map);

Maybe we can check if the locations recorded by the JSON exporter are themselves correct, apart from the reconstruction?

jywarren commented 1 year ago

You could also try to access the overlay image and run console.log(overlay.getCorners())

jywarren commented 1 year ago

getCorners() for both before exporting, and after importing, to check the corner locations at various stages?

jywarren commented 1 year ago

@7malikk how does this process work for you, can you follow the steps and confirm what we're seeing? Thank you!

jywarren commented 1 year ago

I just realized also - the cached JSON files from legacy MapKnitter use the plain [...] array format 😭

https://web.archive.org/web/20220726175911/https://mapknitter.org/maps/ceres--2/warpables.json

This doesn't have to be a problem for us, certainly not now. The format we've chosen is definitely better. I'll make an issue to have a feature which can detect if the JSON is just an array, and accept that format too, for backwards compatibility. We can work on it later!

https://github.com/publiclab/Leaflet.DistortableImage/issues/1350

7malikk commented 1 year ago

@7malikk how does this process work for you, can you follow the steps and confirm what we're seeing? Thank you!

Yea, using the previous setup for load images from JSON, the images are not placed on the saved coordinates. @jywarren

7malikk commented 1 year ago

I just realized also - the cached JSON files from legacy MapKnitter use the plain [...] array format 😭

https://web.archive.org/web/20220726175911/https://mapknitter.org/maps/ceres--2/warpables.json

This doesn't have to be a problem for us, certainly not now. The format we've chosen is definitely better. I'll make an issue to have a feature which can detect if the JSON is just an array, and accept that format too, for backwards compatibility. We can work on it later!

Detect & accept array-only JSON files from legacy MapKnitter #1350

Sounds good

segun-codes commented 1 year ago

I just realized also - the cached JSON files from legacy MapKnitter use the plain [...] array format 😭

https://web.archive.org/web/20220726175911/https://mapknitter.org/maps/ceres--2/warpables.json

This doesn't have to be a problem for us, certainly not now. The format we've chosen is definitely better. I'll make an issue to have a feature which can detect if the JSON is just an array, and accept that format too, for backwards compatibility. We can work on it later!

Detect & accept array-only JSON files from legacy MapKnitter #1350

Oh! interesting, okay. I can work on this adapter in another PR once work on this PR is completed.

segun-codes commented 1 year ago

getCorners() for both before exporting, and after importing, to check the corner locations at various stages?

Sure, I will give this and the previous two suggestions a shot. Many thanks!

segun-codes commented 1 year ago

Here's the error I got:

Uncaught TypeError: Cannot read properties of undefined (reading '0')
    at i.getCorner (DistortableImageOverlay.js:519:25)
    at i._calculateProjectiveTransform (DistortableImageOverlay.js:535:43)
    at i._animateZoom (DistortableImageOverlay.js:501:34)
    at i.fire (Events.js:203:11)
    at i._animateZoom (Map.js:1699:8)
    at i.<anonymous> (Map.js:1677:9)

And here's the .json:

{"avg_cm_per_pixel":6.763221163059949,
"collection":[{"id":134,"src":"https://archive.org/download/texas-barnraising//IMG_0137.JPG","width":3648,"height":2736,"tooltipText":"IMG_0137","image_file_name":"IMG_0137.JPG",
"nodes":[
{"lat":51.49046894696402,"lon":-0.06231307983398438},
{"lat":51.48565891027461,"lon":-0.03896713256835938},
{"lat":51.46427482966439,"lon":-0.03896713256835938},
{"lat":51.46427482966439,"lon":-0.08462905883789064}
],"cm_per_pixel":3.926851070243194},
{"id":132,"src":"https://archive.org/download/texas-barnraising//IMG_0128.JPG","width":3648,"height":2736,"tooltipText":"IMG_0128","image_file_name":"IMG_0128.JPG",
"nodes":[
{"lat":51.53629915545823,"lon":-0.15552520751953128},
{"lat":51.53629915545823,"lon":-0.10986328125000001},
{"lat":51.5027589576403,"lon":-0.12445449829101564},
{"lat":51.51493882813492,"lon":-0.15552520751953128}
],"cm_per_pixel":7.291666666666667},
{"id":120,"src":"https://archive.org/download/texas-barnraising//IMG_0114.JPG","width":3648,"height":2736,"tooltipText":"IMG_0114","image_file_name":"IMG_0114.JPG",
"nodes":[
{"lat":51.499766912405946,"lon":-0.1694297790527344},
{"lat":51.486834743880806,"lon":-0.11655807495117189},
{"lat":51.47838944760882,"lon":-0.12376785278320314},
{"lat":51.47838944760882,"lon":-0.1694297790527344}
],"cm_per_pixel":9.071145752269986}]}

The JSON looks OK, but I think somehow it's not correctly reading the coordinates?

Hi @jywarren, based on Illustration 8 above, you can see everything works fine on my end following the flow you suggested. Do you want to re-attempt the process again and see if you still get that same error. If no error, I'd suggest you consider this PR for merging while I open another issue for the repositioning of reconstructed images in right coordinates. What do you say?

segun-codes commented 1 year ago

Wow, that is strange. Can you try manually creating the image from the JS console, using:
var image = L.distortableImageOverlay(
      imageURL,
      {
        corners: ARRAY
      }
    ).addTo(map);
Maybe we can check if the locations recorded by the JSON exporter are themselves correct, apart from the reconstruction?

I got the error below while trying to use JS console. So for this reason, I haven't been able to run the tests exactly the way you advised (as above and in the two suggestions you gave). However, I replicated a similar test from within the MK lite code but didn't find any inconsistency in the coordinates.

I'm thinking we spin-off the issue of proper repositioning of reconstructed images into a stand-alone issue and tackle it therefrom. What do you think?

ILLUSTRATION 12 error

jywarren commented 1 year ago

OK! Locations look good to me. I saw this error:

but images were properly placed after interacting with the map and panning to where the images should be.

One challenge was getting back to the same place, so i added this after images are reconstructed:

map.fitBounds(map.imgGroup.getBounds())

I noticed you were setting image height, and followed where you traced the code to see that was used. But then I realized, the code relies on the image being loaded to measure the height and width, and if we're not at the correct location, the images are off-screen, and they never get loaded (i think Leaflet is smart like that, only loading them if they're onscreen). So that's why we didn't get the width and height, and that's why the error happens!

To test this idea, I tried manually panning to the correct location before dragging the JSON file onto the map. And indeed, they loaded correctly with no error then!

OK, so that means, we have to pan to the correct location before we can load the images. We actually need to parse through the image corners and collect an array of all the lat/lon corners, and create a LatLngBounds object:

https://leafletjs.com/reference.html#latlngbounds

Then we should be able to run:

map.fitBounds(cornerBounds)

BEFORE we place the images, and then they should generate properly.

OK, I left a few comments and changes in the code to illustrate this, and where to put the bounds collecting code!

segun-codes commented 1 year ago

OK! Locations look good to me. I saw this error:

but images were properly placed after interacting with the map and panning to where the images should be.

One challenge was getting back to the same place, so i added this after images are reconstructed:

map.fitBounds(map.imgGroup.getBounds())

I noticed you were setting image height, and followed where you traced the code to see that was used. But then I realized, the code relies on the image being loaded to measure the height and width, and if we're not at the correct location, the images are off-screen, and they never get loaded (i think Leaflet is smart like that, only loading them if they're onscreen). So that's why we didn't get the width and height, and that's why the error happens!

To test this idea, I tried manually panning to the correct location before dragging the JSON file onto the map. And indeed, they loaded correctly with no error then!

OK, so that means, we have to pan to the correct location before we can load the images. We actually need to parse through the image corners and collect an array of all the lat/lon corners, and create a LatLngBounds object:

https://leafletjs.com/reference.html#latlngbounds

Then we should be able to run:

map.fitBounds(cornerBounds)

BEFORE we place the images, and then they should generate properly.

OK, I left a few comments and changes in the code to illustrate this, and where to put the bounds collecting code!

Okay, this is a very helpful finding. I applied it and it worked, thank you!!! By the way, I noticed that specifying the height is not a problem but well, I left it out as per your advice.

jywarren commented 1 year ago

Super. Good to merge now? Great job!

segun-codes commented 1 year ago

Reconstructed images (by Dnd or JSON URL) are placed on the right coordinates now, thanks to map.fitBounds(). I tested and everything still works including this precision-related functionality. Thank you for your support. Kindly review and perhaps consider for merging.

Tests were performed using the MK lite-generated JSON files (mkl-01.json & mkl-02.json) and JSON URLs below.

jywarren commented 1 year ago

Excellent! Merging!!

7malikk commented 1 year ago

Great work @segun-codes

publiclab / Leaflet.DistortableImage

Drag and Drop JSON File to Tile Layer to Reconstruct Saved Map #1345