diff --git a/e2e/playwright/snapshot-tests.spec.ts b/e2e/playwright/snapshot-tests.spec.ts index c1f6ed89b..8dcc8a601 100644 --- a/e2e/playwright/snapshot-tests.spec.ts +++ b/e2e/playwright/snapshot-tests.spec.ts @@ -950,7 +950,75 @@ test( test.describe('Grid visibility', { tag: '@snapshot' }, () => { // FIXME: Skip on macos its being weird. - test.skip(process.platform === 'darwin', 'Skip on macos') + // test.skip(process.platform === 'darwin', 'Skip on macos') + + test('Grid turned off to on via command bar', async ({ page }) => { + const u = await getUtils(page) + const stream = page.getByTestId('stream') + const mask = [ + page.locator('#app-header'), + page.locator('#sidebar-top-ribbon'), + page.locator('#sidebar-bottom-ribbon'), + ] + + await page.setViewportSize({ width: 1200, height: 500 }) + await page.goto('/') + await u.waitForAuthSkipAppStart() + + await u.openDebugPanel() + // wait for execution done + await expect( + page.locator('[data-message-type="execution-done"]') + ).toHaveCount(1) + await u.closeDebugPanel() + await u.closeKclCodePanel() + // TODO: Find a way to truly know that the objects have finished + // rendering, because an execution-done message is not sufficient. + await page.waitForTimeout(1000) + + // Open the command bar. + await page + .getByRole('button', { name: 'Commands', exact: false }) + .or(page.getByRole('button', { name: '⌘K' })) + .click() + const commandName = 'show scale grid' + const commandOption = page.getByRole('option', { + name: commandName, + exact: false, + }) + const cmdSearchBar = page.getByPlaceholder('Search commands') + // This selector changes after we set the setting + await cmdSearchBar.fill(commandName) + await expect(commandOption).toBeVisible() + await commandOption.click() + + const toggleInput = page.getByPlaceholder('Off') + await expect(toggleInput).toBeVisible() + await expect(toggleInput).toBeFocused() + + // Select On + await page.keyboard.press('ArrowDown') + await expect(page.getByRole('option', { name: 'Off' })).toHaveAttribute( + 'data-headlessui-state', + 'active selected' + ) + await page.keyboard.press('ArrowUp') + await expect(page.getByRole('option', { name: 'On' })).toHaveAttribute( + 'data-headlessui-state', + 'active' + ) + await page.keyboard.press('Enter') + + // Check the toast appeared + await expect( + page.getByText(`Set show scale grid to "true" as a user default`) + ).toBeVisible() + + await expect(stream).toHaveScreenshot({ + maxDiffPixels: 100, + mask, + }) + }) test('Grid turned off', async ({ page }) => { const u = await getUtils(page) diff --git a/e2e/playwright/snapshot-tests.spec.ts-snapshots/Grid-visibility-Grid-turned-off-to-on-via-command-bar-1-Google-Chrome-linux.png b/e2e/playwright/snapshot-tests.spec.ts-snapshots/Grid-visibility-Grid-turned-off-to-on-via-command-bar-1-Google-Chrome-linux.png new file mode 100644 index 000000000..016d3c74c Binary files /dev/null and b/e2e/playwright/snapshot-tests.spec.ts-snapshots/Grid-visibility-Grid-turned-off-to-on-via-command-bar-1-Google-Chrome-linux.png differ diff --git a/e2e/playwright/snapshot-tests.spec.ts-snapshots/Grid-visibility-Grid-turned-off-to-on-via-command-bar-1-Google-Chrome-win32.png b/e2e/playwright/snapshot-tests.spec.ts-snapshots/Grid-visibility-Grid-turned-off-to-on-via-command-bar-1-Google-Chrome-win32.png new file mode 100644 index 000000000..e54c946b6 Binary files /dev/null and b/e2e/playwright/snapshot-tests.spec.ts-snapshots/Grid-visibility-Grid-turned-off-to-on-via-command-bar-1-Google-Chrome-win32.png differ diff --git a/src/components/LspProvider.tsx b/src/components/LspProvider.tsx index a302290d9..dd5b2d5b9 100644 --- a/src/components/LspProvider.tsx +++ b/src/components/LspProvider.tsx @@ -69,14 +69,7 @@ export const LspProvider = ({ children }: { children: React.ReactNode }) => { const [isKclLspReady, setIsKclLspReady] = useState(false) const [isCopilotLspReady, setIsCopilotLspReady] = useState(false) - const { - auth, - settings: { - context: { - modeling: { defaultUnit }, - }, - }, - } = useSettingsAuthContext() + const { auth } = useSettingsAuthContext() const token = auth?.context.token const navigate = useNavigate() @@ -92,7 +85,6 @@ export const LspProvider = ({ children }: { children: React.ReactNode }) => { const initEvent: KclWorkerOptions = { wasmUrl: wasmUrl(), token: token, - baseUnit: defaultUnit.current, apiBaseUrl: VITE_KC_API_BASE_URL, } lspWorker.postMessage({ diff --git a/src/components/SettingsAuthProvider.tsx b/src/components/SettingsAuthProvider.tsx index d04f2b930..4b1cd74f0 100644 --- a/src/components/SettingsAuthProvider.tsx +++ b/src/components/SettingsAuthProvider.tsx @@ -23,7 +23,6 @@ import { engineCommandManager, sceneEntitiesManager, } from 'lib/singletons' -import { uuidv4 } from 'lib/utils' import { IndexLoaderData } from 'lib/types' import { settings } from 'lib/settings/initialSettings' import { @@ -129,27 +128,11 @@ export const SettingsAuthProviderBase = ({ .setTheme(context.app.theme.current) .catch(reportRejection) }, - setEngineScaleGridVisibility: ({ context }) => { - engineCommandManager.setScaleGridVisibility( - context.modeling.showScaleGrid.current - ) - }, setClientTheme: ({ context }) => { const opposingTheme = getOppositeTheme(context.app.theme.current) sceneInfra.theme = opposingTheme sceneEntitiesManager.updateSegmentBaseColor(opposingTheme) }, - setEngineEdges: ({ context }) => { - // eslint-disable-next-line @typescript-eslint/no-floating-promises - engineCommandManager.sendSceneCommand({ - cmd_id: uuidv4(), - type: 'modeling_cmd_req', - cmd: { - type: 'edge_lines_visible' as any, // TODO update kittycad.ts to get this new command type - hidden: !context.modeling.highlightEdges.current, - }, - }) - }, toastSuccess: ({ event }) => { if (!('data' in event)) return const eventParts = event.type.replace(/^set./, '').split('.') as [ @@ -175,17 +158,27 @@ export const SettingsAuthProviderBase = ({ }, 'Execute AST': ({ context, event }) => { try { + const relevantSetting = (s: typeof settings) => { + return ( + s.modeling?.defaultUnit?.current !== + context.modeling.defaultUnit.current || + s.modeling.showScaleGrid.current !== + context.modeling.showScaleGrid.current || + s.modeling?.highlightEdges.current !== + context.modeling.highlightEdges.current + ) + } + const allSettingsIncludesUnitChange = event.type === 'Set all settings' && - event.settings?.modeling?.defaultUnit?.current !== - context.modeling.defaultUnit.current + relevantSetting(event.settings) const resetSettingsIncludesUnitChange = - event.type === 'Reset settings' && - context.modeling.defaultUnit.current !== - settings?.modeling?.defaultUnit?.default + event.type === 'Reset settings' && relevantSetting(settings) if ( event.type === 'set.modeling.defaultUnit' || + event.type === 'set.modeling.showScaleGrid' || + event.type === 'set.modeling.highlightEdges' || allSettingsIncludesUnitChange || resetSettingsIncludesUnitChange ) { diff --git a/src/editor/plugins/lsp/types.ts b/src/editor/plugins/lsp/types.ts index 417a5b6fe..b512de30f 100644 --- a/src/editor/plugins/lsp/types.ts +++ b/src/editor/plugins/lsp/types.ts @@ -1,7 +1,5 @@ import { LspWorkerEventType } from '@kittycad/codemirror-lsp-client' -import { UnitLength } from 'wasm-lib/kcl/bindings/UnitLength' - export enum LspWorker { Kcl = 'kcl', Copilot = 'copilot', @@ -9,7 +7,6 @@ export enum LspWorker { export interface KclWorkerOptions { wasmUrl: string token: string - baseUnit: UnitLength apiBaseUrl: string } diff --git a/src/editor/plugins/lsp/worker.ts b/src/editor/plugins/lsp/worker.ts index 950f8a435..ed91c5d20 100644 --- a/src/editor/plugins/lsp/worker.ts +++ b/src/editor/plugins/lsp/worker.ts @@ -17,7 +17,6 @@ import { KclWorkerOptions, CopilotWorkerOptions, } from 'editor/plugins/lsp/types' -import { EngineCommandManager } from 'lang/std/engineConnection' import { err, reportRejection } from 'lib/trap' const intoServer: IntoServer = new IntoServer() @@ -46,14 +45,12 @@ export async function copilotLspRun( export async function kclLspRun( config: ServerConfig, - engineCommandManager: EngineCommandManager | null, token: string, - baseUnit: string, baseUrl: string ) { try { console.log('start kcl lsp') - await kcl_lsp_run(config, engineCommandManager, baseUnit, token, baseUrl) + await kcl_lsp_run(config, null, undefined, token, baseUrl) } catch (e: any) { console.log('kcl lsp failed', e) // We can't restart here because a moved value, we should do this another way. @@ -82,13 +79,7 @@ onmessage = function (event: MessageEvent) { switch (worker) { case LspWorker.Kcl: const kclData = eventData as KclWorkerOptions - await kclLspRun( - config, - null, - kclData.token, - kclData.baseUnit, - kclData.apiBaseUrl - ) + await kclLspRun(config, kclData.token, kclData.apiBaseUrl) break case LspWorker.Copilot: let copilotData = eventData as CopilotWorkerOptions diff --git a/src/hooks/useSetupEngineManager.ts b/src/hooks/useSetupEngineManager.ts index a8095b1b3..cb23e666f 100644 --- a/src/hooks/useSetupEngineManager.ts +++ b/src/hooks/useSetupEngineManager.ts @@ -2,7 +2,7 @@ import { useLayoutEffect, useEffect, useRef } from 'react' import { engineCommandManager, kclManager } from 'lib/singletons' import { deferExecution } from 'lib/utils' import { Themes } from 'lib/theme' -import { makeDefaultPlanes, modifyGrid } from 'lang/wasm' +import { makeDefaultPlanes } from 'lang/wasm' import { useModelingContext } from './useModelingContext' import { useNetworkContext } from 'hooks/useNetworkContext' import { useAppState, useAppStream } from 'AppState' @@ -56,9 +56,6 @@ export function useSetupEngineManager( makeDefaultPlanes: () => { return makeDefaultPlanes(kclManager.engineCommandManager) }, - modifyGrid: (hidden: boolean) => { - return modifyGrid(kclManager.engineCommandManager, hidden) - }, }) hasSetNonZeroDimensions.current = true } diff --git a/src/lang/modifyAst/addEdgeTreatment.test.ts b/src/lang/modifyAst/addEdgeTreatment.test.ts index d30d57389..481b965f3 100644 --- a/src/lang/modifyAst/addEdgeTreatment.test.ts +++ b/src/lang/modifyAst/addEdgeTreatment.test.ts @@ -40,7 +40,6 @@ beforeAll(async () => { makeDefaultPlanes: () => makeDefaultPlanes(engineCommandManager), setMediaStream: () => {}, setIsStreamReady: () => {}, - modifyGrid: async () => {}, callbackOnEngineLiteConnect: () => { resolve(true) }, diff --git a/src/lang/std/artifactGraph.test.ts b/src/lang/std/artifactGraph.test.ts index 8f841a1d7..fd4117b17 100644 --- a/src/lang/std/artifactGraph.test.ts +++ b/src/lang/std/artifactGraph.test.ts @@ -139,7 +139,6 @@ beforeAll(async () => { makeDefaultPlanes: () => makeDefaultPlanes(engineCommandManager), setMediaStream: () => {}, setIsStreamReady: () => {}, - modifyGrid: async () => {}, // eslint-disable-next-line @typescript-eslint/no-misused-promises callbackOnEngineLiteConnect: async () => { const cacheEntries = Object.entries(codeToWriteCacheFor) as [ diff --git a/src/lang/std/engineConnection.ts b/src/lang/std/engineConnection.ts index 8aed52896..5a92dd155 100644 --- a/src/lang/std/engineConnection.ts +++ b/src/lang/std/engineConnection.ts @@ -1399,7 +1399,6 @@ export class EngineCommandManager extends EventTarget { } private makeDefaultPlanes: () => Promise | null = () => null - private modifyGrid: (hidden: boolean) => Promise | null = () => null private onEngineConnectionOpened = () => {} private onEngineConnectionClosed = () => {} @@ -1432,7 +1431,6 @@ export class EngineCommandManager extends EventTarget { height, token, makeDefaultPlanes, - modifyGrid, settings = { pool: null, theme: Themes.Dark, @@ -1452,14 +1450,12 @@ export class EngineCommandManager extends EventTarget { height: number token?: string makeDefaultPlanes: () => Promise - modifyGrid: (hidden: boolean) => Promise settings?: SettingsViaQueryString }) { if (settings) { this.settings = settings } this.makeDefaultPlanes = makeDefaultPlanes - this.modifyGrid = modifyGrid if (width === 0 || height === 0) { return } @@ -1539,21 +1535,15 @@ export class EngineCommandManager extends EventTarget { type: 'default_camera_get_settings', }, }) - // We want modify the grid first because we don't want it to flash. - // Ideally these would already be default hidden in engine (TODO do - // that) https://github.com/KittyCAD/engine/issues/2282 - // eslint-disable-next-line @typescript-eslint/no-floating-promises - this.modifyGrid(!this.settings.showScaleGrid)?.then(async () => { - await this.initPlanes() - setIsStreamReady(true) + await this.initPlanes() + setIsStreamReady(true) - // Other parts of the application should use this to react on scene ready. - this.dispatchEvent( - new CustomEvent(EngineCommandManagerEvents.SceneReady, { - detail: this.engineConnection, - }) - ) - }) + // Other parts of the application should use this to react on scene ready. + this.dispatchEvent( + new CustomEvent(EngineCommandManagerEvents.SceneReady, { + detail: this.engineConnection, + }) + ) } this.engineConnection.addEventListener( @@ -2212,15 +2202,6 @@ export class EngineCommandManager extends EventTarget { }).catch(reportRejection) } - /** - * Set the visibility of the scale grid in the engine scene. - * @param visible - whether to show or hide the scale grid - */ - setScaleGridVisibility(visible: boolean) { - // eslint-disable-next-line @typescript-eslint/no-floating-promises - this.modifyGrid(!visible) - } - // Some "objects" have the same source range, such as sketch_mode_start and start_path. // So when passing a range, we need to also specify the command type mapRangeToObjectId( diff --git a/src/lang/wasm.ts b/src/lang/wasm.ts index ddc599f36..4a16adb0c 100644 --- a/src/lang/wasm.ts +++ b/src/lang/wasm.ts @@ -1,14 +1,13 @@ import init, { parse_wasm, recast_wasm, - execute_wasm, + execute, kcl_lint, modify_ast_for_sketch_wasm, is_points_ccw, get_tangential_arc_to_info, program_memory_init, make_default_planes, - modify_grid, coredump, toml_stringify, default_app_settings, @@ -43,7 +42,9 @@ import { Environment } from '../wasm-lib/kcl/bindings/Environment' import { Node } from 'wasm-lib/kcl/bindings/Node' import { CompilationError } from 'wasm-lib/kcl/bindings/CompilationError' import { SourceRange as RustSourceRange } from 'wasm-lib/kcl/bindings/SourceRange' +import { getChangedSettingsAtLevel } from 'lib/settings/settingsUtils' +export type { Configuration } from 'wasm-lib/kcl/bindings/Configuration' export type { Program } from '../wasm-lib/kcl/bindings/Program' export type { Expr } from '../wasm-lib/kcl/bindings/Expr' export type { ObjectExpression } from '../wasm-lib/kcl/bindings/ObjectExpression' @@ -493,18 +494,20 @@ export const _executor = async ( return Promise.reject(programMemoryOverride) try { - let baseUnit = 'mm' + let jsAppSettings = default_app_settings() if (!TEST) { const getSettingsState = import('components/SettingsAuthProvider').then( (module) => module.getSettingsState ) - baseUnit = - (await getSettingsState)()?.modeling.defaultUnit.current || 'mm' + const settings = (await getSettingsState)() + if (settings) { + jsAppSettings = getChangedSettingsAtLevel(settings, 'user') + } } - const execState: RawExecState = await execute_wasm( + const execState: RawExecState = await execute( JSON.stringify(node), JSON.stringify(programMemoryOverride?.toRaw() || null), - baseUnit, + JSON.stringify({ settings: jsAppSettings }), engineCommandManager, fileSystemManager ) @@ -552,20 +555,6 @@ export const makeDefaultPlanes = async ( } } -export const modifyGrid = async ( - engineCommandManager: EngineCommandManager, - hidden: boolean -): Promise => { - try { - await modify_grid(engineCommandManager, hidden) - return - } catch (e) { - // TODO: do something real with the error. - console.log('modify grid error', e) - return Promise.reject(e) - } -} - export const modifyAstForSketch = async ( engineCommandManager: EngineCommandManager, ast: Node, diff --git a/src/lib/testHelpers.ts b/src/lib/testHelpers.ts index 4f545ad11..7453086ae 100644 --- a/src/lib/testHelpers.ts +++ b/src/lib/testHelpers.ts @@ -112,9 +112,6 @@ export async function executor( makeDefaultPlanes: () => { return new Promise((resolve) => resolve(defaultPlanes)) }, - modifyGrid: (hidden: boolean) => { - return new Promise((resolve) => resolve()) - }, }) return new Promise((resolve) => { diff --git a/src/machines/settingsMachine.ts b/src/machines/settingsMachine.ts index 571aa606d..1c432c923 100644 --- a/src/machines/settingsMachine.ts +++ b/src/machines/settingsMachine.ts @@ -42,8 +42,6 @@ export const settingsMachine = setup({ setClientTheme: () => {}, 'Execute AST': () => {}, toastSuccess: () => {}, - setEngineEdges: () => {}, - setEngineScaleGridVisibility: () => {}, setClientSideSceneUnits: () => {}, persistSettings: () => {}, resetSettings: assign(({ context, event }) => { @@ -172,7 +170,7 @@ export const settingsMachine = setup({ 'set.modeling.highlightEdges': { target: 'persisting settings', - actions: ['setSettingAtLevel', 'toastSuccess', 'setEngineEdges'], + actions: ['setSettingAtLevel', 'toastSuccess', 'Execute AST'], }, 'Reset settings': { @@ -201,11 +199,7 @@ export const settingsMachine = setup({ 'set.modeling.showScaleGrid': { target: 'persisting settings', - actions: [ - 'setSettingAtLevel', - 'toastSuccess', - 'setEngineScaleGridVisibility', - ], + actions: ['setSettingAtLevel', 'toastSuccess', 'Execute AST'], }, }, }, diff --git a/src/wasm-lib/kcl/src/engine/mod.rs b/src/wasm-lib/kcl/src/engine/mod.rs index f41cbaf67..9e1d315f9 100644 --- a/src/wasm-lib/kcl/src/engine/mod.rs +++ b/src/wasm-lib/kcl/src/engine/mod.rs @@ -120,6 +120,61 @@ pub trait EngineManager: std::fmt::Debug + Send + Sync + 'static { Ok(()) } + /// Set the visibility of edges. + async fn set_edge_visibility( + &self, + visible: bool, + source_range: SourceRange, + ) -> Result<(), crate::errors::KclError> { + self.batch_modeling_cmd( + uuid::Uuid::new_v4(), + source_range, + &ModelingCmd::from(mcmd::EdgeLinesVisible { hidden: !visible }), + ) + .await?; + + Ok(()) + } + + async fn set_units( + &self, + units: crate::UnitLength, + source_range: SourceRange, + ) -> Result<(), crate::errors::KclError> { + // Before we even start executing the program, set the units. + self.batch_modeling_cmd( + uuid::Uuid::new_v4(), + source_range, + &ModelingCmd::from(mcmd::SetSceneUnits { unit: units.into() }), + ) + .await?; + + Ok(()) + } + + /// Re-run the command to apply the settings. + async fn reapply_settings( + &self, + settings: &crate::ExecutorSettings, + source_range: SourceRange, + ) -> Result<(), crate::errors::KclError> { + // Set the edge visibility. + self.set_edge_visibility(settings.highlight_edges, source_range).await?; + + // Change the units. + self.set_units(settings.units, source_range).await?; + + // Send the command to show the grid. + self.modify_grid(!settings.show_grid, source_range).await?; + + // We do not have commands for changing ssao on the fly. + + // Flush the batch queue, so the settings are applied right away. + self.flush_batch(false, source_range).await?; + + Ok(()) + } + // Add a modeling command to the batch but don't fire it right away. async fn batch_modeling_cmd( &self, @@ -504,11 +559,11 @@ pub trait EngineManager: std::fmt::Debug + Send + Sync + 'static { })) } - async fn modify_grid(&self, hidden: bool) -> Result<(), KclError> { + async fn modify_grid(&self, hidden: bool, source_range: SourceRange) -> Result<(), KclError> { // Hide/show the grid. self.batch_modeling_cmd( uuid::Uuid::new_v4(), - Default::default(), + source_range, &ModelingCmd::from(mcmd::ObjectVisible { hidden, object_id: *GRID_OBJECT_ID, @@ -519,7 +574,7 @@ pub trait EngineManager: std::fmt::Debug + Send + Sync + 'static { // Hide/show the grid scale text. self.batch_modeling_cmd( uuid::Uuid::new_v4(), - Default::default(), + source_range, &ModelingCmd::from(mcmd::ObjectVisible { hidden, object_id: *GRID_SCALE_TEXT_OBJECT_ID, @@ -527,8 +582,6 @@ pub trait EngineManager: std::fmt::Debug + Send + Sync + 'static { ) .await?; - self.flush_batch(false, Default::default()).await?; - Ok(()) } diff --git a/src/wasm-lib/kcl/src/execution/cache.rs b/src/wasm-lib/kcl/src/execution/cache.rs new file mode 100644 index 000000000..ea8ed1b5c --- /dev/null +++ b/src/wasm-lib/kcl/src/execution/cache.rs @@ -0,0 +1,50 @@ +//! Functions for helping with caching an ast and finding the parts the changed. + +use schemars::JsonSchema; +use serde::{Deserialize, Serialize}; + +use crate::{ + execution::ExecState, + parsing::ast::types::{Node, Program}, +}; + +/// Information for the caching an AST and smartly re-executing it if we can. +#[derive(Debug, Clone, Deserialize, Serialize, PartialEq, ts_rs::TS, JsonSchema)] +#[ts(export)] +pub struct CacheInformation { + /// The old information. + pub old: Option, + /// The new ast to executed. + pub new_ast: Node, +} + +/// The old ast and program memory. +#[derive(Debug, Clone, Deserialize, Serialize, PartialEq, ts_rs::TS, JsonSchema)] +#[ts(export)] +pub struct OldAstState { + /// The ast. + pub ast: Node, + /// The exec state. + pub exec_state: ExecState, + /// The last settings used for execution. + pub settings: crate::execution::ExecutorSettings, +} + +impl From for CacheInformation { + fn from(program: crate::Program) -> Self { + CacheInformation { + old: None, + new_ast: program.ast, + } + } +} + +/// The result of a cache check. +#[derive(Debug, Clone, Deserialize, Serialize, PartialEq, ts_rs::TS, JsonSchema)] +#[ts(export)] +pub struct CacheResult { + /// Should we clear the scene and start over? + pub clear_scene: bool, + /// The program that needs to be executed. + pub program: Node, +} diff --git a/src/wasm-lib/kcl/src/execution/mod.rs b/src/wasm-lib/kcl/src/execution/mod.rs index b9bb28040..07c3e75ba 100644 --- a/src/wasm-lib/kcl/src/execution/mod.rs +++ b/src/wasm-lib/kcl/src/execution/mod.rs @@ -23,15 +23,18 @@ type Point3D = kcmc::shared::Point3d; pub use function_param::FunctionParam; pub use kcl_value::{KclObjectFields, KclValue}; +pub(crate) mod cache; +mod exec_ast; +mod function_param; +mod kcl_value; + use crate::{ engine::{EngineManager, ExecutionKind}, errors::{KclError, KclErrorDetails}, + execution::cache::{CacheInformation, CacheResult}, fs::{FileManager, FileSystem}, - parsing::ast::{ - cache::{get_changed_program, CacheInformation}, - types::{ - BodyItem, Expr, FunctionExpression, ImportSelector, ItemVisibility, Node, NodeRef, TagDeclarator, TagNode, - }, + parsing::ast::types::{ + BodyItem, Expr, FunctionExpression, ImportSelector, ItemVisibility, Node, NodeRef, TagDeclarator, TagNode, }, settings::types::UnitLength, source_range::{ModuleId, SourceRange}, @@ -39,10 +42,6 @@ use crate::{ ExecError, Program, }; -mod exec_ast; -mod function_param; -mod kcl_value; - /// State for executing a program. #[derive(Debug, Default, Clone, Deserialize, Serialize, PartialEq, ts_rs::TS, JsonSchema)] #[ts(export)] @@ -1654,17 +1653,6 @@ impl ExecutorContext { let engine: Arc> = Arc::new(Box::new(crate::engine::conn::EngineConnection::new(ws).await?)); - // Set the edge visibility. - engine - .batch_modeling_cmd( - uuid::Uuid::new_v4(), - SourceRange::default(), - &ModelingCmd::from(mcmd::EdgeLinesVisible { - hidden: !settings.highlight_edges, - }), - ) - .await?; - Ok(Self { engine, fs: Arc::new(FileManager::new()), @@ -1691,7 +1679,7 @@ impl ExecutorContext { pub async fn new( engine_manager: crate::engine::conn_wasm::EngineCommandManager, fs_manager: crate::fs::wasm::FileSystemManager, - units: UnitLength, + settings: ExecutorSettings, ) -> Result { Ok(ExecutorContext { engine: Arc::new(Box::new( @@ -1701,16 +1689,16 @@ impl ExecutorContext { )), fs: Arc::new(FileManager::new(fs_manager)), stdlib: Arc::new(StdLib::new()), - settings: ExecutorSettings { - units, - ..Default::default() - }, + settings, context_type: ContextType::Live, }) } #[cfg(target_arch = "wasm32")] - pub async fn new_mock(fs_manager: crate::fs::wasm::FileSystemManager, units: UnitLength) -> Result { + pub async fn new_mock( + fs_manager: crate::fs::wasm::FileSystemManager, + settings: ExecutorSettings, + ) -> Result { Ok(ExecutorContext { engine: Arc::new(Box::new( crate::engine::conn_mock::EngineConnection::new() @@ -1719,10 +1707,7 @@ impl ExecutorContext { )), fs: Arc::new(FileManager::new(fs_manager)), stdlib: Arc::new(StdLib::new()), - settings: ExecutorSettings { - units, - ..Default::default() - }, + settings, context_type: ContextType::Mock, }) } @@ -1811,6 +1796,71 @@ impl ExecutorContext { // AND if we aren't in wasm it doesn't really matter. Ok(()) } + // Given an old ast, old program memory and new ast, find the parts of the code that need to be + // re-executed. + // This function should never error, because in the case of any internal error, we should just pop + // the cache. + pub async fn get_changed_program(&self, info: CacheInformation) -> Option { + let Some(old) = info.old else { + // We have no old info, we need to re-execute the whole thing. + return Some(CacheResult { + clear_scene: true, + program: info.new_ast, + }); + }; + + // If the settings are different we might need to bust the cache. + // We specifically do this before checking if they are the exact same. + if old.settings != self.settings { + // If the units are different we need to re-execute the whole thing. + if old.settings.units != self.settings.units { + return Some(CacheResult { + clear_scene: true, + program: info.new_ast, + }); + } + + // If anything else is different we do not need to re-execute, but rather just + // run the settings again. + + if self + .engine + .reapply_settings(&self.settings, Default::default()) + .await + .is_err() + { + // Bust the cache, we errored. + return Some(CacheResult { + clear_scene: true, + program: info.new_ast, + }); + } + } + + // If the ASTs are the EXACT same we return None. + // We don't even need to waste time computing the digests. + if old.ast == info.new_ast { + return None; + } + + let mut old_ast = old.ast.inner; + old_ast.compute_digest(); + let mut new_ast = info.new_ast.inner.clone(); + new_ast.compute_digest(); + + // Check if the digest is the same. + if old_ast.digest == new_ast.digest { + return None; + } + + // Check if the changes were only to Non-code areas, like comments or whitespace. + + // For any unhandled cases just re-execute the whole thing. + Some(CacheResult { + clear_scene: true, + program: info.new_ast, + }) + } /// Perform the execution of a program. /// You can optionally pass in some initialization memory. @@ -1831,7 +1881,7 @@ impl ExecutorContext { let _stats = crate::log::LogPerfStats::new("Interpretation"); // Get the program that actually changed from the old and new information. - let cache_result = get_changed_program(cache_info.clone(), &self.settings); + let cache_result = self.get_changed_program(cache_info.clone()).await; // Check if we don't need to re-execute. let Some(cache_result) = cache_result else { @@ -1848,23 +1898,9 @@ impl ExecutorContext { // TODO: Use the top-level file's path. exec_state.add_module(std::path::PathBuf::from("")); - // Before we even start executing the program, set the units. - self.engine - .batch_modeling_cmd( - exec_state.id_generator.next_uuid(), - SourceRange::default(), - &ModelingCmd::from(mcmd::SetSceneUnits { - unit: match self.settings.units { - UnitLength::Cm => kcmc::units::UnitLength::Centimeters, - UnitLength::Ft => kcmc::units::UnitLength::Feet, - UnitLength::In => kcmc::units::UnitLength::Inches, - UnitLength::M => kcmc::units::UnitLength::Meters, - UnitLength::Mm => kcmc::units::UnitLength::Millimeters, - UnitLength::Yd => kcmc::units::UnitLength::Yards, - }, - }), - ) - .await?; + + // Re-apply the settings, in case the cache was busted. + self.engine.reapply_settings(&self.settings, Default::default()).await?; self.inner_execute(&cache_result.program, exec_state, crate::execution::BodyType::Root) .await?; @@ -2141,23 +2177,8 @@ impl ExecutorContext { self.settings.units = units; } - /// Execute the program, then get a PNG screenshot. - pub async fn execute_and_prepare_snapshot( - &self, - program: &Program, - exec_state: &mut ExecState, - ) -> std::result::Result { - self.execute_and_prepare(program, exec_state).await - } - - /// Execute the program, return the interpreter and outputs. - pub async fn execute_and_prepare( - &self, - program: &Program, - exec_state: &mut ExecState, - ) -> std::result::Result { - self.run(program.clone().into(), exec_state).await?; - + /// Get a snapshot of the current scene. + pub async fn prepare_snapshot(&self) -> std::result::Result { // Zoom to fit. self.engine .send_modeling_cmd( @@ -2193,6 +2214,17 @@ impl ExecutorContext { }; Ok(contents) } + + /// Execute the program, then get a PNG screenshot. + pub async fn execute_and_prepare_snapshot( + &self, + program: &Program, + exec_state: &mut ExecState, + ) -> std::result::Result { + self.run(program.clone().into(), exec_state).await?; + + self.prepare_snapshot().await + } } /// For each argument given, @@ -2289,9 +2321,12 @@ mod tests { use pretty_assertions::assert_eq; use super::*; - use crate::parsing::ast::types::{DefaultParamVal, Identifier, Node, Parameter}; + use crate::{ + parsing::ast::types::{DefaultParamVal, Identifier, Node, Parameter}, + OldAstState, + }; - pub async fn parse_execute(code: &str) -> Result { + pub async fn parse_execute(code: &str) -> Result<(Program, ExecutorContext, ExecState)> { let program = Program::parse_no_errs(code)?; let ctx = ExecutorContext { @@ -2302,9 +2337,9 @@ mod tests { context_type: ContextType::Mock, }; let mut exec_state = ExecState::default(); - ctx.run(program.into(), &mut exec_state).await?; + ctx.run(program.clone().into(), &mut exec_state).await?; - Ok(exec_state.memory) + Ok((program, ctx, exec_state)) } /// Convenience function to get a JSON value from memory and unwrap. @@ -2715,36 +2750,39 @@ let shape = layer() |> patternTransform(10, transform, %) #[tokio::test(flavor = "multi_thread")] async fn test_math_execute_with_functions() { let ast = r#"const myVar = 2 + min(100, -1 + legLen(5, 3))"#; - let memory = parse_execute(ast).await.unwrap(); - assert_eq!(5.0, mem_get_json(&memory, "myVar").as_f64().unwrap()); + let (_, _, exec_state) = parse_execute(ast).await.unwrap(); + assert_eq!(5.0, mem_get_json(&exec_state.memory, "myVar").as_f64().unwrap()); } #[tokio::test(flavor = "multi_thread")] async fn test_math_execute() { let ast = r#"const myVar = 1 + 2 * (3 - 4) / -5 + 6"#; - let memory = parse_execute(ast).await.unwrap(); - assert_eq!(7.4, mem_get_json(&memory, "myVar").as_f64().unwrap()); + let (_, _, exec_state) = parse_execute(ast).await.unwrap(); + assert_eq!(7.4, mem_get_json(&exec_state.memory, "myVar").as_f64().unwrap()); } #[tokio::test(flavor = "multi_thread")] async fn test_math_execute_start_negative() { let ast = r#"const myVar = -5 + 6"#; - let memory = parse_execute(ast).await.unwrap(); - assert_eq!(1.0, mem_get_json(&memory, "myVar").as_f64().unwrap()); + let (_, _, exec_state) = parse_execute(ast).await.unwrap(); + assert_eq!(1.0, mem_get_json(&exec_state.memory, "myVar").as_f64().unwrap()); } #[tokio::test(flavor = "multi_thread")] async fn test_math_execute_with_pi() { let ast = r#"const myVar = pi() * 2"#; - let memory = parse_execute(ast).await.unwrap(); - assert_eq!(std::f64::consts::TAU, mem_get_json(&memory, "myVar").as_f64().unwrap()); + let (_, _, exec_state) = parse_execute(ast).await.unwrap(); + assert_eq!( + std::f64::consts::TAU, + mem_get_json(&exec_state.memory, "myVar").as_f64().unwrap() + ); } #[tokio::test(flavor = "multi_thread")] async fn test_math_define_decimal_without_leading_zero() { let ast = r#"let thing = .4 + 7"#; - let memory = parse_execute(ast).await.unwrap(); - assert_eq!(7.4, mem_get_json(&memory, "thing").as_f64().unwrap()); + let (_, _, exec_state) = parse_execute(ast).await.unwrap(); + assert_eq!(7.4, mem_get_json(&exec_state.memory, "thing").as_f64().unwrap()); } #[tokio::test(flavor = "multi_thread")] @@ -2783,11 +2821,11 @@ fn check = (x) => { } check(false) "#; - let mem = parse_execute(ast).await.unwrap(); - assert_eq!(false, mem_get_json(&mem, "notTrue").as_bool().unwrap()); - assert_eq!(true, mem_get_json(&mem, "notFalse").as_bool().unwrap()); - assert_eq!(true, mem_get_json(&mem, "c").as_bool().unwrap()); - assert_eq!(false, mem_get_json(&mem, "d").as_bool().unwrap()); + let (_, _, exec_state) = parse_execute(ast).await.unwrap(); + assert_eq!(false, mem_get_json(&exec_state.memory, "notTrue").as_bool().unwrap()); + assert_eq!(true, mem_get_json(&exec_state.memory, "notFalse").as_bool().unwrap()); + assert_eq!(true, mem_get_json(&exec_state.memory, "c").as_bool().unwrap()); + assert_eq!(false, mem_get_json(&exec_state.memory, "d").as_bool().unwrap()); } #[tokio::test(flavor = "multi_thread")] @@ -3167,4 +3205,310 @@ let w = f() + f() let json = serde_json::to_string(&mem).unwrap(); assert_eq!(json, r#"{"type":"Solids","value":[]}"#); } + + // Easy case where we have no old ast and memory. + // We need to re-execute everything. + #[tokio::test(flavor = "multi_thread")] + async fn test_get_changed_program_no_old_information() { + let new = r#"// Remove the end face for the extrusion. +firstSketch = startSketchOn('XY') + |> startProfileAt([-12, 12], %) + |> line([24, 0], %) + |> line([0, -24], %) + |> line([-24, 0], %) + |> close(%) + |> extrude(6, %) + +// Remove the end face for the extrusion. +shell({ faces = ['end'], thickness = 0.25 }, firstSketch)"#; + let (program, ctx, _) = parse_execute(new).await.unwrap(); + + let result = ctx + .get_changed_program(CacheInformation { + old: None, + new_ast: program.ast.clone(), + }) + .await; + + assert!(result.is_some()); + + let result = result.unwrap(); + + assert_eq!(result.program, program.ast); + assert!(result.clear_scene); + } + + #[tokio::test(flavor = "multi_thread")] + async fn test_get_changed_program_same_code() { + let new = r#"// Remove the end face for the extrusion. +firstSketch = startSketchOn('XY') + |> startProfileAt([-12, 12], %) + |> line([24, 0], %) + |> line([0, -24], %) + |> line([-24, 0], %) + |> close(%) + |> extrude(6, %) + +// Remove the end face for the extrusion. +shell({ faces = ['end'], thickness = 0.25 }, firstSketch)"#; + + let (program, ctx, exec_state) = parse_execute(new).await.unwrap(); + + let result = ctx + .get_changed_program(CacheInformation { + old: Some(OldAstState { + ast: program.ast.clone(), + exec_state, + settings: Default::default(), + }), + new_ast: program.ast.clone(), + }) + .await; + + assert_eq!(result, None); + } + + #[tokio::test(flavor = "multi_thread")] + async fn test_get_changed_program_same_code_changed_whitespace() { + let old = r#" // Remove the end face for the extrusion. +firstSketch = startSketchOn('XY') + |> startProfileAt([-12, 12], %) + |> line([24, 0], %) + |> line([0, -24], %) + |> line([-24, 0], %) + |> close(%) + |> extrude(6, %) + +// Remove the end face for the extrusion. +shell({ faces = ['end'], thickness = 0.25 }, firstSketch) "#; + + let new = r#"// Remove the end face for the extrusion. +firstSketch = startSketchOn('XY') + |> startProfileAt([-12, 12], %) + |> line([24, 0], %) + |> line([0, -24], %) + |> line([-24, 0], %) + |> close(%) + |> extrude(6, %) + +// Remove the end face for the extrusion. +shell({ faces = ['end'], thickness = 0.25 }, firstSketch)"#; + + let (program_old, ctx, exec_state) = parse_execute(old).await.unwrap(); + + let program_new = crate::Program::parse_no_errs(new).unwrap(); + + let result = ctx + .get_changed_program(CacheInformation { + old: Some(OldAstState { + ast: program_old.ast.clone(), + exec_state, + settings: Default::default(), + }), + new_ast: program_new.ast.clone(), + }) + .await; + + assert_eq!(result, None); + } + + #[tokio::test(flavor = "multi_thread")] + async fn test_get_changed_program_same_code_changed_code_comment_start_of_program() { + let old = r#" // Removed the end face for the extrusion. +firstSketch = startSketchOn('XY') + |> startProfileAt([-12, 12], %) + |> line([24, 0], %) + |> line([0, -24], %) + |> line([-24, 0], %) + |> close(%) + |> extrude(6, %) + +// Remove the end face for the extrusion. +shell({ faces = ['end'], thickness = 0.25 }, firstSketch) "#; + + let new = r#"// Remove the end face for the extrusion. +firstSketch = startSketchOn('XY') + |> startProfileAt([-12, 12], %) + |> line([24, 0], %) + |> line([0, -24], %) + |> line([-24, 0], %) + |> close(%) + |> extrude(6, %) + +// Remove the end face for the extrusion. +shell({ faces = ['end'], thickness = 0.25 }, firstSketch)"#; + + let (program, ctx, exec_state) = parse_execute(old).await.unwrap(); + + let program_new = crate::Program::parse_no_errs(new).unwrap(); + + let result = ctx + .get_changed_program(CacheInformation { + old: Some(OldAstState { + ast: program.ast.clone(), + exec_state, + settings: Default::default(), + }), + new_ast: program_new.ast.clone(), + }) + .await; + + assert_eq!(result, None); + } + + #[tokio::test(flavor = "multi_thread")] + async fn test_get_changed_program_same_code_changed_code_comments() { + let old = r#" // Removed the end face for the extrusion. +firstSketch = startSketchOn('XY') + |> startProfileAt([-12, 12], %) + |> line([24, 0], %) + |> line([0, -24], %) + |> line([-24, 0], %) // my thing + |> close(%) + |> extrude(6, %) + +// Remove the end face for the extrusion. +shell({ faces = ['end'], thickness = 0.25 }, firstSketch) "#; + + let new = r#"// Remove the end face for the extrusion. +firstSketch = startSketchOn('XY') + |> startProfileAt([-12, 12], %) + |> line([24, 0], %) + |> line([0, -24], %) + |> line([-24, 0], %) + |> close(%) + |> extrude(6, %) + +// Remove the end face for the extrusion. +shell({ faces = ['end'], thickness = 0.25 }, firstSketch)"#; + + let (program, ctx, exec_state) = parse_execute(old).await.unwrap(); + + let program_new = crate::Program::parse_no_errs(new).unwrap(); + + let result = ctx + .get_changed_program(CacheInformation { + old: Some(OldAstState { + ast: program.ast.clone(), + exec_state, + settings: Default::default(), + }), + new_ast: program_new.ast.clone(), + }) + .await; + + assert!(result.is_some()); + + let result = result.unwrap(); + + assert_eq!(result.program, program_new.ast); + assert!(result.clear_scene); + } + + // Changing the units with the exact same file should bust the cache. + #[tokio::test(flavor = "multi_thread")] + async fn test_get_changed_program_same_code_but_different_units() { + let new = r#"// Remove the end face for the extrusion. +firstSketch = startSketchOn('XY') + |> startProfileAt([-12, 12], %) + |> line([24, 0], %) + |> line([0, -24], %) + |> line([-24, 0], %) + |> close(%) + |> extrude(6, %) + +// Remove the end face for the extrusion. +shell({ faces = ['end'], thickness = 0.25 }, firstSketch)"#; + + let (program, mut ctx, exec_state) = parse_execute(new).await.unwrap(); + + // Change the settings to cm. + ctx.settings.units = crate::UnitLength::Cm; + + let result = ctx + .get_changed_program(CacheInformation { + old: Some(OldAstState { + ast: program.ast.clone(), + exec_state, + settings: Default::default(), + }), + new_ast: program.ast.clone(), + }) + .await; + + assert!(result.is_some()); + + let result = result.unwrap(); + + assert_eq!(result.program, program.ast); + assert!(result.clear_scene); + } + + // Changing the grid settings with the exact same file should NOT bust the cache. + #[tokio::test(flavor = "multi_thread")] + async fn test_get_changed_program_same_code_but_different_grid_setting() { + let new = r#"// Remove the end face for the extrusion. +firstSketch = startSketchOn('XY') + |> startProfileAt([-12, 12], %) + |> line([24, 0], %) + |> line([0, -24], %) + |> line([-24, 0], %) + |> close(%) + |> extrude(6, %) + +// Remove the end face for the extrusion. +shell({ faces = ['end'], thickness = 0.25 }, firstSketch)"#; + + let (program, mut ctx, exec_state) = parse_execute(new).await.unwrap(); + + // Change the settings. + ctx.settings.show_grid = !ctx.settings.show_grid; + + let result = ctx + .get_changed_program(CacheInformation { + old: Some(OldAstState { + ast: program.ast.clone(), + exec_state, + settings: Default::default(), + }), + new_ast: program.ast.clone(), + }) + .await; + + assert_eq!(result, None); + } + + // Changing the edge visibility settings with the exact same file should NOT bust the cache. + #[tokio::test(flavor = "multi_thread")] + async fn test_get_changed_program_same_code_but_different_edge_visiblity_setting() { + let new = r#"// Remove the end face for the extrusion. +firstSketch = startSketchOn('XY') + |> startProfileAt([-12, 12], %) + |> line([24, 0], %) + |> line([0, -24], %) + |> line([-24, 0], %) + |> close(%) + |> extrude(6, %) + +// Remove the end face for the extrusion. +shell({ faces = ['end'], thickness = 0.25 }, firstSketch)"#; + + let (program, mut ctx, exec_state) = parse_execute(new).await.unwrap(); + + // Change the settings. + ctx.settings.highlight_edges = !ctx.settings.highlight_edges; + + let result = ctx + .get_changed_program(CacheInformation { + old: Some(OldAstState { + ast: program.ast.clone(), + exec_state, + settings: Default::default(), + }), + new_ast: program.ast.clone(), + }) + .await; + + assert_eq!(result, None); + } } diff --git a/src/wasm-lib/kcl/src/lib.rs b/src/wasm-lib/kcl/src/lib.rs index 529a12cb8..35f190b54 100644 --- a/src/wasm-lib/kcl/src/lib.rs +++ b/src/wasm-lib/kcl/src/lib.rs @@ -82,16 +82,15 @@ mod wasm; pub use coredump::CoreDump; pub use engine::{EngineManager, ExecutionKind}; pub use errors::{CompilationError, ConnectionError, ExecError, KclError}; -pub use execution::{ExecState, ExecutorContext, ExecutorSettings}; +pub use execution::{ + cache::{CacheInformation, OldAstState}, + ExecState, ExecutorContext, ExecutorSettings, +}; pub use lsp::{ copilot::Backend as CopilotLspBackend, kcl::{Backend as KclLspBackend, Server as KclLspServerSubCommand}, }; -pub use parsing::ast::{ - cache::{CacheInformation, OldAstState}, - modify::modify_ast_for_sketch, - types::FormatOptions, -}; +pub use parsing::ast::{modify::modify_ast_for_sketch, types::FormatOptions}; pub use settings::types::{project::ProjectConfiguration, Configuration, UnitLength}; pub use source_range::{ModuleId, SourceRange}; diff --git a/src/wasm-lib/kcl/src/lsp/kcl/mod.rs b/src/wasm-lib/kcl/src/lsp/kcl/mod.rs index c440d9061..20e9580ff 100644 --- a/src/wasm-lib/kcl/src/lsp/kcl/mod.rs +++ b/src/wasm-lib/kcl/src/lsp/kcl/mod.rs @@ -45,14 +45,11 @@ use crate::{ errors::Suggestion, lsp::{backend::Backend as _, util::IntoDiagnostic}, parsing::{ - ast::{ - cache::{CacheInformation, OldAstState}, - types::{Expr, Node, VariableKind}, - }, + ast::types::{Expr, Node, VariableKind}, token::TokenType, PIPE_OPERATOR, }, - ModuleId, Program, SourceRange, + CacheInformation, ModuleId, OldAstState, Program, SourceRange, }; lazy_static::lazy_static! { diff --git a/src/wasm-lib/kcl/src/parsing/ast/cache.rs b/src/wasm-lib/kcl/src/parsing/ast/cache.rs deleted file mode 100644 index 37383e6e5..000000000 --- a/src/wasm-lib/kcl/src/parsing/ast/cache.rs +++ /dev/null @@ -1,373 +0,0 @@ -//! Functions for helping with caching an ast and finding the parts the changed. - -use schemars::JsonSchema; -use serde::{Deserialize, Serialize}; - -use crate::{ - execution::ExecState, - parsing::ast::types::{Node, Program}, -}; - -/// Information for the caching an AST and smartly re-executing it if we can. -#[derive(Debug, Clone, Deserialize, Serialize, PartialEq, ts_rs::TS, JsonSchema)] -#[ts(export)] -pub struct CacheInformation { - /// The old information. - pub old: Option, - /// The new ast to executed. - pub new_ast: Node, -} - -/// The old ast and program memory. -#[derive(Debug, Clone, Deserialize, Serialize, PartialEq, ts_rs::TS, JsonSchema)] -#[ts(export)] -pub struct OldAstState { - /// The ast. - pub ast: Node, - /// The exec state. - pub exec_state: ExecState, - /// The last settings used for execution. - pub settings: crate::execution::ExecutorSettings, -} - -impl From for CacheInformation { - fn from(program: crate::Program) -> Self { - CacheInformation { - old: None, - new_ast: program.ast, - } - } -} - -/// The result of a cache check. -#[derive(Debug, Clone, Deserialize, Serialize, PartialEq, ts_rs::TS, JsonSchema)] -#[ts(export)] -pub struct CacheResult { - /// Should we clear the scene and start over? - pub clear_scene: bool, - /// The program that needs to be executed. - pub program: Node, -} - -// Given an old ast, old program memory and new ast, find the parts of the code that need to be -// re-executed. -// This function should never error, because in the case of any internal error, we should just pop -// the cache. -pub fn get_changed_program( - info: CacheInformation, - new_settings: &crate::execution::ExecutorSettings, -) -> Option { - let Some(old) = info.old else { - // We have no old info, we need to re-execute the whole thing. - return Some(CacheResult { - clear_scene: true, - program: info.new_ast, - }); - }; - - // If the settings are different we need to bust the cache. - // We specifically do this before checking if they are the exact same. - if old.settings != *new_settings { - return Some(CacheResult { - clear_scene: true, - program: info.new_ast, - }); - } - - // If the ASTs are the EXACT same we return None. - // We don't even need to waste time computing the digests. - if old.ast == info.new_ast { - return None; - } - - let mut old_ast = old.ast.inner; - old_ast.compute_digest(); - let mut new_ast = info.new_ast.inner.clone(); - new_ast.compute_digest(); - - // Check if the digest is the same. - if old_ast.digest == new_ast.digest { - return None; - } - - // Check if the changes were only to Non-code areas, like comments or whitespace. - - // For any unhandled cases just re-execute the whole thing. - Some(CacheResult { - clear_scene: true, - program: info.new_ast, - }) -} - -#[cfg(test)] -mod tests { - use std::sync::Arc; - - use anyhow::Result; - use pretty_assertions::assert_eq; - - use super::*; - - async fn execute(program: &crate::Program) -> Result { - let ctx = crate::execution::ExecutorContext { - engine: Arc::new(Box::new(crate::engine::conn_mock::EngineConnection::new().await?)), - fs: Arc::new(crate::fs::FileManager::new()), - stdlib: Arc::new(crate::std::StdLib::new()), - settings: Default::default(), - context_type: crate::execution::ContextType::Mock, - }; - let mut exec_state = crate::execution::ExecState::default(); - ctx.run(program.clone().into(), &mut exec_state).await?; - - Ok(exec_state) - } - - // Easy case where we have no old ast and memory. - // We need to re-execute everything. - #[test] - fn test_get_changed_program_no_old_information() { - let new = r#"// Remove the end face for the extrusion. -firstSketch = startSketchOn('XY') - |> startProfileAt([-12, 12], %) - |> line([24, 0], %) - |> line([0, -24], %) - |> line([-24, 0], %) - |> close(%) - |> extrude(6, %) - -// Remove the end face for the extrusion. -shell({ faces = ['end'], thickness = 0.25 }, firstSketch)"#; - let program = crate::Program::parse_no_errs(new).unwrap().ast; - - let result = get_changed_program( - CacheInformation { - old: None, - new_ast: program.clone(), - }, - &Default::default(), - ); - - assert!(result.is_some()); - - let result = result.unwrap(); - - assert_eq!(result.program, program); - assert!(result.clear_scene); - } - - #[tokio::test(flavor = "multi_thread")] - async fn test_get_changed_program_same_code() { - let new = r#"// Remove the end face for the extrusion. -firstSketch = startSketchOn('XY') - |> startProfileAt([-12, 12], %) - |> line([24, 0], %) - |> line([0, -24], %) - |> line([-24, 0], %) - |> close(%) - |> extrude(6, %) - -// Remove the end face for the extrusion. -shell({ faces = ['end'], thickness = 0.25 }, firstSketch)"#; - let program = crate::Program::parse_no_errs(new).unwrap(); - - let executed = execute(&program).await.unwrap(); - - let result = get_changed_program( - CacheInformation { - old: Some(OldAstState { - ast: program.ast.clone(), - exec_state: executed, - settings: Default::default(), - }), - new_ast: program.ast.clone(), - }, - &Default::default(), - ); - - assert_eq!(result, None); - } - - #[tokio::test(flavor = "multi_thread")] - async fn test_get_changed_program_same_code_changed_whitespace() { - let old = r#" // Remove the end face for the extrusion. -firstSketch = startSketchOn('XY') - |> startProfileAt([-12, 12], %) - |> line([24, 0], %) - |> line([0, -24], %) - |> line([-24, 0], %) - |> close(%) - |> extrude(6, %) - -// Remove the end face for the extrusion. -shell({ faces = ['end'], thickness = 0.25 }, firstSketch) "#; - - let new = r#"// Remove the end face for the extrusion. -firstSketch = startSketchOn('XY') - |> startProfileAt([-12, 12], %) - |> line([24, 0], %) - |> line([0, -24], %) - |> line([-24, 0], %) - |> close(%) - |> extrude(6, %) - -// Remove the end face for the extrusion. -shell({ faces = ['end'], thickness = 0.25 }, firstSketch)"#; - let program_old = crate::Program::parse_no_errs(old).unwrap(); - - let executed = execute(&program_old).await.unwrap(); - - let program_new = crate::Program::parse_no_errs(new).unwrap(); - - let result = get_changed_program( - CacheInformation { - old: Some(OldAstState { - ast: program_old.ast.clone(), - exec_state: executed, - settings: Default::default(), - }), - new_ast: program_new.ast.clone(), - }, - &Default::default(), - ); - - assert_eq!(result, None); - } - - #[tokio::test(flavor = "multi_thread")] - async fn test_get_changed_program_same_code_changed_code_comment_start_of_program() { - let old = r#" // Removed the end face for the extrusion. -firstSketch = startSketchOn('XY') - |> startProfileAt([-12, 12], %) - |> line([24, 0], %) - |> line([0, -24], %) - |> line([-24, 0], %) - |> close(%) - |> extrude(6, %) - -// Remove the end face for the extrusion. -shell({ faces = ['end'], thickness = 0.25 }, firstSketch) "#; - - let new = r#"// Remove the end face for the extrusion. -firstSketch = startSketchOn('XY') - |> startProfileAt([-12, 12], %) - |> line([24, 0], %) - |> line([0, -24], %) - |> line([-24, 0], %) - |> close(%) - |> extrude(6, %) - -// Remove the end face for the extrusion. -shell({ faces = ['end'], thickness = 0.25 }, firstSketch)"#; - let program_old = crate::Program::parse_no_errs(old).unwrap(); - - let executed = execute(&program_old).await.unwrap(); - - let program_new = crate::Program::parse_no_errs(new).unwrap(); - - let result = get_changed_program( - CacheInformation { - old: Some(OldAstState { - ast: program_old.ast.clone(), - exec_state: executed, - settings: Default::default(), - }), - new_ast: program_new.ast.clone(), - }, - &Default::default(), - ); - - assert_eq!(result, None); - } - - #[tokio::test(flavor = "multi_thread")] - async fn test_get_changed_program_same_code_changed_code_comments() { - let old = r#" // Removed the end face for the extrusion. -firstSketch = startSketchOn('XY') - |> startProfileAt([-12, 12], %) - |> line([24, 0], %) - |> line([0, -24], %) - |> line([-24, 0], %) // my thing - |> close(%) - |> extrude(6, %) - -// Remove the end face for the extrusion. -shell({ faces = ['end'], thickness = 0.25 }, firstSketch) "#; - - let new = r#"// Remove the end face for the extrusion. -firstSketch = startSketchOn('XY') - |> startProfileAt([-12, 12], %) - |> line([24, 0], %) - |> line([0, -24], %) - |> line([-24, 0], %) - |> close(%) - |> extrude(6, %) - -// Remove the end face for the extrusion. -shell({ faces = ['end'], thickness = 0.25 }, firstSketch)"#; - let program_old = crate::Program::parse_no_errs(old).unwrap(); - - let executed = execute(&program_old).await.unwrap(); - - let program_new = crate::Program::parse_no_errs(new).unwrap(); - - let result = get_changed_program( - CacheInformation { - old: Some(OldAstState { - ast: program_old.ast.clone(), - exec_state: executed, - settings: Default::default(), - }), - new_ast: program_new.ast.clone(), - }, - &Default::default(), - ); - - assert!(result.is_some()); - - let result = result.unwrap(); - - assert_eq!(result.program, program_new.ast); - assert!(result.clear_scene); - } - - // Changing the units with the exact same file should bust the cache. - #[tokio::test(flavor = "multi_thread")] - async fn test_get_changed_program_same_code_but_different_units() { - let new = r#"// Remove the end face for the extrusion. -firstSketch = startSketchOn('XY') - |> startProfileAt([-12, 12], %) - |> line([24, 0], %) - |> line([0, -24], %) - |> line([-24, 0], %) - |> close(%) - |> extrude(6, %) - -// Remove the end face for the extrusion. -shell({ faces = ['end'], thickness = 0.25 }, firstSketch)"#; - let program = crate::Program::parse_no_errs(new).unwrap(); - - let executed = execute(&program).await.unwrap(); - - let result = get_changed_program( - CacheInformation { - old: Some(OldAstState { - ast: program.ast.clone(), - exec_state: executed, - settings: Default::default(), - }), - new_ast: program.ast.clone(), - }, - &crate::ExecutorSettings { - units: crate::UnitLength::Cm, - ..Default::default() - }, - ); - - assert!(result.is_some()); - - let result = result.unwrap(); - - assert_eq!(result.program, program.ast); - assert!(result.clear_scene); - } -} diff --git a/src/wasm-lib/kcl/src/parsing/ast/mod.rs b/src/wasm-lib/kcl/src/parsing/ast/mod.rs index f6ba6e245..c44a59265 100644 --- a/src/wasm-lib/kcl/src/parsing/ast/mod.rs +++ b/src/wasm-lib/kcl/src/parsing/ast/mod.rs @@ -1,4 +1,3 @@ -pub(crate) mod cache; pub(crate) mod digest; pub mod modify; pub mod types; diff --git a/src/wasm-lib/kcl/src/test_server.rs b/src/wasm-lib/kcl/src/test_server.rs index f2ae47f32..f7246145f 100644 --- a/src/wasm-lib/kcl/src/test_server.rs +++ b/src/wasm-lib/kcl/src/test_server.rs @@ -55,7 +55,11 @@ async fn do_execute_and_snapshot( program: Program, ) -> Result<(crate::execution::ExecState, image::DynamicImage), ExecError> { let mut exec_state = Default::default(); - let snapshot_png_bytes = ctx.execute_and_prepare(&program, &mut exec_state).await?.contents.0; + let snapshot_png_bytes = ctx + .execute_and_prepare_snapshot(&program, &mut exec_state) + .await? + .contents + .0; // Decode the snapshot, return it. let img = image::ImageReader::new(std::io::Cursor::new(snapshot_png_bytes)) diff --git a/src/wasm-lib/src/wasm.rs b/src/wasm-lib/src/wasm.rs index 5fb9cb5e2..022efb835 100644 --- a/src/wasm-lib/src/wasm.rs +++ b/src/wasm-lib/src/wasm.rs @@ -1,6 +1,6 @@ //! Wasm bindings for `kcl`. -use std::{str::FromStr, sync::Arc}; +use std::sync::Arc; use futures::stream::TryStreamExt; use gloo_utils::format::JsValueSerdeExt; @@ -56,10 +56,10 @@ pub async fn clear_scene_and_bust_cache( // wasm_bindgen wrapper for execute #[wasm_bindgen] -pub async fn execute_wasm( +pub async fn execute( program_ast_json: &str, program_memory_override_str: &str, - units: &str, + settings: &str, engine_manager: kcl_lib::wasm_engine::EngineCommandManager, fs_manager: kcl_lib::wasm_engine::FileSystemManager, ) -> Result { @@ -73,11 +73,11 @@ pub async fn execute_wasm( // You cannot override the memory in non-mock mode. let is_mock = program_memory_override.is_some(); - let units = kcl_lib::UnitLength::from_str(units).map_err(|e| e.to_string())?; + let settings: kcl_lib::Configuration = serde_json::from_str(settings).map_err(|e| e.to_string())?; let ctx = if is_mock { - kcl_lib::ExecutorContext::new_mock(fs_manager, units).await? + kcl_lib::ExecutorContext::new_mock(fs_manager, settings.into()).await? } else { - kcl_lib::ExecutorContext::new(engine_manager, fs_manager, units).await? + kcl_lib::ExecutorContext::new(engine_manager, fs_manager, settings.into()).await? }; let mut exec_state = ExecState::default(); @@ -168,23 +168,6 @@ pub async fn make_default_planes( JsValue::from_serde(&default_planes).map_err(|e| e.to_string()) } -// wasm_bindgen wrapper for modifying the grid -#[wasm_bindgen] -pub async fn modify_grid( - engine_manager: kcl_lib::wasm_engine::EngineCommandManager, - hidden: bool, -) -> Result<(), String> { - console_error_panic_hook::set_once(); - // deserialize the ast from a stringified json - - let engine = kcl_lib::wasm_engine::EngineConnection::new(engine_manager) - .await - .map_err(|e| format!("{:?}", e))?; - engine.modify_grid(hidden).await.map_err(String::from)?; - - Ok(()) -} - // wasm_bindgen wrapper for execute #[wasm_bindgen] pub async fn modify_ast_for_sketch_wasm( @@ -296,7 +279,7 @@ impl ServerConfig { pub async fn kcl_lsp_run( config: ServerConfig, engine_manager: Option, - units: &str, + settings: Option, token: String, baseurl: String, ) -> Result<(), JsValue> { @@ -309,8 +292,12 @@ pub async fn kcl_lsp_run( } = config; let executor_ctx = if let Some(engine_manager) = engine_manager { - let units = kcl_lib::UnitLength::from_str(units).map_err(|e| e.to_string())?; - Some(kcl_lib::ExecutorContext::new(engine_manager, fs.clone(), units).await?) + let settings: kcl_lib::Configuration = if let Some(settings) = settings { + serde_json::from_str(&settings).map_err(|e| e.to_string())? + } else { + Default::default() + }; + Some(kcl_lib::ExecutorContext::new(engine_manager, fs.clone(), settings.into()).await?) } else { None }; diff --git a/src/wasm-lib/tests/executor/cache.rs b/src/wasm-lib/tests/executor/cache.rs new file mode 100644 index 000000000..6c6a76174 --- /dev/null +++ b/src/wasm-lib/tests/executor/cache.rs @@ -0,0 +1,216 @@ +//! Cache testing framework. + +use anyhow::Result; +use kcl_lib::ExecError; + +struct Variation<'a> { + code: &'a str, + settings: &'a kcl_lib::ExecutorSettings, +} + +async fn cache_test(test_name: &str, variations: Vec>) -> Result> { + let first = variations + .first() + .ok_or_else(|| anyhow::anyhow!("No variations provided for test '{}'", test_name))?; + + let mut ctx = kcl_lib::ExecutorContext::new_with_client(first.settings.clone(), None, None).await?; + let mut exec_state = kcl_lib::ExecState::default(); + + let mut old_ast_state = None; + let mut img_results = Vec::new(); + for (index, variation) in variations.iter().enumerate() { + let program = kcl_lib::Program::parse_no_errs(variation.code)?; + + // set the new settings. + ctx.settings = variation.settings.clone(); + + ctx.run( + kcl_lib::CacheInformation { + old: old_ast_state, + new_ast: program.ast.clone(), + }, + &mut exec_state, + ) + .await?; + let snapshot_png_bytes = ctx.prepare_snapshot().await?.contents.0; + + // Decode the snapshot, return it. + let img = image::ImageReader::new(std::io::Cursor::new(snapshot_png_bytes)) + .with_guessed_format() + .map_err(|e| ExecError::BadPng(e.to_string())) + .and_then(|x| x.decode().map_err(|e| ExecError::BadPng(e.to_string())))?; + // Save the snapshot. + let path = crate::assert_out(&format!("cache_{}_{}", test_name, index), &img); + + img_results.push((path, img)); + + // Prepare the last state. + old_ast_state = Some(kcl_lib::OldAstState { + ast: program.ast, + exec_state: exec_state.clone(), + settings: variation.settings.clone(), + }); + } + + Ok(img_results) +} + +#[tokio::test(flavor = "multi_thread")] +async fn kcl_test_cache_change_units_changes_output() { + let code = r#"part001 = startSketchOn('XY') + |> startProfileAt([5.5229, 5.25217], %) + |> line([10.50433, -1.19122], %) + |> line([8.01362, -5.48731], %) + |> line([-1.02877, -6.76825], %) + |> line([-11.53311, 2.81559], %) + |> close(%) + |> extrude(4, %) +"#; + + let result = cache_test( + "change_units_changes_output", + vec![ + Variation { + code, + settings: &kcl_lib::ExecutorSettings { + units: kcl_lib::UnitLength::In, + ..Default::default() + }, + }, + Variation { + code, + settings: &kcl_lib::ExecutorSettings { + units: kcl_lib::UnitLength::Mm, + ..Default::default() + }, + }, + ], + ) + .await + .unwrap(); + + let first = result.first().unwrap(); + let second = result.last().unwrap(); + + assert!(first.1 != second.1); +} + +#[tokio::test(flavor = "multi_thread")] +async fn kcl_test_cache_change_grid_visualizes_grid_off_to_on() { + let code = r#"part001 = startSketchOn('XY') + |> startProfileAt([5.5229, 5.25217], %) + |> line([10.50433, -1.19122], %) + |> line([8.01362, -5.48731], %) + |> line([-1.02877, -6.76825], %) + |> line([-11.53311, 2.81559], %) + |> close(%) + |> extrude(4, %) +"#; + + let result = cache_test( + "change_grid_visualizes_grid_off_to_on", + vec![ + Variation { + code, + settings: &kcl_lib::ExecutorSettings { + show_grid: false, + ..Default::default() + }, + }, + Variation { + code, + settings: &kcl_lib::ExecutorSettings { + show_grid: true, + ..Default::default() + }, + }, + ], + ) + .await + .unwrap(); + + let first = result.first().unwrap(); + let second = result.last().unwrap(); + + assert!(first.1 != second.1); +} + +#[tokio::test(flavor = "multi_thread")] +async fn kcl_test_cache_change_grid_visualizes_grid_on_to_off() { + let code = r#"part001 = startSketchOn('XY') + |> startProfileAt([5.5229, 5.25217], %) + |> line([10.50433, -1.19122], %) + |> line([8.01362, -5.48731], %) + |> line([-1.02877, -6.76825], %) + |> line([-11.53311, 2.81559], %) + |> close(%) + |> extrude(4, %) +"#; + + let result = cache_test( + "change_grid_visualizes_grid_on_to_off", + vec![ + Variation { + code, + settings: &kcl_lib::ExecutorSettings { + show_grid: true, + ..Default::default() + }, + }, + Variation { + code, + settings: &kcl_lib::ExecutorSettings { + show_grid: false, + ..Default::default() + }, + }, + ], + ) + .await + .unwrap(); + + let first = result.first().unwrap(); + let second = result.last().unwrap(); + + assert!(first.1 != second.1); +} + +#[tokio::test(flavor = "multi_thread")] +async fn kcl_test_cache_change_highlight_edges_changes_visual() { + let code = r#"part001 = startSketchOn('XY') + |> startProfileAt([5.5229, 5.25217], %) + |> line([10.50433, -1.19122], %) + |> line([8.01362, -5.48731], %) + |> line([-1.02877, -6.76825], %) + |> line([-11.53311, 2.81559], %) + |> close(%) + |> extrude(4, %) +"#; + + let result = cache_test( + "change_highlight_edges_changes_visual", + vec![ + Variation { + code, + settings: &kcl_lib::ExecutorSettings { + highlight_edges: true, + ..Default::default() + }, + }, + Variation { + code, + settings: &kcl_lib::ExecutorSettings { + highlight_edges: false, + ..Default::default() + }, + }, + ], + ) + .await + .unwrap(); + + let first = result.first().unwrap(); + let second = result.last().unwrap(); + + assert!(first.1 != second.1); +} diff --git a/src/wasm-lib/tests/executor/main.rs b/src/wasm-lib/tests/executor/main.rs index 6aa5c3a02..3006fcf1c 100644 --- a/src/wasm-lib/tests/executor/main.rs +++ b/src/wasm-lib/tests/executor/main.rs @@ -1,3 +1,5 @@ +mod cache; + use kcl_lib::{ test_server::{execute_and_snapshot, execute_and_snapshot_no_auth}, UnitLength, @@ -5,7 +7,7 @@ use kcl_lib::{ /// The minimum permissible difference between asserted twenty-twenty images. /// i.e. how different the current model snapshot can be from the previous saved one. -const MIN_DIFF: f64 = 0.99; +pub(crate) const MIN_DIFF: f64 = 0.99; macro_rules! kcl_input { ($file:literal) => { @@ -13,8 +15,11 @@ macro_rules! kcl_input { }; } -fn assert_out(test_name: &str, result: &image::DynamicImage) { - twenty_twenty::assert_image(format!("tests/executor/outputs/{test_name}.png"), result, MIN_DIFF); +pub(crate) fn assert_out(test_name: &str, result: &image::DynamicImage) -> String { + let path = format!("tests/executor/outputs/{test_name}.png"); + twenty_twenty::assert_image(&path, result, MIN_DIFF); + + path } #[tokio::test(flavor = "multi_thread")] diff --git a/src/wasm-lib/tests/executor/outputs/cache_change_grid_visualizes_grid_off_to_on_0.png b/src/wasm-lib/tests/executor/outputs/cache_change_grid_visualizes_grid_off_to_on_0.png new file mode 100644 index 000000000..8f97850b7 Binary files /dev/null and b/src/wasm-lib/tests/executor/outputs/cache_change_grid_visualizes_grid_off_to_on_0.png differ diff --git a/src/wasm-lib/tests/executor/outputs/cache_change_grid_visualizes_grid_off_to_on_1.png b/src/wasm-lib/tests/executor/outputs/cache_change_grid_visualizes_grid_off_to_on_1.png new file mode 100644 index 000000000..3425e99c6 Binary files /dev/null and b/src/wasm-lib/tests/executor/outputs/cache_change_grid_visualizes_grid_off_to_on_1.png differ diff --git a/src/wasm-lib/tests/executor/outputs/cache_change_grid_visualizes_grid_on_to_off_0.png b/src/wasm-lib/tests/executor/outputs/cache_change_grid_visualizes_grid_on_to_off_0.png new file mode 100644 index 000000000..10a5d3ae3 Binary files /dev/null and b/src/wasm-lib/tests/executor/outputs/cache_change_grid_visualizes_grid_on_to_off_0.png differ diff --git a/src/wasm-lib/tests/executor/outputs/cache_change_grid_visualizes_grid_on_to_off_1.png b/src/wasm-lib/tests/executor/outputs/cache_change_grid_visualizes_grid_on_to_off_1.png new file mode 100644 index 000000000..8f97850b7 Binary files /dev/null and b/src/wasm-lib/tests/executor/outputs/cache_change_grid_visualizes_grid_on_to_off_1.png differ diff --git a/src/wasm-lib/tests/executor/outputs/cache_change_highlight_edges_changes_visual_0.png b/src/wasm-lib/tests/executor/outputs/cache_change_highlight_edges_changes_visual_0.png new file mode 100644 index 000000000..8f97850b7 Binary files /dev/null and b/src/wasm-lib/tests/executor/outputs/cache_change_highlight_edges_changes_visual_0.png differ diff --git a/src/wasm-lib/tests/executor/outputs/cache_change_highlight_edges_changes_visual_1.png b/src/wasm-lib/tests/executor/outputs/cache_change_highlight_edges_changes_visual_1.png new file mode 100644 index 000000000..0deafe039 Binary files /dev/null and b/src/wasm-lib/tests/executor/outputs/cache_change_highlight_edges_changes_visual_1.png differ diff --git a/src/wasm-lib/tests/executor/outputs/cache_change_units_changes_output_0.png b/src/wasm-lib/tests/executor/outputs/cache_change_units_changes_output_0.png new file mode 100644 index 000000000..fa87beb72 Binary files /dev/null and b/src/wasm-lib/tests/executor/outputs/cache_change_units_changes_output_0.png differ diff --git a/src/wasm-lib/tests/executor/outputs/cache_change_units_changes_output_1.png b/src/wasm-lib/tests/executor/outputs/cache_change_units_changes_output_1.png new file mode 100644 index 000000000..8f97850b7 Binary files /dev/null and b/src/wasm-lib/tests/executor/outputs/cache_change_units_changes_output_1.png differ