Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Sprite slicing and tiling #10588
Sprite slicing and tiling #10588
Changes from 9 commits
cc9ae89
5e27898
c520369
d857b32
1f55f8c
3589a08
272d43b
6d124d7
42b29cc
3c98c03
3b89390
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could use a custom shader here that draws all the slices in one pass, then only need one entity id is needed.
With a shader based solution, the whole texture_slice module wouldn't be needed even. All it would need is just the ImageScaleMode component, extraction function, some pipeline specialisation stuff, and a fragment shader.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also it isn't very difficult either to modify batching to group sprites together with the same entity id when there isn't any need to sort the group internally. I thought I even wrote a PR somewhere that implemented it for use with Text2d.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure about the name
ImageScaleMode
but it's not that much a problem I guess, I can't think of anything better.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ImageScalingMode or ImageScalingBehavior is a marginal improvement I think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe instead of scale, because the other variants aren't really scalings, something like
ImageExtendMode
orImageExtensionMode
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
change to:
/// The image will repeat horizontally.
...
/// The image will repeat vertically.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couldn't this type reflect Default and PartialEq?
#[reflect(Default, PartialEq)]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These comments probably aren't necessary (like the UiRect fields).