diff --git a/frontend/src/app/main/data/workspace/modifiers.cljs b/frontend/src/app/main/data/workspace/modifiers.cljs index 2cee1752a2..328b366bfb 100644 --- a/frontend/src/app/main/data/workspace/modifiers.cljs +++ b/frontend/src/app/main/data/workspace/modifiers.cljs @@ -21,7 +21,6 @@ [app.common.types.component :as ctk] [app.common.types.container :as ctn] [app.common.types.modifiers :as ctm] - [app.common.types.path :as path] [app.common.types.shape-tree :as ctst] [app.common.types.shape.attrs :refer [editable-attrs]] [app.common.types.shape.layout :as ctl] @@ -885,6 +884,18 @@ ids (into (set (keys modif-tree)) xf:without-uuid-zero (keys transforms)) + options + (cond-> options + translation? + (assoc :resize-ids + (into [] + (remove (fn [id] + (let [parent-id (dm/get-in objects [id :parent-id])] + (and (contains? ids parent-id) + (= (get transforms id) + (get transforms parent-id)))))) + ids))) + update-shape (fn [shape] (let [shape-id (dm/get-prop shape :id) @@ -894,14 +905,6 @@ (gsh/apply-transform transform) (ctm/apply-structure-modifiers modifiers)))) - bool-ids - (into #{} - (comp - (mapcat (partial cfh/get-parents-with-self objects)) - (filter cfh/bool-shape?) - (map :id)) - ids) - undo-id (js/Symbol)] (rx/concat @@ -912,16 +915,7 @@ (clear-local-transform) (ptk/event ::dwg/move-frame-guides {:ids ids :transforms transforms}) (ptk/event ::dwcm/move-frame-comment-threads transforms) - (dwsh/update-shapes ids update-shape options) - - ;; The update to the bool path needs to be in a different operation because it - ;; needs to have the updated children info. - ;; `update-layout? false`: recalculating a bool path can never change - ;; `:hidden`, and the layout check would recompute the whole boolean - ;; path in WASM once per bool shape just to find that out. - (dwsh/update-shapes bool-ids path/update-bool-shape (assoc options - :with-objects? true - :update-layout? false))) + (dwsh/update-shapes ids update-shape options)) (if undo-transation? (rx/of (dwu/commit-undo-transaction undo-id)) diff --git a/frontend/src/app/main/data/workspace/shapes.cljs b/frontend/src/app/main/data/workspace/shapes.cljs index 447a192a7d..0b195be68b 100644 --- a/frontend/src/app/main/data/workspace/shapes.cljs +++ b/frontend/src/app/main/data/workspace/shapes.cljs @@ -110,7 +110,7 @@ ([ids update-fn {:keys [reg-objects? save-undo? stack-undo? attrs ignore-tree page-id ignore-touched undo-group with-objects? changed-sub-attr changed-item-index - translation? skip-grid-reassignment? skip-component-sync?] + translation? skip-grid-reassignment? skip-component-sync? resize-ids] :or {reg-objects? false save-undo? true stack-undo? false @@ -153,7 +153,7 @@ :ignore-touched ignore-touched :with-objects? with-objects? :skip-grid-reassignment? skip-grid-reassignment?}) - (cond-> reg-objects? (pcb/resize-parents ids)) + (cond-> reg-objects? (pcb/resize-parents (or resize-ids ids))) (pcb/set-translation? translation?) (pcb/set-skip-component-sync? skip-component-sync?))))] ;; Check buffered text candidates when the buffer is committed. @@ -191,7 +191,7 @@ {:as props :keys [reg-objects? save-undo? stack-undo? attrs ignore-tree page-id ignore-touched undo-group with-objects? changed-sub-attr changed-item-index translation? - skip-grid-reassignment? skip-component-sync?] + skip-grid-reassignment? skip-component-sync? resize-ids] :or {reg-objects? false save-undo? true stack-undo? false @@ -250,7 +250,7 @@ (rx/empty)) (if (seq (:redo-changes changes)) - (let [changes (cond-> changes reg-objects? (pcb/resize-parents ids))] + (let [changes (cond-> changes reg-objects? (pcb/resize-parents (or resize-ids ids)))] (rx/of (dch/commit-changes changes))) (rx/empty)) diff --git a/frontend/test/frontend_tests/logic/bool_move_test.cljs b/frontend/test/frontend_tests/logic/bool_move_test.cljs new file mode 100644 index 0000000000..5191d4e98e --- /dev/null +++ b/frontend/test/frontend_tests/logic/bool_move_test.cljs @@ -0,0 +1,99 @@ +;; This Source Code Form is subject to the terms of the Mozilla Public +;; License, v. 2.0. If a copy of the MPL was not distributed with this +;; file, You can obtain one at http://mozilla.org/MPL/2.0/. +;; +;; Copyright (c) KALEIDOS SUBSIDIARY SL + +(ns frontend-tests.logic.bool-move-test + "Moving shapes must only recalculate booleans whose children moved + relative to them, and leave every boolean equal to a fresh calculation." + (:require + [app.common.geom.point :as gpt] + [app.common.test-helpers.compositions :as ctho] + [app.common.test-helpers.files :as cthf] + [app.common.test-helpers.ids-map :as cthi] + [app.common.test-helpers.shapes :as cths] + [app.common.types.modifiers :as ctm] + [app.common.types.pages-list :as ctpl] + [app.common.types.path :as path] + [app.common.types.shape-tree :as ctst] + [app.main.data.workspace.modifiers :as dwm] + [app.render-wasm.api :as wasm.api] + [cljs.test :as t :include-macros true] + [frontend-tests.helpers.state :as ths] + [frontend-tests.helpers.wasm :as thw])) + +(def ^:private bool-calculations (atom 0)) + +(defn- counting-calc-bool-content + [shape objects] + (swap! bool-calculations inc) + (path/calc-bool-content shape objects)) + +(def ^:private original-calculate-bool wasm.api/calculate-bool) + +(t/use-fixtures :each + {:before (fn [] + (cthi/reset-idmap!) + (thw/setup-wasm-mocks!) + (reset! bool-calculations 0) + (set! wasm.api/calculate-bool counting-calc-bool-content) + (set! path/wasm:calc-bool-content counting-calc-bool-content)) + :after (fn [] + (set! wasm.api/calculate-bool original-calculate-bool) + (set! path/wasm:calc-bool-content nil) + (thw/teardown-wasm-mocks!))}) + +(defn- file-with-bool-in-board + [] + (let [file (-> (cthf/sample-file :file1) + (ctho/add-frame :board :x 0 :y 0 :width 400 :height 400) + (cths/add-sample-shape :bool {:type :bool + :bool-type :union + :parent-label :board}) + (ctho/add-rect :rect1 :x 10 :y 10 :width 100 :height 100 :parent-label :bool) + (ctho/add-rect :rect2 :x 60 :y 60 :width 100 :height 100 :parent-label :bool)) + page (cthf/current-page file) + bool (path/update-bool-shape (cths/get-shape file :bool) (:objects page))] + (update file :data ctpl/update-page (:id page) #(ctst/set-shape % bool)))) + +(defn- disable-pixel-grid + [] + (fn [state] + (update state :workspace-layout disj :snap-pixel-grid))) + +(defn- run-move + [done labels delta expected-calculations & {:keys [pixel-grid?] :or {pixel-grid? true}}] + (let [file (file-with-bool-in-board) + store (ths/setup-store file) + ids (map #(:id (cths/get-shape file %)) labels) + modif-tree (dwm/create-modif-tree ids (ctm/move-modifiers delta)) + events (cond->> [(dwm/apply-wasm-modifiers modif-tree)] + (not pixel-grid?) (cons (disable-pixel-grid)))] + (reset! bool-calculations 0) + (ths/run-store + store done events + (fn [new-state] + (let [calculations @bool-calculations + file' (ths/get-file-from-state new-state) + objects' (:objects (cthf/current-page file')) + after (cths/get-shape file' :bool) + fresh (path/update-bool-shape after objects')] + (t/is (= expected-calculations calculations)) + (t/is (= (:selrect fresh) (:selrect after))) + (t/is (= (vec (:content fresh)) (vec (:content after))))))))) + +(t/deftest moving-a-board-does-not-recalculate-the-booleans-inside + (t/async + done + (run-move done [:board :bool :rect1 :rect2] (gpt/point 10 20) 0))) + +(t/deftest moving-a-boolean-child-recalculates-the-boolean + (t/async + done + (run-move done [:rect1] (gpt/point 10 20) 1))) + +(t/deftest moving-a-board-without-pixel-grid-does-not-recalculate-its-booleans + (t/async + done + (run-move done [:board] (gpt/point 10.5 20.25) 0 :pixel-grid? false))) diff --git a/frontend/test/frontend_tests/runner.cljs b/frontend/test/frontend_tests/runner.cljs index 1f78325a70..07f4cb0e8a 100644 --- a/frontend/test/frontend_tests/runner.cljs +++ b/frontend/test/frontend_tests/runner.cljs @@ -39,6 +39,7 @@ [frontend-tests.errors-test] [frontend-tests.fonts-test] [frontend-tests.helpers-shapes-test] + [frontend-tests.logic.bool-move-test] [frontend-tests.logic.comp-remove-swap-slots-test] [frontend-tests.logic.components-and-tokens] [frontend-tests.logic.copy-paste-typography-test] @@ -174,6 +175,7 @@ 'frontend-tests.errors-test 'frontend-tests.fonts-test 'frontend-tests.helpers-shapes-test + 'frontend-tests.logic.bool-move-test 'frontend-tests.logic.comp-remove-swap-slots-test 'frontend-tests.logic.components-and-tokens 'frontend-tests.logic.copy-paste-typography-test diff --git a/render-wasm/src/render.rs b/render-wasm/src/render.rs index b840563f4f..26a99a499e 100644 --- a/render-wasm/src/render.rs +++ b/render-wasm/src/render.rs @@ -1202,7 +1202,9 @@ impl RenderState { // This avoids clearing Cache on renders that don't actually paint tiles (e.g. hover/UI), // while still preventing stale pixels from surviving across full-quality renders. if !self.cache_cleared_this_render { - self.surfaces.clear_cache(self.background_color); + if self.options.is_debug_visible() { + self.surfaces.clear_cache(self.background_color); + } self.cache_cleared_this_render = true; } let tile_rect = self.get_current_aligned_tile_bounds()?; @@ -1220,7 +1222,7 @@ impl RenderState { &self.tile_viewbox, ¤t_tile, &tile_rect, - false, + !self.options.is_debug_visible(), self.render_area, self.get_scale(), self.viewbox.area, @@ -2333,10 +2335,7 @@ impl RenderState { .is_none_or(|s| s.width() < win_w || s.height() < win_h); if needs_alloc { scratch_surface = get_gpu_state() - .create_surface_with_isize( - "drag_crop_scratch".to_string(), - skia::ISize::new(win_w, win_h), - ) + .create_surface_with_isize(skia::ISize::new(win_w, win_h)) .ok(); } let Some(scratch) = scratch_surface.as_mut() else { @@ -2512,11 +2511,13 @@ impl RenderState { s.canvas().scale((scale, scale)); }); - self.surfaces.resize_cache_from_viewbox( - &self.viewbox, - &self.cached_viewbox, - self.options.dpr_viewport_interest_area_threshold, - )?; + if self.options.is_debug_visible() { + self.surfaces.resize_cache_from_viewbox( + &self.viewbox, + &self.cached_viewbox, + self.options.dpr_viewport_interest_area_threshold, + )?; + } // FIXME - review debug // debug::render_debug_tiles_for_viewbox(self); @@ -4319,11 +4320,16 @@ impl RenderState { if !is_empty || self.current_tile_had_shapes { if self.options.is_interactive_transform() { // During drag, avoid snapshot-based caching. Draw Current directly - // into Target (and Cache) to reduce stalls. + // into Target (and Cache, for the debug views) to reduce stalls. + let draw_on_cache = if self.options.is_debug_visible() { + surfaces::DrawOnCache::Yes + } else { + surfaces::DrawOnCache::No + }; self.surfaces.draw_current_tile_into_backbuffer( &tile_rect, self.background_color, - surfaces::DrawOnCache::Yes, + draw_on_cache, ); } else { self.apply_render_to_final_canvas()?; diff --git a/render-wasm/src/render/gpu_state.rs b/render-wasm/src/render/gpu_state.rs index 7a158b5fc6..1c0bdcf314 100644 --- a/render-wasm/src/render/gpu_state.rs +++ b/render-wasm/src/render/gpu_state.rs @@ -88,93 +88,35 @@ impl GpuState { None } - fn delete_gl_texture(&mut self, texture_id: gl::types::GLuint) -> bool { - unsafe { - gl::DeleteTextures(1, &texture_id); - gl::GetError() == 0 - } - } - - fn create_gl_texture(&mut self, width: i32, height: i32) -> gl::types::GLuint { - let mut texture_id: gl::types::GLuint = 0; - - unsafe { - gl::GenTextures(1, &mut texture_id); - gl::BindTexture(gl::TEXTURE_2D, texture_id); - - gl::TexParameteri(gl::TEXTURE_2D, gl::TEXTURE_MIN_FILTER, gl::LINEAR as i32); - gl::TexParameteri(gl::TEXTURE_2D, gl::TEXTURE_MAG_FILTER, gl::LINEAR as i32); - gl::TexParameteri(gl::TEXTURE_2D, gl::TEXTURE_WRAP_S, gl::CLAMP_TO_EDGE as i32); - gl::TexParameteri(gl::TEXTURE_2D, gl::TEXTURE_WRAP_T, gl::CLAMP_TO_EDGE as i32); - - gl::TexImage2D( - gl::TEXTURE_2D, - 0, - gl::RGBA8 as i32, - width, - height, - 0, - gl::RGBA, - gl::UNSIGNED_BYTE, - std::ptr::null(), - ); - } - - texture_id - } - - pub fn delete_surface(&mut self, surface: &mut skia::Surface) -> bool { - let Some(texture) = skia::gpu::surfaces::get_backend_texture( - surface, - skia_safe::surface::BackendHandleAccess::FlushRead, - ) else { - return false; - }; - let Some(texture_info) = gpu::backend_textures::get_gl_texture_info(&texture) else { - return false; - }; - self.delete_gl_texture(texture_info.id) - } - - pub fn create_surface_with_isize( - &mut self, - label: String, - size: ISize, - ) -> Result { - self.create_surface_with_dimensions(label, size.width, size.height) + pub fn create_surface_with_isize(&mut self, size: ISize) -> Result { + self.create_surface_with_dimensions(size.width, size.height) } pub fn create_surface_with_dimensions( &mut self, - label: String, width: i32, height: i32, ) -> Result { - let backend_texture = unsafe { - let texture_id = self.create_gl_texture(width, height); - let texture_info = TextureInfo { - target: gl::TEXTURE_2D, - id: texture_id, - format: gl::RGBA8, - protected: skia::gpu::Protected::No, - }; - gpu::backend_textures::make_gl((width, height), gpu::Mipmapped::No, texture_info, label) - }; + let image_info = skia::ImageInfo::new( + (width, height), + skia::ColorType::RGBA8888, + skia::AlphaType::Premul, + None, + ); - let surface = gpu::surfaces::wrap_backend_texture( + gpu::surfaces::render_target( &mut self.context, - &backend_texture, + gpu::Budgeted::No, + &image_info, + None, gpu::SurfaceOrigin::BottomLeft, None, - skia::ColorType::RGBA8888, - None, + false, None, ) .ok_or(Error::CriticalError( "Failed to create Skia surface".to_string(), - ))?; - - Ok(surface) + )) } /// Create a Skia surface that will be used for rendering. diff --git a/render-wasm/src/render/surfaces.rs b/render-wasm/src/render/surfaces.rs index 761b2d02d6..be6ccb1fb5 100644 --- a/render-wasm/src/render/surfaces.rs +++ b/render-wasm/src/render/surfaces.rs @@ -165,8 +165,7 @@ impl DocAtlas { pub fn try_new() -> Result { // Keep atlas as a regular surface like the rest. Start with a tiny // transparent surface and grow it on demand. - let mut surface = - get_gpu_state().create_surface_with_dimensions("atlas".to_string(), 1, 1)?; + let mut surface = get_gpu_state().create_surface_with_dimensions(1, 1)?; surface.canvas().clear(skia::Color::TRANSPARENT); @@ -276,8 +275,7 @@ impl DocAtlas { return Ok(()); } - let mut new_surface = - gpu_state.create_surface_with_dimensions("atlas".to_string(), new_w, new_h)?; + let mut new_surface = gpu_state.create_surface_with_dimensions(new_w, new_h)?; new_surface.canvas().clear(skia::Color::TRANSPARENT); // Copy old atlas into the new one with offset. @@ -307,7 +305,6 @@ impl DocAtlas { self.origin = skia::Point::new(new_left, new_top); self.size = skia::ISize::new(new_w, new_h); self.scale = new_scale; - gpu_state.delete_surface(&mut self.surface); self.surface = new_surface; Ok(()) } @@ -528,35 +525,25 @@ impl Surfaces { let margins = skia::ISize::new(extra_tile_dims.width / 4, extra_tile_dims.height / 4); let target = gpu_state.create_target_surface(width, height)?; - let filter = gpu_state.create_surface_with_isize("filter".to_string(), extra_tile_dims)?; - let cache = gpu_state.create_surface_with_dimensions("cache".to_string(), width, height)?; - let backbuffer = - gpu_state.create_surface_with_dimensions("backbuffer".to_string(), width, height)?; + let filter = gpu_state.create_surface_with_isize(extra_tile_dims)?; + let cache = gpu_state.create_surface_with_dimensions(width, height)?; + let backbuffer = gpu_state.create_surface_with_dimensions(width, height)?; let max_texture_size = gpu_state.max_texture_size(); - let tile_atlas = gpu_state.create_surface_with_dimensions( - "tile_atlas".to_string(), - max_texture_size, - max_texture_size, - )?; + let tile_atlas = + gpu_state.create_surface_with_dimensions(max_texture_size, max_texture_size)?; - let current = - gpu_state.create_surface_with_isize("current".to_string(), extra_tile_dims)?; + let current = gpu_state.create_surface_with_isize(extra_tile_dims)?; - let drop_shadows = - gpu_state.create_surface_with_isize("drop_shadows".to_string(), extra_tile_dims)?; - let inner_shadows = - gpu_state.create_surface_with_isize("inner_shadows".to_string(), extra_tile_dims)?; - let text_drop_shadows = gpu_state - .create_surface_with_isize("text_drop_shadows".to_string(), extra_tile_dims)?; - let shape_fills = - gpu_state.create_surface_with_isize("shape_fills".to_string(), extra_tile_dims)?; - let shape_strokes = - gpu_state.create_surface_with_isize("shape_strokes".to_string(), extra_tile_dims)?; - let export = gpu_state.create_surface_with_isize("export".to_string(), extra_tile_dims)?; + let drop_shadows = gpu_state.create_surface_with_isize(extra_tile_dims)?; + let inner_shadows = gpu_state.create_surface_with_isize(extra_tile_dims)?; + let text_drop_shadows = gpu_state.create_surface_with_isize(extra_tile_dims)?; + let shape_fills = gpu_state.create_surface_with_isize(extra_tile_dims)?; + let shape_strokes = gpu_state.create_surface_with_isize(extra_tile_dims)?; + let export = gpu_state.create_surface_with_isize(extra_tile_dims)?; - let ui = gpu_state.create_surface_with_dimensions("ui".to_string(), width, height)?; - let debug = gpu_state.create_surface_with_dimensions("debug".to_string(), width, height)?; + let ui = gpu_state.create_surface_with_dimensions(width, height)?; + let debug = gpu_state.create_surface_with_dimensions(width, height)?; let tiles = TileTextureCache::new(tile_atlas.width(), tile_atlas.height()); let atlas = DocAtlas::try_new()?; @@ -677,12 +664,10 @@ impl Surfaces { )); canvas.scale((s / scale, s / scale)); - self.atlas.surface.draw( - canvas, - (0.0, 0.0), - self.sampling_options, - Some(&skia::Paint::default()), - ); + let sampling = skia::SamplingOptions::new(skia::FilterMode::Linear, skia::MipmapMode::None); + self.atlas + .surface + .draw(canvas, (0.0, 0.0), sampling, Some(&skia::Paint::default())); canvas.restore(); } diff --git a/render-wasm/src/state.rs b/render-wasm/src/state.rs index 5b2684ecb4..42653f876a 100644 --- a/render-wasm/src/state.rs +++ b/render-wasm/src/state.rs @@ -50,18 +50,16 @@ impl State { .to_string(), )); } - self.saved_shapes = Some(self.shapes.clone()); - self.shapes = ShapesPool::new(); + self.saved_shapes = Some(std::mem::take(&mut self.shapes)); Ok(()) } // Disposes of the temporary shapes pool restoring the normal pool // Will panic if a there is no temporary pool. pub fn end_temp_objects(&mut self) -> Result<()> { - self.shapes = self.saved_shapes.clone().ok_or(Error::CriticalError( + self.shapes = self.saved_shapes.take().ok_or(Error::CriticalError( "Tried to end temp objects but not content to be restored is present".to_string(), ))?; - self.saved_shapes = None; Ok(()) }