Mobile: Uncomment code required for fog rendering on clear color #79776
Conversation
This file contains 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
Calinou
added
bug
topic:rendering
topic:3d
cherrypick:4.1
clayjohn
reviewed
Jul 24, 2023
servers/rendering/renderer_rd/forward_mobile/render_forward_mobile.cpp
Outdated
Show resolved
Hide resolved
servers/rendering/renderer_rd/forward_mobile/render_forward_mobile.cpp
Outdated
Show resolved
Hide resolved
|
Oh, and by the way, there shouldn't be any issues. There are other checks to ensure that the volumetric fog is actually enabled before updating shader params etc. |
|
I implemented the suggestions as an amended commit to avoid cluttering the git history. That does make sense as a change, so thanks for the suggestion. I'm still not entirely caught up on what's not available between the different renderers, so it wasn't something I would have caught on my own. |
clayjohn
approved these changes
Jul 27, 2023
|
Thanks! |
|
Cherry-picked for 4.1.3. |
YuriSizov
removed
the
cherrypick:4.1
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This should fix #74187. I noticed these lines of code were commented out while looking into an unrelated issue. The equivalent lines of code are not commented out in the Forward+ (forward clustered) renderer, so I assume it's an error here,
I'm not entirely sure why the code here was commented out originally, only that it was commented out when the Mobile renderer was first added with #47059. Perhaps there was some issue this was working around? I wasn't able to identify any issues with uncommenting it now, so I assume there's no problem with this PR. I would like to know why it was commented out originally if anybody still remembers, so I can be certain there are no issues with uncommenting it now.