Skip to content

Feature/editmode cartesian rectangle#654

Open
eKerney wants to merge 7 commits into
visgl:masterfrom
eKerney:feature/editmode-cartesian-rectangle
Open

Feature/editmode cartesian rectangle#654
eKerney wants to merge 7 commits into
visgl:masterfrom
eKerney:feature/editmode-cartesian-rectangle

Conversation

@eKerney

@eKerney eKerney commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

First crack at adding a Cartesian implementation for draw-rectangle-using-three-points-mode.ts. Added a few functions to cartesian-utils.ts, and added props.coordinateSyste pass through from ThreeClickPolygonMode. Should add tests for cartesian util functions.
DONT MERGE

@charlieforward9

Copy link
Copy Markdown
Member

Worked #641 in lets get this branch up to date and see where we stand

@eKerney

eKerney commented Jun 17, 2026

Copy link
Copy Markdown
Contributor Author

@charlieforward9 resolved minor merge conflicts and ran tests locally

return Math.abs(orientation(p1, p2, p)) / len;
}

export function generatePointsParallelToLinePointsCartesian(

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding a little comment/illustration elaborating on this function behavior or making the name a little clear would be great.

My guess is these would eventually make it into math.gl and we would want to make it clear where else it may be reusable.

coordinateSystem: EditModeCoordinateSystem
): Feature<Polygon> | null | undefined {
const [p3, p4] = generatePointsParallelToLinePoints(coord1, coord2, coord3);
// const [p3, p4] = generatePointsParallelToLinePoints(coord1, coord2, coord3);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment is not necessary

return event.picks.length && event.picks.find(p => p.featureType === 'points');
}

// Dispatch function to call function based on coord system, defaults to geo mode

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for implies a loop, can we think of a better name suffix?

  • onCoordinateFunc
  • ...branch

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at the scope of this utility file, it feels like coordinate conditional math should be packaged into a coordinate-math.ts file, instead of exporting the geospatial coordinate math & forCoord from this file and an independent cartesian-coordinate.ts file.

props: ModeProps<SimpleFeatureCollection>
): {handled: boolean} {
const outer = getPolygonFeature(feature.geometry.coordinates, props);
const cartesian = props.coordinateSystem instanceof CartesianCoordinateSystem;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants