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

many_buttons enhancements #9712

Merged
merged 13 commits into from
Sep 8, 2023

Conversation

ickshonpe
Copy link
Contributor

@ickshonpe ickshonpe commented Sep 6, 2023

Objective

many_buttons enhancements:

  • use argh to manage the commandline arguments like the other stress tests
  • add an option to set the number of buttons
  • add a grid layout option
  • centre the grid properly
  • use viewport coords for the layout's style constraints
  • replace use of absolute positioning

includes the changes from #9636

Displaying an image isn't actually about stress testing image rendering. Without a second texture (the first is used by the text) the entire grid will be drawn in a single batch. The extra texture used by the image forces the renderer to break up the batches at every button displaying an image, where it has to switch between the font atlas texture and the image texture.

Solution

many_buttons_new

Changelog

many_buttons stress test example enhancements:

  • uses argh to the manage the commandline arguments.
  • New commandline args:
    • --help display info & list all commandline options
    • --buttons set the number of buttons.
    • --image-freq set the frequency of buttons displaying images
    • --grid use a grid layout
  • style constraints are specified in viewport coords insead of percentage values
  • margins and nested bundles are used to construct the layout, instead of absolute positioning
  • the button grid centered in the window, the empty gap along the bottom and right is removed
  • an image is drawn as the background to every Nth button where N is set using the --image-freq commandline option.

* uses `argh` to the manage the commandline arguments.
* New commandline args:
  - `--help` display help
  - `--buttons`  set the number of buttons.
  - `--image-freq` set the frequency of buttons displaying images
*
* uses `argh` to the manage the commandline arguments.
* New commandline args:
  - `--help` display help
  - `--buttons`  set the number of buttons.
  - `--image-freq` set the frequency of buttons displaying images
* style constraints are specified in viewport coords insead of percentage values
* margins and nested bundles are used to construct the layout, instead of absolute positioning
* the grid of buttons is centered in the window, the empty gap along the bottom and right is removed
@ickshonpe ickshonpe changed the title Many buttons enhancements many_buttons enhancements Sep 6, 2023
Copy link
Member

@alice-i-cecile alice-i-cecile left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The CLI interface looks nice :) And the enhancements will be useful for more precise benchmarking without exploding our set of stress tests.

@rparrett
Copy link
Contributor

rparrett commented Sep 6, 2023

Some thoughts:

  • Any particular reason to not be using the default window size here? The online examples page just looks nicer when everything is the same aspect ratio.
  • What about using Display::Grid? Seems like the natural way to lay out a big ol' grid but I guess that changes the characteristics of the test a bit. edit: I see you added a grid option while I was reviewing.
  • Basing border size and gaps between buttons on button dimensions effectively erases the borders/gaps with large amounts of buttons.

Here's a branch with Display::Grid and static borders / gutters / gaps if you want to experiment with it.

@ickshonpe
Copy link
Contributor Author

Some thoughts:

  • Any particular reason to not be using the default window size here? The online examples page just looks nicer when everything is the same aspect ratio.

Not really, with a square window you get square buttons but it doesn't matter. I hadn't considered the examples page, I'll change it to the default.

@alice-i-cecile alice-i-cecile added A-Rendering Drawing game state to the screen C-Examples An addition or correction to our examples C-Performance A change motivated by improving speed, memory usage or compile times A-UI Graphical user interfaces, styles, layouts, and widgets labels Sep 7, 2023
@ickshonpe
Copy link
Contributor Author

ickshonpe commented Sep 7, 2023

Some thoughts:

  • Any particular reason to not be using the default window size here? The online examples page just looks nicer when everything is the same aspect ratio.
  • What about using Display::Grid? Seems like the natural way to lay out a big ol' grid but I guess that changes the characteristics of the test a bit. edit: I see you added a grid option while I was reviewing.
  • Basing border size and gaps between buttons on button dimensions effectively erases the borders/gaps with large amounts of buttons.

Here's a branch with Display::Grid and static borders / gutters / gaps if you want to experiment with it.

Won't this do the opposite, will the buttons disappear with a lot of buttons?
The margins are slightly different between the grid and flex modes too, it'd be nice to fix that as well.
I guess I don't want to worry too much about the visual aspects and styling in this PR though and as it's primarily a benchmark it doesn't matter how it looks in the end.

Maybe it would be better to merge this as it is? The changes here (if not ideal) are a big improvement on main. Then you could create a second PR as I'm sure you know more about the grid layout model than me etc.

@rparrett
Copy link
Contributor

rparrett commented Sep 7, 2023

Won't this do the opposite, will the buttons disappear with a lot of buttons?

Yeah, that's fair, especially at the particular sizes I chose.

I guess I don't want to worry too much about the visual aspects and styling in this PR though and as it's primarily a benchmark it doesn't matter how it looks in the end.

I had some vague notion that rendering zero-pixel-width geometry might have different performance characteristics but I doubt that is how it works or that it matters if it is. (and even at 100k buttons the borders are at least ~0.1 pixels)

Maybe it would be better to merge this as it is?

Not a blocker for me.

@rparrett
Copy link
Contributor

rparrett commented Sep 7, 2023

The margins are slightly different between the grid and flex modes too, it'd be nice to fix that as well.

It might add some additional utility if those modes were producing identical output, but I am not completely convinced that's even possible. Might require a greater flexbox wizard than me.

@alice-i-cecile alice-i-cecile added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Sep 8, 2023
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Sep 8, 2023
Merged via the queue into bevyengine:main with commit 73447b6 Sep 8, 2023
26 checks passed
@cart cart mentioned this pull request Oct 13, 2023
43 tasks
rdrpenguin04 pushed a commit to rdrpenguin04/bevy that referenced this pull request Jan 9, 2024
# Objective

`many_buttons` enhancements:
* use `argh` to manage the commandline arguments like the other stress
tests
* add an option to set the number of buttons
* add a grid layout option
* centre the grid properly
* use viewport coords for the layout's style constraints
* replace use of absolute positioning

includes the changes from bevyengine#9636

Displaying an image isn't actually about stress testing image rendering.
Without a second texture (the first is used by the text) the entire grid
will be drawn in a single batch. The extra texture used by the image
forces the renderer to break up the batches at every button displaying
an image, where it has to switch between the font atlas texture and the
image texture.

## Solution

<img width="401" alt="many_buttons_new"
src="https://github.com/bevyengine/bevy/assets/27962798/82140c6d-d72c-4e4f-b9b6-dd204176e51d">

---

## Changelog

 `many_buttons` stress test example enhancements:
* uses `argh` to the manage the commandline arguments.
* New commandline args:
  - `--help` display info & list all commandline options
  - `--buttons`  set the number of buttons.
  - `--image-freq` set the frequency of buttons displaying images
  - `--grid` use a grid layout
* style constraints are specified in viewport coords insead of
percentage values
* margins and nested bundles are used to construct the layout, instead
of absolute positioning
* the button grid centered in the window, the empty gap along the bottom
and right is removed
* an image is drawn as the background to every Nth button where N is set
using the `--image-freq` commandline option.

---------

Co-authored-by: Rob Parrett <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Rendering Drawing game state to the screen A-UI Graphical user interfaces, styles, layouts, and widgets C-Examples An addition or correction to our examples C-Performance A change motivated by improving speed, memory usage or compile times S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants