KhronosGroup / EGL-Registry

EGL API and Extension Registry
115 stars 105 forks source link

`QueryDmaBufModifiersEXT` does not guarantee obtained modifiers are actually usable #201

Open yshui opened 4 months ago

yshui commented 4 months ago

QueryDmaBufModifiersEXT lets me query which modifiers are supported for a given format, but the information I provide to this API (i.e. format) is not sufficient to determine whether a modifier will actually work. For example, at allocation time, mesa AMDGPU will also check if the size of the buffer is within supported range (see: https://gitlab.freedesktop.org/mesa/mesa/-/blob/2487a875527f636565a7b39036690fbf7c5d46db/src/gallium/drivers/radeonsi/si_texture.c#L1564), and will fail if I try to allocate a texture that's too large.

Can QueryDmaBufModifiersEXT be extended to incorporate size info into the query as well? This will make it "complete" w.r.t, e.g. gbm_bo_create_with_modifiers.

stonesthrow commented 4 months ago

Reference: https://registry.khronos.org/EGL/extensions/EXT/EGL_EXT_image_dma_buf_import_modifiers.txt

We need original authors or from the dmabuf community to comment and provide feedback.

If I recall documentation, its possible the size of the buffer is too small and a fault will happen (MESA or Weston). So I guess the original authors realized the "size" could be a lie. Either its the right size or there is a fault. Although size checks are nice check to add.

As for max texture size, you can query GL_MAX_TEXTURE_SIZE

yshui commented 1 month ago

As for max texture size, you can query GL_MAX_TEXTURE_SIZE

GL_MAX_TEXTURE_SIZE is the global maximum texture size, right? But there are specific texture size limits for a given modifier. Vulkan has VkImageFormatProperties::maxExtent, which can be queried by chaining a VkPhysicalDeviceImageDrmFormatModifierInfoEXT when calling vkGetPhysicalDeviceImageFormatProperties2

stonesthrow commented 1 month ago

That's Vulkan API. And if you have Vulkan and EGL/GL these are likely the same - maximum the GPU can handle.

yshui commented 1 month ago

This is a useful piece of information, can we add this to EGL as well?

stonesthrow commented 1 month ago

You can query max rendering surface from the EGL Configs, that will match up with Max renderer able image of Vulkan. eglGetConfigAttrib: EGL_MAX_PBUFFER_WIDTH Returns the maximum width of a pixel buffer surface in pixels.

EGL_MAX_PBUFFER_HEIGHT Returns the maximum height of a pixel buffer surface in pixels.

EGL_MAX_PBUFFER_PIXELS Returns the maximum size of a pixel buffer surface in pixels.

yshui commented 1 month ago

That is not what I want though. What I want to query is the maximum possible image size for a given DRM format modifier, or whether a given image dimension is supported by a DRM format modifier.

stonesthrow commented 1 month ago

EGL Images created from textures, and textures from EGL Images would be limited to that GL_MAX_TEXTURE_SIZE. Since you are talking DRM formats, then you are talking https://registry.khronos.org/EGL/extensions/EXT/EGL_EXT_image_dma_buf_import.txt and https://registry.khronos.org/EGL/extensions/EXT/EGL_EXT_image_dma_buf_import_modifiers.txt There is no mention of size limits with importing dmabufs into EGLImages or importing any native buffer. However, once that EGLImage is glEGLTargetTexture2DOES to import into a texture - then the texture limit will apply. Maybe EGLImage needs a limit on creation - but EGL just identifies the type and properties of the buffer for import/sharing with other khronos APIs.

yshui commented 1 month ago

There is no mention of size limits with importing dmabufs into EGLImages or importing any native buffer.

Yes, that's what the spec says. But in practice drivers will impose size limits, and that information is exposed nowhere in the API, and that is the problem.

Using Vulkan you will be able to query this information, and this is missing from EGL.

stonesthrow commented 1 month ago

I understand what you are saying. With Vulkan, you will use the VkImage for rendering or sampling, such that the GPU may have limits on what it can handle.

EGL could have an EGLImage that is not used with OpenGL/ES. So there may not be a GPU limit - if say used with OpenCL or such. So the limit is only imposed when the importing to a texture - that has a GPU limit.

yshui commented 1 month ago

EGL could have an EGLImage that is not used with OpenGL/ES. So there may not be a GPU limit - if say used with OpenCL or such. So the limit is only imposed when the importing to a texture - that has a GPU limit.

I don't understand what you are saying. There are limits when creating an EGLImage with DRM format modifier.

stonesthrow commented 1 month ago

You'll have to point that out, I don't see what modifiers you are referring to, https://registry.khronos.org/EGL/extensions/EXT/EGL_EXT_image_dma_buf_import.txt There is EGL_WIDTH and EGL_HEIGHT as attributes for creating new image from dmabuf. So other than the MAX_TEXTURE_SIZE there is nothing in spec. Unless as you say there are some modifiers that indicate a limit for this type of client buffer.

yshui commented 1 month ago

You'll have to point that out, I don't see what modifiers you are referring to,

What do you mean? Now I am confused. We are talking about DRM format modifiers, right?

So other than the MAX_TEXTURE_SIZE there is nothing in spec.

Yes! And that is the problem I've been trying to raise this whole time. Drivers in practice impose size limit based on modifiers used, but EGL API does not expose that.

stonesthrow commented 1 month ago

My apologies, I have several concurrent tasks, some related. So yes, need to get original authors or people working in this space to create a new version of this extension and add the queries you need. The EGL work group is not active other than a few people checking in on new issues and proposals. So to get changes requires people to do the proposals.

stonesthrow commented 1 month ago

So the thing to do would be copy https://registry.khronos.org/EGL/extensions/EXT/EGL_EXT_image_dma_buf_import_modifiers.txt into a new spec proposal EGL_EXT_image_dma_buf_import_modifiers2. And in this new spec propose a new way to query the max size. New query function or modifiers to use.

Put that in a pull request and we get as many of the linux participants to review as possible for feedback - needed to justify making this a EXT extension.

https://github.com/KhronosGroup/EGL-Registry/wiki

stonesthrow commented 1 month ago

@linyaa-kiwi add for participant/monitor

yshui commented 1 month ago

Thank you.