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

Cull GUI elements not in scissor bounds #23022

Merged
merged 1 commit into from Apr 20, 2024
Merged

Conversation

sarbes
Copy link
Member

@sarbes sarbes commented Mar 20, 2023

Description

With the PR, we check if the GUI element we want to render is in bounds of the current scissor region. If not, we can avoid the related geometry setup and drawcall(s).

Motivation and context

Drawcalls are expensive. Our current geometry processing is expensive. Both can be avoided for elements outside of the scissor box we want to render to. This is especially important if we apply our "Cost Reduction" (2) dirty region algorithm. But also our "Union" algorithm, as well as full screen rendering will profit from out-of-bounds culling.

Without it, the cost reduction option is absolutely not viable, as it submits all drawcalls for each region, instead of only the ones needed for each region.

I'm guesstimating that the cost of it will be amortized by avoiding one drawcall.

How has this been tested?

Running various skins, I couldn't see any missing UI elements.

What is the effect on users?

Less CPU and GPU utilization. How much depends on the system, skin, and DR scheme.

Screenshots (if appropriate):

In this example, the "Cost Reduction" algorithm is active. You can see a "2" being rendered in a region (marked by the black/white scissor outline). For this region, only a glClear() and one glDrawElements() is submitted. Without the patch, all drawcalls (>30) would be submitted for each region rendered.
image

Types of change

  • Bug fix (non-breaking change which fixes an issue)
  • Clean up (non-breaking change which removes non-working, unmaintained functionality)
  • Improvement (non-breaking change which improves existing functionality)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that will cause existing functionality to change)
  • Cosmetic change (non-breaking change that doesn't touch code)
  • None of the above (please explain below)

Checklist:

  • My code follows the Code Guidelines of this project
  • My change requires a change to the documentation, either Doxygen or wiki
  • I have updated the documentation accordingly
  • I have read the Contributing document
  • I have added tests to cover my change
  • All new and existing tests passed

@sarbes sarbes added Type: Improvement non-breaking change which improves existing functionality Component: GUI rendering v21 Omega labels Mar 20, 2023
@sarbes sarbes added this to the Omega 21.0 Alpha 1 milestone Mar 20, 2023
@CrystalP
Copy link
Contributor

CrystalP commented Mar 20, 2023

Would that help the ControlGroupList that always renders all children, even when outside its area?

oh and... great idea!

xbmc/guilib/GUIControl.cpp Outdated Show resolved Hide resolved
@sarbes
Copy link
Member Author

sarbes commented Mar 20, 2023

Would that help the ControlGroupList that always renders all children, even when outside its area?

oh and... great idea!

I wanted to be on the safe side. I'm not even sure if elements outside of a group boundary are legal.

@CrystalP
Copy link
Contributor

CrystalP commented Apr 12, 2023

I wanted to be on the safe side. I'm not even sure if elements outside of a group boundary are legal.

Maybe not, but I was thinking about the case when there are more items in the grouplist than what can be displayed at once given the group boundaries. In that situation it displays what fits and provides a scrollbar. The rest of the children is clipped but still rendered if I remember correctly from when I was writing my PR about page up/down in grouplists.

@sarbes sarbes merged commit 46d9d48 into xbmc:master Apr 20, 2024
2 checks passed
@sarbes
Copy link
Member Author

sarbes commented Apr 20, 2024

TY!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: GUI rendering Type: Improvement non-breaking change which improves existing functionality v22 "P"
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants