diff --git a/e2e/playwright/editor-tests.spec.ts b/e2e/playwright/editor-tests.spec.ts index a4251e9e2..7bfb41636 100644 --- a/e2e/playwright/editor-tests.spec.ts +++ b/e2e/playwright/editor-tests.spec.ts @@ -76,7 +76,7 @@ test.describe('Editor tests', () => { await u.openDebugPanel() await expect( page.locator('[data-receive-command-type="scene_clear_all"]') - ).toHaveCount(2) + ).toHaveCount(1) await expect( page.locator('[data-message-type="execution-done"]') ).toHaveCount(2) @@ -100,7 +100,60 @@ test.describe('Editor tests', () => { ).toHaveCount(3) await expect( page.locator('[data-receive-command-type="scene_clear_all"]') + ).toHaveCount(1) + }) + + test('ensure we use the cache, and do not clear on append', async ({ + homePage, + page, + }) => { + const u = await getUtils(page) + await page.setBodyDimensions({ width: 1000, height: 500 }) + + await homePage.goToModelingScene() + await u.waitForPageLoad() + + await u.codeLocator.click() + await page.keyboard.type(`sketch001 = startSketchOn('XY') + |> startProfileAt([-10, -10], %) + |> line([20, 0], %) + |> line([0, 20], %) + |> line([-20, 0], %) + |> close(%)`) + + // Ensure we execute the first time. + await u.openDebugPanel() + await expect( + page.locator('[data-receive-command-type="scene_clear_all"]') + ).toHaveCount(1) + await expect( + page.locator('[data-message-type="execution-done"]') ).toHaveCount(2) + + // Add whitespace to the end of the code. + await u.codeLocator.click() + await page.keyboard.press('ArrowDown') + await page.keyboard.press('ArrowDown') + await page.keyboard.press('ArrowDown') + await page.keyboard.press('ArrowDown') + await page.keyboard.press('ArrowDown') + await page.keyboard.press('ArrowDown') + await page.keyboard.press('ArrowDown') + await page.keyboard.press('ArrowDown') + await page.keyboard.press('ArrowDown') + await page.keyboard.press('End') + await page.keyboard.press('Enter') + await page.keyboard.press('Enter') + await page.keyboard.type('const x = 1') + await page.keyboard.press('Enter') + + await u.openDebugPanel() + await expect( + page.locator('[data-message-type="execution-done"]') + ).toHaveCount(3) + await expect( + page.locator('[data-receive-command-type="scene_clear_all"]') + ).toHaveCount(1) }) test('if you click the format button it formats your code', async ({ diff --git a/src/wasm-lib/kcl/src/execution/mod.rs b/src/wasm-lib/kcl/src/execution/mod.rs index c48a71bbe..82378e4ed 100644 --- a/src/wasm-lib/kcl/src/execution/mod.rs +++ b/src/wasm-lib/kcl/src/execution/mod.rs @@ -43,6 +43,7 @@ use crate::{ settings::types::UnitLength, source_range::{ModuleId, SourceRange}, std::{args::Arg, StdLib}, + walk::Node as WalkNode, ExecError, Program, }; @@ -2002,9 +2003,12 @@ impl ExecutorContext { return None; } - let mut old_ast = old.ast.inner; + let mut old_ast = old.ast; + let mut new_ast = info.new_ast; + + // The digests should already be computed, but just in case we don't + // want to compare against none. old_ast.compute_digest(); - let mut new_ast = info.new_ast.inner.clone(); new_ast.compute_digest(); // Check if the digest is the same. @@ -2013,12 +2017,84 @@ impl ExecutorContext { } // Check if the changes were only to Non-code areas, like comments or whitespace. + Some(self.generate_changed_program(old_ast, new_ast)) + } - // For any unhandled cases just re-execute the whole thing. - Some(CacheResult { - clear_scene: true, - program: info.new_ast, - }) + /// Force-generate a new CacheResult, even if one shouldn't be made. The + /// way in which this gets invoked should always be through + /// [Self::get_changed_program]. This is purely to contain the logic on + /// how we construct a new [CacheResult]. + pub fn generate_changed_program(&self, old_ast: Node, new_ast: Node) -> CacheResult { + let mut generated_program = new_ast.clone(); + generated_program.body = vec![]; + + if !old_ast.body.iter().zip(new_ast.body.iter()).all(|(old, new)| { + let old_node: WalkNode = old.into(); + let new_node: WalkNode = new.into(); + old_node.digest() == new_node.digest() + }) { + // If any of the nodes are different in the stretch of body that + // overlaps, we have to bust cache and rebuild the scene. This + // means a single insertion or deletion will result in a cache + // bust. + + return CacheResult { + clear_scene: true, + program: new_ast, + }; + } + + // otherwise the overlapping section of the ast bodies matches. + // Let's see what the rest of the slice looks like. + + match new_ast.body.len().cmp(&old_ast.body.len()) { + std::cmp::Ordering::Less => { + // the new AST is shorter than the old AST -- statements + // were removed from the "current" code in the "new" code. + // + // Statements up until now match which means this is a + // "pure delete" of the remaining slice, when we get to + // supporting that. + + // Cache bust time. + CacheResult { + clear_scene: true, + program: new_ast, + } + } + std::cmp::Ordering::Greater => { + // the new AST is longer than the old AST, which means + // statements were added to the new code we haven't previously + // seen. + // + // Statements up until now are the same, which means this + // is a "pure addition" of the remaining slice. + + generated_program + .body + .extend_from_slice(&new_ast.body[old_ast.body.len()..]); + + CacheResult { + clear_scene: false, + program: generated_program, + } + } + std::cmp::Ordering::Equal => { + // currently unreachable, but lets pretend like the code + // above can do something meaningful here for when we get + // to diffing and yanking chunks of the program apart. + + // We don't actually want to do anything here; so we're going + // to not clear and do nothing. Is this wrong? I don't think + // so but i think many things. This def needs to change + // when the code above changes. + + CacheResult { + clear_scene: false, + program: generated_program, + } + } + } } /// Perform the execution of a program.