[Merged by Bors] - Support arbitrary RenderTarget texture formats#6380
Closed
cart wants to merge 4 commits intobevyengine:mainfrom
Closed
[Merged by Bors] - Support arbitrary RenderTarget texture formats#6380cart wants to merge 4 commits intobevyengine:mainfrom
cart wants to merge 4 commits intobevyengine:mainfrom
Conversation
cart
added
A-Rendering
alice-i-cecile
approved these changes
Oct 26, 2022
Member
alice-i-cecile
left a comment
alice-i-cecile
left a comment
There was a problem hiding this comment.
These changes make sense to me, code quality is good, and the explanation is solid.
mockersf
reviewed
Oct 26, 2022
mockersf
approved these changes
Oct 26, 2022
mockersf
added
the
S-Ready-For-Final-Review
Member
Author
|
bors r+ |
bors bot
pushed a commit
that referenced
this pull request
Oct 26, 2022
# Objective Currently, Bevy only supports rendering to the current "surface texture format". This means that "render to texture" scenarios must use the exact format the primary window's surface uses, or Bevy will crash. This is even harder than it used to be now that we detect preferred surface formats at runtime instead of using hard coded BevyDefault values. ## Solution 1. Look up and store each window surface's texture format alongside other extracted window information 2. Specialize the upscaling pass on the current `RenderTarget`'s texture format, now that we can cheaply correlate render targets to their current texture format 3. Remove the old `SurfaceTextureFormat` and `AvailableTextureFormats`: these are now redundant with the information stored on each extracted window, and probably should not have been globals in the first place (as in theory each surface could have a different format). This means you can now use any texture format you want when rendering to a texture! For example, changing the `render_to_texture` example to use `R16Float` now doesn't crash / properly only stores the red component: 
Contributor
bors
bot
changed the title
JMS55
reviewed
Oct 26, 2022
| } | ||
|
|
||
| /// Retrieves the [`TextureFormat`] of this render target, if it exists. | ||
| pub fn get_texture_format<'a>( |
Contributor
There was a problem hiding this comment.
I'm slightly too late, but shouldn't this be called something like get_render_target_texture_format? And we could probably drop the get prefixes from this and similar functions.
Member
There was a problem hiding this comment.
no firm position on my side on naming, but don't hesitate to open follow up PRs 👍
Member
Author
There was a problem hiding this comment.
We sometimes use "get" when either: (1) an option is returned or (2) it isn't a direct property of the object. This sort of falls into both categories, but I don't have super strong opinions here.
On the topic of get_render_target_texture_format: the "render target" is implicit as this type is RenderTarget. We wouldn't rename Window::position() to Window::window_position for the same reason.
james7132
pushed a commit
to james7132/bevy
that referenced
this pull request
Oct 28, 2022
# Objective Currently, Bevy only supports rendering to the current "surface texture format". This means that "render to texture" scenarios must use the exact format the primary window's surface uses, or Bevy will crash. This is even harder than it used to be now that we detect preferred surface formats at runtime instead of using hard coded BevyDefault values. ## Solution 1. Look up and store each window surface's texture format alongside other extracted window information 2. Specialize the upscaling pass on the current `RenderTarget`'s texture format, now that we can cheaply correlate render targets to their current texture format 3. Remove the old `SurfaceTextureFormat` and `AvailableTextureFormats`: these are now redundant with the information stored on each extracted window, and probably should not have been globals in the first place (as in theory each surface could have a different format). This means you can now use any texture format you want when rendering to a texture! For example, changing the `render_to_texture` example to use `R16Float` now doesn't crash / properly only stores the red component: 
ItsDoot
pushed a commit
to ItsDoot/bevy
that referenced
this pull request
Feb 1, 2023
# Objective Currently, Bevy only supports rendering to the current "surface texture format". This means that "render to texture" scenarios must use the exact format the primary window's surface uses, or Bevy will crash. This is even harder than it used to be now that we detect preferred surface formats at runtime instead of using hard coded BevyDefault values. ## Solution 1. Look up and store each window surface's texture format alongside other extracted window information 2. Specialize the upscaling pass on the current `RenderTarget`'s texture format, now that we can cheaply correlate render targets to their current texture format 3. Remove the old `SurfaceTextureFormat` and `AvailableTextureFormats`: these are now redundant with the information stored on each extracted window, and probably should not have been globals in the first place (as in theory each surface could have a different format). This means you can now use any texture format you want when rendering to a texture! For example, changing the `render_to_texture` example to use `R16Float` now doesn't crash / properly only stores the red component: 
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Objective
Currently, Bevy only supports rendering to the current "surface texture format". This means that "render to texture" scenarios must use the exact format the primary window's surface uses, or Bevy will crash. This is even harder than it used to be now that we detect preferred surface formats at runtime instead of using hard coded BevyDefault values.
Solution
RenderTarget's texture format, now that we can cheaply correlate render targets to their current texture formatSurfaceTextureFormatandAvailableTextureFormats: these are now redundant with the information stored on each extracted window, and probably should not have been globals in the first place (as in theory each surface could have a different format).This means you can now use any texture format you want when rendering to a texture! For example, changing the
render_to_textureexample to useR16Floatnow doesn't crash / properly only stores the red component: