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.
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
Apply codebase changes in preparation for
StandardMaterial
transmission #8704Apply codebase changes in preparation for
StandardMaterial
transmission #8704Changes from all commits
0a47196
eadb217
348e2ce
a12419d
77b94e4
a39364f
b5ddbd7
ac820b4
b6e564a
cec61c9
0aac272
6103ec0
e61f78d
2106a53
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we want some validation here to check that the range is valid.
Also, remind me why render_range() is useful for transmission?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The updated code with
.get(range)
now has validation.We use it to implement transmissive "steps", to achieve more than one layer of transparency for transmission. The items are sorted far-to-near, then split into roughly equal ranges and drawn parceled out, with a texture copy between each draw. This is kinda expensive so we default to a single step (equivalent to not using
render_range()
) but you can configure it viaCamera3d
if higher fidelity is required.Note that there are probably better ways to do this range division, including by determining which items' bounds overlap in screen space, making the steps exponentially spaced (so that near objects get more steps) or prioritizing based on roughness, transmission, and screen space coverage, however to start I just went for the most naive approach.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have you considered using "WhiteFallbackImage" and "TransparentBlackFallbackImage"? Though I understand why you'd want to keep the current names over the ones I'm suggesting.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah... I tried out a bunch of different names/orders, but they all felt weird. I didn't want to rename the existing
FallbackImage
needlessly since that would introduce a breaking change for no good reason, and since there was alreadyFallbackImageCubemap
(with the “qualifier” at the end) I just went with the same pattern for consistency.I can rename all of them if there's significant demand, but since we might want to be able to (eventually) create arbitrary-color fallbacks, maybe we can save the breaking change for then?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's fine as-is for now