EGL-Registry icon indicating copy to clipboard operation
EGL-Registry copied to clipboard

RFC: EGL counterpart to GLX_EXT_swap_control_tear

Open nwnk opened this issue 4 years ago • 15 comments

There's no EGL analogue to GLX_EXT_swap_control_tear, which seems like an easy oversight to fix.

I don't have this quite wired up for Mesa yet, and there's some details to address. The GLX extensions let you name a drawable explicitly (meh) and also query the actually-set swap interval and whether tears can be requested. I'm not convinced that's useful for EGL, since you can't import an EGL surface the way you can a GLX drawable, but maybe we're orthogonality enthusiasts. Feedback welcome.

nwnk avatar Sep 09 '20 20:09 nwnk

I was looking at this same issue for ANGLE, where the Vulkan backend could use FIFO_RELAXED to implement the feature:

https://bugs.chromium.org/p/angleproject/issues/detail?id=4257

Incidentally, the NVIDIA binary driver already accepts negative values for eglSwapInterval, so the extension is somewhat supported already despite not being advertised at all.

flibitijibibo avatar Sep 09 '20 20:09 flibitijibibo

Looks useful. IIUC, FIFO_RELAXED is the equivalent of eglSwapInternal(-1) per this extension, so ANGLE would have to set EGL_MIN_SWAP_INTERVAL to -1.

I'm not sure how problematic that is though for applications. What if an application just uses the value of EGL_MIN_SWAP_INTERVAL knowing that it's >= 0 per spec, but then suddenly that's not true anymore? Then they silently go from no-vsync to vsync mode.

What if a new symbol like EGL_MIN_NEGATIVE_SWAP_INTERVAL is introduced for this purpose so that EGL_MIN_SWAP_INTERVAL can retain its value with or without this extension?

ShabbyX avatar Sep 14 '20 02:09 ShabbyX

EGL Work Group ping'd for comment.

stonesthrow avatar Sep 16 '20 17:09 stonesthrow

Incidentally, the NVIDIA binary driver already accepts negative values for eglSwapInterval, so the extension is somewhat supported already despite not being advertised at all.

As per the specification for eglSwapInterval, the NVIDIA EGL driver silently clamps negative values to EGL_MIN_SWAP_INTERVAL, which is 0. The feature described here is NOT supported.

dkartch-nv avatar Sep 21 '20 21:09 dkartch-nv

I'm not sure how problematic that is though for applications. What if an application just uses the value of EGL_MIN_SWAP_INTERVAL knowing that it's >= 0 per spec, but then suddenly that's not true anymore? Then they silently go from no-vsync to vsync mode.

What if a new symbol like EGL_MIN_NEGATIVE_SWAP_INTERVAL is introduced for this purpose so that EGL_MIN_SWAP_INTERVAL can retain its value with or without this extension?

I agree that is a potential problem which will cause backwards compatibility issues. I think it probably wasn't a consideration for GLX because it only exposes a constant for MAX interval, not MIN.

It seems to be that a better way to support this feature would be as a separate flag, rather than overloading the swap interval. But if there's a need to make this mirror the GLX extension, then perhaps a compromise is that it requires explicitly setting a new surface attribute to enable the feature, and then eglSwapInterval will accept negative values for that surface.

dkartch-nv avatar Sep 21 '20 21:09 dkartch-nv

Are we aware of any scenario where the max swap interval would not be possible as a negative swap interval? If not, the attribute might be enough, without having to worry for something like MIN_NEGATIVE_INTERVAL - having a new flag would be a bit fussy for us using SDL, for example, since we only have SDL_GL_SetSwapInterval available to us, so any new EGL functionality would have to be tacked onto the EGL backend for that call.

flibitijibibo avatar Sep 21 '20 22:09 flibitijibibo

In Mesa the "max" swap interval is just INT_MAX. Presumably the max is there to accommodate implementations where the limit is imposed by the hardware, but if any hardware actually exposes that kind of interval control we don't take advantage of it in any of the existing drivers (and I don't think we ever did for any of the drivers we've dropped).

Internally Mesa uses the negative-interval convention since it's unambiguous and matched what GLX did. I don't have any particular need for the EGL API to do the same, and given the existing language about clamping to EGL_MIN_SWAP_INTERVAL I'm leaning towards a new surface attribute so apps would need to opt in:

eglSurfaceAttrib(dpy, surface, EGL_LATE_SWAPS_TEAR_XXX, EGL_TRUE);

