KhronosGroup / OpenGL-API

OpenGL and OpenGL ES API Issue Tracker
34 stars 5 forks source link

Occlusion query counter only cares about depth tests #19

Closed NicolBolas closed 6 years ago

NicolBolas commented 6 years ago

The standard is very strange about when occlusion query counters get updated. It clearly states that occlusion queries happen after depth and stencil tests. However, it also states:

Occlusion queries use query objects to track the number of fragments or samples that pass the depth test.

When an occlusion query is active, the samples-passed count is incremented for each fragment that passes the depth test.

When an occlusion query is started with the target ANY_SAMPLES_PASSED, the samples-boolean state maintained by the GL is set to FALSE. While that occlusion query is active, the samples-boolean state is set to TRUE if any fragment or sample passes the depth test.

The standard always talks about this in terms of passing the depth test, never in terms of a fragment reaching this particular stage in the pipeline. By contrast, the Vulkan standard is explicit about sample counters counting what passes all tests.

Is it really the intent that the sample count only gets updated for any fragment that passes the depth test? That stencil tests, fragment shader discarding (if early fragment tests are not used) and the like will never be taken into account? Or is it the idea that a fragment that fails the stencil test would never have reached the depth test at all, and thus would never have had its sample count incremented?

If the latter is the idea, then this only works for early fragment tests. The standard says in 14.9:

If a fragment is discarded during any of these operations, it will not be processed by any subsequent stage, including fragment shader execution.

But "these operations" seems to refer to the subject of 14.9: early fragment tests. The behavior of discarding fragments from non-early fragment tests only is defined as:

Finally, if the fragment was not discarded, it is used to update the framebuffer at the fragment’s window coordinates.

Which does not explicitly forbid a fragment which gets discarded from further processing. And therefore updating the sample count.

ghost commented 6 years ago

The standard always talks about this in terms of passing the depth test, never in terms of a fragment reaching this particular stage in the pipeline. By contrast, the Vulkan standard is explicit about sample counters counting what passes all tests.

Sounds like we need something like "fragments that aren't discarded by earlier stages" in the occlusion query description, rather than "fragments that pass the depth test".

If the latter is the idea, then this only works for early fragment tests. The standard says in 14.9:

If a fragment is discarded during any of these operations, it will not be processed by any subsequent stage, including fragment shader execution.

Ok so whilst these extra stages say something about discarding, you think we need a blanket statement at the top of "Per-Fragment Operations" that says a similar thing about discarding? Seems reasonable.

This needs discussing by both the ES and GL groups, so assigning to both myself and piers for now.

pdaniell-nv commented 6 years ago

Touched on this briefly in the OpenGL WG meeting. What Tobias suggested about makes sense to generalize what is counted beyond depth test.

ghost commented 6 years ago

Just to wrap this up, here's a specific proposal:

Replace:

...fragments or samples that pass the depth test.

in the first paragraph with:

...fragments that aren't discarded by earlier stages.

and then replace:

...if any fragment or sample passes the depth test.

in the last paragraph with:

...for each fragment still being processed after the depth test.

Those instructions work against section 17.3.5 of the OpenGL 4.6 spec as well, though you also need to replace:

...for each fragment that passes the depth test.

in the second paragraph with:

...for each fragment still being processed after the depth test.

pdaniell-nv commented 6 years ago

OpenGL|ES meeting today approved Tobias' suggestion. Assigned to @oddhack to add to spec updates.

oddhack commented 6 years ago

The fixes identified by @pdaniell-nv will be in the next GL & ES spec updates, coming soon.