Skip to content

Conversation

@voximity
Copy link

@voximity voximity commented Dec 13, 2024

This PR is a migration to Bevy 0.15. I took some liberty in changing the API to feel more ergonomic with 0.15 - if there are any suggestions, let me know. I've tried to detail my changes below.

Fixes #30.

Progress

  • Render text and textures
  • Migrate to 0.15's retained rendering
  • Migrate to 0.15's overhauled text
  • Remove all bundles and replace with required components
  • Update all examples to use required components
  • Investigate strange mesh rendering issue
    • Frequently a texture mesh will appear mis-scaled and upside down, or text will not fully render all of its glyphs - maybe some sort of race condition, since it seems non-deterministic?
  • Update README example

Changelog

Required components

All billboard-related bundles are now required components.

  • If you were using BillboardTextureBundle, instead now use BillboardTexture and BillboardMesh.
  • If you were using BillboardTextBundle, instead now use BillboardText.
  • If you were using BillboardLockAxisBundle, instead now simply add a BillboardLockAxis component to your relevant Billboard* entity.

Renamed components

Some components were renamed to be more in-line with Bevy 0.15.

  • BillboardTextureHandle -> BillboardTexture
  • BillboardMeshHandle -> BillboardMesh

Other

  • Normal Text and Text2d components are not compatible with BillboardText! This is because Text requires Node, and the presence of Node causes the text to be rendered as a normal UI element. Instead, use BillboardText to act as your base text section.
  • BillboardText properly supports TextSpan children, and will be traversed as expected when the mesh is constructed.
  • Added some helper constructor/builder methods to BillboardLockAxis.

@voximity

This comment was marked as resolved.

@kulkalkul
Copy link
Owner

Oups, I accidentally clicked on checkbox, which edited your message. Sorry about that 😅

I don't know when I can review and push it, but thanks a lot for figuring out most of the hard work

@voximity
Copy link
Author

No problem! I just figured out the rendering issue named above, and will have a fix pushed soon. Then this is ready for review/usage by others

@voximity
Copy link
Author

This should be good to go for a review!

@Bendzae
Copy link

Bendzae commented Dec 24, 2024

Thanks a lot! :)

I'm not sure if the problem was introduced in this PR but changing the TextColor after spawn does not cause a rerender. As a workaround I could just set the font_size in TextFont and it works as expected.

@voximity
Copy link
Author

Nice find! This does appear to be a regression since the color is now a separate component instead of stored in text sections directly. Will fix this soon.

@voximity
Copy link
Author

Solving this issue turns out to be a little more complicated than expected:

  • detect_text_needs_rerender from Bevy does not detect when text colors change - presumably this is handled by some other system I can't find, or text is normally drawn in a more immediate-mode manner
  • ComputedTextBlock's needs_rerender field is private, which means we can't trivially flag one for rerender without making an entirely new ComputedTextBlock (and losing whatever is in its buffer field)

My current solution is just to wipe the ComputedTextBlock with a fresh one, since they start with needs_rerender set to true. I'm not sure I like this though. Any thoughts before I push?

@kulkalkul
Copy link
Owner

No technical input on my side, as I lack the time to investigate the issue (and even merge this anytime soon) 🙁 If your current solution works well (even if performance is regressed), I think it is good to go? I know that Alice wants to have billboard functionality inside Bevy itself, so I think that keeping the plugin "good enough" and supporting up-to-date Bevy is good balance in the sense that people still can have easy to access billboard functionality while people contributing are not wasting their time by investing too much.

@evilenzo
Copy link

evilenzo commented Dec 30, 2024

@voximity thanks for your work on this lib. I just want to say that you can always do stuff you think is going to work well and maintainer of this lib will decide which solutions are good or not by just cherry picking commits from your PR when he'll have enough time for this. Currently your pr is the only way for me and other people to use this lib in 0.15 without any investigations so any updates here will be much appreciated too!

@voximity
Copy link
Author

voximity commented Jan 2, 2025

Been busy these past few days but have pushed my (working) solution to the problem. It uses a separate sparse-set marker component that is inserted/removed when the text should be rerendered, so it shouldn't pollute your entity with components that don't matter to you. Haven't done any benchmarking compared to resetting ComputedTextBlock as per my original idea but I'd guess this approach is better.

@corintho
Copy link

corintho commented Jan 5, 2025

@voximity thank you for this PR. I started a project a few weeks ago in Bevy 0.15, and I needed this feature. I am currently using this PR as an override of the official release. I will report back if I see anything weird.

@Shatur
Copy link

Shatur commented Jan 5, 2025

Maybe implement Deref and DerefMut for BillboardText?

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.

Bevy v0.15

6 participants