Incorporate EXIF functionality (and some refactors) #169

rexagod · 2019-03-14T16:51:18Z

Follow-up on #156. Based off #161. Refer this.

src/edit/tools/getEXIFdata.js

jywarren · 2019-03-17T21:54:32Z

Cool! We can do altitude in follow-up, it's pretty imprecise but we have some prior info on this here and some ideas for refining it too. publiclab/mapknitter#159 publiclab/mapknitter#176 publiclab/mapknitter#67 In one of those I think we have a sample image with altitude data to work from. Thanks, I'll review tomorrow! Much appreciated!!!!

…

On Sun, Mar 17, 2019, 5:42 PM Pranshu Srivastava ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In src/edit/tools/getEXIFdata.js <#169 (comment)> : > @@ -0,0 +1,85 @@ +L.EXIF = function getEXIFdata(img) { move the image to that location on the planet resize it, rotate it using the available data, as in the below code pan the map to the new position so the user can see it @jywarren <https://github.com/jywarren> I've implemented all of the above except the resizing (scaling/distort) one since there is no available data for that in the EXIF tags. What do you think? Thanks! — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#169 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AABfJ4EpA43Bbi5SUUQNbhabKartyfGjks5vXra-gaJpZM4b0u3d> .

jywarren

OK, this looks good to go, once we remove the alert(). We could also add a confirm() instead of alert() so that people are prompted "Geolocate this image and move it to the location found?" with Ok and Cancel options. But we don't have to; we could just move it immediately.

Thanks a lot for this one, it's going to be great!

dist/leaflet.distortableimage.js

test/karma.conf.js

rexagod · 2019-04-25T14:11:07Z

rexagod · 2019-04-30T22:34:23Z

Pinging @jywarren.

rexagod · 2019-05-03T16:39:03Z

Just mentioning here that his PR was referenced on se18023's question that "Can geotagged aerial images be automatically placed in the right location on the map according to their coordinates?"

sashadev-sky

@rexagod @jywarren Reviewing this now!

This is really cool. A few things to address:

For some reason it isn't working with select.html, but I guess the 1st step would be to rebase this and then look into that. If it's complicated to fix I think that would be ok to address in a new PR if preferred.
It is still printing to the console. @jywarren @rexagod did we still want to add a returned value instead? I think that would be good to fix here, and removing all the console.logs because I have noticed those upstreamed in mapknitter already!
Minor separate suggestions below (sorry I know you probably were planning on refactoring this after @jywarren confirmed implementation but just in case)
- potentially move dependency restructuring to new PR?

And last thing: is this still meant to be addressing the refactoring of the src/edit/DistortableImage.EditToolbar.js file? Is the idea now to move all of the toolbar tool action code being executed into separate files in /tools? So for example, all the toggle___ methods?

src/edit/tools/L.tools.EXIF.js

package.json

Co-Authored-By: Sasha Boginsky <[email protected]>

refer: publiclab#169 (review)

rexagod · 2019-07-12T22:00:38Z

@sashadev-sky I'm seeing asynchronous bugs for the getEXIFdata method after updating this branch today. This seems strange since the code related to that is untouched (check commits above), which leads me to believe that there might have been some changes to the package (exif-js) while I was away? I even tried wrapping this in a Promise but no luck. What do you think?

PS. To summarize, this branch has been rebased and @sashadev-sky's suggestions have been implemented, and everything looks good except the EXIF bug, which is at the core of this PR. 😕

rexagod · 2019-07-12T22:40:08Z

Update: Just verified. The package (exif-js) is still at the same version In this PR and the main branch, so that shouldn't be the source of this bug. Can't figure out what exactly is causing this?

jywarren · 2019-07-24T21:18:32Z

And last thing: is this still meant to be addressing the refactoring of the src/edit/DistortableImage.EditToolbar.js file? Is the idea now to move all of the toolbar tool action code being executed into separate files in /tools? So for example, all the toggle___ methods?

It'd be nice but we have too many inter-dependent PRs so let's try to solve things one at a time. That may mean #225 can be merged if we don't need to solve these all at once.

@rexagod can you elaborate on the exact timing issue? If we need to pin the exif-js ref to a specific commit from when it was working before, we can do that for now to unblock this. Like, whatever commit from back in time. But it was last published 2 years ago so seems unlikely? https://www.npmjs.com/package/exif-js

jywarren · 2019-07-24T21:57:37Z

Hey all, i wondered if you're ever needing cross-reviews or troubleshooting maybe it would be good to connect @Rishabh570 with you all here (see recent work here: publiclab/community-toolbox#236). You're all doing complex JS architecture so i wanted to make the connection in case it's helpful, for example with the timing bug here! 🙌

