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

Removing ExtractAtlasNode from UIRender Extract Schedule #11756

Closed
wants to merge 2 commits into from

Conversation

valenotary
Copy link

…nt when building the UI render

Objective

Solution

  • Just removed the parts of the render_app chain that involved the allegedly unnecessary ExtractAtlasNode enum. Wasn't sure if I was also supposed to remove the enum as well -- can restore this easily if so.

Copy link
Contributor

github-actions bot commented Feb 7, 2024

Welcome, new contributor!

Please make sure you've read our contributing guide and we look forward to reviewing your pull request shortly ✨

@ManevilleF ManevilleF mentioned this pull request Feb 7, 2024
3 tasks
@alice-i-cecile alice-i-cecile added A-Rendering Drawing game state to the screen C-Code-Quality A section of code that is hard to understand or change M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide labels Feb 7, 2024
Copy link
Contributor

github-actions bot commented Feb 7, 2024

It looks like your PR is a breaking change, but you didn't provide a migration guide.

Could you add some context on what users should update when this change get released in a new version of Bevy?
It will be used to help writing the migration guide for the version. Putting it after a ## Migration Guide will help it get automatically picked up by our tooling.

github-merge-queue bot pushed a commit that referenced this pull request Feb 7, 2024
> Follow up to #10588 
> Closes #11749 (Supersedes #11756)

Enable Texture slicing for the following UI nodes:
- `ImageBundle`
- `ButtonBundle`

<img width="739" alt="Screenshot 2024-01-29 at 13 57 43"
src="https://github.com/bevyengine/bevy/assets/26703856/37675681-74eb-4689-ab42-024310cf3134">

I also added a collection of `fantazy-ui-borders` from
[Kenney's](www.kenney.nl) assets, with the appropriate license (CC).
If it's a problem I can use the same textures as the `sprite_slice`
example

# Work done

Added the `ImageScaleMode` component to the targetted bundles, most of
the logic is directly reused from `bevy_sprite`.
The only additional internal component is the UI specific
`ComputedSlices`, which does the same thing as its spritee equivalent
but adapted to UI code.

Again the slicing is not compatible with `TextureAtlas`, it's something
I need to tackle more deeply in the future

# Fixes

* [x] I noticed that `TextureSlicer::compute_slices` could infinitely
loop if the border was larger that the image half extents, now an error
is triggered and the texture will fallback to being stretched
* [x] I noticed that when using small textures with very small *tiling*
options we could generate hundred of thousands of slices. Now I set a
minimum size of 1 pixel per slice, which is already ridiculously small,
and a warning will be sent at runtime when slice count goes above 1000
* [x] Sprite slicing with `flip_x` or `flip_y` would give incorrect
results, correct flipping is now supported to both sprites and ui image
nodes thanks to @odecay observation

# GPU Alternative

I create a separate branch attempting to implementing 9 slicing and
tiling directly through the `ui.wgsl` fragment shader. It works but
requires sending more data to the GPU:
- slice border
- tiling factors

And more importantly, the actual quad *scale* which is hard to put in
the shader with the current code, so that would be for a later iteration
@mockersf
Copy link
Member

mockersf commented Feb 7, 2024

this was done in #11600

@mockersf mockersf closed this Feb 7, 2024
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 C-Code-Quality A section of code that is hard to understand or change M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ordering constraints referencing RenderUiSystem::ExtractAtlasNode aren't doing anything
3 participants