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

Fix PerfQuery inconsistencies across backends. #11647

Merged
merged 2 commits into from Mar 15, 2023

Conversation

AdmiralCurtiss
Copy link
Contributor

This was discovered by someone on Twitter:
https://twitter.com/sup39x1207/status/1632813805213454337
I was promised a PR on Discord a few days ago but it never showed up, so here I am with it instead so it doesn't get forgotten. @TellowKrinkle

We're still not accurate to hardware but let's at least fix this obvious error...




@Dentomologist
Copy link
Contributor

Once these changes are added GetQueryResult will have the same implementation in all the real backends, and it doesn't seem to me that there's likely to be a reason for them to diverge in the future.

Given that, is there a reason we shouldn't just use that implementation in PerfQueryBase to prevent future problems of this type, and let the couple specialized backends that need a different implementation keep their overrides?

@TellowKrinkle
Copy link
Contributor

TellowKrinkle commented Mar 12, 2023

One more mismatch

if (g_ActiveConfig.iMultisamples > 1)
result /= g_ActiveConfig.iMultisamples;

Is only in the OGL and Metal renderers
I would think this is the correct thing to do (as in, perf query results are per sample and not per shader invocation), but it would be good to test MSAA and verify

Edit: Test done, OpenGL is correct, we need to divide by the number of samples: https://twitter.com/sup39x1207/status/1635109919606337536

@AdmiralCurtiss AdmiralCurtiss changed the title Return quarter value for PerfQuery in all backends. Fix PerfQuery inconsistencies across backends. Mar 14, 2023
@AdmiralCurtiss
Copy link
Contributor Author

Like that?

@TellowKrinkle
Copy link
Contributor

Like that?

Yep

@delroth delroth merged commit 91fca07 into dolphin-emu:master Mar 15, 2023
14 checks passed
@AdmiralCurtiss AdmiralCurtiss deleted the perfquery-inconsistency branch March 15, 2023 14:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
4 participants