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

courtessy @Lopuska: opengl occlusion query fix #1505

Merged
merged 1 commit into from Feb 15, 2016

Conversation

Projects
None yet
3 participants
@Azaezel
Contributor

Azaezel commented Jan 18, 2016

No description provided.

@Azaezel

This comment has been minimized.

Show comment
Hide comment
@Azaezel

Azaezel Jan 18, 2016

Contributor

better version of #1441

Contributor

Azaezel commented Jan 18, 2016

better version of #1441

@Lopuska

This comment has been minimized.

Show comment
Hide comment
@Lopuska

Lopuska Jan 18, 2016

Member

Thank you @Azaezel
Here is a quick description if someone wants to know more about:

This fixes error messages from OpenGL render device during queries requests.
To reproduce this issue, just make sure to use OpenGL rendering, then put a point light in the world with at least 1 opaque object in the scene and set the castShadow flag to on from the point light properties.
The problem is related in the fact that OpenGL has some differences in 'when' we can perform a query to the GPU. In DirectX you can make it at any time, also more than one time in a single drawcall. In OpenGL you can't. In that case, if you have a query active in a target (for example GL_SAMPLES_PASSED), you have to wait that this query finish it's work before you can perform a new one.

If someone want to know how I fixed that, here is a quick but clear explanation:
This is what OpenGL doesn't like: (but it's ok for DirectX)
if ( curEntry.shadowMap )
curEntry.shadowMap->preLightRender();
if ( curEntry.dynamicShadowMap ) curEntry.dynamicShadowMap->preLightRender(); // argh! can't call it again.

Occlusion query is used here to know if a light is occluded by the geometry. If so, we don't need to render the shadow map.
However we have 2 kind of shadowmap drawed together, 1 static and 1 dynamic.
So instead to store GFXOcclusionQuery object per shadow instance, I move that object inside the ShadowMapParams which is a shared object between static and dynamic shadows. The light has the ownership of this object, so it's perfect to make it our query one single time (not 2) at the beginnning of drawing lights and check it later just to know if we need to render the shadow related to each light.
So it's a good way to make OpenGL happy. DirectX works always perfectly as before.
As last thing, this PR give also just a little of more performance, because we don't have to query 2 objects but only 1, and GPU queries can be really expensive if not used in right way!

This has been already tested deeply.

Before the fix, wrong:
http://i.imgur.com/CGpwRsp.png
After the fix, correct result:
http://i.imgur.com/U8oxgEm.png

Thank you! :)

Member

Lopuska commented Jan 18, 2016

Thank you @Azaezel
Here is a quick description if someone wants to know more about:

This fixes error messages from OpenGL render device during queries requests.
To reproduce this issue, just make sure to use OpenGL rendering, then put a point light in the world with at least 1 opaque object in the scene and set the castShadow flag to on from the point light properties.
The problem is related in the fact that OpenGL has some differences in 'when' we can perform a query to the GPU. In DirectX you can make it at any time, also more than one time in a single drawcall. In OpenGL you can't. In that case, if you have a query active in a target (for example GL_SAMPLES_PASSED), you have to wait that this query finish it's work before you can perform a new one.

If someone want to know how I fixed that, here is a quick but clear explanation:
This is what OpenGL doesn't like: (but it's ok for DirectX)
if ( curEntry.shadowMap )
curEntry.shadowMap->preLightRender();
if ( curEntry.dynamicShadowMap ) curEntry.dynamicShadowMap->preLightRender(); // argh! can't call it again.

Occlusion query is used here to know if a light is occluded by the geometry. If so, we don't need to render the shadow map.
However we have 2 kind of shadowmap drawed together, 1 static and 1 dynamic.
So instead to store GFXOcclusionQuery object per shadow instance, I move that object inside the ShadowMapParams which is a shared object between static and dynamic shadows. The light has the ownership of this object, so it's perfect to make it our query one single time (not 2) at the beginnning of drawing lights and check it later just to know if we need to render the shadow related to each light.
So it's a good way to make OpenGL happy. DirectX works always perfectly as before.
As last thing, this PR give also just a little of more performance, because we don't have to query 2 objects but only 1, and GPU queries can be really expensive if not used in right way!

This has been already tested deeply.

Before the fix, wrong:
http://i.imgur.com/CGpwRsp.png
After the fix, correct result:
http://i.imgur.com/U8oxgEm.png

Thank you! :)

@Areloch Areloch added the Bugfix label Jan 18, 2016

@Lopuska Lopuska self-assigned this Feb 14, 2016

@Lopuska Lopuska added this to the 3.9 milestone Feb 14, 2016

Lopuska added a commit that referenced this pull request Feb 15, 2016

@Lopuska Lopuska merged commit b009d67 into GarageGames:development Feb 15, 2016

@Azaezel Azaezel deleted the Azaezel:OcclusionQueryFixGL_Clean branch Feb 16, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment