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

ContentFit support for images #1219

Merged
merged 12 commits into from
Feb 23, 2022
Merged

ContentFit support for images #1219

merged 12 commits into from
Feb 23, 2022

Conversation

Alch-Emi
Copy link
Contributor

@Alch-Emi Alch-Emi commented Jan 23, 2022

Closes #1179
Closes #1014

Adds support for various image fit modes, documented and explained in the new fit module and explained in #1179.

Additionally, the tour example has been updated to show off the new functionality. To this end, an additional image was added to the tour example which satisfies the need for a background (to clearly denote the bounds of the image) and a size smaller than the screen (so that the special behavior of the ScaleDown mode can be seen).

Breaking changes:

  • Images now take up space slightly differently, although defaults are a very close approximation of original behavior
  • The public fit field was added to the Image struct
  • Images on web now follow their height property properly, if set

Non-breaking Changes:

  • Addition of ContentFit to iced_core (and appropriate re-exports)
  • Update of the tour example
  • Addition of the fit(ContentFit) method to the Image widget
  • Adjustments to the native rendering of images to support the new ContentFits

Recommended future work:

  • This PR does not provide any logic pertaining to the alignment of images within the space alotted to them, and chooses to always align images into the top-left corner, as this behavior is closest to the original behavior. However, varied alignment behavior (or at least the ability to center the content within its frame) will doubtlessly be desired.
fit2.mp4

Copy link
Member

@hecrj hecrj left a comment

Choose a reason for hiding this comment

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

This is great! Thank you 🥳

I just have a couple of small suggestions!

core/src/fit.rs Outdated Show resolved Hide resolved
src/lib.rs Show resolved Hide resolved
examples/tour/src/main.rs Outdated Show resolved Hide resolved
core/src/fit.rs Outdated Show resolved Hide resolved
core/src/fit.rs Outdated Show resolved Hide resolved
Alch-Emi added a commit to Alch-Emi/iced that referenced this pull request Feb 12, 2022
@Alch-Emi
Copy link
Contributor Author

Alright, that should be everything! Any other thoughts?

@Alch-Emi Alch-Emi requested a review from hecrj February 13, 2022 13:58
@Alch-Emi
Copy link
Contributor Author

Alch-Emi commented Feb 15, 2022

Should I include a patch for the iced_pure image widget in this PR too now that that exists, or should that be a separate PR?

Copy link
Member

@hecrj hecrj left a comment

Choose a reason for hiding this comment

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

Looks great! 🥳 Thank you 🙇

I made some changes here and there, since I realized we could add ContentFit support for the Svg widget as well!

Let me know what you think and we can merge this! 🚢

Should I include a patch for the iced_pure image widget in this PR too now that that exists, or should that be a separate PR?

Don't worry! The changes there haven't landed yet. I can deal with the conflicts :)

@Alch-Emi
Copy link
Contributor Author

Looks great! My two concerns with the large & transparent Ferris image are that, because she doesn't have a background, it's hard to tell where the border is, and that it can't showcase the OnlyScaleDown option, since it can't scale up. The border wouldn't normally be a huge deal, and to be fair, it still isn't, but an important part of understanding the image modes is understanding how they affect the border. At the very least, I think we need to use a smaller Ferris, and preferably one with a non-white background.

@hecrj
Copy link
Member

hecrj commented Feb 23, 2022

I think Contain, Cover, and Fill are easy to grasp.

I have removed the other two from the tour, since we don't really need to be that exhaustive. Maybe we can come up with a dedicated image example to showcase all of the ContentFit variants properly.

Thank you again! 🙇 Let's merge this!

@hecrj hecrj merged commit 9fe5080 into iced-rs:master Feb 23, 2022
@hecrj hecrj added this to the 0.4.0 milestone Apr 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature Request] Control image fit / scaling in image widget How do I stretch an image
2 participants