Tweaks to adjust to new pygfx blending #838
Conversation
| @@ -560,6 +560,11 @@ def _render(self, draw=True): | |||
| # draw the underlay planes | |||
| self.renderer.render(self._underlay_scene, self._underlay_camera, flush=False) | |||
|
|
|||
| # With new pygfx' blending, the depth buffer is only cleared after each flush, we need a manual depth | |||
| # clear to erase the depth values set by the underlay. | |||
There was a problem hiding this comment.
Good point, you may want to use a different depth range in the camera, so that the fragments all have a very small depth value, so they don't get behind stuff in the scene. Or simply disable depth-tests.
There was a problem hiding this comment.
Or you can clear the depth buffer again, if you don't need it for post processing or things like that.
There was a problem hiding this comment.
What are depth tests? Is there a performance difference between this or clearing again? What's your recommendation, I'm not too familiar with how the depth buffer works in this context, only how it's important for knowing what objects are above what in a 3D scene. Eventually the overlay scene will probably have axes when using an orthographic projection, along with legends etc.
There was a problem hiding this comment.
The depth test makes sure that objects are not occluded by objects that are behind it (but are rendered later).
The depth buffer needs to be cleared before using it, otherwise the depth test makes no sense. Since there is an underlay pass and an overlay pass, we need extra manual clears.
I added a clear before the overlay pass. That's definitely the simplest solution. The only caveat is that if users want access to the depth buffer (of the scene) they can't because the depth buffer will represent the content of the overlay. If we ever want to change that, we can look into that later. There are more upcoming changes in pyfx that will likely make that easier.
There was a problem hiding this comment.
The only caveat is that if users want access to the depth buffer (of the scene) they can't because the depth buffer will represent the content of the overlay. If we ever want to change that, we can look into that later. There are more upcoming changes in pyfx that will likely make that easier.
do you want to make a note of it in the pygfx issue if you don't have this planned already?
|
I think this should be merged soon after we decide how to deal with the depth buffer for the overlay pass? |
|
There are some more upcoming changes. I'll likely make some more changes this week. |
|
Let's merge this, so we keep |
|
Failures are due to pygfx/wgpu-py#723 And I think due to AA being disabled for tests now? Some diffs below:
|
|
The image tests fail because the downsample filter is changed (to go from hirez to target rez). So indeed related to SSAA. See pygfx/pygfx#1121 |
The env var that disables PPAA is for a different kind of aa 😉, which has not been merged yet (at the time of writing): pygfx/pygfx#1117 That env var will make sure that the screenshots don't change when that PR is in effect. |
|
📚 Docs preview built and uploaded! https://www.fastplotlib.org/ver/pygfx-blending |
So if I update the ground truth screenshots good to go!? |
I'm half wondering if I want to add a similar env var to override the pixel_ratio for screenshot tests, now that we have to update all screenshots ... I'm sorry it takes so long. Is this stopping other prs from progressing? |
|
EDIT: ok now I understand how github actions pushes generated screenshots artifacts. If an image hasn't been re-generated it still gets sent because it exists in the repo already. I should do separate dirs for imgui and non-imgui screenshots or filter what's upload to the artifact. |
For upcoming pygfx/pygfx#1002
May not be complete, can leave open until the pygfx-pr is merged.