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

Add automated image matching with matcher-core #547

Open
rexagod opened this issue Jun 14, 2019 · 21 comments
Open

Add automated image matching with matcher-core #547

rexagod opened this issue Jun 14, 2019 · 21 comments

Comments

@rexagod
Copy link
Member

rexagod commented Jun 14, 2019

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

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

/cc @jywarren

@jywarren
Copy link
Member

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
Copy link
Member Author

rexagod commented Jun 14, 2019

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

@jywarren
Copy link
Member

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

image

@jywarren
Copy link
Member

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

@jywarren
Copy link
Member

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

@rexagod
Copy link
Member Author

rexagod commented Jun 14, 2019

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

@rexagod
Copy link
Member Author

rexagod commented Jun 14, 2019

@rexagod
Copy link
Member Author

rexagod commented Jun 14, 2019

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

@jywarren
Copy link
Member

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
Copy link
Member

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
Copy link
Member

This is very cool, @rexagod !!!

@jywarren
Copy link
Member

jywarren commented Jun 14, 2019

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
Copy link
Member Author

rexagod commented Jun 17, 2019

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
Copy link
Member

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
Copy link
Member Author

rexagod commented Jun 22, 2019

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

Added docs in publiclab/matcher-core@a1ec607

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
Copy link
Member Author

rexagod commented Jun 22, 2019

Also, here's the PR comparison: main...rexagod:rexa-soc-ldi

@jywarren
Copy link
Member

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
Copy link
Member

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 -- #312 -- great!

@rexagod rexagod closed this as completed Jun 27, 2019
@rexagod
Copy link
Member Author

rexagod commented Jun 27, 2019

Moved over to #312

@jywarren jywarren transferred this issue from publiclab/matcher-core Feb 8, 2020
@jywarren jywarren changed the title Enable matcher.js for Leaflet.DistortableImage Add automated image matching with matcher-core Feb 8, 2020
@jywarren jywarren reopened this Feb 8, 2020
@jywarren
Copy link
Member

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

@jywarren jywarren pinned this issue Nov 17, 2020
@cesswairimu
Copy link
Collaborator

cesswairimu commented Oct 12, 2022

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

@cesswairimu cesswairimu unpinned this issue Oct 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants