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

Missing implementation for rotating Image using software renderer #3068

Open
kivimango opened this issue Jul 9, 2023 · 2 comments
Open

Missing implementation for rotating Image using software renderer #3068

kivimango opened this issue Jul 9, 2023 · 2 comments
Labels
a:renderer-software Software Renderer (mO,bF) enhancement New feature or request

Comments

@kivimango
Copy link

Hello !

I'm receiving a panic when i want to rotate the image in slint using software renderer (other platforms works ok)

thread 'main' panicked at 'not yet implemented', /home/user/.cargo/git/checkouts/slint-8153123e5dffa129/4a56531/internal/core/software_renderer.rs:1954:9

Could you please implement the code for rotating images for sw renderer too ?
https://github.com/slint-ui/slint/blob/master/internal/core/software_renderer.rs#L1954

Also, i don't know if it is the intended way of work, but if i don't set the rotation-angle explicitly to 0 on the Image widget,
changing it elsewhere, (for instance in the clicked() callback of a button) wont do anything.

Run it with

SLINT_BACKEND=winit-software cargo run
// Minimal, Reproducible Example 
slint::slint! {
import { Button, VerticalBox } from "std-widgets.slint";
export component Demo {
    VerticalBox {
        alignment: start;
        img := Image {
            source: @image-url("https://slint.dev/logo/slint-logo-full-light.svg");
            // when rotation-angle property is not set explicitly, setting it elsewhere wont do anything 
            rotation-angle: 0;
            height: 100px;
        }
        HorizontalLayout {
            alignment: center;
            Button {
                text: "Rotate";
                clicked => {
                img.rotation-angle += 90deg;
                }
            }
        }
    }
}
}

fn main() {
    Demo::new().unwrap().run().unwrap();
}
ogoffart added a commit that referenced this issue Jul 10, 2023
... if they are not set as actual binding before

As reported in #3068

The problem is that the pass will properly create the Rotation or
Opacity item, but will not create the two way binding if there is no
existing binding. This causes code like `img.rotation-angle = ...` to
change the rotation angle of the image, but not its parent Rotation
item.
Fix it by making sure there is always a binding.

Since the change only affect visual representation, I abused one of the
screenshot test to test this feature. And I also patched another bug
that some #[allow(unused_parens)] was not set in the 'init' callback and
would cause a warning in the test
@ogoffart ogoffart added the a:renderer-software Software Renderer (mO,bF) label Jul 10, 2023
@ogoffart
Copy link
Member

Thanks for your bug report!

The second part about binding not working when not initialized is indeed a bug. This is being fixed in #3077
Let's keep track only of the fact that the rotation is not implemented in the software renderer.

The software renderer is indeed lacking behind in therms of features.

In order to implement rotated image. This is the things that needs to be done:

  1. One need to implement the todo!() to store the angle in the self.current_state.

  2. And handle this rotation in SceneBuilder::draw_image_impl. This means computing the geometry of the bounding rectangle of the rotated image and store the rotation angle in the SceneTexture.

  3. adapt draw_functions::draw_texture_line to handle this angle. This means that for each pixel, we would have a pos which is computed with a matrix multiplication instead of having a contsant y_pos. Since this code is performence sensitive, we probably want to have two different code path. Instead of doing a matrix multiplication of each pixel, one can just cache a delta x and an delta y or something like that.

ogoffart added a commit that referenced this issue Jul 11, 2023
... if they are not set as actual binding before

As reported in #3068

The problem is that the pass will properly create the Rotation or
Opacity item, but will not create the two way binding if there is no
existing binding. This causes code like `img.rotation-angle = ...` to
change the rotation angle of the image, but not its parent Rotation
item.
Fix it by making sure there is always a binding.

Since the change only affect visual representation, I abused one of the
screenshot test to test this feature. And I also patched another bug
that some #[allow(unused_parens)] was not set in the 'init' callback and
would cause a warning in the test
@kivimango
Copy link
Author

Should have been open a two, separate issue.
Will do next time.
Thanks anyway.

@ogoffart ogoffart added the enhancement New feature or request label Feb 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a:renderer-software Software Renderer (mO,bF) enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants