publiclab / Leaflet.DistortableImage

A Leaflet extension to distort or "rubber sheet" images
https://publiclab.github.io/Leaflet.DistortableImage/examples/
BSD 2-Clause "Simplified" License
271 stars 283 forks source link

Add automated image matching with matcher-core #547

Open rexagod opened 5 years ago

rexagod commented 5 years ago

https://github.com/publiclab/matcher-core/

Discussion thread for matcher.js's integration issues into Leaflet.DistortableImage.

/cc @jywarren

jywarren commented 5 years ago

Exciting!!!!!!

Testing at https://rexagod.github.io/Leaflet.DistortableImage/examples/ --

when i press the puzzle piece button:

image

I get:

leaflet.distortableimage.js:1285 err: check if matcher is initialized properly and correct parameters are supplied 
 ReferenceError: processedPoints is not defined
    at e.addHooks (leaflet.distortableimage.js:1283)
    at enable (leaflet.toolbar.js:1)
    at enable (leaflet.toolbar.js:1)
    at HTMLAnchorElement.r (leaflet.js:5)
rexagod commented 5 years ago

Hey @jywarren! Just to confirm, did you enable the matcher first (from the bottom left button)?

jywarren commented 5 years ago

Aha! I didn't see this down there! OK:

image

jywarren commented 5 years ago

So, this is all running locally in my browser tab?

jywarren commented 5 years ago

Hm, it seems to hang, with green screen and Uncaught (in promise) utils async error!

rexagod commented 5 years ago

Okay, did you clone it or are you running it from the gh-pages link?

rexagod commented 5 years ago

This one https://rexagod.github.io/Leaflet.DistortableImage/examples/

rexagod commented 5 years ago

I just double checked this and the demo link seems to work just fine. Should I record the sequence of steps?

jywarren commented 5 years ago

OK awesome!!

image

Can you show a pull request for this code in LDI? I'd love to learn more about how it's organized! Thanks!

jywarren commented 5 years ago

Also, what about using some of these photos, for a more real-world scenario? I wonder if the flower petals are giving it some trouble?

https://photos.app.goo.gl/SMWuhiieCCqc9W246

jywarren commented 5 years ago

This is very cool, @rexagod !!!

jywarren commented 5 years ago

Let's get a good look at the architecture of the code first, but I'm interested in exploring some different UI ideas to see what is most natural for people. Maybe we could open a new issue to discuss UI ideas like:

  1. potentially updating the lines connecting dots in realtime as you drag, even?
  2. running matches against any image your image is closest to instead of having to click 2?
  3. thinking about where to put the button to "enable" matching
  4. maybe showing a spinner icon on the image itself as points are identified, instead of over the whole map?
  5. What's actually happening while the spinner is displayed?

What takes the most time, generating the points?

Thanks! This is so cool!

rexagod commented 5 years ago

Thanks, @jywarren! Briefly responding to the concerns above,

potentially updating the lines connecting dots in realtime as you drag, even?

This would actually require the matcher to update the coordinates on every mouseover, mousemove or drag event, all of which require re-reruns of the same as soon as the cursor moves one pixel on the map, which could lead to performance loss on older systems. I test ran this on my 2012 i3 edition and the lag was really noticeable. Maybe we can brainstorm an alternative that satisfies performance on all sorts of rigs?

test

running matches against any image your image is closest to instead of having to click 2?

Okay, this should require a complete refactor of the algorithm from clicks to proximity, maybe we can brainstorm the pros and cons before proceeding with such a big code refactor? What do you think?

thinking about where to put the button to "enable" matching

Hmm, there are a number of approaches we can take here, I guess. Maybe we can assign it a keybinding, and log that inside the keymapper, or perhaps a separate UI button (similar to current one, but more dynamic)? I'll open up an issue on this.

maybe showing a spinner icon on the image itself as points are identified, instead of over the whole map? What's actually happening while the spinner is displayed? What takes the most time, generating the points?

Yeah, so actually, the loading, or the green spinner splash screen basically is there to compensate for the time taken for the constructor to instantiate the matcher class (which is the main reason for overhead), after which, the actual "matching" is a matter of microseconds, no matter which images you pass to it, since the reference has now been loaded into the cache, which as of now requires the browser to reload in order to access that (ad actually "match" and "stitch" images).

So essentially, we can definitely shift to image spinner from screen spinner given that we're good with the browser reloading once the algorithm has been loaded.

I'm really excited to see your interest into this, and the future implementations of this module, and just want to brainstorm a bit and be sure about our plans regarding the same before implementing them.

Thank you!

jywarren commented 5 years ago

This would actually require the matcher to update the coordinates on every mouseover, mousemove or drag event, all of which require re-reruns of the same as soon as the cursor moves one pixel on the map, which could lead to performance loss on older systems. I test ran this on my 2012 i3 edition and the lag was really noticeable. Maybe we can brainstorm an alternative that satisfies performance on all sorts of rigs?

Can we break down the steps the matcher takes, and can we document them in the README? This will really help!

I was thinking that we could just redraw the connecting lines in realtime, not recalculate the coordinates. Just redrawing Leaflet lines shouldn't be too expensive!

Okay, this should require a complete refactor of the algorithm from clicks to proximity, maybe we can brainstorm the pros and cons before proceeding with such a big code refactor? What do you think?

Hm, shouldn't the event that triggers matching be independent from the algorithm that is triggered? Again, i think documenting how the code is modularized should help us make good decisions here. For example, documenting the lifecycle of the whole interaction would expand on what you've written here and link to specific lines of code:

Yeah, so actually, the loading, or the green spinner splash screen basically is there to compensate for the time taken for the constructor to instantiate the matcher class (which is the main reason for overhead), after which, the actual "matching" is a matter of microseconds, no matter which images you pass to it, since the reference has now been loaded into the cache, which as of now requires the browser to reload in order to access that (ad actually "match" and "stitch" images).

Does that make sense? Thanks!

rexagod commented 5 years ago

Can we break down the steps the matcher takes, and can we document them in the README? This will really help!

Added docs in https://github.com/publiclab/matcher-core/commit/a1ec607fdcddf49df406811ccbffdcca3c9ce844

Hm, shouldn't the event that triggers matching be independent from the algorithm that is triggered?

Sorry, I actually meant the projector and stitcher functions. But let me know if you consider shifting to proximity-based measures, and I'll make it work!

Thanks!

rexagod commented 5 years ago

Also, here's the PR comparison: https://github.com/publiclab/Leaflet.DistortableImage/compare/main...rexagod:rexa-soc-ldi

jywarren commented 5 years ago

Can we break down the steps the matcher takes, and can we document them in the README? This will really help!

Added docs in a1ec607

Actually i was hoping you could break down the orbify command more, because it seems it has sub-components for a) point finding, b) point matching -- this gets at the distinction in publiclab/matcher-core#4 -- and is a really useful thing to separate out if we can!

jywarren commented 5 years ago

Sorry, I actually meant the projector and stitcher functions. But let me know if you consider shifting to proximity-based measures, and I'll make it work!

Shall we move over to that repository to discuss? I've turned it into a PR so there's a place to talk -- https://github.com/publiclab/Leaflet.DistortableImage/pull/312 -- great!

rexagod commented 5 years ago

Moved over to https://github.com/publiclab/Leaflet.DistortableImage/pull/312

jywarren commented 3 years ago

Noting that this got stuck when we needed ES6 and I believe that is now resolved. Thanks all!

cesswairimu commented 1 year ago

unpinning this to pin the welcome issue, will back pin after