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

Hook up Quality in wgpu and add quality options to context menu & cli #9400

Merged
merged 10 commits into from
Feb 6, 2023

Conversation

Dinnerbone
Copy link
Contributor

@Dinnerbone Dinnerbone commented Feb 3, 2023

WGPU also doesn't support 16x MSAA right now so those will turn into 8x. I'm working on a PR to fix that.

It's not a guarantee that devices support x2 even if they support x4, so it's possible Medium turns into Low for some devices.

I've added "Quality: Low" / "Quality: Medium" / "Quality: High" to the context menu. A future TODO is to turn that into a submenu.

In the future we may switch from MSAA to our own downscaling methods, which might solve those.

@danielhjacobs
Copy link
Contributor

I remember, after I hooked up quality in the embed and passed the values to AS as is, I learned of the existence of the options autohigh and autolow (in the embed, I don't think they can be set in the AS): https://helpx.adobe.com/flash/kb/flash-object-embed-tag-attributes.html. I never prioritized fixing that issue because quality didn't work anyway.

You may want to look into the behavior of autohigh and autolow and make sure they work as expected.

@danielhjacobs
Copy link
Contributor

danielhjacobs commented Feb 3, 2023

The issue being since autohigh and autolow don't exist in the AS (or if they do they were never implemented in Ruffle), they won't be set properly if set on the embed since quality is passed as-is. I assume it may be best to just treat autohigh as high and autolow as low, unless you can actually measure the performance like Flash did (and assuming MSAA affects speed in Ruffle).

@Dinnerbone
Copy link
Contributor Author

MSAA absolutely impacts speed, but I'm not sure I want to tackle somehow automatically monitoring and adjusting performance in this PR 😅 The behaviour should be identical to what it was before, at least

@danielhjacobs
Copy link
Contributor

danielhjacobs commented Feb 3, 2023

Yeah, I think technically autohigh and autolow would be ignored (since they'd be considered invalid quality values). They'd both therefore function like high (I believe/hope), since that's the default. They both have the potential to function as high already, so that's probably fine.

Just thought they were worth noting.

@Dinnerbone Dinnerbone changed the title Hook up Quality in wgpu and add --quality option to desktop Hook up Quality in wgpu and add quality options to context menu & cli Feb 3, 2023
@Dinnerbone
Copy link
Contributor Author

Created gfx-rs/wgpu#3454 for wgpu x16 support

target: T,
preferred_sample_count: u32,
) -> Result<Self, Error> {
pub fn new(descriptors: Arc<Descriptors>, target: T) -> Result<Self, Error> {
Copy link
Member

Choose a reason for hiding this comment

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

Could this take in the quality as an argument? That would allow us to avoid re-creating the surface immediately when the command-line quality argument is something other than StageQuality::Low

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We have to recreate it (and the actual wgpu Surface it's for) immediately for the size of the swf and other player configs that may affect it. I think there's a better way to do the whole bootstrapping process overall, probably by passing in render builder to the PlayerBuilder, but I'm not sure.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants