Task Grid #26

Bonkles · 2019-08-29T19:17:38Z

Currently, If a URL query param such as gpx=https://tm.nsosm.com/project/1915/task/1897.gpx&map=15.18/-4.0972/131.3184

is supplied to Rapid, we draw a task box around the area according to the gpx bounding box.

This PR adds the ability to subdivide the bounding box into a 2x2, 3x3, 4x4, 5x5, or 6x6 grid.

Updated .gif: As per @gaoxm's excellent feedback I've made some refinements, and the grid options will only be available when the task boundary is a rectangle.

gaoxm

Looks good overall - the major thing to consider I think is whether to limit this to only rectangle tasks. Also could you check the build failures? From past experience builds with node.js 8 and 10 should pass.

modules/operations/delete.js

modules/renderer/map.js

modules/ui/background.js

Bonkles · 2019-09-06T21:57:40Z

Now that this PR checks for rectangular geometry of the task box, I ran a negative test case by manually creating my own local .gpx file with one odd side.

The grid/grid options do not render in the UI as per this screenshot:

modules/core/rapid_context.js

gaoxm · 2019-09-09T21:12:17Z

modules/ui/background.js

@@ -259,6 +271,58 @@ export function uiBackground(context) {
        updateOverlayList();
    }

+
+    function renderGridList(selection) {
+


resurfacing - let's follow the convention in the same file and avoid empty line at beginning of function

Refresh master from upstream.

… etc) can be found in the background controls.

…d options on whether the task area is rectangular.

… boxes.

Bonkles · 2019-09-18T18:47:28Z

modules/core/rapid_context.js

