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

Imagery Offset Database #4166

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
89 changes: 88 additions & 1 deletion modules/services/imagery_offset.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,95 @@ export default {
inflight = {};
offsetCache = rbush();
},
getImageryID: function(url, imageryType) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@Zverik could I get your review for this piece of code, I tried to convert the Java code you wrote in Javascript.

If you could also point me to some unit test cases for this function in your code base ..

Copy link
Contributor

Choose a reason for hiding this comment

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

JavaScript probably contains more (or better) files for handling URLs. I'm wondering if it makes sense to write this differently, particularly when parsing the parameters part of the URL

if (url == null) return;
console.log('url=', url)
debugger;
url = url.toLowerCase();
if (imageryType === 'bing' || url.indexOf('tiles.virtualearth.net') > -1)
return 'bing';
// if (url.indexOf("scanex_irs") > )
if (
imageryType === 'TMS' &&
url.match(/.+tiles\.mapbox\.com\/v[3-9]\/openstreetmap\.map.*/)
Copy link
Contributor

Choose a reason for hiding this comment

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

A sed pattern (s/foo/bar/) can use any character to delimit, and it's normal to use something other than / when dealing with URLs, e.g. s@foo@bar@. This avoids excessive escaping. Is this possible with Javascript's regex implementation?

)
return 'mapbox';

search: function(location, callback) {
// does iD support WMS? if yes, how to detect
var isWMS = false;

// Remove protocol
var i = url.indexOf('://');
url = url.substring(i + 3);

var query = '';
// Split URL into address and query string
var questionMarkIndex = url.indexOf('?');
if (questionMarkIndex > 0) {
query = url.slice(questionMarkIndex);
url = url.slice(0, questionMarkIndex);
}

var removeWMSParams = [
Copy link
Contributor

Choose a reason for hiding this comment

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

Not needed for iD

'srs',
'width',
'height',
'bbox',
'service',
'request',
'version',
'format',
'styles',
'transparent'
];

var qparams = {};
var qparamsStr = query.length > 1 ? query.slice(1).split('&') : '';

qparamsStr.forEach(function(param) {
var kv = param.split('=');
console.log(kv);
kv[0] = kv[0].toLowerCase();
// WMS: if this is WMS, remove all parameters except map and layers
Copy link
Contributor

Choose a reason for hiding this comment

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

Not needed for iD

if (isWMS && removeWMSParams.indexOf(kv[0]) > -1) {
return;
}
// TMS: skip parameters with variable values and Mapbox's access token
if (
(kv.length > 1 &&
kv[1].indexOf('{') >= 0 &&
kv[1].indexOf('}') > 0) ||
kv[0] === 'access_token'
) {
return;
}
console.log('here')
Copy link
Contributor

Choose a reason for hiding this comment

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

Left over debugging statement?

qparams[kv[0].toLowerCase()] = kv.length > 1 ? kv[1] : null;
});

var sb = '';
Object.keys(qparams).forEach(function(qk) {
if (sb.length > 0) sb += '&';
else if (query.length > 0) sb += '?';
sb += qk + '=' + qparams[qk];
});

// TMS: remove /{zoom} and /{y}.png parts
url = url.replace(/\/\{[^}]+\}(?:\.\w+)?/g, '');

// TMS: remove variable parts
url = url.replace(/\{[^}]+\}/g, '');

while (url.indexOf('..') > -1) {
url = url.replace('..', '.');
}

if (url.startsWith('.')) url = url.substring(1);

return url + query;
},
search: function(location, url, callback) {
console.log(this.getImageryID(url))
var cached = offsetCache.search({
minX: location[0],
minY: location[1],
Expand Down
2 changes: 1 addition & 1 deletion modules/ui/background.js
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ export function uiBackground(context) {
function update() {
backgroundList.call(drawList, 'radio', clickSetSource, function(d) { return !d.overlay; });
overlayList.call(drawList, 'checkbox', clickSetOverlay, function(d) { return d.overlay; });

Copy link
Contributor

Choose a reason for hiding this comment

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

Stray whitespace

selectLayer();
updateOffsetVal();
}
Expand Down
39 changes: 37 additions & 2 deletions modules/ui/panels/background.js
Original file line number Diff line number Diff line change
@@ -1,14 +1,17 @@
import * as d3 from 'd3';
import _ from 'lodash';
import {services} from '../../services'
import { t } from '../../util/locale';

var searchOffset = services.imageryOffset;
searchOffset.init();

export function uiPanelBackground(context) {
var background = context.background();
var currSource = null;
var currZoom = '';
var currVintage = '';

debugger;
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems out of place


function redraw(selection) {
if (currSource !== background.baseLayerSource().name()) {
Expand Down Expand Up @@ -44,7 +47,7 @@ export function uiPanelBackground(context) {
if (!currVintage) {
debouncedGetVintage(selection);
}

debouncedGetOffset(selection);
var toggle = context.getDebug('tile') ? 'hide_tiles' : 'show_tiles';

selection
Expand All @@ -61,6 +64,8 @@ export function uiPanelBackground(context) {


var debouncedGetVintage = _.debounce(getVintage, 250);
var debouncedGetOffset = _.debounce(getOffset, 250);

function getVintage(selection) {
var tile = d3.select('.layer-background img.tile-center'); // tile near viewport center
if (tile.empty()) return;
Expand All @@ -81,6 +86,35 @@ export function uiPanelBackground(context) {
});
}

function getOffset(selection) {
var tile = d3.select('.layer-background img.tile-center'); // tile near viewport center
if (tile.empty()) return;

var d = tile.datum();
var zoom = d[2];
var url = d[3];
var center = context.map().center();
console.log(d);
searchOffset.search(center,url, console.log);
// zoom = (d && d.length >= 3 && d[2]) || Math.floor(context
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this commented code be removed?

// .map()
// .zoom()),
// center = context.map().center();

// currZoom = String(zoom);
// selection.selectAll('.zoom').text(currZoom);

// if (!d || !d.length >= 3) return;
// background
// .baseLayerSource()
// .getVintage(center, d, function(err, result) {
// currVintage =
// (result && result.range) ||
// t('info_panels.background.unknown');
// selection.selectAll('.vintage').text(currVintage);
// });
}


var panel = function(selection) {
selection.call(redraw);
Expand All @@ -91,6 +125,7 @@ export function uiPanelBackground(context) {
})
.on('move.info-background', function() {
selection.call(debouncedGetVintage);
selection.call(debouncedGetOffset);
});

};
Expand Down