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

Image: data url support #7201

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

task-jp
Copy link
Contributor

@task-jp task-jp commented Dec 23, 2024

Data url format is useful when one tries slintpad with images.

Image {
source: @image-url("data:image/png;base64,iV....=");
}

@task-jp
Copy link
Contributor Author

task-jp commented Dec 23, 2024

Does dataurl I added depend on some old version of crates that didn't support no_std yet? In such case how do we solve it in general?

Copy link
Member

@ogoffart ogoffart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR.
This is an usefull feature to have.
I'm thinking the decoding could be done in the compiler instead. (and stored in EmbeddedData)

@task-jp
Copy link
Contributor Author

task-jp commented Dec 24, 2024

Thanks for your feedback.

Does "in the compiler" mean decoding the image in from_at_image_url_node?

In the current version, I wanted to use image::load_from_memory_with_format() as same place as image::open().
Another reason is that in the case of wasm, we can skip the decoding as below.

#[cfg(target_arch = "wasm32")]
return self.lookup_image_in_cache_or_create(cache_key, |_| {
    return Some(ImageInner::HTMLImage(vtable::VRc::new(
        super::htmlimage::HTMLImage::new(&str),
    )));
});

@task-jp
Copy link
Contributor Author

task-jp commented Dec 24, 2024

It may be better not to introduce the DataUrl for ImageReference. Adding "data:/" block below the "builtin:/" block could be good enough.

Data url format is useful when one tries slintpad with images.

Image {
    source: @image-url("data:image/png;base64,iV....=");
}

Fixes: slint-ui#4905
@tronical
Copy link
Member

Thanks for the PR. This is an usefull feature to have. I'm thinking the decoding could be done in the compiler instead. (and stored in EmbeddedData)

I agree. This feature can most likely be handled entirely in the compiler, without introducing any additional functions and dependencies in the runtime.

Is there a particular benefit to letting the browser do the data url decoding at runtime for wasm builds?

@task-jp
Copy link
Contributor Author

task-jp commented Dec 24, 2024

Is there a particular benefit to letting the browser do the data url decoding at runtime for wasm builds?

I don't have any strong reason for that, it just looked cleaner for me. I'll move the logic in the compiler.

@task-jp
Copy link
Contributor Author

task-jp commented Dec 25, 2024

I need more advise to make it right.

As far as I checked, there is only one place that EmbeddedData is created.

_ => ImageReference::EmbeddedData {

The code here is called during compilation but if I use slint-viewer, it won't be called because embed_images returns immediately in the first if statement.

pub async fn embed_images(
    ...
) {
    if embed_files == EmbedResourcesKind::Nothing && resource_url_mapper.is_none() {
        return;
    }

Is that correct?

@ogoffart
Copy link
Member

Indeed, it may need to be handled separately depending on the language.

  • In rust, we generate an include_bytes! in the embedded_file_tokens function. Instead, we need to generate a slice. (Like we do already for builtins).

    let data = embedded_file_tokens(path);

  • In C++, we do include the data in the binary so we should do the dame for data, but without opening the file:

    let resource_file = crate::fileaccess::load_file(std::path::Path::new(path)).unwrap(); // embedding pass ensured that the file exists

  • in the interpreter we'd open the file there, and we would need to handle the data url similarly to the builtin

    i_slint_compiler::expression_tree::ImageReference::AbsolutePath(path) => {

I guess there could be an helper function in i_slint_compiler to share some of the decoding.
Maybe ImageReference can get a new enum value like ImageReference::Data { data: String, extension: String }` (which contain the actual data (base-64 decoded))

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants