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

Bug: Disparity in quality levels #4

Open
MatusT opened this issue Aug 4, 2020 · 2 comments
Open

Bug: Disparity in quality levels #4

MatusT opened this issue Aug 4, 2020 · 2 comments
Assignees

Comments

@MatusT
Copy link

MatusT commented Aug 4, 2020

Hello,

I found some disparities in quality levels in the code. Could you comment on It whether I am missing something, or It is indeed a bug?

In fx_cacao.cpp on line 1974 the importance map is generated only if the level is Highest (4) which seems correct.

if (context->settings.qualityLevel == FFX_CACAO_QUALITY_HIGHEST) {
  // generate importance map code
}

BUT, in fx_cacaco.hlsl for level HIGH (3) you call GenerateSSAOShadowsInternal with qualityLevel = 3 and adaptiveBase = false which results in:

static const uint g_numTaps[5] = { 3, 5, 12, 0, 0 };
// ... later
const int numberOfTaps = (adaptiveBase) ? (SSAO_ADAPTIVE_TAP_BASE_COUNT) : (g_numTaps[qualityLevel]);

^ number of taps being 0! which I believe should be 12 as in https://software.intel.com/content/www/us/en/develop/articles/adaptive-screen-space-ambient-occlusion.html.

and

if ((qualityLevel != 3) || adaptiveBase)
{

}
else // if ( qualityLevel == 3 ) adaptive approach
{
 // ...
 float importance = g_ImportanceMap.SampleLevel(g_LinearClampSampler, fullResUV, 0.0).x;
 // ...
}

^ you sample from a map that is not generated. Because level 3 (High) is not the adaptive one. That is 4. I believe the if should be qualityLevel !=4.

@MatusT MatusT changed the title Disparity in quality levels Bug: Disparity in quality levels Aug 4, 2020
@rys rys self-assigned this Apr 28, 2021
@rys
Copy link
Contributor

rys commented Apr 28, 2021

@jayfraser can you take a look and check if @MatusT's comment is valid and explain why or why not?

@MatusT
Copy link
Author

MatusT commented Apr 29, 2021

I almost forgot that I opened this issue. Looking at the code, it had major revisions since I looked at it. It might be possible that this issue is now obsolete.

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

No branches or pull requests

2 participants