Rishabh570 · 2019-07-25T13:24:05Z

I would love to contribute to this in any way possible for me, seems like there's some unresolved bit here which has put this to halt.

I'm seeing asynchronous bugs for the getEXIFdata method after updating this branch

So getEXIFdata is throwing errors as of now? Do we have the option of going back to the commit where it was working fine (like approx 14 days ago)?

sashadev-sky · 2019-08-28T02:18:08Z

examples/select.html

@@ -13,11 +13,14 @@

  <!-- for full-res export -->
  <script src="../node_modules/jquery/dist/jquery.js"></script>
-  <script defer src="../node_modules/webgl-distort/dist/webgl-distort.js"></script>


@rexagod Do you feel its better not to have the defer? I put it there to speed up initial page load

sashadev-sky · 2019-09-03T02:36:23Z

Gruntfile.js

@@ -160,6 +161,7 @@ module.exports = function(grunt) {
        'concat:dist'
    ]);

+    // disable for now?


@rexagod why did you want this one disabled? is this still the case?

sashadev-sky · 2019-09-03T02:39:19Z

src/edit/tools/DistortableImage.PopupBar.js

-    var image = this._overlay.getElement();
+  addHooks: function () {
+    var overlay = this._overlay;
+    var image = overlay._image;


Suggested change

var image = overlay._image;

var image = overlay.getElement();

sashadev-sky · 2019-09-03T02:44:48Z

src/edit/tools/L.tools.EXIF.js

+L.tools = L.tools || {};
+
+L.tools.EXIF = function getEXIFdata(ref, overlay) {
+  var GPS = ref.exifdata,


we would want to update this part to use 3 vars as per new code standardization rules

sashadev-sky · 2019-09-03T02:46:38Z

src/edit/getEXIFdata.js

-      typeof GPS.GPSLatitude !== "undefined" &&
-      typeof GPS.GPSLongitude !== "undefined"
-    ) {
-      // sadly, encoded in [degrees,minutes,seconds]


you didn't copy over this code comment I just wanted to confirm if its irrelavent now or could still be useful

sashadev-sky · 2019-09-03T02:57:14Z

src/edit/tools/L.tools.EXIF.js

+
+  var panTo = L.latLng(lat, lng);
+
+  var x_diff = overlay.getCorner(0).lng - overlay.getCorner(1).lng; // width


Lets update this and below to deltaX and deltaY to abide to JS camelcase convention?

sashadev-sky · 2019-09-03T04:17:02Z

@rexagod I have a different implementation in mind to make exif work, I opened the PR in #421 . Basically overlay.getElement() typically won't have any EXIF data because its a compressed version of the original element, which has stripped EXIF metadata. So I am writing an implementation where the fullResolutionSrc option will be used as a way to provide a fullRes download and a src with metadata (if it is passed, we will check for metadata from it not the image element currently in the DOM)

So I was thinking we could close this PR and work from #421 instead. What do you think?

rexagod added 4 commits February 25, 2019 20:58

First fn included//Refactoring and second fn remaining

8815eb3

Modularize EXIF

e4421f5

rebase

3e0a46c

move to /tools

57d400c

rexagod changed the title ~~Extract exif~~ Move EXIF to /tools Mar 14, 2019

rexagod added 2 commits March 14, 2019 22:22

Merge branch 'main' into extract-EXIF

9c106ee

build /dist

2fa5bab

jywarren reviewed Mar 15, 2019

View reviewed changes

src/edit/tools/getEXIFdata.js Outdated Show resolved Hide resolved

add EXIF geolocating feature

9ae27d5

jywarren mentioned this pull request Mar 30, 2019

exif.js doesn't read orientation tags properly publiclab/mapknitter#181

Open

rexagod mentioned this pull request Apr 11, 2019

implement DistortableImage.toGeoJSON() #4

Open

jywarren requested changes Apr 19, 2019

View reviewed changes

dist/leaflet.distortableimage.js Outdated Show resolved Hide resolved

test/karma.conf.js Outdated Show resolved Hide resolved

rexagod added 5 commits April 25, 2019 04:10

refactor package.json and include coverage

c1dc6e2

remove race condition and add confirm()

a609be4

introduce tools class, refactor dependancies, and some minor changes

d1bc993

recreate lock file

a96266c

remove jquery duplicate

9de8d75

rexagod requested a review from jywarren April 25, 2019 14:08

rexagod changed the title ~~Move EXIF to /tools~~ Incorporate EXIF functionality (and some refactors) Apr 25, 2019

rexagod added the needs-review label May 2, 2019

rexagod mentioned this pull request May 3, 2019

(WIP)Add ".tiff" as content upload type publiclab/mapknitter#577

Closed

5 tasks

sashadev-sky requested changes May 12, 2019

View reviewed changes

src/edit/tools/L.tools.EXIF.js Outdated Show resolved Hide resolved

src/edit/tools/L.tools.EXIF.js Outdated Show resolved Hide resolved

src/edit/tools/L.tools.EXIF.js Outdated Show resolved Hide resolved

src/edit/tools/L.tools.EXIF.js Outdated Show resolved Hide resolved

sashadev-sky added enhancement in progress and removed needs-review labels May 12, 2019

sashadev-sky reviewed May 12, 2019

View reviewed changes

package.json Show resolved Hide resolved

sashadev-sky mentioned this pull request May 23, 2019

Implement dynamic sub-menus #225

Closed

rexagod and others added 3 commits May 25, 2019 19:07

Update src/edit/tools/L.tools.EXIF.js

1bac383

Co-Authored-By: Sasha Boginsky <[email protected]>

improve on @sashadev-sky's review

f704bac

refer: publiclab#169 (review)

rebase: EXIF bug

6ccd182

sashadev-sky mentioned this pull request Aug 1, 2019

add rotate and scale actions #373

Merged

5 tasks

sashadev-sky added this to the Version 1.0.0 Release milestone Aug 13, 2019

rexagod self-assigned this Aug 15, 2019

This was referenced Aug 28, 2019

Dependencies update #412

Merged

Dependencies update #413

Closed

Refactor dependancy types in package.json #411

Closed

sashadev-sky reviewed Sep 3, 2019

View reviewed changes

sashadev-sky mentioned this pull request Sep 3, 2019

EnableExif #421

Open

5 tasks

jywarren mentioned this pull request Oct 1, 2022

Add GPS/EXIF metadata detection for rough auto-placement using lat/lon, compass, elevation #1024

Open

10 tasks

rexagod closed this Jan 22, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Incorporate EXIF functionality (and some refactors) #169

Incorporate EXIF functionality (and some refactors) #169

rexagod commented Mar 14, 2019 •

edited

Loading

jywarren commented Mar 17, 2019 via email

jywarren left a comment

rexagod commented Apr 25, 2019

rexagod commented Apr 30, 2019

rexagod commented May 3, 2019 •

edited

Loading

sashadev-sky left a comment •

edited

Loading

rexagod commented Jul 12, 2019 •

edited

Loading

rexagod commented Jul 12, 2019

jywarren commented Jul 24, 2019

jywarren commented Jul 24, 2019

Rishabh570 commented Jul 25, 2019

sashadev-sky Aug 28, 2019

sashadev-sky Sep 3, 2019

sashadev-sky Sep 3, 2019

sashadev-sky Sep 3, 2019

sashadev-sky Sep 3, 2019

sashadev-sky Sep 3, 2019

sashadev-sky commented Sep 3, 2019

	var image = overlay._image;
	var image = overlay.getElement();


		var panTo = L.latLng(lat, lng);

		var x_diff = overlay.getCorner(0).lng - overlay.getCorner(1).lng; // width

Incorporate EXIF functionality (and some refactors) #169

Incorporate EXIF functionality (and some refactors) #169

Conversation

rexagod commented Mar 14, 2019 • edited Loading

jywarren commented Mar 17, 2019 via email

jywarren left a comment

Choose a reason for hiding this comment

rexagod commented Apr 25, 2019

rexagod commented Apr 30, 2019

rexagod commented May 3, 2019 • edited Loading

sashadev-sky left a comment • edited Loading

Choose a reason for hiding this comment

rexagod commented Jul 12, 2019 • edited Loading

rexagod commented Jul 12, 2019

jywarren commented Jul 24, 2019

jywarren commented Jul 24, 2019

Rishabh570 commented Jul 25, 2019

sashadev-sky Aug 28, 2019

Choose a reason for hiding this comment

sashadev-sky Sep 3, 2019

Choose a reason for hiding this comment

sashadev-sky Sep 3, 2019

Choose a reason for hiding this comment

sashadev-sky Sep 3, 2019

Choose a reason for hiding this comment

sashadev-sky Sep 3, 2019

Choose a reason for hiding this comment

sashadev-sky Sep 3, 2019

Choose a reason for hiding this comment

sashadev-sky commented Sep 3, 2019

rexagod commented Mar 14, 2019 •

edited

Loading

rexagod commented May 3, 2019 •

edited

Loading

sashadev-sky left a comment •

edited

Loading

rexagod commented Jul 12, 2019 •

edited

Loading