Skip to content

Improve TAA example#10183

Open
DGriffin91 wants to merge 2 commits intobevyengine:mainfrom
DGriffin91:improve_taa_example
Open

Improve TAA example#10183
DGriffin91 wants to merge 2 commits intobevyengine:mainfrom
DGriffin91:improve_taa_example

Conversation

@DGriffin91
Copy link
Contributor

@DGriffin91 DGriffin91 commented Oct 19, 2023

Objective

This PR updates the anti_aliasing example with a noise texture and camera movement to illustrate potential issues with TAA. It also features a screenshot mode to consistently capture the scene with camera movent independent of the frame rate.

main

@DGriffin91 DGriffin91 mentioned this pull request Oct 19, 2023
@JMS55
Copy link
Contributor

JMS55 commented Oct 19, 2023

I'm still not convinced a noise texture is a good idea. TAA heavily relies on color clamping to prevent ghosting, which just utterly fails on such tiny high frequency detail. I don't think it's a realistic test of TAA.

Moving transparencies would be a more realistic example of a torture test imo.

@DGriffin91
Copy link
Contributor Author

The artifacts that this test reveal are representative of artifacts I've seen using the TAA implementation in bevy main in actual highly detailed scenes. (though they are somewhat exaggerated in this test scene)

I think it would be beneficial to additionally have tests that involve alpha blend materials, but TAA features designed for handling alpha blend are largely unrelated to the issues I've been having, that are illustrated by this example scene.

@alice-i-cecile alice-i-cecile added A-Rendering Drawing game state to the screen C-Examples An addition or correction to our examples labels Oct 19, 2023
Copy link
Contributor

@superdump superdump left a comment

Choose a reason for hiding this comment

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

Aside from leaning on argh and the bevy_ci_testing feature for screenshots and exiting after a number of frames, I think the noise texture is a useful stress test for 'high frequency' (noisy) textures, and the moving camera are for sure useful.

.insert_resource(Msaa::Off)
let mut app = App::new();
let mut movement_settings = CameraMovementSettings::default();
if std::env::args().nth(1).unwrap_or_default() == "--screenshot_taa" {
Copy link
Contributor

@superdump superdump Oct 19, 2023

Choose a reason for hiding this comment

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

You could use the argh crate for CLI stuff if you prefer. It may be useful for continuous integration / regression test automation to be able to specify which anti-aliasing mode is used. The CI functionality also supports making screenshots at a particular frame. So maybe that's the route to take here.

For example, if you build with the bevy_ci_testing feature, you can write (exit_after: Some(1500), screenshot_frames: [1234], frame_time: Some(0.0167)) to a .ron file, and then run the example with CI_TESTING_CONFIG=/path/to/my_config.ron and it will run the example for that many frames, with a fixed delta time, taking screenshots at the specified frame(s).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would like the test (at least in its current state) to use the frame I specified. Should I add exit_after, screenshot_frames to the CLI options, and also like a test flag, or just have the frame I care about the the default? Do these CLI options need to exist for bevy_ci_testing to work or does that work regardless of what options I add?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think I ultimately want to have a set of test scenarios. Like moving the camera side to side, moving the helmets around but keeping the camera in place, etc...

@JMS55 JMS55 added this to the 0.13 milestone Nov 23, 2023
@alice-i-cecile alice-i-cecile removed this from the 0.13 milestone Jan 24, 2024
@janhohenheim janhohenheim added the S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged label Sep 15, 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-Examples An addition or correction to our examples S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged

Projects

No open projects
Status: No status

Development

Successfully merging this pull request may close these issues.

5 participants