Under certain conditions, ModifyMode can modify an incorrect vertex

[rjwats]

Module

• deck.gl-community/arrow-layers
• deck.gl-community/bing-maps
• deck.gl-community/editable-layers
• deck.gl-community/experimental
• deck.gl-community/graph-layers
• deck.gl-community/layers
• deck.gl-community/react
• deck.gl-community/react-graph-layer

Description

When modifying a geometry using ModifyMode there is an issue which can result in the wrong vertex being updated. This occurs when you "drop" a vertex close to another vertex (specifically one with a higher ordinal within coordinate list).

I believe getPickedEditHandle in handleStopDragging is returning the vertex with the highest ordinal from the geometry, which is why the handleStopDragging modifieds the wrong vertex in this situation.

I'm not 100% confident in the correct implementation, however the following override appears to solve the issue for me:

export default class CustomModifyMode extends ModifyMode {

  handleStopDragging(event: StopDraggingEvent, props: ModeProps<FeatureCollection>) {
    const selectedFeatureIndexes = props.selectedIndexes;
    const editHandle = getPickedEditHandle(event.pointerDownPicks);

    if (selectedFeatureIndexes.length && editHandle) {
      this._dragEditHandle('finishMovePosition', props, editHandle, event);
    }
  }

}

Attached is a video of the issue. See the below codesandbox for an interactive example of the problem with instructions and a visual aid to help observe the problem during editing:

https://codesandbox.io/p/sandbox/fwhvn7

example.mp4

Thanks :)

Expected Behavior

Only the dragged vertex should be updated to reflect the location where the user has dropped it.

Steps to Reproduce

Open the codesandbox:

https://codesandbox.io/p/sandbox/fwhvn7

Drag the left most vertex and place it so it overlaps one of the other vertexes. Observe how the the vertex you have dropped the point onto is modified unexpectedly. I believe getPickedEditHandle in handleStopDragging is returning the vertex with the highest ordinal from the linestring, which is why the handleStopDragging modifieds the wrong vertex in this situation.

Environment

• Framework version: @deck.gl-community/editable-layers 9.0.3
• Browser: Any
• OS: Any

Logs

No response

[timnyborg]

I think your solution is correct - it's the same as I landed on when reporting this to nebula.gl ages back: https://github.com/uber/nebula.gl/pull/868/files

[ibgreen]

Thanks for reporting.

Unfortunately we don't have an up-to-speed, in-depth maintainer of this module that can weigh in and say what is correct or not, which makes it tricky for us to decide whether to accept such fixes.

But I'd be open to accepting a PR since we now have two voices in favor of this change.

(If we wanted to play it defensively we could add an option to ModifyMode so that users have to opt in to the improved version, that way we won't break anyone's code, depend on how confident you guys feel about this change).

[rjwats]

Thanks for the reply, it's much appreciated as our project makes extensive use of this library

I'm confident that the issue was an implementation mistake, and that the fix is correct and won't cause any side effects (other than fixing the bug, of course). I ought to implement a regression test - which I'm guessing should probably live here:

/modules/editable-layers/test/edit-modes/lib/modify-mode.spec.ts

I'll raise a PR this working week.