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
Add background blur #5720
base: master
Are you sure you want to change the base?
Add background blur #5720
Changes from 13 commits
8b872d0
9b31df7
2072326
f8741e8
aad651b
f61f815
aca64b6
4a2e3bc
476ab02
9a7f3f5
785b30c
e087506
d7bf936
3364cbb
8d806f0
32a1d9e
de46cce
4b0bfbb
7dbb73d
183a356
9fb7d8e
7e4624e
ef8633d
adf3799
250ae91
25d81a5
81cc1fa
da80a2b
c42e895
f8d66c4
ebbc183
1ee7d7c
558bee8
b6d16b2
08fd6c7
4eefd74
fb1a295
8f263f3
edd8fad
279b0fb
d07857c
a921ee9
175db9a
a9812a0
1e63466
3d22c5e
05df472
3a04bdc
d093fc6
c54034b
ad8fa84
5b6c4c9
cd4f082
7ec3587
95ea4f7
3ec4406
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.
Could this be implemented as a second render pass or directly baked into the background shader? I ask because we'll hit a rendering performance problem at some point if we just throw subviewports at any problem.
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.
Actually, it seems possible to re-implement this without viewports. I've just found out that it's possible for a shader to get it's texture from a CanvasLayer instead of a viewport (the docs), which should be more performant.
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.
In case that's not usable, I think putting the blur code directly into the background shader would be the right way to go. That would also quite neatly I think expose how expensive in terms of texture reads it is actually to perform a blur.
Edit: and as an added benefit the blur could be only applied to the layer that has the distortion on it (so only layer0).
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.
Putting the blur code into the background shader would be bad for performance, because it would then need n*n texture reads (for blur diameter of n), and for each of those reads it would also need to calculate distortion.
The current implementation does blur in 2 passes (vertical and horizontal), and because of that only needs 2*n reads. This difference in calculation costs should probably compensate for viewports' performance.
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.
Still it will now blur the moving bubble layers that are separate from the furthest background layer as well, which is probably not the best. Also it could maybe be possible to use a cheaper blur method combined into the distortion math to do a reasonable enough blur with high performance at the same time as applying the distortion math?
I guess I'll let you investigate first what is actually possible to do before dumping more ideas.
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.
Apparently the way of making multi-pass shaders with CanvasLayers requires reading the screen texture, so there still needs to be at least one viewport.
The background plane can also be optimized into a ColorRect (which needs a scale argument passed to it, but it's probably still more performant than rendering 3D geometry).
This is what the resulting scene hierarchy looks like:
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.
After thinking about the situation more: I guess the most optimal way to implement this feature is to use the subviewports, but I think that to balance that out we should add a graphics option to control the blur amount and if the blur is zero the subviewports should be skipped entirely so that the game is no slower than it currently is on computers where rendering to texture ends up harming performance a lot.