Repository: slint-ui/slint (language: rust)

Issue:
swrenderer: Window background change don't mark the whole area as dirty for partial rendering
<!-- Thanks for opening an issue! 🤗 -->
<!-- IMPORTANT: This issue tracker is for tracking bugs and features request. For questions or help on how to use Slint, please go to the "Discussions" tab -->
<!-- Please mention your platform and the programming language you are using Slint with -->
<!-- For bugs, please give steps on how to reproduce. What is the expected behavior and what do you see instead. -->
<!-- If possible, please include relevant code snippets, in code blocks (```  ```)-->

I'm using ESP32C3 and following the [Official MCU guide](https://releases.slint.dev/1.5.1/docs/rust/slint/docs/mcu/) to build a demo. It works but i encounter two problems.

My Cargo.toml slint part
``` toml
[dependencies.slint]
version = "1.5.1"
default-features = false
features = ["compat-1-2", "unsafe-single-threaded", "libm", "renderer-software"]
[build-dependencies]
slint-build = "1.5.1"
```

My `.slint` file
```slint
import { VerticalBox, Switch } from "std-widgets.slint";
export component Demo inherits Window{
    width: 128px;
    height: 128px;
    in-out property<bool> check: false;
    background: sw.checked ? #db073d:#07dba5;
    VerticalBox {
        alignment: center;
        Text {
            text: "Bg color test";
            font-size: 15px;
            horizontal-alignment: center;
        }
        sw := Switch {
            checked: check;
            text: "Switch";
        }
        Slider {
            minimum: 0;
            value: sw.checked ? 0 : 100;
            maximum: 100;
        }
    }
}

```
**I'm using linebuffer method with super loop approach by following the guide.**

1.  Can't use `Slider` widget, when add `Slider` in my `.slint`, compiler will error out with:
     >  called `Result::unwrap()` on an `Err` value: CompileError(["/home/kortan/Documents/code/rust/esp32/slint/ui/main.slint:21: Unknown type Slider"])

2.  When toggled switch, not full background freshed, only switch area changed.

    https://github.com/slint-ui/slint/assets/12783634/9ceffc3a-a483-4240-8a08-949f6900917b

    And when config `animate` to `background`,  display get even weird.
   ```slint
import { VerticalBox, Switch } from "std-widgets.slint";
export component Demo inherits Window{
    width: 128px;
    height: 128px;
    in-out property<bool> check: false;
    background: sw.checked ? #db073d:#07dba5;
    animate background {
         duration: 300ms;
    }
    VerticalBox {
        alignment: center;
        Text {
            text: "Bg color test";
            font-size: 15px;
            horizontal-alignment: center;
        }
        sw := Switch {
            checked: check;
            text: "Switch";
        }
    }
}
  ```
   
https://github.com/slint-ui/slint/assets/12783634/bf587c83-a24f-455d-ae07-76ee24a3602f


    






Discussion / hints:
> Can't use Slider widget, when add Slider in my .slint, compiler will error out with:

Did you import the Slider? Adding it into the `import {...}` should work.

> When toggled switch, not full background freshed, only switch area changed.

That sounds like a bug to me...
> Did you import the Slider? Adding it into the import {...} should work.

Just realize that i've made a really dumb question. Sorry for that.

>That sounds like a bug to me...

The slint code works well on slintpad. So there may be something wrong inside `draw_if_needed`.
After update to slint 1.6.0, things are not getting better much.

https://github.com/slint-ui/slint/assets/12783634/1cde03bc-f676-496a-8625-c8fea856240c


How are you rendering to the screen? We made a change in 1.6 regarding the dirty regions. Render now returns the region that has multiple rectangles, all of which need to be flushed to the screen.
Do you use the return value of render() somewhere?
@tronical 
> How are you rendering to the screen? 

