Retain sketch selection segment color after adding a constraint to the segment (#2700)

* Start of basic test (not yet failing)

* Add a little logging to get the lay of the scene object land

* Move test colors to utility, test broken

* Get accurate test that is only broken with highlighted color behavior

* Working implementation but now initial segment color sticks around too long

* Make sure segment base color is always the theme color

* Remove console logs, refactor a couple lines to use if statements instead of inline booleans

* Fix new test

* Make origin color update like the other segment types

* fmt

* Fix issue where initially-selected segments lose highlight after hover

* Undo this tweaking of the selection logic, this is really only about the clientSideEntities

* Remove unused exports

* Remove unnecessary code change from ModelingMachineProvider

* Remove newline

* Update src/clientSideScene/sceneEntities.ts

Co-authored-by: Kurt Hutten <k.hutten@protonmail.ch>

---------

Co-authored-by: Kurt Hutten <k.hutten@protonmail.ch>
Co-authored-by: Jess Frazelle <jessfraz@users.noreply.github.com>

e2e/playwright/flow-tests.spec.ts

@@ -6,6 +6,7 @@ import {
   wiggleMove,
   doExport,
   metaModifier,
+  TEST_COLORS,
 } from './test-utils'
 import waitOn from 'wait-on'
 import { XOR, roundOff, uuidv4 } from 'lib/utils'
@@ -136,13 +137,11 @@ test('Basic sketch', async ({ page }) => {
   await page.waitForTimeout(100)
 
   const line1 = await u.getSegmentBodyCoords(`[data-overlay-index="${0}"]`, 0)
-  await expect(await u.getGreatestPixDiff(line1, [249, 249, 249])).toBeLessThan(
+  expect(await u.getGreatestPixDiff(line1, TEST_COLORS.WHITE)).toBeLessThan(3)
-    3
-  )
   // click between first two clicks to get center of the line
   await page.mouse.click(startXPx + PUR * 15, 500 - PUR * 10)
   await page.waitForTimeout(100)
-  await expect(await u.getGreatestPixDiff(line1, [0, 0, 255])).toBeLessThan(3)
+  expect(await u.getGreatestPixDiff(line1, TEST_COLORS.BLUE)).toBeLessThan(3)
 
   // hold down shift
   await page.keyboard.down('Shift')
@@ -3969,6 +3968,81 @@ const part002 = startSketchOn('XZ')
       })
     }
   })
