Improve PerfQuery accuracy #3893
Conversation
|
@dolphin-emu-bot rebuild |
|
D3D is unchanged, assumed still broken (I'm on OS X) |
|
Fair enough. It's just good to note it. |
|
I can test Super Mario Sunshine; I do wonder if this will make it more/less sensitive. I remember a long time ago thinking that it wasn't quite right to console. |
|
Thanks! I'd be happy to change the code to try rounding up instead (which sounds like it might be more accurate - at the moment it rounds down, and I was hoping it wouldn't matter). Commit 4d8d86b introduced the original "divide-by-four" code, from which I inferred the rest of the logic. @neobrain: If you remember and don't mind my asking, how did you figure out it needed to count 2x2 quads instead of pixels? |
|
I'm not the greatest Super Mario Sunshine player, but, I couldn't notice a discernibly difference between master and this pull request in Super Mario Sunshine while scrubbing Sirena Beach. |
|
@hhh I don't recall returning the number of 2x2 quads. The intention of my code was to return the number of pixels with a 2x2 granularity (i.e. |
|
Thanks for the explanation @neobrain :) - if I don't assume the existing code is necessary, I could just try not-dividing-by-four in each backend, which would be a much tidier solution. (And fortunately it sounds like we'd only need to test Super Mario Sunshine - which I'm guessing is just thresholding or comparing to zero and will hopefully be fine.) |
|
SMS check against zero. else it won't work with higher resolutions. |
| @@ -157,7 +158,13 @@ void PerfQueryGL::FlushOne() | |||
| glGetQueryObjectuiv(entry.query_id, GL_QUERY_RESULT, &result); | |||
|
|
|||
| // NOTE: Reported pixel metrics should be referenced to native resolution | |||
| m_results[entry.query_type] += (u64)result * EFB_WIDTH / g_renderer->GetTargetWidth() * EFB_HEIGHT / g_renderer->GetTargetHeight(); | |||
| result = (u64)result * EFB_WIDTH / g_renderer->GetTargetWidth() * EFB_HEIGHT / g_renderer->GetTargetHeight(); | |||
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
hthh
changed the title
VideoCommon actually divides the output by the EFB scale, hence your reasoning is incorrect. In fact, SMS compares against a threshold as far as I know. |
|
For reference, dropping the lower 2 bits from this count should be closer to actual hardware behavior and hence kept around in some way, at least until a hwtest proves the opposite conjecture that the performance queries actually do count pixels. |
|
Can this be finished up? Fixes https://bugs.dolphin-emu.org/issues/8181 |
In TimeSplitters: Future Perfect, PerfQuery is used to detect the visibility of lights and draw coronas. 25 points are drawn for each light. However, the returned count was incorrectly being divided by four leading to dim coronas. Using 4x antialiasing was a workaround because of a bug where antialiasing multiplied the PerfQuery results. This commit fixes that bug too (but only for OpenGL).
|
@JMC47 Sorry, been a bit busy, but I think this is finished up now. I don't think dropping the low two bits makes sense for PRIMITIVE_POINTS, and it would probably produce noticeable artefacts as only 25 points are drawn (so it would be discretized to six brightness levels). Getting the draw type passed through is tricky, and I don't think it will change behaviour in the games known to use this feature, so I've only kept it around in a TODO comment for now. There's room for improvement by passing through the draw type (points/lines/triangles), and possibly dividing by the AA oversampling factor in the other backends, but I'll leave that for later PRs (possibly to be done by people who are set up to test on Windows). This change fixes TimeSplitters on OpenGL, and should fix it on the other backends. It shouldn't change Super Mario Sunshine noticeably, but it might be worth retesting to be sure. |
|
What's the status of this PR? |
|
Needs review, but ready to be merged (in my opinion). |
|
I'd be fine with merging this now that I tested everything. Please review peoples. |
|
Sounds fine IMO. |
In TimeSplitters: Future Perfect, PRIMITIVE_POINTS is used with PerfQuery to detect the visibility of lights and draw coronas. The point count was incorrectly being divided by four leading to dim coronas.
Using 4x antialiasing was a workaround because of another bug where antialiasing multiplied the PerfQuery results. This fixes that bug too (for OpenGL).
This should address https://bugs.dolphin-emu.org/issues/8181 in the OpenGL backend. This can probably be pretty closely copied to the other backends, but it's a bit of a hassle for me to test them, so I didn't try. I moved the location of the division, potentially increasing rounding error in other games. Can anyone test this on Super Mario Sunshine?
This change is