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

Migrate motion blur, TAA, SSAO, and SSR to required components #15572

Conversation

Jondolf
Copy link
Contributor

@Jondolf Jondolf commented Oct 1, 2024

Objective

Again, a step forward in the migration to required components: a bunch of camera rendering cormponents!

Note that this does not include the camera components themselves yet, because the naming and API for Camera hasn't been fully decided yet.

Solution

As per the selected proposals:

  • Deprecate MotionBlurBundle in favor of the MotionBlur component
  • Deprecate TemporalAntiAliasBundle in favor of the TemporalAntiAliasing component
  • Deprecate ScreenSpaceAmbientOcclusionBundle in favor of the ScreenSpaceAmbientOcclusion component
  • Deprecate ScreenSpaceReflectionsBundle in favor of the ScreenSpaceReflections component

Migration Guide

MotionBlurBundle, TemporalAntiAliasBundle, ScreenSpaceAmbientOcclusionBundle, and ScreenSpaceReflectionsBundle have been deprecated in favor of the MotionBlur, TemporalAntiAliasing, ScreenSpaceAmbientOcclusion, and ScreenSpaceReflections components instead. Inserting them will now also insert the other components required by them automatically.

@Jondolf Jondolf added D-Trivial Nice and easy! A great choice to get started with Bevy A-Rendering Drawing game state to the screen A-ECS Entities, components, systems, and events M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide X-Blessed Has a large architectural impact or tradeoffs, but the design has been endorsed by decision makers S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Oct 1, 2024
@Jondolf Jondolf added this to the 0.15 milestone Oct 1, 2024
Comment on lines +31 to +36
type TaaComponents = (
TemporalAntiAliasing,
TemporalJitter,
DepthPrepass,
MotionVectorPrepass,
);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is kinda scuffed, but component removal is a bit more annoying without bundles 🤔

(of course I could also make this an actual Bundle like TemporalAntiAliasBundle too, but the properties aren't needed in this case)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does seem like we're missing a camera.remove_with_requires::<TemporalAntiAliasing>() function in bevy_ecs

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tracked in #15580.

examples/3d/anti_aliasing.rs Outdated Show resolved Hide resolved
examples/3d/ssao.rs Outdated Show resolved Hide resolved
Copy link
Member

@cart cart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We definitely needs some form of remove_with_requires, but that shouldn't hold up progress here.

@alice-i-cecile alice-i-cecile added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Oct 1, 2024
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Oct 1, 2024
Merged via the queue into bevyengine:main with commit 22af24a Oct 1, 2024
27 checks passed
@Jondolf Jondolf deleted the camera-render-features-required-components branch October 2, 2024 06:27
robtfm pushed a commit to robtfm/bevy that referenced this pull request Oct 4, 2024
…ngine#15572)

# Objective

Again, a step forward in the migration to required components: a bunch
of camera rendering cormponents!

Note that this does not include the camera components themselves yet,
because the naming and API for `Camera` hasn't been fully decided yet.

## Solution

As per the [selected
proposals](https://hackmd.io/@bevy/required_components/%2FpiqD9GOdSFKZZGzzh3C7Uw):

- Deprecate `MotionBlurBundle` in favor of the `MotionBlur` component
- Deprecate `TemporalAntiAliasBundle` in favor of the
`TemporalAntiAliasing` component
- Deprecate `ScreenSpaceAmbientOcclusionBundle` in favor of the
`ScreenSpaceAmbientOcclusion` component
- Deprecate `ScreenSpaceReflectionsBundle` in favor of the
`ScreenSpaceReflections` component

---

## Migration Guide

`MotionBlurBundle`, `TemporalAntiAliasBundle`,
`ScreenSpaceAmbientOcclusionBundle`, and `ScreenSpaceReflectionsBundle`
have been deprecated in favor of the `MotionBlur`,
`TemporalAntiAliasing`, `ScreenSpaceAmbientOcclusion`, and
`ScreenSpaceReflections` components instead. Inserting them will now
also insert the other components required by them automatically.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ECS Entities, components, systems, and events A-Rendering Drawing game state to the screen D-Trivial Nice and easy! A great choice to get started with Bevy M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it X-Blessed Has a large architectural impact or tradeoffs, but the design has been endorsed by decision makers
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants