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

Mapillary js #3128

Merged
merged 14 commits into from Jun 1, 2016
Merged

Mapillary js #3128

merged 14 commits into from Jun 1, 2016

Conversation

kepta
Copy link
Collaborator

@kepta kepta commented May 23, 2016

Continuing from #3014 , thanks to @peterneubauer.

Things remaining

@kepta kepta added the wip Work in progress label May 23, 2016
@kepta kepta force-pushed the mapillary_js branch 2 times, most recently from 16a5ca3 to 7384480 Compare May 23, 2016 14:18
@kepta
Copy link
Collaborator Author

kepta commented May 24, 2016

Build Process Issues:

  1. The mapillary-js.min.js file is 1.1MB in size. To give some perspective, the entire iD.js file is roughly 2MB in size. If I try to add mapillary-js.min.js to js/lib folder the make process takes forever to complete. Currently, I am directly copying the mapillary-js.min.js to dist folder in order to avoid concatenating it with iD.js
  2. The svg dependencies of mapillaryJS require them to be hosted on the same level as that of the Mapillary css file. So I am directly copying these files to dist folder as of now :(.
  3. Resource not found when using mapillaryJS mapillary/mapillary-js#133

@bhousel @peterneubauer your views on this?

@peterneubauer
Copy link
Contributor

Hi there,
I upgraded to MapillaryJS 1.3 in a2f3063 which makes the local artifacts unnecessary, also the SVGs can be removed.

Regarding the API call- the image you are looking for is from today, we have some lag in the system. Try anything older than a day, e.g. https://a.mapillary.com/v2/nav/im/1gE6B94DP0iDmUHjtRFKcQ?client_id=NzNRM2otQkR2SHJzaXJmNmdQWVQ0dzo1ZWYyMmYwNjdmNDdlNmVi

/peter

if (!imageKey) return;

// console.log('hello world');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

commented out debug comment left in here

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh thanks for catching that pnorman 👍

@peterneubauer
Copy link
Contributor

I see that the in line mapillary icons and svg is still in here. Is that mandatory or could we just refer to the js and css in index.html for easier upgrade?

@kepta
Copy link
Collaborator Author

kepta commented May 25, 2016

@peterneubauer we generally keep js/css libraries in the code base itself.

@bhousel when asynchronously loading css/js do we want to use their CDN or host a copy within iD?

@bhousel
Copy link
Member

bhousel commented May 25, 2016

@bhousel when asynchronously loading css/js do we want to use their CDN or host a copy within iD?

Probably better to send this stuff to the CDN.. That helps the OSM folks a bit.

@kepta
Copy link
Collaborator Author

kepta commented May 27, 2016

The implementation is almost complete.
Here is a gif to show how it looks like currently

id_demo_mapillary

…nsitions

Old behavior would redraw only after the map stops moving for 300ms
New behavior will force redraw within 750ms, regardless of map stability
@bhousel
Copy link
Member

bhousel commented May 27, 2016

The gif doesn't do it justice, but centerEase is now 👌
Before it was just firing off 10 little map moves.. Now we smoothly interpolate to the new location, adjusting the projection each animation frame and queueing redraw for later.

centerease

@bhousel
Copy link
Member

bhousel commented Jun 1, 2016

This is working alright!
Thanks @kepta and @peterneubauer for your work on this...
Merging 👍

mapillary-viewer

@bhousel bhousel merged commit 30dfdeb into master Jun 1, 2016
@bhousel bhousel deleted the mapillary_js branch June 1, 2016 04:29
@bhousel bhousel removed the wip Work in progress label Jun 1, 2016
@peterneubauer
Copy link
Contributor

Super work @kepta ! Also, is this working even for street signs correctly?

@peterneubauer
Copy link
Contributor

I guess we should soon implement submission of changes to the locations/angles to Mapillary :)

@bhousel
Copy link
Member

bhousel commented Jun 1, 2016

Also, is this working even for street signs correctly?

Yes, it's working for the signs layer also.. The one change we made is that now the photo layer must be activated before the signs layer can be activated.

I guess we should soon implement submission of changes to the locations/angles to Mapillary :)

Sure! If there are some more API calls that would be useful to make, like for tagging or blurring or moving the markers, open a new issue so we can keep track of it..

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

Successfully merging this pull request may close these issues.

None yet

4 participants