From 89d8d37576b813b31e94eb44ab470b23522fcca2 Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Fri, 15 Dec 2017 17:28:20 -0500 Subject: [PATCH] Drawing all the correct vertices now where I want them, simplify classes Some highlights - `getSiblingAndChildVertices` are expensive, so they're saved and called less frequently - draw touch targets for all the visible vertices - remove redundant css classes and `setClass` function --- css/20_map.css | 25 +------ modules/modes/drag_node.js | 2 +- modules/renderer/map.js | 8 +- modules/svg/vertices.js | 149 +++++++++++++++++++++++-------------- test/spec/svg/midpoints.js | 2 +- 5 files changed, 100 insertions(+), 86 deletions(-) diff --git a/css/20_map.css b/css/20_map.css index 4ab4dae1c..cbff2a7f5 100644 --- a/css/20_map.css +++ b/css/20_map.css @@ -14,6 +14,7 @@ use { pointer-events: none; } .layer-points-group * { pointer-events: none; } +.layer-points-group.layer-points-midpoints *, .layer-points-group.layer-points-targets * { pointer-events: all; } @@ -96,30 +97,6 @@ g.midpoint .shadow { fill: currentColor; } -/*g.vertex.vertex-hover { - display: none; -} - -.mode-draw-area g.vertex.vertex-hover, -.mode-draw-line g.vertex.vertex-hover, -.mode-add-area g.vertex.vertex-hover, -.mode-add-line g.vertex.vertex-hover, -.mode-add-point g.vertex.vertex-hover, -.mode-drag-node g.vertex.vertex-hover { - display: block; - color: #f00; -} - -.mode-draw-area .hover-disabled g.vertex.vertex-hover, -.mode-draw-line .hover-disabled g.vertex.vertex-hover, -.mode-add-area .hover-disabled g.vertex.vertex-hover, -.mode-add-line .hover-disabled g.vertex.vertex-hover, -.mode-add-point .hover-disabled g.vertex.vertex-hover, -.mode-drag-node .hover-disabled g.vertex.vertex-hover { - display: none; -} -*/ - g.vertex.related:not(.selected) .shadow, g.vertex.hover:not(.selected) .shadow, g.midpoint.related:not(.selected) .shadow, diff --git a/modules/modes/drag_node.js b/modules/modes/drag_node.js index 50cd8231b..bd5bb5a23 100644 --- a/modules/modes/drag_node.js +++ b/modules/modes/drag_node.js @@ -249,7 +249,7 @@ export function modeDragNode(context) { var behavior = behaviorDrag() - .selector('g.node, g.point, g.midpoint') + .selector('g.node, g.midpoint') .surface(d3_select('#map').node()) .origin(origin) .on('start', start) diff --git a/modules/renderer/map.js b/modules/renderer/map.js index f9de7d460..39996d04f 100644 --- a/modules/renderer/map.js +++ b/modules/renderer/map.js @@ -207,7 +207,7 @@ export function rendererMap(context) { all = context.features().filter(all, graph); surface.selectAll('.data-layer-osm') - .call(drawVertices, graph, all, filter, map.extent()) + .call(drawVertices.drawSelected, graph, all, map.extent()) .call(drawMidpoints, graph, all, filter, map.trimmedExtent()); dispatch.call('drawn', this, { full: false }); } @@ -268,6 +268,7 @@ export function rendererMap(context) { var graph = context.graph(); var features = context.features(); var all = context.intersects(map.extent()); + var fullRedraw = false; var data; var filter; @@ -291,6 +292,7 @@ export function rendererMap(context) { } else { data = all; + fullRedraw = true; filter = utilFunctor(true); } } @@ -298,11 +300,11 @@ export function rendererMap(context) { data = features.filter(data, graph); surface.selectAll('.data-layer-osm') - .call(drawVertices, graph, data, filter, map.extent()) + .call(drawVertices, graph, data, filter, map.extent(), fullRedraw) .call(drawLines, graph, data, filter) .call(drawAreas, graph, data, filter) .call(drawMidpoints, graph, data, filter, map.trimmedExtent()) - .call(drawLabels, graph, data, filter, dimensions, !difference && !extent) + .call(drawLabels, graph, data, filter, dimensions, fullRedraw) .call(drawPoints, graph, data, filter); dispatch.call('drawn', this, {full: true}); diff --git a/modules/svg/vertices.js b/modules/svg/vertices.js index 85ab774ca..e09eb40ea 100644 --- a/modules/svg/vertices.js +++ b/modules/svg/vertices.js @@ -1,5 +1,4 @@ import _assign from 'lodash-es/assign'; -import _clone from 'lodash-es/clone'; import _values from 'lodash-es/values'; import { select as d3_select } from 'd3-selection'; @@ -21,12 +20,21 @@ export function svgVertices(projection, context) { fill: [1, 1.5, 1.5, 1.5] }; - var _currHover; - var _currHoverSiblings = {}; + var _currHoverTarget; + var _currPersistent = {}; + var _currHover = {}; + var _prevHover = {}; + var _currSelected = {}; + var _prevSelected = {}; - function draw(selection, graph, vertices, klass, siblings, filter) { - siblings = siblings || {}; + function sortY(a, b) { + return b.loc[1] - a.loc[1]; + } + + + function draw(selection, graph, vertices, sets, filter) { + sets = sets || { selected: {}, important: {}, hovered: {} }; var icons = {}; var directions = {}; var wireframe = context.surface().classed('fill-wireframe'); @@ -54,16 +62,8 @@ export function svgVertices(projection, context) { } - function setClass(klass) { - return function(entity) { - d3_select(this) - .attr('class', 'node vertex ' + klass + ' ' + entity.id); - }; - } - - function updateAttributes(selection) { - ['shadow','stroke','fill'].forEach(function(klass) { + ['shadow', 'stroke', 'fill'].forEach(function(klass) { var rads = radiuses[klass]; selection.selectAll('.' + klass) .each(function(entity) { @@ -88,8 +88,9 @@ export function svgVertices(projection, context) { .attr('visibility', (z === 0 ? 'hidden' : null)); } + vertices.sort(sortY); - var groups = selection.selectAll('.vertex.' + klass) + var groups = selection.selectAll('g.vertex') .filter(filter) .data(vertices, osmEntity.key); @@ -100,41 +101,42 @@ export function svgVertices(projection, context) { // enter var enter = groups.enter() .append('g') - .attr('class', function(d) { return 'node vertex ' + klass + ' ' + d.id; }); + .attr('class', function(d) { return 'node vertex ' + d.id; }) + .order(); enter .append('circle') - .each(setClass('shadow')); + .attr('class', 'shadow'); enter .append('circle') - .each(setClass('stroke')); + .attr('class', 'stroke'); // Vertices with icons get a `use`. enter.filter(function(d) { return getIcon(d); }) .append('use') + .attr('class', 'icon') + .attr('width', '11px') + .attr('height', '11px') .attr('transform', 'translate(-5, -6)') .attr('xlink:href', function(d) { var picon = getIcon(d); var isMaki = dataFeatureIcons.indexOf(picon) !== -1; return '#' + picon + (isMaki ? '-11' : ''); - }) - .attr('width', '11px') - .attr('height', '11px') - .each(setClass('icon')); + }); // Vertices with tags get a fill. enter.filter(function(d) { return d.hasInterestingTags(); }) .append('circle') - .each(setClass('fill')); + .attr('class', 'fill'); // update groups = groups .merge(enter) .attr('transform', svgPointTransform(projection)) - .classed('sibling', function(entity) { return entity.id in siblings; }) - .classed('shared', function(entity) { return graph.isShared(entity); }) - .classed('endpoint', function(entity) { return entity.isEndpoint(graph); }) + .classed('sibling', function(d) { return d.id in sets.selected; }) + .classed('shared', function(d) { return graph.isShared(d); }) + .classed('endpoint', function(d) { return d.isEndpoint(graph); }) .call(updateAttributes); @@ -150,7 +152,7 @@ export function svgVertices(projection, context) { // enter/update dgroups = dgroups.enter() .insert('g', '.shadow') - .each(setClass('viewfieldgroup')) + .attr('class', 'viewfieldgroup') .merge(dgroups); var viewfields = dgroups.selectAll('.viewfield') @@ -174,6 +176,7 @@ export function svgVertices(projection, context) { function drawTargets(selection, graph, entities, filter) { var debugClass = 'pink'; var targets = selection.selectAll('.target') + .filter(filter) .data(entities, osmEntity.key); // exit @@ -202,7 +205,7 @@ export function svgVertices(projection, context) { } - function getSiblingAndChildVertices(ids, graph, extent, wireframe, zoom) { + function getSiblingAndChildVertices(ids, graph, wireframe, zoom) { var results = {}; function addChildVertices(entity) { @@ -223,7 +226,7 @@ export function svgVertices(projection, context) { addChildVertices(member); } } - } else if (renderAsVertex(entity, graph, wireframe, zoom) && entity.intersects(extent, graph)) { + } else if (renderAsVertex(entity, graph, wireframe, zoom)) { results[entity.id] = entity; } } @@ -249,66 +252,98 @@ export function svgVertices(projection, context) { } - function drawVertices(selection, graph, entities, filter, extent) { + function drawVertices(selection, graph, entities, filter, extent, fullRedraw) { var wireframe = context.surface().classed('fill-wireframe'); var zoom = ktoz(projection.scale()); + var mode = context.mode(); + var isDrawing = mode && /^(add|draw|drag)/.test(mode.id); - var selected = getSiblingAndChildVertices(context.selectedIDs(), graph, extent, wireframe, zoom); + // Collect important vertices from the `entities` list.. + // (during a paritial redraw, it will not contain everything) + if (fullRedraw) { + _currPersistent = {}; + } - // interesting vertices from the `entities` list.. - var interesting = {}; for (var i = 0; i < entities.length; i++) { var entity = entities[i]; var geometry = entity.geometry(graph); if ((geometry === 'point') && renderAsVertex(entity, graph, wireframe, zoom)) { - interesting[entity.id] = entity; + _currPersistent[entity.id] = entity; } else if ((geometry === 'vertex') && (entity.hasInterestingTags() || entity.isEndpoint(graph) || entity.isConnected(graph)) ) { - interesting[entity.id] = entity; + _currPersistent[entity.id] = entity; + + } else if (!fullRedraw) { + delete _currPersistent[entity.id]; } } - // 3 sets of vertices to consider - // - selected + siblings - // - hovered + siblings - // - interesting entities passed in - var all = _assign(selected, interesting, _currHoverSiblings); + // 3 sets of vertices to consider: + var sets = { + persistent: _currPersistent, // persistent = important vertices (render always) + selected: _currSelected, // selected + siblings of selected (render always) + hovered: _currHover // hovered + siblings of hovered (render only in draw modes) + }; - var filterWithSiblings = function(d) { - return d.id in selected || d.id in _currHoverSiblings || filter(d); + var all = _assign({}, _currPersistent, _currSelected, (isDrawing ? _currHover : {})); + + // Draw the vertices.. + // The filter function controls the scope of what objects d3 will touch (exit/enter/update) + // It's important to adjust the filter function to expand the scope beyond whatever entities were passed in. + var filterRendered = function(d) { + return d.id in _currPersistent || d.id in _currSelected || d.id in _currHover || filter(d); }; selection.selectAll('.layer-points .layer-points-vertices') - .call(draw, graph, _values(all), 'vertex-persistent', {}, filterWithSiblings); + .call(draw, graph, visible(all), sets, filterRendered); - - // draw touch targets for the hovered items only - var filterWithHover = function(d) { - return d.id in _currHoverSiblings || filter(d); - }; + // Draw touch targets.. selection.selectAll('.layer-points .layer-points-targets') - .call(drawTargets, graph, _values(_currHoverSiblings), filterWithHover); + .call(drawTargets, graph, visible(all), filterRendered); + + + function visible(which) { + return _values(which).filter(function (entity) { + return entity.intersects(extent, graph); + }); + } } + // partial redraw - only update the selected items.. + drawVertices.drawSelected = function(selection, graph, target, extent) { + var wireframe = context.surface().classed('fill-wireframe'); + var zoom = ktoz(projection.scale()); + + _prevSelected = _currSelected || {}; + _currSelected = getSiblingAndChildVertices(context.selectedIDs(), graph, wireframe, zoom); + + // note that drawVertices will add `_currSelected` automatically if needed.. + var filter = function(d) { return d.id in _prevSelected; }; + drawVertices(selection, graph, _values(_prevSelected), filter, extent, false); + }; + + + // partial redraw - only update the hovered items.. drawVertices.drawHover = function(selection, graph, target, extent) { - if (target === _currHover) return; + if (target === _currHoverTarget) return; // continue only if something changed var wireframe = context.surface().classed('fill-wireframe'); var zoom = ktoz(projection.scale()); - var prevHoverSiblings = _currHoverSiblings || {}; - var filter = function(d) { return d.id in prevHoverSiblings; }; - _currHover = target; + _prevHover = _currHover || {}; + _currHoverTarget = target; - if (_currHover) { - _currHoverSiblings = getSiblingAndChildVertices([_currHover.id], graph, extent, wireframe, zoom); + if (_currHoverTarget) { + _currHover = getSiblingAndChildVertices([_currHoverTarget.id], graph, wireframe, zoom); } else { - _currHoverSiblings = {}; + _currHover = {}; } - drawVertices(selection, graph, _values(prevHoverSiblings), filter, extent); + // note that drawVertices will add `_currHover` automatically if needed.. + var filter = function(d) { return d.id in _prevHover; }; + drawVertices(selection, graph, _values(_prevHover), filter, extent, false); }; return drawVertices; diff --git a/test/spec/svg/midpoints.js b/test/spec/svg/midpoints.js index bb4442415..230ad8446 100644 --- a/test/spec/svg/midpoints.js +++ b/test/spec/svg/midpoints.js @@ -20,7 +20,7 @@ describe('iD.svgMidpoints', function () { selectedIDs: function() { return _selectedIDs; } }); - var map = d3.select(document.createElement('div')) + d3.select(document.createElement('div')) .attr('id', 'map') .call(context.map().centerZoom([0, 0], 17));