fix: Correct picking formulas#142
Conversation
| } | ||
| } | ||
| } | ||
| export { |
There was a problem hiding this comment.
To change - We can avoid the re-export, neuroglancer doesn't guarantee a stable API
| element.clientLeft; | ||
| const mouseY = | ||
| ((clientY - bounds.top) / bounds.height) * element.offsetHeight + | ||
| ((clientY - bounds.top) / bounds.height) * element.offsetHeight - |
There was a problem hiding this comment.
Note for PR - definite problem - the border around the panels is accidentally getting added to mouse position. Result would be a 4 pixel off selection in Y, and moreso if the border size is expanded.
| const y = startY + relativeY; | ||
| if (x < 0 || y < 0 || x >= viewportWidth || y >= viewportHeight) { | ||
| buffer[ | ||
| baseOffset + (relativeY * pickDiameter + relativeX) * stride |
There was a problem hiding this comment.
definite problem, the buffer clearing was not using the relative index into the buffer, but instead using x and y window locations. Probably never really came up because picking there happens very rarely
| } | ||
| const { renderViewport } = this; | ||
| const glWindowX = | ||
| const glWindowX = Math.floor( |
There was a problem hiding this comment.
Note for when we make PR - Problem, without rounding to int (floor makes sense) it is possible we could end up with a floating point value here. This immediately goes into readPixels which is expecting an int, and how our float is handled is likely implementation dependent. It is also used in clearOutOfBoundsPickData, but with our other fix to use relative X and Y that could handle fractional data so it's less of a problem there.
This change is related to the change to add 0.5 to the pick because now we're flooring and using the 0.5 to use the center. But I think we can remove the 0.5 add and land at the pixel edge instead of the pixel centre. In neuroglancer, the origin of a voxel is the lower corner anyway, not the centre. I've noted this bit elsewhere
| } | ||
| } | ||
|
|
||
| export function getCenteredPickWindowCoordinate( |
There was a problem hiding this comment.
I'm not sure we need this due to the origin of a voxel in neuroglancer not being its centre. Think we could remove this fn and revert the places where it is called to the old in place
| return glWindowCoordinate + relativeCoordinate - pickRadius + 0.5; | ||
| } | ||
|
|
||
| export function resolveNearestPanelPickSample( |
There was a problem hiding this comment.
I think this could use a js doc comment explaining it. Even the description of the params would help a lot
| }; | ||
| for (let i = 0; i < numOffsets; ++i) { | ||
| const offset = pickOffsetSequence[i]; | ||
| const pickId = data[4 * i]; |
There was a problem hiding this comment.
Definite problem. This explains why we had trouble picking the nodes in polylines in 2D. The 3D code was doing this correctly, but specifically in 2D it was wrong.
2 participants
Cherry picks the fix for picking from the feature/edit-mode. See https://metacell.atlassian.net/browse/NA-629 for details.
Closes https://metacell.atlassian.net/browse/NA-629