Re: [Piglit] [PATCH 2/2] EGL_CHROMIUM_get_sync_values: Add conformance test.
Well, I find your arguments convincing. For whatever my opinion is worth.
:-)
On Apr 24, 2014 1:03 PM, "Chad Versace"
wrote:
> On Thu, Apr 24, 2014 at 09:17:50AM -0700, Jamey Sharp wrote:
> > On Apr 23, 2014 1:36 PM, "Chad Versace"
wrote:
> > > As for eglSwapBuffers being synchronized to vertical retraces, again
it
> > > is on all platforms I've used.
> >
> > Modulo bugs, presumably, since it certainly isn't reliably synchronized
in X
> > right now. :-)
>
> How is it not synchronized in X? And how unreliabe is its unreliability?
> Please tell. This storm cloud troubles my soul.
I should qualify, I've only been testing glX, not EGL, but since the bugs
I'm seeing look to me like they're in the user space drivers on the X
server side, I expect them to show up when using either client API.
You can try the Piglit patch series that Theo and I posted yesterday and
see whether you get any better results than I do. Our earlier ad-hoc tests
suggested that none of the open source drivers do well at OML_sync_control,
using either DRI2 or Present. Our Piglit tests confirm that result for
current git master of xserver and xf86-video-intel using either UXA or SNA
with either DRI2 or DRI3.
I speculate that only some of the code paths are buggy, and it depends on
whether the driver decides to use page flipping, blits, or exchanges, which
in turn follows complicated rules I don't understand. One symptom is that
often the WaitForSbc or WaitForMsc call will return 0 for both UST and MSC,
which occurs when the driver calls DRI2SwapComplete with zeroes, and that
tends to correlate with bad behavior. But it isn't a cause or a reliable
effect of the bugs.
I recommend trying without a compositor first as compositing managers seem
to make the situation quite a bit worse, though under compositing it's
unreliably unreliable. :-\ Without a compositor, stable results from the
Piglit tests. With a compositor, sometimes both the compositor and the GL
client crash simultaneously, and the rest of the time I can't predict which
tests will fail.
> > Ooh, and does Mesa set EGL_MAX_SWAP_INTERVAL == 0 if vblank_mode is 0?
>
> Yep. This code block, present in the X11 and Wayland code, does it. I
verified
> it actually does it job with my wfl-swapinterval toy.
>
>switch (vblank_mode) {
>...
Ah, very good!
Jamey
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit
Re: [Piglit] [PATCH 2/2] EGL_CHROMIUM_get_sync_values: Add conformance test.
On Thu, Apr 24, 2014 at 09:17:50AM -0700, Jamey Sharp wrote: > On Apr 23, 2014 1:36 PM, "Chad Versace" wrote: > > On Tue, Apr 22, 2014 at 12:22:12PM -0700, Sarah Sharp wrote: > > > On Fri, Apr 18, 2014 at 06:21:31PM -0700, Jamey Sharp wrote: > > > > - I don't see that MSC has to change at all even after two > > > > eglSwapBuffers calls, if SwapBuffers doesn't sync to vertical retrace. > > > > > > I looked at the EGL 1.5 spec with Chad, and if you look at the > > > definition of eglSwapInterval, you'll see that the "minimum number of > > > video frame periods per buffer swap" is set to 1 by default. I think > > > one frame period is the time between vertical retraces, but the spec > > > isn't clear on that. > > > > The spec is sadly ambiguous on that. It's also ambiguous on whether > > eglSwapBuffers blocks or immediately returns false if too many > > outstanding swaps are pending. > > > > I think it's safe to assume that it blocks. because that's what all EGL > > implementations do that I've used: X11, Android, Wayland, and > > (vacuously) GBM. (More below on why this it's vacuously true that GBM > > implements the blocking behavior). > > I'd just feel more comfortable, in a conformance test, if there were > specification text you could point to instead of a de facto standard. I note > you only mention Linux based GL implementations, after all. All Linux, but not all Mesa. Android's eglSwapBuffers is supposed to block across all Android platforms. On the Khronos EGL Teleconference yesterday, several EGL vendors coincidentally had a lengthy conversation about bugs that arise in applications due to eglSwapBuffers undesirably blocking. (The conversation's motivation was not this Piglit test, though). An implicit axiom that underpinned the conversation was that eglSwapBuffers does block. That's just what implementations do. The blocking itself wasn't up for debate, but how applications should deal with it rare corner cases. So I stand by my claim. The de facto standard is the standard here. > > As for eglSwapBuffers being synchronized to vertical retraces, again it > > is on all platforms I've used. > > Modulo bugs, presumably, since it certainly isn't reliably synchronized in X > right now. :-) But I agree that it's intended to be, especially considering: How is it not synchronized in X? And how unreliabe is its unreliability? Please tell. This storm cloud troubles my soul. > Theo has since pointed out this sentence in the OML_sync_control spec: > > "If there are multiple outstanding swaps for the same window, at most one such > swap can be satisfied per increment of MSC." > > So I retract my first concern. :-) > > (Technically this doesn't say the swap has to happen during vertical retrace, > but for the purposes of Sarah's tests that doesn't matter.) I found the EGL text that mandates synchronization to vertical retrace! From the EGL 1.5 spec: If interval is set to a value of 0, buffer swaps are not synchronized to a video frame, [...] Well sorta... Though not explicitly stated, I believe spec clearly intends to mandate the inverse also: "If the invterval is non-zero, buffer swaps are synchronized to a video frame". > > I played with that test app on X11, Wayland, and GBM to discover exactly > > how the swap interval behaves on each platform. Wayland was the > > weirdest. > > Out of curiosity, how was Wayland weird? The swap interval range in Wayland is [EGL_MIN_SWAP_INTERVAL=0, EGL_MAX_SWAP_INTERVAL=1]. I was surprised that EGL_MAX_SWAP_INTERVAL < 2. > > I think the test should do this: > > > > Create the EGLConfig as it currently does. That is, do not specify > > EGL_[MIN|MAX]_SWAP_INTERVAL. Then immediately query > > eglGetConfigAttrib(EGL_MIN_SWAP_INTERVAL) and > > eglGetConfigAttrib(EGL_MAX_SWAP_INTERVAL). > > > > If the max swap interval is 0, then eglSwapBuffers will not block. > > So report PIGLIT_RESULT_SKIP. > > > > Else, the EGL spec ensures that the surface's swap interval is > > EGL_MIN_SWAP_INTERVAL. Calculate the expected MSC and SBC from that. > > > > For bonus points, let the desired swap_interval be an input > > parameter to the test. The test will skip if and only if > > requested_swap_interval is outside of [EGL_MIN_SWAP_INTERVAL, > > EGL_MAX_SWAP_INTERVAL]. The test sets the surface's swap interval > > with eglSwapInterval(requested_swap_interval), calculates expected > > values of of MSC and SBC, and tests for them. > > > > Why is GBM a special case? Because EGLConfigs on GBM have > > EGL_MIN_SWAP_INTERVAL == EGL_MAX_SWAP_INTERVAL == 0. So eglSwapBuffers > > never throttles. > > This strikes me as a very satisfying way to handle it, my lingering concerns > about unspecified throttling/blocking behavior notwithstanding. > > Ooh, and does Mesa set EGL_MAX_SWAP_INTERVAL == 0 if vblank_mode is 0? Yep. This code block, present in the X11 and Wayland code, does it. I verified it actually does it job with my
Re: [Piglit] [PATCH 2/2] EGL_CHROMIUM_get_sync_values: Add conformance test.
On Apr 23, 2014 1:36 PM, "Chad Versace" wrote: > On Tue, Apr 22, 2014 at 12:22:12PM -0700, Sarah Sharp wrote: > > On Fri, Apr 18, 2014 at 06:21:31PM -0700, Jamey Sharp wrote: > > > - I don't see that MSC has to change at all even after two > > > eglSwapBuffers calls, if SwapBuffers doesn't sync to vertical retrace. > > > > I looked at the EGL 1.5 spec with Chad, and if you look at the > > definition of eglSwapInterval, you'll see that the "minimum number of > > video frame periods per buffer swap" is set to 1 by default. I think > > one frame period is the time between vertical retraces, but the spec > > isn't clear on that. > > The spec is sadly ambiguous on that. It's also ambiguous on whether > eglSwapBuffers blocks or immediately returns false if too many > outstanding swaps are pending. > > I think it's safe to assume that it blocks. because that's what all EGL > implementations do that I've used: X11, Android, Wayland, and > (vacuously) GBM. (More below on why this it's vacuously true that GBM > implements the blocking behavior). I'd just feel more comfortable, in a conformance test, if there were specification text you could point to instead of a de facto standard. I note you only mention Linux based GL implementations, after all. Any day one can use a word like "vacuously" is a good day, though. > As for eglSwapBuffers being synchronized to vertical retraces, again it > is on all platforms I've used. Modulo bugs, presumably, since it certainly isn't reliably synchronized in X right now. :-) But I agree that it's intended to be, especially considering: > Like Sarah said. If you set env vars that provide nonconformant > behavior, then your conformance testsuite may fail. It wasn't obvious to me that that was non-conforming behavior. However, Theo has since pointed out this sentence in the OML_sync_control spec: "If there are multiple outstanding swaps for the same window, at most one such swap can be satisfied per increment of MSC." So I retract my first concern. :-) (Technically this doesn't say the swap has to happen during vertical retrace, but for the purposes of Sarah's tests that doesn't matter.) > I played with that test app on X11, Wayland, and GBM to discover exactly > how the swap interval behaves on each platform. Wayland was the > weirdest. Out of curiosity, how was Wayland weird? > I think the test should do this: > > Create the EGLConfig as it currently does. That is, do not specify > EGL_[MIN|MAX]_SWAP_INTERVAL. Then immediately query > eglGetConfigAttrib(EGL_MIN_SWAP_INTERVAL) and > eglGetConfigAttrib(EGL_MAX_SWAP_INTERVAL). > > If the max swap interval is 0, then eglSwapBuffers will not block. > So report PIGLIT_RESULT_SKIP. > > Else, the EGL spec ensures that the surface's swap interval is > EGL_MIN_SWAP_INTERVAL. Calculate the expected MSC and SBC from that. > > For bonus points, let the desired swap_interval be an input > parameter to the test. The test will skip if and only if > requested_swap_interval is outside of [EGL_MIN_SWAP_INTERVAL, > EGL_MAX_SWAP_INTERVAL]. The test sets the surface's swap interval > with eglSwapInterval(requested_swap_interval), calculates expected > values of of MSC and SBC, and tests for them. > > Why is GBM a special case? Because EGLConfigs on GBM have > EGL_MIN_SWAP_INTERVAL == EGL_MAX_SWAP_INTERVAL == 0. So eglSwapBuffers > never throttles. This strikes me as a very satisfying way to handle it, my lingering concerns about unspecified throttling/blocking behavior notwithstanding. Ooh, and does Mesa set EGL_MAX_SWAP_INTERVAL == 0 if vblank_mode is 0? Jamey ___ Piglit mailing list Piglit@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/piglit
Re: [Piglit] [PATCH 2/2] EGL_CHROMIUM_get_sync_values: Add conformance test.
On Tue, Apr 22, 2014 at 12:22:12PM -0700, Sarah Sharp wrote: > On Fri, Apr 18, 2014 at 06:21:31PM -0700, Jamey Sharp wrote: > > I'm no expert on OML_sync_control, but in a funny coincidence I've > > been reading that spec and Xorg's DRI2 implementation of it pretty > > carefully recently. (I blame keithp.) > > > > In test_eglGetSyncValuesCHROMIUM_msc_and_sbc_test, I think what the > > spec allows and requires might be a little different than what's > > tested, although I'm not sure. > > > > - I don't see that MSC has to change at all even after two > > eglSwapBuffers calls, if SwapBuffers doesn't sync to vertical retrace. > > I looked at the EGL 1.5 spec with Chad, and if you look at the > definition of eglSwapInterval, you'll see that the "minimum number of > video frame periods per buffer swap" is set to 1 by default. I think > one frame period is the time between vertical retraces, but the spec > isn't clear on that. The spec is sadly ambiguous on that. It's also ambiguous on whether eglSwapBuffers blocks or immediately returns false if too many outstanding swaps are pending. I think it's safe to assume that it blocks. because that's what all EGL implementations do that I've used: X11, Android, Wayland, and (vacuously) GBM. (More below on why this it's vacuously true that GBM implements the blocking behavior). As for eglSwapBuffers being synchronized to vertical retraces, again it is on all platforms I've used. > > I think you can test this by setting the DRI config option > > vblank_mode=0 when you run your Piglit test. (I've read that you can > > set that as an environment variable but I can't find code in Mesa that > > would do that, so maybe you have to use drirc.) > > I asked Chad about this, and his argument was if you're setting random > environment variables, the test suite is not guaranteed to work. Jamey, Mesa does respect the environment var vblank_mode=0. You likely failed to find the code in Mesa because it lives in magic XML spaghetti. Like Sarah said. If you set env vars that provide nonconformant behavior, then your conformance testsuite may fail. > > - I'm not sure how you can ensure any SwapBuffers calls completed > > without using any of GLX_OML_sync_control, GLX_INTEL_swap_event, or > > implementation-specific knowledge. It isn't obvious to me that > > eglSwapBuffers can be relied on to throttle the client, even on a > > specific implementation like Xorg/DRI2. (Again, perhaps with > > vblank_mode=0; I'd expect turning off vsync to also turn off > > throttling.) Per the de facto standard behavior discussed above (that eglSwapBuffers blocks if swap_interval >= 1 and there exists outstanding swaps), if two calls to eglSwapBuffers have returned then at least one swap has completed. > It's apparently up to the driver to enforce the throttling of > eglSwapBuffers. Chad and I discussed explicitly setting > EGL_MIN_SWAP_INTERVAL and EGL_MAX_SWAP_INTERVAL to 1 in the EGL config. > The driver is supposed to throw an error if it can't accommodate that > swap interval, and that's a bug if it doesn't. I suggested adding a > piglit log warning if the test fails, something to the effect of: > > "MSC and SBC test failed, does the driver throttle eglSwapBuffers?" > > That way if the test fails, people will at least know what part of the > driver to look at. I've been playing with that idea. I even wrote a test app: https://github.com/chadversary/waffle/blob/wfl-swapinterval/src/utils/wfl-swapinterval.c I played with that test app on X11, Wayland, and GBM to discover exactly how the swap interval behaves on each platform. Wayland was the weirdest. I think the test should do this: Create the EGLConfig as it currently does. That is, do not specify EGL_[MIN|MAX]_SWAP_INTERVAL. Then immediately query eglGetConfigAttrib(EGL_MIN_SWAP_INTERVAL) and eglGetConfigAttrib(EGL_MAX_SWAP_INTERVAL). If the max swap interval is 0, then eglSwapBuffers will not block. So report PIGLIT_RESULT_SKIP. Else, the EGL spec ensures that the surface's swap interval is EGL_MIN_SWAP_INTERVAL. Calculate the expected MSC and SBC from that. For bonus points, let the desired swap_interval be an input parameter to the test. The test will skip if and only if requested_swap_interval is outside of [EGL_MIN_SWAP_INTERVAL, EGL_MAX_SWAP_INTERVAL]. The test sets the surface's swap interval with eglSwapInterval(requested_swap_interval), calculates expected values of of MSC and SBC, and tests for them. Why is GBM a special case? Because EGLConfigs on GBM have EGL_MIN_SWAP_INTERVAL == EGL_MAX_SWAP_INTERVAL == 0. So eglSwapBuffers never throttles. ___ Piglit mailing list Piglit@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/piglit
Re: [Piglit] [PATCH 2/2] EGL_CHROMIUM_get_sync_values: Add conformance test.
On Fri, Apr 18, 2014 at 06:21:31PM -0700, Jamey Sharp wrote: > I'm no expert on OML_sync_control, but in a funny coincidence I've > been reading that spec and Xorg's DRI2 implementation of it pretty > carefully recently. (I blame keithp.) > > In test_eglGetSyncValuesCHROMIUM_msc_and_sbc_test, I think what the > spec allows and requires might be a little different than what's > tested, although I'm not sure. > > - I don't see that MSC has to change at all even after two > eglSwapBuffers calls, if SwapBuffers doesn't sync to vertical retrace. I looked at the EGL 1.5 spec with Chad, and if you look at the definition of eglSwapInterval, you'll see that the "minimum number of video frame periods per buffer swap" is set to 1 by default. I think one frame period is the time between vertical retraces, but the spec isn't clear on that. > I think you can test this by setting the DRI config option > vblank_mode=0 when you run your Piglit test. (I've read that you can > set that as an environment variable but I can't find code in Mesa that > would do that, so maybe you have to use drirc.) I asked Chad about this, and his argument was if you're setting random environment variables, the test suite is not guaranteed to work. > - I'm not sure how you can ensure any SwapBuffers calls completed > without using any of GLX_OML_sync_control, GLX_INTEL_swap_event, or > implementation-specific knowledge. It isn't obvious to me that > eglSwapBuffers can be relied on to throttle the client, even on a > specific implementation like Xorg/DRI2. (Again, perhaps with > vblank_mode=0; I'd expect turning off vsync to also turn off > throttling.) It's apparently up to the driver to enforce the throttling of eglSwapBuffers. Chad and I discussed explicitly setting EGL_MIN_SWAP_INTERVAL and EGL_MAX_SWAP_INTERVAL to 1 in the EGL config. The driver is supposed to throw an error if it can't accommodate that swap interval, and that's a bug if it doesn't. I suggested adding a piglit log warning if the test fails, something to the effect of: "MSC and SBC test failed, does the driver throttle eglSwapBuffers?" That way if the test fails, people will at least know what part of the driver to look at. Sarah Sharp ___ Piglit mailing list Piglit@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/piglit
Re: [Piglit] [PATCH 2/2] EGL_CHROMIUM_get_sync_values: Add conformance test.
I'm no expert on OML_sync_control, but in a funny coincidence I've
been reading that spec and Xorg's DRI2 implementation of it pretty
carefully recently. (I blame keithp.)
In test_eglGetSyncValuesCHROMIUM_msc_and_sbc_test, I think what the
spec allows and requires might be a little different than what's
tested, although I'm not sure.
- I don't see that MSC has to change at all even after two
eglSwapBuffers calls, if SwapBuffers doesn't sync to vertical retrace.
I think you can test this by setting the DRI config option
vblank_mode=0 when you run your Piglit test. (I've read that you can
set that as an environment variable but I can't find code in Mesa that
would do that, so maybe you have to use drirc.)
- I'm not sure how you can ensure any SwapBuffers calls completed
without using any of GLX_OML_sync_control, GLX_INTEL_swap_event, or
implementation-specific knowledge. It isn't obvious to me that
eglSwapBuffers can be relied on to throttle the client, even on a
specific implementation like Xorg/DRI2. (Again, perhaps with
vblank_mode=0; I'd expect turning off vsync to also turn off
throttling.)
And of course I'd be curious to know why ChromeOS wants GetSyncValues
without any of the APIs that make those values meaningful, but that
isn't a question that unit testing can answer. ;-)
All my confusion aside, this seems like a fine set of tests for this extension.
Jamey
On Fri, Apr 18, 2014 at 3:37 PM, Sarah Sharp
wrote:
> Test the EGL_CHROMIUM_get_sync_values extension, which is equivalent to
> the glXGetSyncValuesOML function in the GLX OML_sync_control extension:
>
> http://www.opengl.org/registry/specs/OML/glx_sync_control.txt
>
> It's difficult to test the Media Stream Counter (MSC) without the
> equivalent function to get the MSC rate (glXGetMscRateOML). The test at
> least makes sure MSC and SBC increment after two SwapBuffers() calls.
> UST is system-dependent, so it may behave differently on Windows, Linux,
> and Apple. The test just makes sure it increments monotonically.
>
> The test uses Chad's new subtest infrastructure and the EGL convenience
> functions from egl-util.c.
>
> Signed-off-by: Sarah Sharp
> Cc: Chad Versace
> Cc: Rob Bradford
> ---
> tests/all.py | 4 +
> tests/egl/spec/CMakeLists.txt | 1 +
> .../CMakeLists.gles2.txt | 7 +
> .../egl_chromium_get_sync_values/CMakeLists.txt| 1 +
> .../egl_chromium_get_sync_values.c | 278
> +
> 5 files changed, 291 insertions(+)
> create mode 100644
> tests/egl/spec/egl_chromium_get_sync_values/CMakeLists.gles2.txt
> create mode 100644 tests/egl/spec/egl_chromium_get_sync_values/CMakeLists.txt
> create mode 100644
> tests/egl/spec/egl_chromium_get_sync_values/egl_chromium_get_sync_values.c
>
> diff --git a/tests/all.py b/tests/all.py
> index 49c801a..439cd02 100644
> --- a/tests/all.py
> +++ b/tests/all.py
> @@ -3835,6 +3835,10 @@ egl_khr_fence_sync = {}
> spec['EGL_KHR_fence_sync'] = egl_khr_fence_sync
> egl_khr_fence_sync['conformance'] = concurrent_test('egl_khr_fence_sync')
>
> +egl_chromium_get_sync_values = {}
> +spec['EGL_CHROMIUM_get_sync_values'] = egl_chromium_get_sync_values
> +egl_chromium_get_sync_values['conformance'] =
> concurrent_test('egl_chromium_get_sync_values')
> +
> gles20 = {}
> spec['!OpenGL ES 2.0'] = gles20
> gles20['glsl-fs-pointcoord'] = concurrent_test('glsl-fs-pointcoord_gles2')
> diff --git a/tests/egl/spec/CMakeLists.txt b/tests/egl/spec/CMakeLists.txt
> index 84c3fa0..ac3fe9c 100644
> --- a/tests/egl/spec/CMakeLists.txt
> +++ b/tests/egl/spec/CMakeLists.txt
> @@ -2,3 +2,4 @@ add_subdirectory (egl-1.4)
> add_subdirectory (egl_ext_client_extensions)
> add_subdirectory (egl_khr_create_context)
> add_subdirectory (egl_khr_fence_sync)
> +add_subdirectory (egl_chromium_get_sync_values)
> diff --git a/tests/egl/spec/egl_chromium_get_sync_values/CMakeLists.gles2.txt
> b/tests/egl/spec/egl_chromium_get_sync_values/CMakeLists.gles2.txt
> new file mode 100644
> index 000..d707647
> --- /dev/null
> +++ b/tests/egl/spec/egl_chromium_get_sync_values/CMakeLists.gles2.txt
> @@ -0,0 +1,7 @@
> +link_libraries(
> + piglitutil_${piglit_target_api}
> +)
> +
> +piglit_add_executable(egl_chromium_get_sync_values ../../egl-util.c
> egl_chromium_get_sync_values.c)
> +
> +# vim: ft=cmake:
> diff --git a/tests/egl/spec/egl_chromium_get_sync_values/CMakeLists.txt
> b/tests/egl/spec/egl_chromium_get_sync_values/CMakeLists.txt
> new file mode 100644
> index 000..144a306
> --- /dev/null
> +++ b/tests/egl/spec/egl_chromium_get_sync_values/CMakeLists.txt
> @@ -0,0 +1 @@
> +piglit_include_target_api()
> diff --git
> a/tests/egl/spec/egl_chromium_get_sync_values/egl_chromium_get_sync_values.c
> b/tests/egl/spec/egl_chromium_get_sync_values/egl_chromium_get_sync_values.c
> new file mode 100644
> index 000..adb26aa
> --- /dev/null
> +++
> b/tests/egl/spec/egl_chromium_get_