+                    if (maxlon === undefined || lon > maxlon) maxlon = lon;                    
+                }
+
+                if (f.geometry.type === 'LineString') {


Perhaps I should leave a comment here explaining that the 'LineString' portion of the task geoJson explicitly defines just the task bbox.

Actually, @gaoxm @zlavergne is that a valid assumption to make? Are there other formats to consider for task bboxes/gpx urls?

not sure what could possibly be returned by TM - I think we should also allow 'Polygon'. @zlavergne could you help check what could be returned by TM?

Okay, I did some further digging and it does look like I need to keep the 'LineString' constraint here. Here's why- the .gpx data fed to us by TM also contains points that show the task id/ state. These points do not necessarily share the same lats/lngs as the four corners of the task box- sometimes the points will be along the interior of a line segment.

modules/renderer/map.js

Bonkles · 2019-09-18T18:50:50Z

modules/svg/data.js

@@ -475,7 +475,7 @@ export function svgData(projection, context, dispatch) {
            d3_text(url)
                .then(function(data) {
                    drawData.setFile(extension, data);
-                    var isTaskBoundsUrl = extension === '.gpx' && url.indexOf('project') > 0 && url.indexOf('task') > 0;


@gaoxm @zlavergne @jeffdefacto this is one thing we definitely need to discuss- is this change okay to make? Seems like it would totally break stuff, but I see the value in allowing somebody to just send .gpx data without a project/task associated with it.

In that case we will be unsure about whether their gpx URL is for the purpose of defining the boundary of a mapping task; e.g. some users may load GPS traces through gpx URL and use them to guide mapping roads. I'm leaning towards keeping the checks.

modules/ui/background.js

Bonkles · 2019-09-18T19:10:27Z

modules/core/rapid_context.js


 export function coreRapidContext(context) {
    var rapidContext = {};
    rapidContext.version = '1.0.1';
+    var _isRect = undefined; 
+    var dispatch = d3_dispatch('task_extent_set');


I see you there, wondering why I need to use pub-sub to display a static option correctly.

The simple answer is- the gpx task URL is read sometime AFTER the first few renders are called for the layer sidebar. That means at time t=0, no task gpx exists, so we render the control with no grid options.

The 'task_extent_set' event fires once the extent info has been read from the URL and is available for the sidebar to re-render.

If the task extent is not rectangular, we don't bother re-rendering the sidebar. If it IS rectangular, then we re-render the sidebar with the grid controls in place.

modules/core/rapid_context.js

gaoxm · 2019-09-18T20:11:00Z

modules/core/rapid_context.js

+                    if (maxlon === undefined || lon > maxlon) maxlon = lon;                    
+                }
+
+                if (f.geometry.type === 'LineString') {


not sure what could possibly be returned by TM - I think we should also allow 'Polygon'. @zlavergne could you help check what could be returned by TM?

modules/core/rapid_context.js

gaoxm · 2019-09-18T20:15:25Z

modules/core/rapid_context.js

+                    //If there are only two unique lngs and two unique lats in the geometry of the 
+                    //task area, congrats- we have a rectangle! 
+                    if (uniqueLatCount === 2 && uniqueLngCount === 2) {
+                        _isRect = true; 


besides this condition, we should also make sure that along each unique lat/lon value, there are at least 2 points. e.g. one counter-case could be a linestring containing only two points: uniqueLatCount === 2 && uniqueLngCount === 2 will be true but it's not a rectangle. Using a two dicts keyed on lat/lon values might be the easies way to help with that.

modules/geo/extent.js

gaoxm · 2019-09-18T20:27:18Z

modules/svg/data.js

@@ -475,7 +475,7 @@ export function svgData(projection, context, dispatch) {
            d3_text(url)
                .then(function(data) {
                    drawData.setFile(extension, data);
-                    var isTaskBoundsUrl = extension === '.gpx' && url.indexOf('project') > 0 && url.indexOf('task') > 0;


In that case we will be unsure about whether their gpx URL is for the purpose of defining the boundary of a mapping task; e.g. some users may load GPS traces through gpx URL and use them to guide mapping roads. I'm leaning towards keeping the checks.

modules/ui/background.js

modules/core/rapid_context.js

Pulll lated changes into master.

… etc) can be found in the background controls.

…d options on whether the task area is rectangular.

… boxes.

gaoxm

👍 Looks good now - only one minor inline comment about naming of _isRect. Don't forget to rebase and squash commits before merging :).

modules/core/rapid_context.js

… will not be considered for drawing grids.

* Grid has been added if a GPX url is supplied- Grid options (2x2, 3x3, etc) can be found in the background controls if and only if the bounding area is a rectangle.

Task Grid (Copy of PR #26, originally merged into the v2 branch).

Bonkles requested review from chrisklaiber and gaoxm August 29, 2019 19:17

Bonkles changed the title ~~Add an NxN grid to the task box if a gpx URL is supplied.~~ Task Grid Aug 29, 2019

Bonkles assigned Bonkles, jeffdefacto and zlavergne Sep 4, 2019

gaoxm suggested changes Sep 5, 2019

View reviewed changes

Bonkles mentioned this pull request Sep 6, 2019

Ai road halo (optional feature) - add AI magenta shadow to any road that used to be an AI road #28

Merged

gaoxm suggested changes Sep 9, 2019

View reviewed changes

Bonkles force-pushed the grid branch from 5fe6047 to 169774f Compare September 13, 2019 17:47

Bonkles and others added 2 commits September 13, 2019 16:00

Merge pull request #3 from facebookincubator/master

82a41ed

Refresh master from upstream.

Grid has been added if a GPX url is supplied- Grid options (2x2, 3x3,…

3c57872

… etc) can be found in the background controls.

Bonkles mentioned this pull request Sep 13, 2019

resurfacing - let's follow the convention in the same file and avoid empty line at beginning of function #36

Closed

Bonkles added 2 commits September 16, 2019 14:25

Incorporate review feedback, also conditionally render the grid / gri…

a5ef0d9

…d options on whether the task area is rectangular.

Get grids working again.

2f05662

Bonkles force-pushed the grid branch from 169774f to 2f05662 Compare September 17, 2019 20:02

Get some correct behavior around rectangular vs. non-rectangular task…

8825577

… boxes.

Bonkles commented Sep 18, 2019

View reviewed changes

Fix tests, some very small tweaks in spacing/logic.

7788663

Bonkles commented Sep 18, 2019

View reviewed changes

gaoxm suggested changes Sep 18, 2019

View reviewed changes

Add more stringent check for rectangle-ness.

ebbded4

gaoxm suggested changes Sep 19, 2019

View reviewed changes

modules/core/rapid_context.js Show resolved Hide resolved

Bonkles and others added 6 commits September 19, 2019 13:45

Merge pull request #4 from facebookincubator/master

1613b82

Pulll lated changes into master.

Grid has been added if a GPX url is supplied- Grid options (2x2, 3x3,…

2c07143

… etc) can be found in the background controls.

Incorporate review feedback, also conditionally render the grid / gri…

c6b07ef

…d options on whether the task area is rectangular.

Get grids working again.

87fc237

Get some correct behavior around rectangular vs. non-rectangular task…

fb6ca38

… boxes.

Fix tests, some very small tweaks in spacing/logic.

97d769f

Bonkles added 2 commits September 19, 2019 13:49

Add more stringent check for rectangle-ness.

eb9e30d

Merge branch 'grid' of github.com:bonkles-worldai/RapiD into grid

73da17a

Bonkles changed the base branch from master to track_id_v2.15 September 23, 2019 15:28

restore project and task requirements in the URL.

5176747

gaoxm approved these changes Oct 4, 2019

View reviewed changes

modules/core/rapid_context.js Outdated Show resolved Hide resolved

Add more stringent check to ensure that multipolygonal bounding areas…

c3d09b8

… will not be considered for drawing grids.

Bonkles force-pushed the grid branch from 5541b3d to c3d09b8 Compare October 8, 2019 21:20

Bonkles merged commit 37c6b93 into facebook:track_id_v2.15 Oct 8, 2019

Bonkles mentioned this pull request Jan 7, 2020

v3: port grids functionality over from v2. #95

Closed

bhousel added a commit that referenced this pull request Jul 7, 2020

Merge pull request #135 from bonkles-worldai/v3_grid

66fc10d

Task Grid (Copy of PR #26, originally merged into the v2 branch).

Bonkles mentioned this pull request Jan 23, 2023

Task Boundary and Grid re-implementation #768

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Task Grid #26

Task Grid #26

Bonkles commented Aug 29, 2019 •

edited

Loading

gaoxm left a comment •

edited

Loading

Bonkles commented Sep 6, 2019

gaoxm Sep 9, 2019

Bonkles Sep 18, 2019

gaoxm Sep 18, 2019

Bonkles Oct 4, 2019

Bonkles Sep 18, 2019

gaoxm Sep 18, 2019

Bonkles Sep 18, 2019

Bonkles Sep 18, 2019

gaoxm Sep 18, 2019

gaoxm Sep 18, 2019

gaoxm Sep 18, 2019

gaoxm left a comment

Task Grid #26

Task Grid #26

Conversation

Bonkles commented Aug 29, 2019 • edited Loading

gaoxm left a comment • edited Loading

Choose a reason for hiding this comment

Bonkles commented Sep 6, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gaoxm left a comment

Choose a reason for hiding this comment

Bonkles commented Aug 29, 2019 •

edited

Loading

gaoxm left a comment •

edited

Loading