Fix disappearing cached shapes (#6458)

* Fix GL parameter
* Don't leak GL Framebuffers
* Resize: Don't rebuild screen-size-independent passes
* Don't drop Texture while in use
* Fix concurrency bug

lib/rust/ensogl/core/src/display/render/composer.rs

@@ -51,12 +51,28 @@ impl {
         self.init_passes();
     }
 
-    /// Resize the composer and reinitialize all of its layers.
+    /// Resize the composer and reinitialize all of its screen-size-dependent layers.
     pub fn resize(&mut self, width: i32, height: i32, pixel_ratio: f32) {
+        if width == self.width && height == self.height && pixel_ratio == self.pixel_ratio {
+            // Some resize events are spurious; it is not necessary to reinitialize the passes if
+            // the size hasn't actually changed.
+            return;
+        }
         self.width = width;
         self.height = height;
         self.pixel_ratio = pixel_ratio;
-        self.init_passes();
+        let ctx = &self.context;
+        let vars = &self.variables;
+        let defs = self.pipeline.passes_clone();
+        let old_passes = self.passes.drain(..);
+        let passes = defs.into_iter().zip(old_passes).map(|(def, pass)| {
+            if def.is_screen_size_independent() {
+                pass
+            } else {
+                ComposerPass::new(ctx, vars, def, width, height, pixel_ratio)
+            }
+        }).collect_vec();
+        self.passes = passes;
     }
 
     /// Initialize all pass definitions from the [`Pipeline`].

lib/rust/ensogl/core/src/display/render/pass.rs

@@ -20,6 +20,9 @@ use crate::system::gpu::data::texture::class::TextureOps;
 pub trait Definition: CloneBoxedForDefinition + Debug + 'static {
     fn initialize(&mut self, _instance: &Instance) {}
     fn run(&mut self, _instance: &Instance, update_status: UpdateStatus);
+    fn is_screen_size_independent(&self) -> bool {
+        false
+    }
 }
 
 clone_boxed!(Definition);