+
+  test('Horizontally constrained line remains selected after applying constraint', async ({
+    page,
+  }) => {
+    await page.addInitScript(async () => {
+      localStorage.setItem(
+        'persistCode',
+        `const sketch001 = startSketchOn('XY')
+  |> startProfileAt([-1.05, -1.07], %)
+  |> line([3.79, 2.68], %, 'seg01')
+  |> line([3.13, -2.4], %)`
+      )
+    })
+    const u = await getUtils(page)
+    await page.setViewportSize({ width: 1200, height: 500 })
+    await page.goto('/')
+    await u.waitForAuthSkipAppStart()
+
+    await page.getByText("line([3.79, 2.68], %, 'seg01')").click()
+    await page.getByRole('button', { name: 'Edit Sketch' }).click()
+
+    await page.waitForTimeout(100)
+    const lineBefore = await u.getSegmentBodyCoords(
+      `[data-overlay-index="1"]`,
+      0
+    )
+    expect(
+      await u.getGreatestPixDiff(lineBefore, TEST_COLORS.WHITE)
+    ).toBeLessThan(3)
+    await page.mouse.move(lineBefore.x, lineBefore.y)
+    await page.waitForTimeout(50)
+    await page.mouse.click(lineBefore.x, lineBefore.y)
+    expect(
+      await u.getGreatestPixDiff(lineBefore, TEST_COLORS.BLUE)
+    ).toBeLessThan(3)
+
+    await page
+      .getByRole('button', {
+        name: 'Constrain',
+      })
+      .click()
+    await page.getByRole('button', { name: 'horizontal', exact: true }).click()
+
+    let activeLinesContent = await page.locator('.cm-activeLine').all()
+    await expect(activeLinesContent[0]).toHaveText(`|> xLine(3.13, %)`)
+
+    // If the overlay-angle is updated the THREE.js scene is in a good state
+    await expect(
+      await page.locator('[data-overlay-index="1"]')
+    ).toHaveAttribute('data-overlay-angle', '0')
+
+    const lineAfter = await u.getSegmentBodyCoords(
+      `[data-overlay-index="1"]`,
+      0
+    )
+    expect(
+      await u.getGreatestPixDiff(lineAfter, TEST_COLORS.BLUE)
+    ).toBeLessThan(3)
+
+    await page
+      .getByRole('button', {
+        name: 'Constrain',
+      })
+      .click()
+    await page.getByRole('button', { name: 'length', exact: true }).click()
+
+    await page.getByLabel('length Value').fill('10')
+    await page.getByRole('button', { name: 'Add constraining value' }).click()
+
+    activeLinesContent = await page.locator('.cm-activeLine').all()
+    await expect(activeLinesContent[0]).toHaveText(`|> xLine(length001, %)`)
+
+    // checking the count of the overlays is a good proxy check that the client sketch scene is in a good state
+    await expect(page.getByTestId('segment-overlay')).toHaveCount(2)
+  })
 })
 
 test.describe('Testing segment overlays', () => {

e2e/playwright/test-utils.ts

@@ -8,6 +8,13 @@ import { Protocol } from 'playwright-core/types/protocol'
 import type { Models } from '@kittycad/lib'
 import { APP_NAME } from 'lib/constants'
 
+type TestColor = [number, number, number]
+export const TEST_COLORS = {
+  WHITE: [249, 249, 249] as TestColor,
+  YELLOW: [255, 255, 0] as TestColor,
+  BLUE: [0, 0, 255] as TestColor,
+} as const
+
 async function waitForPageLoad(page: Page) {
   // wait for 'Loading stream...' spinner
   await page.getByTestId('loading-stream').waitFor()

src/clientSideScene/sceneEntities.ts

@@ -73,7 +73,7 @@ import {
   changeSketchArguments,
   updateStartProfileAtArgs,
 } from 'lang/std/sketch'
-import { normaliseAngle, roundOff, throttle } from 'lib/utils'
+import { isOverlap, normaliseAngle, roundOff, throttle } from 'lib/utils'
 import {
   createArrayExpression,
   createCallExpressionStdLib,
@@ -83,6 +83,7 @@ import {
   findUniqueName,
 } from 'lang/modifyAst'
 import {
+  Selections,
   getEventForSegmentSelection,
   sendSelectEventToEngine,
 } from 'lib/selections'
@@ -300,6 +301,7 @@ export class SceneEntities {
     position,
     maybeModdedAst,
     draftExpressionsIndices,
+    selectionRanges,
   }: {
     sketchPathToNode: PathToNode
     maybeModdedAst: Program
@@ -307,6 +309,7 @@ export class SceneEntities {
     forward: [number, number, number]
     up: [number, number, number]
     position?: [number, number, number]
+    selectionRanges?: Selections
   }): Promise<{
     truncatedAst: Program
     programMemoryOverride: ProgramMemory
@@ -396,6 +399,12 @@ export class SceneEntities {
         draftExpressionsIndices &&
         index <= draftExpressionsIndices.end &&
         index >= draftExpressionsIndices.start
+      const isSelected = selectionRanges?.codeBasedSelections.some(
+        (selection) => {
+          return isOverlap(selection.range, segment.__geoMeta.sourceRange)
+        }
+      )
+
       let seg
       const callExpName = getNodeFromPath<CallExpression>(
         maybeModdedAst,
@@ -413,6 +422,7 @@ export class SceneEntities {
           scale: factor,
           texture: sceneInfra.extraSegmentTexture,
           theme: sceneInfra._theme,
+          isSelected,
         })
         callbacks.push(
           this.updateTangentialArcToSegment({
@@ -434,6 +444,7 @@ export class SceneEntities {
           callExpName,
           texture: sceneInfra.extraSegmentTexture,
           theme: sceneInfra._theme,
+          isSelected,
         })
         callbacks.push(
           this.updateStraightSegment({

src/clientSideScene/segments.ts

@@ -45,18 +45,21 @@ export function profileStart({
   pathToNode,
   scale = 1,
   theme,
+  isSelected,
 }: {
   from: Coords2d
   id: string
   pathToNode: PathToNode
   scale?: number
   theme: Themes
+  isSelected?: boolean
 }) {
   const group = new Group()
 
   const geometry = new BoxGeometry(12, 12, 12) // in pixels scaled later
   const baseColor = getThemeColorForThreeJs(theme)
-  const body = new MeshBasicMaterial({ color: baseColor })
+  const color = isSelected ? 0x0000ff : baseColor
+  const body = new MeshBasicMaterial({ color })
   const mesh = new Mesh(geometry, body)
 
   group.add(mesh)
@@ -66,7 +69,8 @@ export function profileStart({
     id,
     from,
     pathToNode,
-    isSelected: false,
+    isSelected,
+    baseColor,
   }
   group.name = PROFILE_START
   group.position.set(from[0], from[1], 0)
@@ -84,6 +88,7 @@ export function straightSegment({
   callExpName,
   texture,
   theme,
+  isSelected = false,
 }: {
   from: Coords2d
   to: Coords2d
@@ -94,6 +99,7 @@ export function straightSegment({
   callExpName: string
   texture: Texture
   theme: Themes
+  isSelected?: boolean
 }): Group {
   const group = new Group()
 
@@ -119,7 +125,8 @@ export function straightSegment({
 
   const baseColor =
     callExpName === 'close' ? 0x444444 : getThemeColorForThreeJs(theme)
-  const body = new MeshBasicMaterial({ color: baseColor })
+  const color = isSelected ? 0x0000ff : baseColor
+  const body = new MeshBasicMaterial({ color })
   const mesh = new Mesh(geometry, body)
   mesh.userData.type = isDraftSegment
     ? STRAIGHT_SEGMENT_DASH
@@ -132,7 +139,7 @@ export function straightSegment({
     from,
     to,
     pathToNode,
-    isSelected: false,
+    isSelected,
     callExpName,
     baseColor,
   }
@@ -141,7 +148,7 @@ export function straightSegment({
   const length = Math.sqrt(
     Math.pow(to[0] - from[0], 2) + Math.pow(to[1] - from[1], 2)
   )
-  const arrowGroup = createArrowhead(scale, theme)
+  const arrowGroup = createArrowhead(scale, theme, color)
   arrowGroup.position.set(to[0], to[1], 0)
   const dir = new Vector3()
     .subVectors(new Vector3(to[0], to[1], 0), new Vector3(from[0], from[1], 0))
@@ -169,9 +176,10 @@ export function straightSegment({
   return group
 }
 
-function createArrowhead(scale = 1, theme: Themes): Group {
+function createArrowhead(scale = 1, theme: Themes, color?: number): Group {
+  const baseColor = getThemeColorForThreeJs(theme)
   const arrowMaterial = new MeshBasicMaterial({
-    color: getThemeColorForThreeJs(theme),
+    color: color || baseColor,
   })
   // specify the size of the geometry in pixels (i.e. cone height = 20px, cone radius = 4.5px)
   // we'll scale the group to the correct size later to match these sizes in screen space
@@ -232,6 +240,7 @@ export function tangentialArcToSegment({
   scale = 1,
   texture,
   theme,
+  isSelected,
 }: {
   prevSegment: SketchGroup['value'][number]
   from: Coords2d
@@ -242,6 +251,7 @@ export function tangentialArcToSegment({
   scale?: number
   texture: Texture
   theme: Themes
+  isSelected?: boolean
 }): Group {
   const group = new Group()
 
@@ -273,7 +283,8 @@ export function tangentialArcToSegment({
   })
 
   const baseColor = getThemeColorForThreeJs(theme)
-  const body = new MeshBasicMaterial({ color: baseColor })
+  const color = isSelected ? 0x0000ff : baseColor
+  const body = new MeshBasicMaterial({ color })
   const mesh = new Mesh(geometry, body)
   mesh.userData.type = isDraftSegment
     ? TANGENTIAL_ARC_TO__SEGMENT_DASH
@@ -286,12 +297,12 @@ export function tangentialArcToSegment({
     to,
     prevSegment,
     pathToNode,
-    isSelected: false,
+    isSelected,
     baseColor,
   }
   group.name = TANGENTIAL_ARC_TO_SEGMENT
 
-  const arrowGroup = createArrowhead(scale, theme)
+  const arrowGroup = createArrowhead(scale, theme, color)
   arrowGroup.position.set(to[0], to[1], 0)
   const arrowheadAngle = endAngle + (Math.PI / 2) * (ccw ? 1 : -1)
   arrowGroup.quaternion.setFromUnitVectors(

src/lib/selections.ts

@@ -278,7 +278,7 @@ export function processCodeMirrorRanges({
   }
 }
 
-function updateSceneObjectColors(codeBasedSelections: Selection[]) {
+export function updateSceneObjectColors(codeBasedSelections: Selection[]) {
   let updated: Program
   try {
     updated = parse(recast(kclManager.ast))
@@ -301,6 +301,7 @@ function updateSceneObjectColors(codeBasedSelections: Selection[]) {
     const groupHasCursor = codeBasedSelections.some((selection) => {
       return isOverlap(selection.range, [node.start, node.end])
     })
+
     const color = groupHasCursor
       ? 0x0000ff
       : segmentGroup?.userData?.baseColor || 0xffffff

src/machines/modelingMachine.ts

@@ -900,7 +900,10 @@ export const modelingMachine = createMachine(
           sceneInfra.modelingSend('Equip Line tool')
         }
       },
-      'setup client side sketch segments': ({ sketchDetails }) => {
+      'setup client side sketch segments': ({
+        sketchDetails,
+        selectionRanges,
+      }) => {
         if (!sketchDetails) return
         ;(async () => {
           if (Object.keys(sceneEntitiesManager.activeSegments).length > 0) {
@@ -913,6 +916,7 @@ export const modelingMachine = createMachine(
             up: sketchDetails.yAxis,
             position: sketchDetails.origin,
             maybeModdedAst: kclManager.ast,
+            selectionRanges,
           })
           sceneInfra.resetMouseListeners()
           sceneEntitiesManager.setupSketchIdleCallbacks({