Skip to content
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

Copy on present in OpenGL backend #3469

Open
kvark opened this issue Nov 7, 2020 · 0 comments
Open

Copy on present in OpenGL backend #3469

kvark opened this issue Nov 7, 2020 · 0 comments

Comments

@kvark
Copy link
Member

kvark commented Nov 7, 2020

Currently, the GL backend is built in such a way that we copy the full frame on present(). This is a waste of fill-rate and performance that we've been trying to eliminate. The plan was to modify surfman in a mystical way that would let us do this - servo/surfman#202. However, digging into EGL as well as asking around on Khronos slack proved this to be impossible. The best we can do is - avoid context sharing in GL. So the problem of copy on present remains unsolved.

I've been thinking that perhaps in most cases where the swapchain is used, there should be no need for a copy at all. Most applications would render to the swapchain image in a single pass, and the depth is discarded after it. So what if we create the EGL surface with all of color, depth, stencil, just in case, and try to use it aggressively? Given the render pass boundaries in this API, it seems like it could cover most cases:

Cases

Case A: rendering to color only.
GL backend sees that an application only renders to the color, it binds the main framebuffer, and the copy is avoided.

Case B: rendering to color and depth, the depth is cleared and discarded at the end.
We see the clear+discard on the depth, and we figure out that we don't actually need to consider the user-provided depth image. So we just use the default framebuffer again, and the copy is avoided.

Case C: rendering to color and depth, depth is test-only, loaded from a depth pre-pass.
We see that the depth is discarded, so we can use the default framebuffer. However, we also see that the depth is loaded, so we issue a copy from the user-provided depth into the main FBO depth, first, before starting a pass.

Case D: rendering to color and depth, and depth is stored (not discarded).
We proceed with the logic of B/C, but after the pass we issue a copy from the framebuffer depth to the user-provided depth.

Case X: rendering with other colors alongside the swapchain. I don't see how we can reasonably support that in GL backend. We'll have to document and assert on that.

Case Y: user-provided depth is not a 24-bit depth. I don't think we can deal with that, unfortunately.

Analysis

This requires some logic to be implemented, for tight integration between EGL and rendering. The benefit is avoiding the copy of color in all cases. However, another cost is that some cases would involve a copy of the depth, which roughly have the same size...

@kvark kvark mentioned this issue Nov 9, 2020
3 tasks
bors bot added a commit that referenced this issue Nov 9, 2020
3470: [gl] replace Surfman by EGL r=grovesNL a=kvark

Fixes #3468

This is a BIG change. It re-architects WSI of the GL backend to unconditionally use EGL (via `khronos-egl` crate atm, but we can change that). This means the backend is *only* supported on Linux/BSD/Android, it's no longer supported on Apple and Microsoft platforms.

One of the consequences - we throw Surfman away. There was too much complication and too little benefit from it anyway.

Another consequence - we are locked to GLES, given that I experienced issues trying to get a desktop GL context via EGL. We can revisit this, but really I think going with GLES 3.1 makes total sense for the backend, unconditionally. if the target platform is powerful above what GLES 3.1 offers, it should just support Vulkan.

Most importantly, it solves GL context sharing, so presentation is much more robust. However, it doesn't solve the extra copy - #3469. For this, we'll need to follow-up by directly using platform extensions, such as [EGL_WL_create_wayland_buffer_from_image](https://www.khronos.org/registry/EGL/extensions/WL/EGL_WL_create_wayland_buffer_from_image.txt).

PR checklist:
- [x] `make` succeeds (on *nix)
- [x] `make reftests` succeeds
- [x] tested examples with the following backends: Linux


Co-authored-by: Dzmitry Malyshau <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant