LinuxKMS: Add support for the Slint software renderer #4334

tronical · 2024-01-13T13:50:32Z

No description provided.

ogoffart

As seen in the CI result, the screenshot test need to be adjusted for the change of rotation direction. (by swapping Rotate90 and rotate 270 in the test

ogoffart · 2024-01-13T15:27:54Z

internal/backends/linuxkms/renderer/sw.rs

+
+        let renderer = Box::new(Self { renderer: SoftwareRenderer::new(), display, size });
+
+        eprintln!("Using Software renderer");


Intentional , all the renderers currently print when they’re in use.

ok. Althoug they probably shouldn't in a production binary. (it would be shown with the FPS env variable, right?)

ogoffart · 2024-01-13T15:46:28Z

internal/core/software_renderer.rs

+        &self,
+        buffer: &mut [impl TargetPixel],
+        pixel_stride: usize,
+        post_render_callback: Option<&dyn Fn(&mut dyn ItemRenderer)>,


Not a fan of having that in the pubic API (even if hidden) of the software renderer.
Also because we will never end up "stabilizing" this since it uses the ItemRenderer which is even more private.

I'd prefer if you would just blit the pixel manually in linuxkms on top of the surface.

The alternative would be to move the cursor handling in i-slint-core and having it as an Image{} item on top of the scene. I think this may be usefull for other backend too such as the uefi.

Otherwise if you don't have time for that, please add an InternalToken parameter to be sure.

Agreed. I'll try the approach with an Image on top, wrapped in a hand-written ItemTree and rendered inline like an inline popup on top of the scene, using the slice passed to draw_contents.

tronical · 2024-01-13T21:33:39Z

Pushed only the CI fix. The cursor drawing is not implemented yet, but I'd like to do that in this PR. I don't want to add more technical debt than absolutely necessary.

github-actions · 2024-06-06T13:22:45Z

internal/backends/linuxkms/renderer/sw.rs

@@ -0,0 +1,144 @@
+// Copyright © SixtyFPS GmbH <info@slint.dev>
+// SPDX-License-Identifier: GPL-3.0-only OR LicenseRef-Slint-Royalty-free-1.1 OR LicenseRef-Slint-commercial


[xtask license_header] _{reported by reviewdog 🐶}

Suggested change

// SPDX-License-Identifier: GPL-3.0-only OR LicenseRef-Slint-Royalty-free-1.1 OR LicenseRef-Slint-commercial

// SPDX-License-Identifier: GPL-3.0-only OR LicenseRef-Slint-Royalty-free-2.0 OR LicenseRef-Slint-Software-3.0

ogoffart · 2024-06-06T14:58:01Z

The reason not to merge that is the mouse cursor API, right?

I suggest, to avoid further bitrot, to take the mouse cursor out for now, and merge it with that limitation. (i'd assume this is meant to be used with touchscreen anyway that don't have mouse)

tronical · 2024-06-06T15:34:31Z

That sounds good to me. I’ll remove those bits.

Two features are still missing: - mouse cursor support - partial rendering

…erer Collect the errors and print the lot of them.

…enderer

tronical · 2024-06-06T19:22:48Z

internal/backends/linuxkms/fullscreenwindowadapter.rs

+                        let cursor_rect = LogicalRect::new(
+                            euclid::point2(mouse_position.x, mouse_position.y),
+                            euclid::Size2D::from_untyped(cursor_image.size().cast()),
+                        );
+                        self.renderer.as_core_renderer().mark_dirty_region(cursor_rect.into());


I think this is harmless, but I can also remove it if desired. It's a no-op as there won't be a mouse with the sw renderer, and otherwise mark_dirty_region is a no-op with the GPU renderers.

But for strictly speaking this is the correct thing to do while we have the code for this approach here (to be removed later with the image we discussed).

ogoffart · 2024-06-06T21:56:56Z

internal/backends/linuxkms/renderer/sw.rs

+
+#[repr(transparent)]
+#[derive(Copy, Clone, bytemuck::Pod, bytemuck::Zeroable)]
+struct DumbBufferPixel(pub u32);


Why can't the existing argb8pixel be used?

I don't see an argb8pixel in our code base.

I see these:

Rgba8Pixel: TargetPixel is not implemented for it and it's the wrong order. (Softbuffer expects 0RGB)

PremultipliedRgbaColor: TargetPixel is implemented for it, but it's the wrong order.

This is basically the same as in commit 46ec787 . If you have a suggestion where it could be shared, then I'm all for it. Should I introduce a "softbuffer-pixel" feature in i-slint-core perhaps? I wonder in which module to place it.

ogoffart · 2024-06-06T21:57:54Z

internal/backends/linuxkms/renderer/sw.rs

+
+        let renderer = Box::new(Self { renderer: SoftwareRenderer::new(), display, size });
+
+        eprintln!("Using Software renderer");


ok. Althoug they probably shouldn't in a production binary. (it would be shown with the FPS env variable, right?)

ogoffart · 2024-06-06T21:59:52Z

internal/backends/linuxkms/Cargo.toml

@@ -20,6 +20,7 @@ renderer-skia = ["renderer-skia-opengl"]
 renderer-skia-vulkan = ["i-slint-renderer-skia/vulkan", "vulkano"]
 renderer-skia-opengl = ["i-slint-renderer-skia/opengl", "drm", "gbm", "gbm-sys", "glutin", "raw-window-handle"]
 renderer-femtovg = ["i-slint-renderer-femtovg", "drm", "gbm", "gbm-sys", "glutin", "raw-window-handle"]
+renderer-software = ["i-slint-core/software-renderer-systemfonts", "i-slint-core/software-renderer-rotation", "dep:bytemuck"]


enabling rotation here means that the rotation becomes public API if the user enable (public) linuxkms feature of Slint :-(

We have the following option:

don't care and still do breaking change later

stabilize it as it (we can still discuss small changes before the release)

Actually polish the rotation feature sooner

use a different way to gate it to make sure it's not callable without importing i-slint-core directly. (using InternalToken or so)

Ooops, good point about it becoming public API. That's not intentional in this PR.

It's a bummer, because the feature itself works well (used in mcu as well as here). But we're not sure yet about the exact API, i.e. explicit in SoftwareRenderer as property, as a parameter to render(), or implicit in the Window, right?

ogoffart reviewed Jan 13, 2024

View reviewed changes

tronical force-pushed the simon/linuxkms-swr branch from de833e1 to 4a522b0 Compare June 6, 2024 13:14

github-actions bot reviewed Jun 6, 2024

View reviewed changes

tronical force-pushed the simon/linuxkms-swr branch from 4a522b0 to 45814eb Compare June 6, 2024 13:54

tronical added 4 commits June 6, 2024 21:19

LinuxKMS: Initial support for rendering with the Slint software renderer

453eb57

Two features are still missing: - mouse cursor support - partial rendering

Document availability of the linuxkms-software backend choice

4bf6b11

LinuxKMS: Improve error reporting when failing to initialize any rend…

da8bc75

…erer Collect the errors and print the lot of them.

LinuxKMS: Add support for partial rendering with the Slint software r…

9bcf779

…enderer

tronical force-pushed the simon/linuxkms-swr branch from 45814eb to 9bcf779 Compare June 6, 2024 19:20

tronical commented Jun 6, 2024

View reviewed changes

ogoffart reviewed Jun 6, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

LinuxKMS: Add support for the Slint software renderer #4334

LinuxKMS: Add support for the Slint software renderer #4334

tronical commented Jan 13, 2024

ogoffart left a comment

ogoffart Jan 13, 2024

tronical Jun 6, 2024 •

edited

ogoffart Jun 6, 2024

ogoffart Jan 13, 2024

tronical Jan 13, 2024

tronical commented Jan 13, 2024

github-actions bot Jun 6, 2024

ogoffart commented Jun 6, 2024

tronical commented Jun 6, 2024

tronical Jun 6, 2024 •

edited

ogoffart Jun 6, 2024

tronical Jun 7, 2024

ogoffart Jun 6, 2024

ogoffart Jun 6, 2024

tronical Jun 7, 2024


		let renderer = Box::new(Self { renderer: SoftwareRenderer::new(), display, size });

		eprintln!("Using Software renderer");

		@@ -0,0 +1,144 @@
		// Copyright © SixtyFPS GmbH <info@slint.dev>
		// SPDX-License-Identifier: GPL-3.0-only OR LicenseRef-Slint-Royalty-free-1.1 OR LicenseRef-Slint-commercial

LinuxKMS: Add support for the Slint software renderer #4334

Are you sure you want to change the base?

LinuxKMS: Add support for the Slint software renderer #4334

Conversation

tronical commented Jan 13, 2024

ogoffart left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tronical Jun 6, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tronical commented Jan 13, 2024

github-actions bot Jun 6, 2024

Choose a reason for hiding this comment

ogoffart commented Jun 6, 2024

tronical commented Jun 6, 2024

tronical Jun 6, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tronical Jun 6, 2024 •

edited

tronical Jun 6, 2024 •

edited