-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
[Merged by Bors] - make TextLayoutInfo a Component #4460
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
Conversation
Weibye
left a comment
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 really like the change, the code looks good as far as I can tell :) Tiny comment before approving:
1d4c4f6 to
eca2452
Compare
|
Brought this up-to-date with the main branch. I wish github displayed the diffs in such a way that it was clearer most of the changed lines are just indentation changes due to removing the surrounding |
|
More merge conflicts :( Sorry @yrns; I was on vacation. If you fix those up for us I'll merge it ASAP. |
|
Merged and tested the ui example. |
|
Thanks! bors r+ |
# Objective Make `TextLayoutInfo` more accessible as a component, rather than internal to `TextPipeline`. I am working on a plugin that manipulates these and there is no (mutable) access to them right now. ## Solution This changes `TextPipeline::queue_text` to return `TextLayoutInfo`'s rather than storing them in a map internally. `text2d_system` and `text_system` now take the returned `TextLayoutInfo` and store it as a component of the entity. I considered adding an accessor to `TextPipeline` (e.g. `get_glyphs_mut`) but this seems like it might be a little faster, and also has the added benefit of cleaning itself up when entities are removed. Right now nothing is ever removed from the glyphs map. ## Changelog Removed `DefaultTextPipeline`. `TextPipeline` no longer has a generic key type. `TextPipeline::queue_text` returns `TextLayoutInfo` directly. ## Migration Guide This might break a third-party crate? I could restore the orginal TextPipeline API as a wrapper around what's in this PR.
|
Pull request successfully merged into main. Build succeeded: |
# Objective Make `TextLayoutInfo` more accessible as a component, rather than internal to `TextPipeline`. I am working on a plugin that manipulates these and there is no (mutable) access to them right now. ## Solution This changes `TextPipeline::queue_text` to return `TextLayoutInfo`'s rather than storing them in a map internally. `text2d_system` and `text_system` now take the returned `TextLayoutInfo` and store it as a component of the entity. I considered adding an accessor to `TextPipeline` (e.g. `get_glyphs_mut`) but this seems like it might be a little faster, and also has the added benefit of cleaning itself up when entities are removed. Right now nothing is ever removed from the glyphs map. ## Changelog Removed `DefaultTextPipeline`. `TextPipeline` no longer has a generic key type. `TextPipeline::queue_text` returns `TextLayoutInfo` directly. ## Migration Guide This might break a third-party crate? I could restore the orginal TextPipeline API as a wrapper around what's in this PR.
# Objective Make `TextLayoutInfo` more accessible as a component, rather than internal to `TextPipeline`. I am working on a plugin that manipulates these and there is no (mutable) access to them right now. ## Solution This changes `TextPipeline::queue_text` to return `TextLayoutInfo`'s rather than storing them in a map internally. `text2d_system` and `text_system` now take the returned `TextLayoutInfo` and store it as a component of the entity. I considered adding an accessor to `TextPipeline` (e.g. `get_glyphs_mut`) but this seems like it might be a little faster, and also has the added benefit of cleaning itself up when entities are removed. Right now nothing is ever removed from the glyphs map. ## Changelog Removed `DefaultTextPipeline`. `TextPipeline` no longer has a generic key type. `TextPipeline::queue_text` returns `TextLayoutInfo` directly. ## Migration Guide This might break a third-party crate? I could restore the orginal TextPipeline API as a wrapper around what's in this PR.
# Objective Make `TextLayoutInfo` more accessible as a component, rather than internal to `TextPipeline`. I am working on a plugin that manipulates these and there is no (mutable) access to them right now. ## Solution This changes `TextPipeline::queue_text` to return `TextLayoutInfo`'s rather than storing them in a map internally. `text2d_system` and `text_system` now take the returned `TextLayoutInfo` and store it as a component of the entity. I considered adding an accessor to `TextPipeline` (e.g. `get_glyphs_mut`) but this seems like it might be a little faster, and also has the added benefit of cleaning itself up when entities are removed. Right now nothing is ever removed from the glyphs map. ## Changelog Removed `DefaultTextPipeline`. `TextPipeline` no longer has a generic key type. `TextPipeline::queue_text` returns `TextLayoutInfo` directly. ## Migration Guide This might break a third-party crate? I could restore the orginal TextPipeline API as a wrapper around what's in this PR.
Objective
Make
TextLayoutInfomore accessible as a component, rather than internal toTextPipeline. I am working on a plugin that manipulates these and there is no (mutable) access to them right now.Solution
This changes
TextPipeline::queue_textto returnTextLayoutInfo's rather than storing them in a map internally.text2d_systemandtext_systemnow take the returnedTextLayoutInfoand store it as a component of the entity. I considered adding an accessor toTextPipeline(e.g.get_glyphs_mut) but this seems like it might be a little faster, and also has the added benefit of cleaning itself up when entities are removed. Right now nothing is ever removed from the glyphs map.Changelog
Removed
DefaultTextPipeline.TextPipelineno longer has a generic key type.TextPipeline::queue_textreturnsTextLayoutInfodirectly.Migration Guide
This might break a third-party crate? I could restore the orginal TextPipeline API as a wrapper around what's in this PR.