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

Gui Stencil Mask Optimization #1740

Open
wants to merge 14 commits into
base: master
Choose a base branch
from

Conversation

ZzzhHe
Copy link
Contributor

@ZzzhHe ZzzhHe commented Jan 7, 2025

Resolve #1538

Changes:

  • Modified Presenter::render() to use three-pass rendering:
    1. First pass: Create stencil mask from GUI
    2. Second pass: Render world with stencil test (skip GUI-covered pixels)
    3. Third pass: Render GUI normally

@heinezen heinezen added area: renderer Concerns our graphics renderer nice new thing ☺ A new feature that was not there before lang: c++ Done in C++ code labels Jan 11, 2025
@heinezen
Copy link
Member

heinezen commented Jan 11, 2025

Hey thanks for contributing again :) Unfortunately, I think the solution you made clashes a bit with our design of the renderer.

In general, the renderer code is separated into two levels (see our renderer docs for details). The (low-level) level 1 interface handles the bare-metal stuff, e.g. interfacing with OpenGL to talk to the GPU, and should generally be inaccessible to other parts of the engine. Instead, the rest of the engine mainly interacts with the level 2 interface of the renderer which abstracts the low-level stuff away and implements systems that can be used for gameplay, e.g. playing sprite animations. The reason we have this level structure is mostly thread-safety, but also easier maintainance, since we can change or replace the level 1 implementaton without changing the entire engine. The latter is important for offering multiple renderer backends simultaneously to the user, e.g. OpenGL and Vulkan.

In your solution, there would be "bare-metal" OpenGL calls in the presenter, even though the presenter is technically part of the level 2 interface. If we do it like this, then the presenter does not work with a level 1 Vulkan renderer backend anymore, for example. Ideally, the presenter does not have to care what backend we initialize the renderer with, whether it's OpenGL or Vulkan.

@heinezen
Copy link
Member

For solving this task properly, we should try to add stencil tests as a more general feature to the level 1 renderer first. If we want to activate it for the GUI, the stencil test write code should be moved into the GUI subsystem. To use the stencil tests in the other render passes, we could add settings to the RenderPass implementations; maybe stencil test reading could be a flag... When the render pass is rendered, we could switch on/off stencil testing based on that flag.

So in summary my thoughts would be:

  1. Move the stencil test writing into the libopenage/renderer/gui subsystem of the level 1 renderer
  2. Add settings to a RenderPass that allows us to configure activating/deactivating stencil tests (and maybe other settings in the future)
  3. Based on the RenderPass settings, activate stencil testing when the render pass becomes active in
    void GlRenderer::render(const std::shared_ptr<RenderPass> &pass) {

@heinezen
Copy link
Member

Apologies btw for askng for so many redesigns 😄 For issues that are not labelled "good first issue", the issue description can sometimes be a bit vague because we are ourselves not sure of what we want as a solution yet. You can always ask me for where to look in the docs before you do another issue like this one, maybe that helps :)

@ZzzhHe
Copy link
Contributor Author

ZzzhHe commented Jan 11, 2025

Thanks a lot for the detailed explanation! It all makes sense now. I’ll ensure to separate level 1 and level 2 as suggested and adjust my solution to follow this structure. I really appreciate your guidance!

@ZzzhHe ZzzhHe force-pushed the gui-stencil-mask-optimization branch from d63438c to 18622d4 Compare January 18, 2025 21:15
@ZzzhHe
Copy link
Contributor Author

ZzzhHe commented Jan 18, 2025

Here are updates:

  1. Added three stencil states in render pass to control rendering behavior:WRITE_STENCIL_MASK, USE_STENCIL_TEST, DISABLE_STENCIL.
  2. Implemented stencil state handling in GlRenderer.
  3. Added stencil_render_pass functionality in the GUI class.
  4. Updated the Presenter class to handle Level 2 rendering abstractly, without OpenGL details.

@ZzzhHe ZzzhHe force-pushed the gui-stencil-mask-optimization branch from eb58af1 to 7eca3b5 Compare January 19, 2025 19:21
@heinezen
Copy link
Member

Finally had time to look at this 😄 I think we are getting there, but after looking at your code, I'm pretty sure that your stencil tests currently won't work. It's hard to test since we currently have deactivated the GUI, but here are my reasons:

  1. Stencil testing is always done per framebuffer, so writing to a specific stencil buffer does not affect stencil testing in other framebuffers. In openage, the world render stage uses a different framebuffer, so you have to actively transfer the stencil buffer state from the GUI framebuffer before you can use stencil testing (i.e. read from the stencil values written by the GUI pass).
  2. You have to actively attach a stencil buffer to the framebuffer in OpenGL which is currently not done in your code. The same applies to depth textures btw, they don't get assigned by default. You can see how we assign a depth buffer for the world render stage framebuffer here:

this->depth_texture = renderer->add_texture(resources::Texture2dInfo(width, height, resources::pixel_format::depth24));

  1. You have introduced a lot of redundancy (e.g. the GUI is rendered twice now) which would counteract all our potential time saves from stencil testing :D

I would recommend that you don't implement this functionality in the presenter yet. A renderer demo is much better suited and allows you to test the level 1 implementation without having to worry about level 2 abstractions logic.

@ZzzhHe
Copy link
Contributor Author

ZzzhHe commented Jan 26, 2025

Oh.. I was too focused on implementing the stencil test functions and completely missed the fundamentals of how framebuffers work in OpenGL. I'll start fresh with a renderer demo this week to re-write and test the core functionality properly. Thanks!

@ZzzhHe
Copy link
Contributor Author

ZzzhHe commented Feb 5, 2025

Here are some new updates:

  1. Added depth_stencil texture (pixel_format::depth24_stencil8) to framebuffer
  2. Created StencilConfig struct to encapsulate stencil test parameters
  3. Implemented stencil testing in render pass and renderer
  4. Added demo8 with a GUI rectangle masking an animated background
    • Configured GUI pass to write to stencil buffer (ref=255, GL_REPLACE)
    • Configured background pass to skip rendering where stencil=255 (GL_NOTEQUAL)

Then I verified it using RenderDoc:

Screenshot 2025-02-05 100818
Screenshot shows stencil buffer during background pass - white areas (255) indicate GUI regions where background rendering is skipped due to stencil test failure.

Current limitation:

GUI isn't visible because each render pass clears the color buffer (glClear in renderer::render).
I'm thinking should we add clear flags to control buffer clearing per pass, or combine GUI and background into single render pass with layers, or maybe other better ideas?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: renderer Concerns our graphics renderer lang: c++ Done in C++ code nice new thing ☺ A new feature that was not there before
Projects
Status: 🔖 TODO
Development

Successfully merging this pull request may close these issues.

Stencil tests for GUI elements
2 participants