My whole project code is [here](https://github.com/KortanZ/esp32c3-slint-st7735-demo), everything about slint is in [ui.rs](https://github.com/KortanZ/esp32c3-slint-st7735-demo/blob/main/src/ui.rs), the render loop looks like this:
```rust
    loop {
        slint::platform::update_timers_and_animations();
        window.draw_if_needed(|renderer| {
            renderer.render_by_line(DisplayWrapper{
                display: &mut display,
                line_buffer: &mut line_buffer
            });
        });

        if !window.has_active_animations() {
            if let Some(duration) = slint::platform::duration_until_next_timer_update() {
                Timer::after(Duration::from_millis(duration.as_millis() as u64)).await;
                continue;
            }
        }
        Timer::after(Duration::from_millis(10)).await;
    }
```
and the `LineBufferProvider`  implemention is like:
```rust
struct DisplayWrapper<'a, T>{
    display: &'a mut T,
    line_buffer: &'a mut [slint::platform::software_renderer::Rgb565Pixel],
}

impl<T: DrawTarget<Color = Rgb565>>
    slint::platform::software_renderer::LineBufferProvider for DisplayWrapper<'_, T>
{
    type TargetPixel = slint::platform::software_renderer::Rgb565Pixel;
    fn process_line(
        &mut self,
        line: usize,
        range: core::ops::Range<usize>,
        render_fn: impl FnOnce(&mut [Self::TargetPixel]),
    ) {
        // Render into the line
        render_fn(&mut self.line_buffer[range.clone()]);

        self.display.fill_contiguous(
            &Rectangle::new(Point::new(range.start as _, line as _), Size::new(range.len() as _, 1)),
            self.line_buffer[range.clone()].iter().map(|p| RawU16::new(p.0).into())
        ).map_err(drop).unwrap();
    }
}
```
>Do you use the return value of render() somewhere?

No, I'm using `render_by_line()` instead and not using return value at all.


Thanks for confirming. So it can’t be that. Does the background change work if you remove the animation on the background property? (Not a solution of course, but helps narrow down a theory I have)
@tronical 
> Does the background change work if you remove the animation on the background property?



https://github.com/slint-ui/slint/assets/12783634/6eddea0a-02bb-438e-859b-45bdd7504d38



Just like when using slint 1.5.1 as i shown on top of this issue, the only difference between animated version and no animation version is that no animation version could change the color correctly rather than a unexpected deeper or lighter color.

Ouch, ok, so my theory of it being related to the time is not true either :(. Since you're calling `render_by_line`, could you try to see via debug output what the values are for the `line: usize` parameter? When toggling the switch, you should get the function invoked also with line values that start at the top of the screen (so zero and onwards).
@tronical I've added debug output in `process_line`, and `line: usize` was start from 70 and end at 82 when toggling the switch. So it seems that only color changed area have been re-rendered.
The problem here is in the Slint implementation of the partial rendering for the software renderer.
The change in the Window background color is not making any area dirty.

As a workaround, you can use a top level Rectangle with a background.
@ogoffart Thank you for your solution : ). I think there are two problem now. One is dirty marking that you mentioned, and the other one is animate problem. As shown in video clip, animation not work as expected either. When configured animate, color dosen't animate but only show a darker or lighter color instead.
I think this is the same problem: the background color is not being tracked, so even if it animates, it doesn't mark anything dirty and so nothing gets repainted. Only other part of the UI that changes get repainted.
@ogoffart 
>I think this is the same problem: the background color is not being tracked, so even if it animates, it doesn't mark anything dirty and so nothing gets repainted. Only other part of the UI that changes get repainted.

You are right, i've tested your workaround, and animation works well too.
The skia renderer with partial rendering is affected, too.
I started working on this, but it's a little trickier, so postponed. But the basic idea I had is this:

1. Teach `WindowItem::render` to look like this:
```rust
  let mut rect = Rectangle::default();
  rect.background.set(self.background());
  backend.draw_rectangle(core::pin::pin!(rect).as_ref(), self_rc, size);
```
2. Teach the FemtoVG and Skia renderers to pick up a solid background color from the window item, if a rendering notifier is set, and use that color as clear color. This is to maintain compatibility. If this code path is taken, then the `draw_rectangle` implementation for those two renderers needs to be taught to *skip* doing its work if `self_rc` is the `WindowItem` which we picked the clear color off of.
3. In the future, change behaviour to require `Window { background: transparent; }` if applications want to render their own underlay in rendering notifiers, and that way eliminate step 2.
>     1. Teach `WindowItem::render` to look like this:
> 
> ```rs
>   let mut rect = Rectangle::default();
>   rect.background.set(self.background());
>   backend.draw_rectangle(core::pin::pin!(rect).as_ref(), self_rc, size);
> ```

Why not implement `impl RenderBorderRectangle for WindowItem`
end then 
```rs
backend.draw_border_rectangle(self, self_rc, size, &self.cached_rendering_data);
```

Seems simple enough.


The SoftwareRenderer also need to be changed so that it doesn't draw the background twice.
Yet, when using a transparent background, the question is whether to clear buffer or not before drawing. Right now, the software renderer fills the buffer with the background color.
But we don't do that, and draw the background as a rectangle, we will blend it over. Which might not be correct if the background is transparent.

Hence the special_case.

Maybe we could have a ItemRenderer::draw_background as a special case.

----

Another idea:
WindowItem::render could be like so:
```
// Record a dependency on the background without actually drawing it.
// This will force the tracker to be dirty.
let _ = self.background();
```

This will fix the issue. But will not simplify the code so that the swrenderer draw gradient background.

Fix patch:
diff --git a/internal/backends/qt/qt_window.rs b/internal/backends/qt/qt_window.rs
index c7783adf7ad..04fe9606b33 100644
--- a/internal/backends/qt/qt_window.rs
+++ b/internal/backends/qt/qt_window.rs
@@ -674,6 +674,16 @@ impl ItemRenderer for QtItemRenderer<'_> {
         );
     }
 
+    fn draw_window_background(
+        &mut self,
+        _rect: Pin<&dyn RenderRectangle>,
+        _self_rc: &ItemRc,
+        _size: LogicalSize,
+        _cache: &CachedRenderingData,
+    ) {
+        // Background is applied via WindowProperties::background()
+    }
+
     fn draw_image(
         &mut self,
         image: Pin<&dyn RenderImage>,
diff --git a/internal/core/item_rendering.rs b/internal/core/item_rendering.rs
index 3373db37e59..208f76fdd87 100644
--- a/internal/core/item_rendering.rs
+++ b/internal/core/item_rendering.rs
@@ -344,6 +344,13 @@ pub trait ItemRenderer {
         _size: LogicalSize,
         _cache: &CachedRenderingData,
     );
+    fn draw_window_background(
+        &mut self,
+        rect: Pin<&dyn RenderRectangle>,
+        self_rc: &ItemRc,
+        size: LogicalSize,
+        cache: &CachedRenderingData,
+    );
     fn draw_image(
         &mut self,
         image: Pin<&dyn RenderImage>,
@@ -462,12 +469,6 @@ pub trait ItemRenderer {
 
     fn draw_image_direct(&mut self, image: crate::graphics::Image);
 
-    /// Fills a rectangle at (0,0) with the given size. This is used for example by the Skia renderer to
-    /// handle window backgrounds with a brush (gradient).
-    fn draw_rect(&mut self, _size: LogicalSize, _brush: Brush) {
-        unimplemented!()
-    }
-
     /// This is called before it is being rendered (before the draw_* function).
     /// Returns
     ///  - if the item needs to be drawn (false means it is clipped or doesn't need to be drawn)
@@ -963,6 +964,7 @@ impl<'a, T: ItemRenderer + ItemRendererFeatures> ItemRenderer for PartialRendere
 
     forward_rendering_call2!(fn draw_rectangle(dyn RenderRectangle));
     forward_rendering_call2!(fn draw_border_rectangle(dyn RenderBorderRectangle));
+    forward_rendering_call2!(fn draw_window_background(dyn RenderRectangle));
     forward_rendering_call2!(fn draw_image(dyn RenderImage));
     forward_rendering_call2!(fn draw_text(dyn RenderText));
     forward_rendering_call!(fn draw_text_input(TextInput));
@@ -1029,10 +1031,6 @@ impl<'a, T: ItemRenderer + ItemRendererFeatures> ItemRenderer for PartialRendere
         self.actual_renderer.draw_image_direct(image)
     }
 
-    fn draw_rect(&mut self, size: LogicalSize, brush: Brush) {
-        self.actual_renderer.draw_rect(size, brush);
-    }
-
     fn window(&self) -> &crate::window::WindowInner {
         self.actual_renderer.window()
     }
diff --git a/internal/core/items.rs b/internal/core/items.rs
index 078f9a83490..c7d33375cc9 100644
--- a/internal/core/items.rs
+++ b/internal/core/items.rs
@@ -1021,14 +1021,21 @@ impl Item for WindowItem {
 
     fn render(
         self: Pin<&Self>,
-        _backend: &mut ItemRendererRef,
-        _self_rc: &ItemRc,
-        _size: LogicalSize,
+        backend: &mut ItemRendererRef,
+        self_rc: &ItemRc,
+        size: LogicalSize,
     ) -> RenderingResult {
+        backend.draw_window_background(self, self_rc, size, &self.cached_rendering_data);
         RenderingResult::ContinueRenderingChildren
     }
 }
 
+impl RenderRectangle for WindowItem {
+    fn background(self: Pin<&Self>) -> Brush {
+        self.background()
+    }
+}
+
 impl WindowItem {
     pub fn font_family(self: Pin<&Self>) -> Option<SharedString> {
         let maybe_family = self.default_font_family();
diff --git a/internal/core/software_renderer.rs b/internal/core/software_renderer.rs
index f071d4368f4..e896d93143f 100644
--- a/internal/core/software_renderer.rs
+++ b/internal/core/software_renderer.rs
@@ -1914,6 +1914,17 @@ impl<'a, T: ProcessScene> crate::item_rendering::ItemRenderer for SceneBuilder<'
         }
     }
 
+    fn draw_window_background(
+        &mut self,
+        rect: Pin<&dyn RenderRectangle>,
+        _self_rc: &ItemRc,
+        _size: LogicalSize,
+        _cache: &CachedRenderingData,
+    ) {
+        // register a dependency for the partial renderer's dirty tracker. The actual rendering is done earlier in the software renderer.
+        let _ = rect.background();
+    }
+
     fn draw_image(
         &mut self,
         image: Pin<&dyn RenderImage>,
diff --git a/internal/core/window.rs b/internal/core/window.rs
index 49c8062a47d..f50fa19b69b 100644
--- a/internal/core/window.rs
+++ b/internal/core/window.rs
@@ -1247,6 +1247,19 @@ impl WindowInner {
         self.strong_component_ref.borrow().is_some()
     }
 
+    /// Returns the window item that is the first item in the component. When Some()
+    /// is returned, it's guaranteed to be safe to downcast to `WindowItem`.
+    pub fn window_item_rc(&self) -> Option<ItemRc> {
+        self.try_component().and_then(|component_rc| {
+            let item_rc = ItemRc::new(component_rc, 0);
+            if item_rc.downcast::<crate::items::WindowItem>().is_some() {
+                Some(item_rc)
+            } else {
+                None
+            }
+        })
+    }
+
     /// Returns the window item that is the first item in the component.
     pub fn window_item(&self) -> Option<VRcMapped<ItemTreeVTable, crate::items::WindowItem>> {
         self.try_component().and_then(|component_rc| {
diff --git a/internal/renderers/femtovg/itemrenderer.rs b/internal/renderers/femtovg/itemrenderer.rs
index e02e14243d8..b8698dbaa31 100644
--- a/internal/renderers/femtovg/itemrenderer.rs
+++ b/internal/renderers/femtovg/itemrenderer.rs
@@ -187,7 +187,23 @@ impl<'a> ItemRenderer for GLItemRenderer<'a> {
         size: LogicalSize,
         _cache: &CachedRenderingData,
     ) {
-        self.draw_rect(size, rect.background());
+        let geometry = PhysicalRect::from(size * self.scale_factor);
+        if geometry.is_empty() {
+            return;
+        }
+        if self.global_alpha_transparent() {
+            return;
+        }
+        // TODO: cache path in item to avoid re-tesselation
+        let path = rect_to_path(geometry);
+        let paint = match self.brush_to_paint(rect.background(), &path) {
+            Some(paint) => paint,
+            None => return,
+        }
+        // Since we're filling a straight rectangle with either color or gradient, save
+        // the extra stroke triangle strip around the edges
+        .with_anti_alias(false);
+        self.canvas.borrow_mut().fill_path(&path, &paint);
     }
 
     fn draw_border_rectangle(
@@ -270,6 +286,17 @@ impl<'a> ItemRenderer for GLItemRenderer<'a> {
         }
     }
 
+    fn draw_window_background(
+        &mut self,
+        rect: Pin<&dyn RenderRectangle>,
+        _self_rc: &ItemRc,
+        _size: LogicalSize,
+        _cache: &CachedRenderingData,
+    ) {
+        // register a dependency for the partial renderer's dirty tracker. The actual rendering is done earlier in SkiaRenderer.
+        let _ = rect.background();
+    }
+
     fn draw_image(
         &mut self,
         image: Pin<&dyn RenderImage>,
@@ -1021,27 +1048,6 @@ impl<'a> ItemRenderer for GLItemRenderer<'a> {
         })
     }
 
-    /// Draws a `Rectangle` using the `GLItemRenderer`.
-    fn draw_rect(&mut self, size: LogicalSize, brush: Brush) {
-        let geometry = PhysicalRect::from(size * self.scale_factor);
-        if geometry.is_empty() {
-            return;
-        }
-        if self.global_alpha_transparent() {
-            return;
-        }
-        // TODO: cache path in item to avoid re-tesselation
-        let path = rect_to_path(geometry);
-        let paint = match self.brush_to_paint(brush, &path) {
-            Some(paint) => paint,
-            None => return,
-        }
-        // Since we're filling a straight rectangle with either color or gradient, save
-        // the extra stroke triangle strip around the edges
-        .with_anti_alias(false);
-        self.canvas.borrow_mut().fill_path(&path, &paint);
-    }
-
     fn window(&self) -> &i_slint_core::window::WindowInner {
         i_slint_core::window::WindowInner::from_pub(self.window)
     }
diff --git a/internal/renderers/femtovg/lib.rs b/internal/renderers/femtovg/lib.rs
index c474021f981..50df786ff85 100644
--- a/internal/renderers/femtovg/lib.rs
+++ b/internal/renderers/femtovg/lib.rs
@@ -260,16 +260,24 @@ impl FemtoVGRenderer {
                     height.get(),
                 );
 
-                // Draws the window background as gradient
-                match window_background_brush {
-                    Some(Brush::SolidColor(..)) | None => {}
-                    Some(brush) => {
-                        item_renderer.draw_rect(
-                            i_slint_core::lengths::logical_size_from_api(
-                                window.size().to_logical(window_inner.scale_factor()),
-                            ),
-                            brush,
-                        );
+                if let Some(window_item_rc) = window_inner.window_item_rc() {
+                    let window_item =
+                        window_item_rc.downcast::<i_slint_core::items::WindowItem>().unwrap();
+                    match window_item.as_pin_ref().background() {
+                        Brush::SolidColor(..) => {
+                            // clear_rect is called earlier
+                        }
+                        _ => {
+                            // Draws the window background as gradient
+                            item_renderer.draw_rectangle(
+                                window_item.as_pin_ref(),
+                                &window_item_rc,
+                                i_slint_core::lengths::logical_size_from_api(
+                                    window.size().to_logical(window_inner.scale_factor()),
+                                ),
+                                &window_item.as_pin_ref().cached_rendering_data,
+                            );
+                        }
                     }
                 }
 
diff --git a/internal/renderers/skia/itemrenderer.rs b/internal/renderers/skia/itemrenderer.rs
index 0df476a1119..4de5889e804 100644
--- a/internal/renderers/skia/itemrenderer.rs
+++ b/internal/renderers/skia/itemrenderer.rs
@@ -376,7 +376,20 @@ impl<'a> ItemRenderer for SkiaItemRenderer<'a> {
         size: LogicalSize,
         _cache: &CachedRenderingData,
     ) {
-        self.draw_rect(size, rect.background());
+        let geometry = PhysicalRect::from(size * self.scale_factor);
+        if geometry.is_empty() {
+            return;
+        }
+
+        let paint = match self.brush_to_paint(
+            rect.background(),
+            geometry.width_length(),
+            geometry.height_length(),
+        ) {
+            Some(paint) => paint,
+            None => return,
+        };
+        self.canvas.draw_rect(to_skia_rect(&geometry), &paint);
     }
 
     fn draw_border_rectangle(
@@ -456,6 +469,16 @@ impl<'a> ItemRenderer for SkiaItemRenderer<'a> {
         }
     }
 
+    fn draw_window_background(
+        &mut self,
+        _rect: Pin<&dyn i_slint_core::item_rendering::RenderRectangle>,
+        _self_rc: &ItemRc,
+        _size: LogicalSize,
+        _cache: &CachedRenderingData,
+    ) {
+        // The background is drawn directly by FemtoVG renderer (via clear_color, if necessary).
+    }
+
     fn draw_image(
         &mut self,
         image: Pin<&dyn RenderImage>,
@@ -929,20 +952,6 @@ impl<'a> ItemRenderer for SkiaItemRenderer<'a> {
         );
     }
 
-    fn draw_rect(&mut self, size: LogicalSize, brush: Brush) {
-        let geometry = PhysicalRect::from(size * self.scale_factor);
-        if geometry.is_empty() {
-            return;
-        }
-
-        let paint =
-            match self.brush_to_paint(brush, geometry.width_length(), geometry.height_length()) {
-                Some(paint) => paint,
-                None => return,
-            };
-        self.canvas.draw_rect(to_skia_rect(&geometry), &paint);
-    }
-
     fn window(&self) -> &i_slint_core::window::WindowInner {
         i_slint_core::window::WindowInner::from_pub(self.window)
     }
diff --git a/internal/renderers/skia/lib.rs b/internal/renderers/skia/lib.rs
index 958fd7f97a5..b3adde3dc38 100644
--- a/internal/renderers/skia/lib.rs
+++ b/internal/renderers/skia/lib.rs
@@ -571,19 +571,24 @@ impl SkiaRenderer {
                 item_renderer = &mut partial_renderer;
             }
 
-            // Draws the window background as gradient
-            match window_inner.window_item().map(|w| w.as_pin_ref().background()) {
-                Some(Brush::SolidColor(clear_color)) => {
-                    skia_canvas.clear(itemrenderer::to_skia_color(&clear_color));
-                }
-                None => {}
-                Some(brush @ _) => {
-                    item_renderer.draw_rect(
-                        i_slint_core::lengths::logical_size_from_api(
-                            window.size().to_logical(window_inner.scale_factor()),
-                        ),
-                        brush,
-                    );
+            if let Some(window_item_rc) = window_inner.window_item_rc() {
+                let window_item =
+                    window_item_rc.downcast::<i_slint_core::items::WindowItem>().unwrap();
+                match window_item.as_pin_ref().background() {
+                    Brush::SolidColor(clear_color) => {
+                        skia_canvas.clear(itemrenderer::to_skia_color(&clear_color));
+                    }
+                    _ => {
+                        // Draws the window background as gradient
+                        item_renderer.draw_rectangle(
+                            window_item.as_pin_ref(),
+                            &window_item_rc,
+                            i_slint_core::lengths::logical_size_from_api(
+                                window.size().to_logical(window_inner.scale_factor()),
+                            ),
+                            &window_item.as_pin_ref().cached_rendering_data,
+                        );
+                    }
                 }
             }
 


Test patch:
diff --git a/api/rs/slint/tests/partial_renderer.rs b/api/rs/slint/tests/partial_renderer.rs
index 69f03df102f..aee4c0f62b1 100644
--- a/api/rs/slint/tests/partial_renderer.rs
+++ b/api/rs/slint/tests/partial_renderer.rs
@@ -475,3 +475,29 @@ fn rotated_image() {
         Some(slint::LogicalPosition { x: 1., y: 50. })
     );
 }
+
+#[test]
+fn window_background() {
+    slint::slint! {
+        export component Ui inherits Window {
+            in property <color> c: yellow;
+            background: c;
+        }
+    }
+
+    slint::platform::set_platform(Box::new(TestPlatform)).ok();
+    let ui = Ui::new().unwrap();
+    let window = WINDOW.with(|x| x.clone());
+    window.set_size(slint::PhysicalSize::new(180, 260));
+    ui.show().unwrap();
+    assert!(window.draw_if_needed(|renderer| {
+        do_test_render_region(renderer, 0, 0, 180, 260);
+    }));
+    assert!(!window.draw_if_needed(|_| { unreachable!() }));
+    ui.set_c(slint::Color::from_rgb_u8(45, 12, 13));
+    assert!(window.draw_if_needed(|renderer| {
+        do_test_render_region(renderer, 0, 0, 180, 260);
+    }));
+    ui.set_c(slint::Color::from_rgb_u8(45, 12, 13));
+    assert!(!window.draw_if_needed(|_| { unreachable!() }));
+}