Adding feature: Image rotation

[DKolter]

Hello dear core team!

For one of my projects, I have found the need to draw a rotated image as a part of an animation in a custom Widget
implementation. I have added a rotation field to the image::Renderer::draw function and have implemented the necessary shader code for the wgpu renderer and tiny_skia renderer.
There is also a small app for testing this behaviour, which currently looks like this:

clip.mp4

scale_factor has been tested too, which caused some issues on the tiny_skia renderer previously.

As discussed with @hecrj, it might be benefical to add a property to the widget::Image too, but there was still some discussion on whether the layout should change when the image is rotated. For example when rotated by 45 degrees, the layout bounds would increase in all directions. This could be a wanted effect, but also unwanted for i.e. animations like in the example app.

Please let me know what you think!

Thanks for your great work on iced

[DKolter]

Proposed syntax:

Image::new("icon.png")
    .rotation(rotation_radians)
    .rotation_layout(RotationLayout::Change),

Output: (Left is RotationLayout::Keep, right is RotationLayout::Change:

clip.mp4

[Gigas002]

The image widget's layout modification changes the way image displayed when loaded for me.

When commented/before changes (\w ContentFit::Contain):

Uncommented, RotationLayout::Change:

Uncommented, RotationLayout::Keep:

Also, I'm not sure if it's expected behavior, but aspect ratio on rotation looks weird. On the screenshots below, the left image is rotated using image crate's rotate function for comparison

Default (no rotation):

90 degree rotation:

[DKolter]

I see, thanks for pointing out. The rotation should probably be done before the content fit layout applies, is that correct?

[Gigas002]

That sounds correct. For now it looks like it's keeping the aspect ration of the created widget, scaling actual content inside on rotation, I think.

[Gigas002]

Hmmm. Loaded image and rotation itself looks correct now, but I have a feeling widget's bounds or axis are not changed on rotation? That's noticeable when changing the size of window with rotated image in it, especially when using ContentFit::Fill:

2024-03-18.21-00-58.mp4

Or ContentFit::Contain:

2024-03-18.21-05-52.mp4

Sorry, if my assumptions are incorrect, I've only tested these changes in my usecase.

[DKolter]

Yes I removed the layout changes, because I was not able to correctly implement all of the edge cases. I will give this another go, but for now the rotation only visually changes the image, not affecting the layout

[DKolter]

I now added support for the content_fit scaling for both the wgpu and tiny_skia backend. Can you double check whether it behaves as expected on your end now @Gigas002 ?

[DKolter]

Example for two images with width Length::Fill and RotationLayout::Change:

clip1.mp4

With RotationLayout::Keep:

clip2.mp4

With RotationLayout::Change and ContentFit::Fill:

clip3.mp4

[Gigas002]

Thank you, @DKolter ! Works great on my end 👍

[Gigas002]

Ah, sorry, @DKolter, I didn't notice it on a first run. After careful view, the ContentFit::None's behavior seems a little bit off.
Resizing the window doesn't change image size, as expected. However, it changes image, or maybe the widget position, while it shouldn't:

The default ContentFit::None behavior:

2024-03-25.19-59-59.mp4

When using this PR's build:

2024-03-25.19-58-03.mp4

I'm not actually sure if it counts as breaking behavior or not, but decided to report it just in case.

I think we can match on content_fit in draw, when creating drawing_bounds and use ..bounds to initialize x/y values when using ContentFit::None, and then it works as expected. But maybe it's not the best solution, it's just the one I came up with.

The others ContentFits works without issues, it seems.

[Gigas002]

One more issue I've noticed is related to "big" images. I can't say for sure how big these should be, but sometimes the image gets weirdly cropped and layout is a bit off. Here's an example of image I can repro this:

And I also use rotated 90 degree rotated version of this image.

Used ContentFit::Fill.

Master branch:

This PR:

Use command to rotate this image 90 degree:

Rotated image:

Use command to rotate this rotated image 90 degree:

[DKolter]

For the first issue: Yes, matching on the content fit is probably the best solution, because it requires special treatment for ContentFit::None.

Second issue: Can you add more details to each image which parameters you passed to which branch? It seems to me, that RotationLayout::Keep is used, which makes the images look weird on purpose (Keeping layout instead of adapting to the rotated image size)

[Gigas002]

The second issue happens for both RotationLayout::Keep and RotationLayout::Change. Here's code that I use to create the widget:

        let handle = Handle::from_path(image_path);
        let image = Image::new(handle)
            .content_fit(ContentFit::Fill)
            .filter_method(FilterMethod::Linear)
            .width(Length::Fill)
            .height(Length::Fill)
            .rotation(self.rotation)
            .rotation_layout(iced::RotationLayout::Change);

The rotation is 90 degrees in radians and triggered on button event. Other parameters are the same. I'm loading the above image and a rotated version of that image. After the image had loaded, I'm playing with window sizes, image rotation and swapping the images.

[DKolter]

I was able to reproduce the issue, it only appears with the wgpu backend, which suggests that there is an issue with either the transformations applied in the shader or with the way the image is being split into parts when it is too large. I will look into it

[DKolter]

Both issues should be resolved now, iced and wgpu backend behave the same now. In the future we could also add the rotation behaviour to the other elements, such as the Svg widget

[Gigas002]

Thank you! Will comment again as I'll be able to run some tests later.

In the future we could also add the rotation behaviour to the other elements

I'm currently testing it with viewer widget, however since it lacks content_fit property I was hoping #2330 would be merged (if it would be merged) first. This PR's rotation functions works mostly same as in image widget, so far looks good

[Gigas002]

Tried out all the ContentFits and some big images, everything works pretty good IMO 👍
Will comment again, if notice something else

[DKolter]

I added the same feature to the Svg widget too. Also I noticed that the behaviour of the ContentFit::None was still not consistent with the same image when rotated in GIMP, now it should be. If you want to, you can run your tests again to verify that it is now more consistent @Gigas002, thank you very much!

[Gigas002]

Hey, @DKolter ! I'm a bit busy this week, so I can't promise I'll be able to try out these changes soon, but I'll try to find some time, maybe on weekends.

[Gigas002]

Hey, @DKolter! Was able to run some tests again, looks great to me so far. Sometimes I notice a small flickering while resizing the window with ContentFit::None, especially when the window is very small, but I guess that's hardly avoidable.

Also, would it be OK to ping you if/when my PR with ContentFit implementation for Viewer merges, and I'll suggest a PR with your rotation feature to it, or would you mind giving an advice?
I'm trying this out in my personal branch atm (don't mind the commit diff, I actually use your branch + custom viewer.rs locally, so only this file is needed), and everything except ContentFit::None works good, but I can't figure out how this can be fixed. Viewer is a bit more complicated: it has to respect zooming capabilities and draggind the image here and there, plus to content fit and resizing functions...

[DKolter]

Hey @Gigas002 , thanks for checking out the changes! I would gladly help you with the Viewer widget, if you want to, you can ping me over discord so we can discuss the issue. Username dkolter

[hecrj]

Thanks! Looks great 🥳

I have simplified and renamed some stuff here and there. The most important change is the removal of the scale argument to draw_image and draw_svg. It seemed redundant since we are already providing specific image bounds.

I have also created a new ferris example to showcase both the ContentFit and Rotation strategies:

2024-05-02.17-12-11.mp4

Let me know what you think!

[hecrj]

Fixed iced_tiny_skia incremental rendering when using rotations:

2024-05-03.07-03-52.mp4