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

Add generic image widget #557

Merged
merged 1 commit into from
Feb 17, 2020
Merged

Conversation

fishrockz
Copy link
Collaborator

I was using this to make plots but it did not scale well.

I am trying a different approach but have been a bit side tracked but made some progress and would like to finish that too.

But this example may be useful to others/(me in the future) so I thought would push it up.

Most of the code is behind feature flags.

Copy link
Member

@cmyr cmyr left a comment

Choose a reason for hiding this comment

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

Okay just a first pass, but this looks reasonable; I think it makes sense for there to be an image widget available, and this is a good start.

If we do merge this we will want to add documentation to all public items.

Thanks for the PR!

druid/Cargo.toml Outdated Show resolved Hide resolved
druid/examples/image.rs Outdated Show resolved Hide resolved
druid/examples/image.rs Outdated Show resolved Hide resolved
druid/examples/image.rs Outdated Show resolved Hide resolved
druid/examples/image.rs Outdated Show resolved Hide resolved
druid/src/widget/images.rs Outdated Show resolved Hide resolved
druid/src/widget/images.rs Outdated Show resolved Hide resolved
druid/src/widget/images.rs Outdated Show resolved Hide resolved
druid/src/widget/images.rs Outdated Show resolved Hide resolved
druid/src/widget/images.rs Outdated Show resolved Hide resolved
@futurepaul
Copy link
Collaborator

Thank you for tackling the various FillStrat options, that's been a todo on SVG that I haven't gotten around to. Perhaps after this lands we can add FillStrat to SVG and use the same type for both.

@fishrockz
Copy link
Collaborator Author

@futurepaul I had loads of changes to svg to allow embedded png/bmp images but it did not scale well enough for my use case. and trying to work out how it should work was hard. many of the references i looked at had different implementations. so i have not tidied it all up as i thought i would try to land this and see how much demand there was for the svg stuff, I am more than happy to look at the svg stuff if its useful.

@fishrockz fishrockz force-pushed the willsalmon/images branch 5 times, most recently from d892679 to 8b4b47a Compare February 15, 2020 19:49
@fishrockz fishrockz force-pushed the willsalmon/images branch 7 times, most recently from cff6224 to 293f750 Compare February 15, 2020 21:10
Copy link
Member

@cmyr cmyr left a comment

Choose a reason for hiding this comment

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

okay a couple more notes here, but looks pretty close to me!

druid/src/widget/images.rs Outdated Show resolved Hide resolved
druid/src/widget/images.rs Outdated Show resolved Hide resolved
druid/src/widget/images.rs Outdated Show resolved Hide resolved
druid/src/widget/images.rs Outdated Show resolved Hide resolved
druid/examples/image.rs Outdated Show resolved Hide resolved
@fishrockz fishrockz force-pushed the willsalmon/images branch 3 times, most recently from e1c6a25 to e9b98ed Compare February 15, 2020 23:16
druid/src/widget/images.rs Outdated Show resolved Hide resolved
@fishrockz fishrockz force-pushed the willsalmon/images branch 5 times, most recently from 9295e08 to bea9efb Compare February 16, 2020 11:27
Copy link
Member

@cmyr cmyr left a comment

Choose a reason for hiding this comment

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

Okay a few more things I've spotted and a few comment fixes, we're getting close tho!

druid/src/widget/mod.rs Outdated Show resolved Hide resolved
druid/src/widget/images.rs Outdated Show resolved Hide resolved
druid/src/widget/images.rs Outdated Show resolved Hide resolved
druid/src/widget/images.rs Outdated Show resolved Hide resolved
druid/src/widget/images.rs Outdated Show resolved Hide resolved
druid/src/widget/images.rs Outdated Show resolved Hide resolved
druid/src/widget/images.rs Outdated Show resolved Hide resolved
druid/src/widget/images.rs Outdated Show resolved Hide resolved
@fishrockz fishrockz force-pushed the willsalmon/images branch 3 times, most recently from 2806066 to 41b8ac4 Compare February 16, 2020 22:47
Copy link
Member

@cmyr cmyr left a comment

Choose a reason for hiding this comment

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

Okay a few more doc fixes 😁

druid/src/widget/image.rs Outdated Show resolved Hide resolved
druid/src/widget/image.rs Outdated Show resolved Hide resolved
druid/src/widget/image.rs Outdated Show resolved Hide resolved
druid/src/widget/image.rs Outdated Show resolved Hide resolved
druid/src/widget/image.rs Outdated Show resolved Hide resolved
druid/src/widget/image.rs Outdated Show resolved Hide resolved
druid/src/widget/image.rs Outdated Show resolved Hide resolved
druid/src/widget/image.rs Outdated Show resolved Hide resolved
druid/src/widget/image.rs Outdated Show resolved Hide resolved
@fishrockz
Copy link
Collaborator Author

I have set this so maintainers can push the branch on my fork. I'm very dyslexic as you have probably noticed and have no issue with others pushing a typo fix etc.

Copy link
Member

@cmyr cmyr left a comment

Choose a reason for hiding this comment

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

Looks good, thanks for your patience!

@cmyr cmyr merged commit a0de2a1 into linebender:master Feb 17, 2020
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.

4 participants