@@ -170,7 +173,7 @@ impl OutputDefinition {
     /// Constructor of the RGBA u8 output with default texture parameters. It is the most popular
     /// option and you should use it to render colors with your passes.
     pub fn new_rgba<Name: Str>(name: Name) -> Self {
-        let internal_format = texture::Rgba;
+        let internal_format = texture::Rgba8;
         let item_type = texture::item_type::u8;
         let texture_parameters = default();
         OutputDefinition::new(name, internal_format, item_type, texture_parameters)
@@ -217,3 +220,9 @@ impl Framebuffer {
         result
     }
 }
+
+impl Drop for Framebuffer {
+    fn drop(&mut self) {
+        self.context.delete_framebuffer(Some(&self.native));
+    }
+}

lib/rust/ensogl/core/src/display/render/passes/cache_shapes.rs

@@ -35,24 +35,31 @@ use crate::gui::component::AnyShapeView;
 #[derive(Clone, Derivative)]
 #[derivative(Debug)]
 pub struct CacheShapesPass {
-    scene:               Scene,
-    framebuffer:         Option<pass::Framebuffer>,
+    scene: Scene,
+    framebuffer: Option<pass::Framebuffer>,
+    texture: Option<crate::system::gpu::data::uniform::AnyTextureUniform>,
     #[derivative(Debug = "ignore")]
-    shapes_to_render:    Vec<Rc<dyn AnyShapeView>>,
+    shapes_to_render: Vec<Rc<dyn AnyShapeView>>,
     /// Texture size in device pixels.
     texture_size_device: Vector2<i32>,
-    layer:               Layer,
+    layer: Layer,
+    camera_ready: Rc<Cell<bool>>,
+    #[derivative(Debug = "ignore")]
+    display_object_update_handler: Option<Rc<enso_callback::Handle>>,
 }
 
 impl CacheShapesPass {
     /// Constructor.
     pub fn new(scene: &Scene) -> Self {
         Self {
-            framebuffer:         default(),
-            shapes_to_render:    default(),
-            layer:               Layer::new("Cached Shapes"),
-            scene:               scene.clone_ref(),
+            framebuffer: default(),
+            texture: default(),
+            shapes_to_render: default(),
+            layer: Layer::new("Cached Shapes"),
+            scene: scene.clone_ref(),
             texture_size_device: default(),
+            camera_ready: default(),
+            display_object_update_handler: default(),
         }
     }
 }
@@ -83,38 +90,60 @@ impl pass::Definition for CacheShapesPass {
         self.layer.camera().update(&self.scene);
         self.scene.display_object.update(&self.scene);
         self.layer.update();
+        // The asynchronous update of the scene's display object initiated above will eventually
+        // set our layer's camera's transformation. Handle the camera update when all
+        // previously-initiated FRP events finish being processed.
+        let handle = frp::microtasks::next_microtask({
+            let camera = self.layer.camera();
+            let scene = self.scene.clone_ref();
+            let camera_ready = Rc::clone(&self.camera_ready);
+            move || {
+                camera.update(&scene);
+                camera_ready.set(true);
+            }
+        });
+        self.display_object_update_handler = Some(Rc::new(handle));
 
         let output = pass::OutputDefinition::new_rgba("cached_shapes");
         let texture =
             instance.new_texture(&output, self.texture_size_device.x, self.texture_size_device.y);
         self.framebuffer = Some(instance.new_framebuffer(&[&texture]));
+        self.texture = Some(texture);
     }
 
     fn run(&mut self, instance: &Instance, _update_status: UpdateStatus) {
-        let is_shader_compiled =
-            |shape: &mut Rc<dyn AnyShapeView>| shape.sprite().symbol.shader().program().is_some();
-        let mut ready_to_render = self.shapes_to_render.drain_filter(is_shader_compiled).peekable();
-        if ready_to_render.peek().is_some() {
-            if let Some(framebuffer) = self.framebuffer.as_ref() {
-                framebuffer.with_bound(|| {
-                    instance.with_viewport(
-                        self.texture_size_device.x,
-                        self.texture_size_device.y,
-                        || {
-                            with_display_mode(DisplayModes::CachedShapesTexture, || {
-                                with_context(|ctx| ctx.set_camera(&self.layer.camera()));
-                                for shape in ready_to_render {
-                                    shape.sprite().symbol.render();
-                                }
-                            })
-                        },
-                    );
-                });
-            } else {
-                reportable_error!("Impossible happened: The CacheShapesPass was run without initialized framebuffer.");
+        if self.camera_ready.get() {
+            let is_shader_compiled = |shape: &mut Rc<dyn AnyShapeView>| {
+                shape.sprite().symbol.shader().program().is_some()
+            };
+            let mut ready_to_render =
+                self.shapes_to_render.drain_filter(is_shader_compiled).peekable();
+            if ready_to_render.peek().is_some() {
+                if let Some(framebuffer) = self.framebuffer.as_ref() {
+                    framebuffer.with_bound(|| {
+                        instance.with_viewport(
+                            self.texture_size_device.x,
+                            self.texture_size_device.y,
+                            || {
+                                with_display_mode(DisplayModes::CachedShapesTexture, || {
+                                    with_context(|ctx| ctx.set_camera(&self.layer.camera()));
+                                    for shape in ready_to_render {
+                                        shape.sprite().symbol.render();
+                                    }
+                                })
+                            },
+                        );
+                    });
+                } else {
+                    reportable_error!("Impossible happened: The CacheShapesPass was run without initialized framebuffer.");
+                }
             }
         }
     }
+
+    fn is_screen_size_independent(&self) -> bool {
+        true
+    }
 }
 
 fn with_display_mode<R>(mode: DisplayModes, f: impl FnOnce() -> R) -> R {

lib/rust/ensogl/core/src/system/gpu/data/texture/class.rs

@@ -58,9 +58,9 @@ impl Drop for TextureBindGuard {
 /// For more background see: https://developer.mozilla.org/en-US/docs/Web/API/WebGLRenderingContext/texParameter
 #[derive(Copy, Clone, Debug, Default)]
 pub struct Parameters {
-    /// Specifies the setting for the texture magnification filter (`Context::TEXTURE_MIN_FILTER`).
+    /// Specifies the setting for the texture minification filter (`Context::TEXTURE_MIN_FILTER`).
     pub min_filter: MinFilter,
-    /// Specifies the setting for the texture minification filter (`Context::TEXTURE_MAG_FILTER`).
+    /// Specifies the setting for the texture magnification filter (`Context::TEXTURE_MAG_FILTER`).
     pub mag_filter: MagFilter,
     /// Specifies the setting for the wrapping function for texture coordinate s
     /// (`Context::TEXTURE_WRAP_S`).
@@ -75,7 +75,7 @@ impl Parameters {
     pub fn apply_parameters(self, context: &Context) {
         let target = Context::TEXTURE_2D;
         context.tex_parameteri(*target, *Context::TEXTURE_MIN_FILTER, *self.min_filter as i32);
-        context.tex_parameteri(*target, *Context::TEXTURE_MIN_FILTER, *self.mag_filter as i32);
+        context.tex_parameteri(*target, *Context::TEXTURE_MAG_FILTER, *self.mag_filter as i32);
         context.tex_parameteri(*target, *Context::TEXTURE_WRAP_S, *self.wrap_s as i32);
         context.tex_parameteri(*target, *Context::TEXTURE_WRAP_T, *self.wrap_t as i32);
     }