Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

API Review: Remove SharedImageBuffer from public API and rename Window::grab_window() to take_snapshot() #5557

Merged
merged 1 commit into from
Jul 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ All notable changes to this project are documented in this file.
- allow all clippy warnings in generated code
- Add `slint::Image::image_buffer()` getter to obtain pixels for a `slint::Image` if available.
- Fix panic in `slint::Timer` when a new timer is started while stopping another.
- Added `slint::Window::grab_window()`
- Added `slint::Window::take_snapshot()`

### Interpreter
- Track model lenght change when accessing a model out of bounds
Expand Down
3 changes: 1 addition & 2 deletions api/rs/slint/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -210,8 +210,7 @@ pub use i_slint_core::component_factory::ComponentFactory;
pub use i_slint_core::graphics::{BorrowedOpenGLTextureBuilder, BorrowedOpenGLTextureOrigin};
// keep in sync with internal/interpreter/api.rs
pub use i_slint_core::graphics::{
Brush, Color, Image, LoadImageError, Rgb8Pixel, Rgba8Pixel, RgbaColor, SharedImageBuffer,
SharedPixelBuffer,
Brush, Color, Image, LoadImageError, Rgb8Pixel, Rgba8Pixel, RgbaColor, SharedPixelBuffer,
};
pub use i_slint_core::model::{
FilterModel, MapModel, Model, ModelExt, ModelNotify, ModelPeer, ModelRc, ModelTracker,
Expand Down
15 changes: 8 additions & 7 deletions internal/backends/qt/qt_window.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ use i_slint_core::graphics::rendering_metrics_collector::{
RenderingMetrics, RenderingMetricsCollector,
};
use i_slint_core::graphics::{
euclid, Brush, Color, FontRequest, IntRect, Point, SharedImageBuffer,
euclid, Brush, Color, FontRequest, IntRect, Point, Rgba8Pixel, SharedImageBuffer,
SharedPixelBuffer,
};
use i_slint_core::input::{KeyEvent, KeyEventType, MouseEvent};
use i_slint_core::item_rendering::{
Expand Down Expand Up @@ -2324,26 +2325,26 @@ impl i_slint_core::renderer::RendererSealed for QtWindow {
// No-op because QtWindow is also the WindowAdapter
}

fn screenshot(&self) -> Result<SharedImageBuffer, PlatformError> {
fn take_snapshot(&self) -> Result<SharedPixelBuffer<Rgba8Pixel>, PlatformError> {
let widget_ptr = self.widget_ptr();

let size = cpp! {unsafe [widget_ptr as "QWidget*"] -> qttypes::QSize as "QSize" {
return widget_ptr->size();
}};

let rgb8_data = cpp! {unsafe [widget_ptr as "QWidget*"] -> qttypes::QByteArray as "QByteArray" {
let rgba8_data = cpp! {unsafe [widget_ptr as "QWidget*"] -> qttypes::QByteArray as "QByteArray" {
QPixmap pixmap = widget_ptr->grab();
QImage image = pixmap.toImage();
image.convertTo(QImage::Format_RGB888);
image.convertTo(QImage::Format_ARGB32);
return QByteArray(reinterpret_cast<const char *>(image.constBits()), image.sizeInBytes());
}};

let buffer = i_slint_core::graphics::SharedPixelBuffer::<i_slint_core::graphics::Rgb8Pixel>::clone_from_slice(
rgb8_data.to_slice(),
let buffer = i_slint_core::graphics::SharedPixelBuffer::<i_slint_core::graphics::Rgba8Pixel>::clone_from_slice(
rgba8_data.to_slice(),
size.width,
size.height,
);
Ok(i_slint_core::graphics::SharedImageBuffer::RGB8(buffer))
Ok(buffer)
}
}

Expand Down
8 changes: 4 additions & 4 deletions internal/backends/testing/slint_systest.proto
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ message RequestSetElementAccessibleValue {
string value = 2;
}

message RequestGrabWindow {
message RequestTakeSnapshot {
Handle window_handle = 1;
}

Expand All @@ -75,7 +75,7 @@ message RequestToAUT {
RequestElementProperties request_element_properties = 4;
RequestInvokeElementAccessibilityAction request_invoke_element_accessibility_action = 5;
RequestSetElementAccessibleValue request_set_element_accessible_value = 6;
RequestGrabWindow request_grab_window = 7;
RequestTakeSnapshot request_take_snapshot = 7;
RequestElementClick request_element_click = 8;
}
}
Expand Down Expand Up @@ -169,7 +169,7 @@ message InvokeElementAccessibilityActionResponse {
message SetElementAccessibleValueResponse {
}

message GrabWindowResponse {
message TakeSnapshotResponse {
bytes window_contents_as_png = 1;
}

Expand All @@ -185,7 +185,7 @@ message AUTResponse {
ElementPropertiesResponse element_properties = 5;
InvokeElementAccessibilityActionResponse invoke_element_accessibility_action_response = 6;
SetElementAccessibleValueResponse set_element_accessible_value_response = 7;
GrabWindowResponse grab_window_response = 8;
TakeSnapshotResponse take_snapshot_response = 8;
ElementClickResponse element_click_response = 9;
}
}
32 changes: 18 additions & 14 deletions internal/backends/testing/systest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ use futures_lite::AsyncReadExt;
use futures_lite::AsyncWriteExt;
use i_slint_core::api::EventLoopError;
use i_slint_core::debug_log;
use i_slint_core::graphics::SharedImageBuffer;
use i_slint_core::item_tree::ItemTreeRc;
use i_slint_core::window::WindowAdapter;
use i_slint_core::window::WindowInner;
Expand Down Expand Up @@ -132,10 +131,10 @@ impl TestingClient {
proto::SetElementAccessibleValueResponse {},
)
}
proto::mod_RequestToAUT::OneOfmsg::request_grab_window(proto::RequestGrabWindow {
window_handle,
}) => proto::mod_AUTResponse::OneOfmsg::grab_window_response(
self.grab_window(handle_to_index(
proto::mod_RequestToAUT::OneOfmsg::request_take_snapshot(
proto::RequestTakeSnapshot { window_handle },
) => proto::mod_AUTResponse::OneOfmsg::take_snapshot_response(
self.take_snapshot(handle_to_index(
window_handle
.ok_or_else(|| "grab window request missing window handle".to_string())?,
))?,
Expand Down Expand Up @@ -172,25 +171,30 @@ impl TestingClient {
})
}

fn grab_window(
fn take_snapshot(
&self,
window_index: generational_arena::Index,
) -> Result<proto::GrabWindowResponse, String> {
) -> Result<proto::TakeSnapshotResponse, String> {
use image::ImageEncoder;

let adapter = self.window_adapter(window_index)?;
let window = adapter.window();
let buffer =
window.grab_window().map_err(|e| format!("Error grabbing window screenshot: {e}"))?;
window.take_snapshot().map_err(|e| format!("Error grabbing window screenshot: {e}"))?;
let mut window_contents_as_png: Vec<u8> = Vec::new();
let cursor = std::io::Cursor::new(&mut window_contents_as_png);
let encoder = image::codecs::png::PngEncoder::new(cursor);
match buffer {
SharedImageBuffer::RGB8(buffer) => encoder.write_image(buffer.as_bytes(), buffer.width(), buffer.height(), image::ColorType::Rgb8),
SharedImageBuffer::RGBA8(buffer) => encoder.write_image(buffer.as_bytes(), buffer.width(), buffer.height(), image::ColorType::Rgba8),
SharedImageBuffer::RGBA8Premultiplied(_) => return Err("window screenshot produced screenshot with pre-multiplied alpha; this is currently not supported".into()),
}.map_err(|encode_err| format!("error encoding png image after screenshot: {encode_err}"))?;
Ok(proto::GrabWindowResponse { window_contents_as_png })
encoder
.write_image(
buffer.as_bytes(),
buffer.width(),
buffer.height(),
image::ColorType::Rgba8,
)
.map_err(|encode_err| {
format!("error encoding png image after screenshot: {encode_err}")
})?;
Ok(proto::TakeSnapshotResponse { window_contents_as_png })
}

fn find_elements_by_id(
Expand Down
8 changes: 4 additions & 4 deletions internal/core/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ This module contains types that are public and re-exported in the slint-rs as we

#[cfg(target_has_atomic = "ptr")]
pub use crate::future::*;
use crate::graphics::SharedImageBuffer;
use crate::graphics::{Rgba8Pixel, SharedPixelBuffer};
use crate::input::{KeyEventType, MouseEvent};
use crate::item_tree::ItemTreeVTable;
use crate::window::{WindowAdapter, WindowInner};
Expand Down Expand Up @@ -656,12 +656,12 @@ impl Window {
}
}

/// Returns an image buffer with the rendered contents.
/// Takes a snapshot of the window contents and returns it as RGBA8 encoded pixel buffer.
///
/// Note that this function may be slow to call. Reading from the framebuffer previously
/// rendered, too, may take a long time.
pub fn grab_window(&self) -> Result<SharedImageBuffer, PlatformError> {
self.0.window_adapter().renderer().screenshot()
pub fn take_snapshot(&self) -> Result<SharedPixelBuffer<Rgba8Pixel>, PlatformError> {
self.0.window_adapter().renderer().take_snapshot()
}
}

Expand Down
94 changes: 85 additions & 9 deletions internal/core/graphics/image.rs
Original file line number Diff line number Diff line change
Expand Up @@ -667,6 +667,86 @@ impl Image {
})
}

/// Returns the pixel buffer for the Image if available in RGB format without alpha.
/// Returns None if the pixels cannot be obtained, for example when the image was created from borrowed OpenGL textures.
pub fn to_rgb8(&self) -> Option<SharedPixelBuffer<Rgb8Pixel>> {
self.0.render_to_buffer(None).and_then(|image| match image {
SharedImageBuffer::RGB8(buffer) => Some(buffer),
_ => None,
})
}

/// Returns the pixel buffer for the Image if available in RGBA format.
/// Returns None if the pixels cannot be obtained, for example when the image was created from borrowed OpenGL textures.
pub fn to_rgba8(&self) -> Option<SharedPixelBuffer<Rgba8Pixel>> {
self.0.render_to_buffer(None).and_then(|image| match image {
SharedImageBuffer::RGB8(buffer) => Some(SharedPixelBuffer::<Rgba8Pixel> {
width: buffer.width,
height: buffer.height,
data: buffer.data.into_iter().map(Into::into).collect(),
}),
SharedImageBuffer::RGBA8(buffer) => Some(buffer),
SharedImageBuffer::RGBA8Premultiplied(buffer) => {
Some(SharedPixelBuffer::<Rgba8Pixel> {
width: buffer.width,
height: buffer.height,
data: buffer
.data
.into_iter()
.map(|rgba_premul| {
if rgba_premul.a == 0 {
Rgba8Pixel::new(0, 0, 0, 0)
} else {
let af = rgba_premul.a as f32 / 255.0;
Rgba8Pixel {
r: (rgba_premul.r as f32 * 255. / af) as u8,
g: (rgba_premul.g as f32 * 255. / af) as u8,
b: (rgba_premul.b as f32 * 255. / af) as u8,
a: rgba_premul.a,
}
}
})
.collect(),
})
}
})
}

/// Returns the pixel buffer for the Image if available in RGBA format, with the alpha channel pre-multiplied
/// to the red, green, and blue channels.
/// Returns None if the pixels cannot be obtained, for example when the image was created from borrowed OpenGL textures.
pub fn to_rgba8_premultiplied(&self) -> Option<SharedPixelBuffer<Rgba8Pixel>> {
self.0.render_to_buffer(None).and_then(|image| match image {
SharedImageBuffer::RGB8(buffer) => Some(SharedPixelBuffer::<Rgba8Pixel> {
width: buffer.width,
height: buffer.height,
data: buffer.data.into_iter().map(Into::into).collect(),
}),
SharedImageBuffer::RGBA8(buffer) => Some(SharedPixelBuffer::<Rgba8Pixel> {
width: buffer.width,
height: buffer.height,
data: buffer
.data
.into_iter()
.map(|rgba| {
if rgba.a == 255 {
rgba
} else {
let af = rgba.a as f32 / 255.0;
Rgba8Pixel {
r: (rgba.r as f32 * af / 255.) as u8,
g: (rgba.g as f32 * af / 255.) as u8,
b: (rgba.b as f32 * af / 255.) as u8,
a: rgba.a,
}
}
})
.collect(),
}),
SharedImageBuffer::RGBA8Premultiplied(buffer) => Some(buffer),
})
}

/// Creates a new Image from an existing OpenGL texture. The texture remains borrowed by Slint
/// for the duration of being used for rendering, such as when assigned as source property to
/// an `Image` element. It's the application's responsibility to delete the texture when it is
Expand Down Expand Up @@ -757,12 +837,6 @@ impl Image {
_ => None,
}
}

/// Returns the pixel buffer for the Image, if available. Returns None if the pixels cannot
/// be obtained, for example when the image was created from borrowed OpenGL textures.
pub fn image_buffer(&self) -> Option<SharedImageBuffer> {
self.0.render_to_buffer(None)
}
}

/// This enum describes the origin to use when rendering a borrowed OpenGL texture.
Expand Down Expand Up @@ -848,13 +922,15 @@ pub fn load_image_from_embedded_data(data: Slice<'static, u8>, format: Slice<'_,
fn test_image_size_from_buffer_without_backend() {
{
assert_eq!(Image::default().size(), Default::default());
assert!(Image::default().image_buffer().is_none());
assert!(Image::default().to_rgb8().is_none());
assert!(Image::default().to_rgba8().is_none());
assert!(Image::default().to_rgba8_premultiplied().is_none());
}
{
let buffer = SharedPixelBuffer::<Rgb8Pixel>::new(320, 200);
let image = Image::from_rgb8(buffer.clone());
assert_eq!(image.size(), [320, 200].into());
assert_eq!(image.image_buffer(), Some(SharedImageBuffer::RGB8(buffer)));
assert_eq!(image.to_rgb8().as_ref().map(|b| b.as_slice()), Some(buffer.as_slice()));
}
}

Expand All @@ -864,7 +940,7 @@ fn test_image_size_from_svg() {
let simple_svg = r#"<svg width="320" height="200" xmlns="http://www.w3.org/2000/svg"></svg>"#;
let image = Image::load_from_svg_data(simple_svg.as_bytes()).unwrap();
assert_eq!(image.size(), [320, 200].into());
assert_eq!(image.image_buffer().unwrap().size(), image.size());
assert_eq!(image.to_rgba8().unwrap().size(), image.size());
}

#[cfg(feature = "svg")]
Expand Down
8 changes: 4 additions & 4 deletions internal/core/renderer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use alloc::rc::Rc;
use core::pin::Pin;

use crate::api::PlatformError;
use crate::graphics::SharedImageBuffer;
use crate::graphics::{Rgba8Pixel, SharedPixelBuffer};
use crate::item_tree::ItemTreeRef;
use crate::items::TextWrap;
use crate::lengths::{LogicalLength, LogicalPoint, LogicalRect, LogicalSize, ScaleFactor};
Expand Down Expand Up @@ -118,9 +118,9 @@ pub trait RendererSealed {
Ok(())
}

/// Re-implement this function to support Window::grab_window(), i.e. return
/// Re-implement this function to support Window::take_snapshot(), i.e. return
/// the contents of the window in an image buffer.
fn screenshot(&self) -> Result<SharedImageBuffer, PlatformError> {
Err("WindowAdapter::grab_window is not implemented by the platform".into())
fn take_snapshot(&self) -> Result<SharedPixelBuffer<Rgba8Pixel>, PlatformError> {
Err("WindowAdapter::take_snapshot is not implemented by the platform".into())
}
}
19 changes: 15 additions & 4 deletions internal/core/software_renderer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@ mod fonts;
use self::fonts::GlyphRenderer;
use crate::api::{PlatformError, Window};
use crate::graphics::rendering_metrics_collector::{RefreshMode, RenderingMetricsCollector};
use crate::graphics::{BorderRadius, PixelFormat, SharedImageBuffer, SharedPixelBuffer};
use crate::graphics::{
BorderRadius, PixelFormat, Rgba8Pixel, SharedImageBuffer, SharedPixelBuffer,
};
use crate::item_rendering::{CachedRenderingData, DirtyRegion, RenderBorderRectangle, RenderImage};
use crate::items::{ItemRc, TextOverflow, TextWrap};
use crate::lengths::{
Expand Down Expand Up @@ -792,7 +794,7 @@ impl RendererSealed for SoftwareRenderer {
self.partial_cache.borrow_mut().clear();
}

fn screenshot(&self) -> Result<SharedImageBuffer, PlatformError> {
fn take_snapshot(&self) -> Result<SharedPixelBuffer<Rgba8Pixel>, PlatformError> {
let Some(window_adapter) =
self.maybe_window_adapter.borrow().as_ref().and_then(|w| w.upgrade())
else {
Expand All @@ -807,7 +809,7 @@ impl RendererSealed for SoftwareRenderer {
let Some((width, height)) = size.width.try_into().ok().zip(size.height.try_into().ok())
else {
// Nothing to render
return Err("grab_window() called on window with invalid size".into());
return Err("take_snapshot() called on window with invalid size".into());
};

let mut target_buffer = SharedPixelBuffer::<crate::graphics::Rgb8Pixel>::new(width, height);
Expand All @@ -817,7 +819,16 @@ impl RendererSealed for SoftwareRenderer {
// ensure that caches are clear for the next call
self.set_repaint_buffer_type(RepaintBufferType::NewBuffer);

Ok(SharedImageBuffer::RGB8(target_buffer))
let mut target_buffer_with_alpha =
SharedPixelBuffer::<Rgba8Pixel>::new(target_buffer.width(), target_buffer.height());
for (target_pixel, source_pixel) in target_buffer_with_alpha
.make_mut_slice()
.iter_mut()
.zip(target_buffer.as_slice().iter())
{
*target_pixel.rgb_mut() = *source_pixel;
}
Ok(target_buffer_with_alpha)
}
}

Expand Down
Loading
Loading