SDL could still honor the negative-interval convention if it wanted, and Mesa would likely convert that to a negative interval internally, but anybody else that happened to be accidentally setting a negative interval would see no change.

The next question would be whether you'd need to have EGL_LATE_SWAPS_MAY_TEAR_XXX as an EGLConfig attribute. Maybe? For Mesa it's a property of the EGLDisplay not the EGLConfig, and the feature's availability would be a function of having the extension string in the display extensions list; if we made it a property of the EGLConfig it'd just be either set for all or for none. I don't have a preference, but I guess I can imagine hardware where it'd be a property of the EGLConfig.

nwnk avatar Sep 22 '20 18:09 nwnk

An opt-in for eglSurfaceAttrib would be great for us. We do already have some backends where we set an error without even calling SwapInterval when interval < 0, so we could do the same for the EGL backend if the function returns EGL_FALSE (or EGL_BAD_ATTRIBUTE if it's an old setup).

@ShabbyX might have a better idea of where to put EGL_LATE_SWAPS_MAY_TEAR_XXX if needed, as the ANGLE implementation did have some issues with feature queries for this iirc.

flibitijibibo avatar Sep 22 '20 20:09 flibitijibibo

In the Vulkan backend of ANGLE, we use swap interval to determine the swapchain's VkPresentModeKHR. That in turn selects from a list of eligible present modes queried from the surface. Given that vkGetPhysicalDeviceSurfacePresentModesKHR depends on the surface, I would say EGL_LATE_SWAPS_MAY_TEAR_XXX may or may not have any effect as technically Vulkan is allowed to return a different set of supported present modes per surface configuration. The same is true for eglSwapInterval.

Given that eglSwapInterval applies to the display, EGL_LATE_SWAPS_MAY_TEAR_XXX should also be a display property. In other words, no to EGLConfig attribute IMO.

FYI @null77

ShabbyX avatar Sep 23 '20 15:09 ShabbyX

Given that eglSwapInterval applies to the display, EGL_LATE_SWAPS_MAY_TEAR_XXX should also be a display property. In other words, no to EGLConfig attribute IMO.

Just to be clear, when I said "For Mesa it's a property of the EGLDisplay not the EGLConfig", I was describing how the implementation works, not the object where the state is attached. The swap interval is a property of the surface. From the spec (emphasis mine):

The function EGLBoolean eglSwapInterval(EGLDisplay dpy, EGLint interval); specifies the minimum number of video frame periods per buffer swap for the draw surface of the current context, for the current rendering API.

That said, in trying to sketch out the spec language adding an EGLConfig attribute and documenting the interactions with the other clever-posting-semantics extensions, I'm not loving it. I'm considering making it only a surface attribute, with some language a la EGL_KHR_mutable_render_buffer or EGL_NV_post_sub_buffer to allow the winsys to just throw errors at you. Something like: eglQuerySurface will return true/false for SWAPS_MAY_TEAR if that's configurable for the surface, and raise EGL_BAD_MATCH otherwise. (This also lets me punt on the issue of what it means to post a tearing swap to a stream or a double-buffered pbuffer.)

nwnk avatar Sep 23 '20 17:09 nwnk

I'm considering making it only a surface attribute

In case I wasn't clear before, that's what ANGLE prefers as well.

ShabbyX avatar Sep 24 '20 21:09 ShabbyX

New revision adds the surface query, some extension interactions, and a few more open issues, and provisionally allocates an enum from Mesa's range. I'm hoping to have it wired up for Mesa later this week, see the Mesa merge request for updates on the code part.

nwnk avatar Sep 29 '20 18:09 nwnk

Updated (atop #114), now with updated XML and headers.

nwnk avatar Oct 01 '20 19:10 nwnk

Need a few MESA supporters to review.

stonesthrow avatar Oct 12 '20 16:10 stonesthrow

Working on a Wayland protocol to allow clients to request asynchronous buffer swaps revealed the problem that EGL does not have a way to differentiate between mailbox and immediate presentation modes (borrowing the Vulkan terms) - swap interval of 0 means mailbox by default (at least on Wayland, and it can't be changed without breaking applications).

This extension seems to provide the desired functionality; interpreting the text literally, a swap interval of 0 means that all frames are late and thus should always tear. Should that maybe be stated explicitly in the extension text?

Zamundaaa avatar Nov 09 '21 22:11 Zamundaaa