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

[Merged by Bors] - make TextLayoutInfo a Component #4460

Closed
wants to merge 3 commits into from

Conversation

yrns
Copy link
Contributor

@yrns yrns commented Apr 11, 2022

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.

@github-actions github-actions bot added the S-Needs-Triage This issue needs to be labelled label Apr 11, 2022
@alice-i-cecile alice-i-cecile added A-Rendering Drawing game state to the screen A-UI Graphical user interfaces, styles, layouts, and widgets C-Usability A targeted quality-of-life change that makes Bevy easier to use and removed S-Needs-Triage This issue needs to be labelled labels Apr 11, 2022
Copy link
Contributor

@Weibye Weibye left a 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:

crates/bevy_text/src/pipeline.rs Outdated Show resolved Hide resolved
@yrns yrns force-pushed the text-layout-info-component branch from 1d4c4f6 to eca2452 Compare August 2, 2022 18:54
@yrns
Copy link
Contributor Author

yrns commented Aug 2, 2022

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 if let conditionals.

@alice-i-cecile alice-i-cecile added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Aug 3, 2022
@alice-i-cecile
Copy link
Member

More merge conflicts :( Sorry @yrns; I was on vacation. If you fix those up for us I'll merge it ASAP.

@yrns
Copy link
Contributor Author

yrns commented Sep 6, 2022

Merged and tested the ui example.

@alice-i-cecile
Copy link
Member

Thanks!

bors r+

bors bot pushed a commit that referenced this pull request Sep 6, 2022
# 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.
@bors bors bot changed the title make TextLayoutInfo a Component [Merged by Bors] - make TextLayoutInfo a Component Sep 6, 2022
@bors bors bot closed this Sep 6, 2022
nicopap pushed a commit to nicopap/bevy that referenced this pull request Sep 12, 2022
# 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.
james7132 pushed a commit to james7132/bevy that referenced this pull request Oct 28, 2022
# 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.
ItsDoot pushed a commit to ItsDoot/bevy that referenced this pull request Feb 1, 2023
# 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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Rendering Drawing game state to the screen A-UI Graphical user interfaces, styles, layouts, and widgets C-Usability A targeted quality-of-life change that makes Bevy easier to use S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants