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.

Sign up for GitHub

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

If a line is invisible, don't find it in a search. #1041

Merged
merged 1 commit into from
Oct 28, 2019
Merged

If a line is invisible, don't find it in a search. #1041

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 3 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@

## Unreleased

### Changes
- Line segments with zero width or zero opacity won't be found by pointSearch or polygonSearch (#1041)

### Bug Fixes
- Removed extra calls to sceneObject constructors (#1039)
- Fixed an issue with rendering on hidden tabs in Chrome (#1042)
Expand Down
6 changes: 3 additions & 3 deletions src/annotation.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,10 +167,10 @@ var annotation = function (type, args) {
/**
* Get or set the label of this annotation.
*
* @param {string|null|undefined} arg If `undefined`, return the label,
* @param {string|null|undefined} [arg] If `undefined`, return the label,
* otherwise change it. `null` to clear the label.
* @param {boolean} noFallback If not truthy and the label is `null`, return
* the name, otherwise return the actual value for label.
* @param {boolean} [noFallback] If not truthy and the label is `null`,
* return the name, otherwise return the actual value for label.
* @returns {this|string} The current label or this annotation.
*/
this.label = function (arg, noFallback) {
Expand Down
45 changes: 33 additions & 12 deletions src/lineFeature.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ var lineFeature = function (arg) {

data.forEach(function (d, index) {
var closed = closedFunc(d, index),
last, lastr, lastr2, first, record = [], min, max;
last, lasti, lastr, lastr2, first, record = [], min, max;

line(d, index).forEach(function (current, j) {
var p = posFunc(current, j, d, index);
Expand All @@ -180,17 +180,18 @@ var lineFeature = function (arg) {
if (max.r === undefined || r > max.r) { max.r = r; }
var r2 = r * r;
if (last) {
record.push({u: p, v: last, r: lastr > r ? lastr : r, r2: lastr2 > r2 ? lastr2 : r2});
record.push({u: p, v: last, r: lastr > r ? lastr : r, r2: lastr2 > r2 ? lastr2 : r2, i: j, j: lasti});
}
last = p;
lasti = j;
lastr = r;
lastr2 = r2;
if (!first && closed) {
first = {p: p, r: r, r2: r2};
first = {p: p, r: r, r2: r2, i: j};
}
});
if (closed && first && (last.x !== first.p.x || last.y !== first.p.y)) {
record.push({u: last, v: first.p, r: lastr > first.r ? lastr : first.r, r2: lastr2 > first.r2 ? lastr2 : first.r2});
record.push({u: last, v: first.p, r: lastr > first.r ? lastr : first.r, r2: lastr2 > first.r2 ? lastr2 : first.r2, i: lasti, j: first.i});
}
record.min = min;
record.max = max;
Expand All @@ -211,7 +212,7 @@ var lineFeature = function (arg) {
*
* @param {geo.geoPosition} p point to search for in map interface gcs.
* @returns {object} An object with `index`: a list of line indices, `found`:
* a list of quads that contain the specified coordinate, and `extra`: an
* a list of lines that contain the specified coordinate, and `extra`: an
* object with keys that are line indices and values that are the first
* segement index for which the line was matched.
*/
Expand All @@ -229,16 +230,26 @@ var lineFeature = function (arg) {
scale = map.unitsPerPixel(map.zoom()),
scale2 = scale * scale,
pt = transform.transformCoordinates(map.ingcs(), map.gcs(), p),
i, j, record;
strokeWidthFunc = m_this.style.get('strokeWidth'),
strokeOpacityFunc = m_this.style.get('strokeOpacity'),
lineFunc = m_this.line(),
line, i, j, record;

for (i = 0; i < m_pointSearchInfo.length; i += 1) {
record = m_pointSearchInfo[i];
line = null;
for (j = 0; j < record.length; j += 1) {
if (util.distance2dToLineSquared(pt, record[j].u, record[j].v) <= record[j].r2 * scale2) {
found.push(data[i]);
indices.push(i);
extra[i] = j;
break;
if (!line) {
line = lineFunc(data[i], i);
}
if ((strokeOpacityFunc(line[record[j].i], record[j].i, data[i], i) > 0 || strokeOpacityFunc(line[record[j].j], record[j].j, data[i], i) > 0) &&
(strokeWidthFunc(line[record[j].i], record[j].i, data[i], i) > 0 || strokeWidthFunc(line[record[j].j], record[j].j, data[i], i) > 0)) {
found.push(data[i]);
indices.push(i);
extra[i] = j;
break;
}
}
}
}
Expand Down Expand Up @@ -268,7 +279,10 @@ var lineFeature = function (arg) {
*/
this.polygonSearch = function (poly, opts) {
var data = m_this.data(), indices = [], found = [], extra = {}, min, max,
map = m_this.layer().map();
map = m_this.layer().map(),
strokeWidthFunc = m_this.style.get('strokeWidth'),
strokeOpacityFunc = m_this.style.get('strokeOpacity'),
lineFunc = m_this.line();
if (!poly.outer) {
poly = {outer: poly, inner: []};
}
Expand Down Expand Up @@ -303,7 +317,7 @@ var lineFeature = function (arg) {
record.min.y > max.y + record.max.r * scale) {
continue;
}
let inside, partial;
let inside, partial, line;
for (j = 0; j < record.length; j += 1) {
u = record[j].u;
v = record[j].v;
Expand All @@ -314,6 +328,13 @@ var lineFeature = function (arg) {
(u.y > max.y + r * scale && v.y > max.y + r * scale)) {
continue;
}
if (!line) {
line = lineFunc(data[i], i);
}
if ((strokeOpacityFunc(line[record[j].i], record[j].i, data[i], i) <= 0 && strokeOpacityFunc(line[record[j].j], record[j].j, data[i], i) <= 0) ||
(strokeWidthFunc(line[record[j].i], record[j].i, data[i], i) <= 0 && strokeWidthFunc(line[record[j].j], record[j].j, data[i], i) <= 0)) {
continue;
}
let dist0 = util.distanceToPolygon2d(u, poly),
dist1 = util.distanceToPolygon2d(v, poly);
if ((dist0 > -r * scale && dist0 < r * scale) || (dist1 > -r * scale && dist1 < r & scale) || dist0 * dist1 < 0) {
Expand Down
6 changes: 5 additions & 1 deletion src/webgl/lineFeature.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,11 @@ var webgl_lineFeature = function (arg) {
strokeColorBuf[dest3] = v1.strokeColor.r;
strokeColorBuf[dest3 + 1] = v1.strokeColor.g;
strokeColorBuf[dest3 + 2] = v1.strokeColor.b;
strokeOpacityBuf[dest] = v1.strokeOpacity;
if ((v1.strokeOpacity <= 0 && v2.strokeOpacity <= 0) || (v1.strokeWidth <= 0 && v2.strokeWidth <= 0)) {
strokeOpacityBuf[dest] = -1;
} else {
strokeOpacityBuf[dest] = v1.strokeOpacity;
}
}
}
}
Expand Down
46 changes: 35 additions & 11 deletions tests/cases/lineFeature.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ describe('geo.lineFeature', function () {
}, {
coord: [{x: 70, y: 10}, {x: 75, y: 12}, {x: 72, y: 15}, {x: 70, y: 15}],
closed: true
}, {
coord: [{x: 60, y: 20, width: 0}, {x: 65, y: 22, width: 0}, {x: 62, y: 25}]
}
];

Expand Down Expand Up @@ -100,7 +102,7 @@ describe('geo.lineFeature', function () {
})
.style({
strokeWidth: function (d) {
return d.width ? d.width : 5;
return d.width !== undefined ? d.width : 5;
},
closed: function (item) {
return item.closed;
Expand Down Expand Up @@ -173,6 +175,12 @@ describe('geo.lineFeature', function () {
}).toThrow(new Error('no width'));
/* Stop throwing the exception */
line.style('strokeWidth', 5);
/* Lines that are transparent shouldn't result in a hit. */
line.style('strokeOpacity', function (p, j, d, i) { return i === 3 ? 0 : 1; });
pt = line.pointSearch({x: 31, y: 12.5});
expect(pt.found.length).toBe(1);
pt = line.pointSearch({x: 31, y: 32.5});
expect(pt.found.length).toBe(0);
});
it('boxSearch', function () {
var map, layer, line, idx;
Expand Down Expand Up @@ -200,6 +208,14 @@ describe('geo.lineFeature', function () {
line.data(testLines)
.line(function (item, itemIdx) {
return item.coord;
})
.style({
strokeWidth: function (d) {
return d.width !== undefined ? d.width : 1;
},
closed: function (item) {
return item.closed;
}
});
result = line.polygonSearch([{x: 16, y: 9}, {x: 36, y: 9}, {x: 36, y: 25}]);
expect(result.index).toEqual([0, 1]);
Expand All @@ -225,6 +241,14 @@ describe('geo.lineFeature', function () {
expect(result.index).toEqual([1]);
result = line.polygonSearch({outer: [{x: 25, y: 5}, {x: 40, y: 5}, {x: 32, y: 20}], inner: [[{x: 30, y: 13}, {x: 35, y: 13}, {x: 32, y: 16}]]}, {partial: true});
expect(result.index).toEqual([1]);
result = line.polygonSearch([{x: 59, y: 19}, {x: 66, y: 22}, {x: 62, y: 26}]);
expect(result.index).toEqual([8]);
// test that lines with zero width aren't returned
result = line.polygonSearch([{x: 59, y: 19}, {x: 60, y: 21}, {x: 61, y: 19}], {partial: true});
expect(result.index).toEqual([]);
line.style({strokeWidth: 1});
result = line.polygonSearch([{x: 59, y: 19}, {x: 60, y: 21}, {x: 61, y: 19}], {partial: true});
expect(result.index).toEqual([8]);
});

describe('rdpSimplifyData', function () {
Expand Down Expand Up @@ -256,28 +280,28 @@ describe('geo.lineFeature', function () {
line._init();
line.data(testLines).line(lineFunc);
counts = countLines(line.data().map(line.style.get('line')));
expect(counts).toEqual({lines: 8, vertices: 23});
expect(counts).toEqual({lines: 9, vertices: 26});
line.rdpSimplifyData(testLines, undefined, undefined, lineFunc);
counts = countLines(line.data().map(line.style.get('line')));
expect(counts).toEqual({lines: 8, vertices: 18});
expect(counts).toEqual({lines: 9, vertices: 21});

// use pixel space for ease of picking tolerance values in tests
map.gcs('+proj=longlat +axis=enu');
map.ingcs('+proj=longlat +axis=esu');
line.rdpSimplifyData(testLines, 2, undefined, lineFunc);
counts = countLines(line.data().map(line.style.get('line')));
expect(counts).toEqual({lines: 8, vertices: 17});
expect(counts).toEqual({lines: 9, vertices: 20});
line.rdpSimplifyData(testLines, 5.01, undefined, lineFunc);
counts = countLines(line.data().map(line.style.get('line')));
expect(counts).toEqual({lines: 8, vertices: 9});
expect(counts).toEqual({lines: 9, vertices: 11});
line.rdpSimplifyData(testLines, 20, undefined, lineFunc);
counts = countLines(line.data().map(line.style.get('line')));
expect(counts).toEqual({lines: 8, vertices: 0});
expect(counts).toEqual({lines: 9, vertices: 0});
line.rdpSimplifyData(testLines, 0.4, function (d) {
return {x: d.x * 0.2, y: d.y * 0.2};
}, lineFunc);
counts = countLines(line.data().map(line.style.get('line')));
expect(counts).toEqual({lines: 8, vertices: 17});
expect(counts).toEqual({lines: 9, vertices: 20});
});
});
});
Expand All @@ -295,7 +319,7 @@ describe('geo.lineFeature', function () {
},
style: {
strokeWidth: function (d) {
return d.width ? d.width : 5;
return d.width !== undefined ? d.width : 5;
},
lineCap: function (d, i, line, idx) {
return ['butt', 'round', 'square'][idx % 3];
Expand All @@ -311,7 +335,7 @@ describe('geo.lineFeature', function () {
}).data(testLines);
line.draw();
stepAnimationFrame();
expect(layer.node().find('path').length).toBe(8);
expect(layer.node().find('path').length).toBe(9);
var paths = layer.node().find('path');
expect(paths.eq(0).css('stroke-linecap')).toBe('butt');
expect(paths.eq(1).css('stroke-linecap')).toBe('round');
Expand All @@ -338,7 +362,7 @@ describe('geo.lineFeature', function () {
},
style: {
strokeWidth: function (d) {
return d.width ? d.width : 5;
return d.width !== undefined ? d.width : 5;
},
lineCap: function (d, i, line, idx) {
return ['butt', 'round', 'square'][idx % 3];
Expand Down Expand Up @@ -377,7 +401,7 @@ describe('geo.lineFeature', function () {
},
style: {
strokeWidth: function (d) {
return d.width ? d.width : 5;
return d.width !== undefined ? d.width : 5;
},
lineCap: function (d, i, line, idx) {
return ['butt', 'round', 'square'][idx % 3];
Expand Down