[Mesa-dev] [PATCH] mesa: add GL_OES_copy_image support
Ilia Mirkin
imirkin at alum.mit.edu
Tue Mar 29 16:36:41 UTC 2016
On Thu, Feb 18, 2016 at 1:24 PM, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
> On Thu, Feb 18, 2016 at 1:14 PM, Ian Romanick <idr at freedesktop.org> wrote:
>> On 02/15/2016 05:41 PM, Ilia Mirkin wrote:
>>> Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
>>> ---
>>>
>>> I ran this with the dEQP tests, and other than the caveats below, they seem to
>>> mostly work.
>>>
>>> The biggest caveat is that this can't actually be enabled for any drivers that
>>> don't implement ETC2 in hardware. So really that's just freedreno and the
>>> super-new desktop hardware. The problem is that you can copy between ETC2 and
>>> non-compressed images, so you need to have the original data around. At least
>>> the way that st/mesa implements this, the original data is not kept around.
>>>
>>> In order to enable this more generally, st/mesa will have to be taught to keep
>>> track of the originally-uploaded data. And support copying (and re-decoding)
>>> of data from another image.
>>>
>>> There also appears to be some unrelated problem relating to copying non-0 levels
>>> but that could well be a nouveau issue, or something unrelated. I don't think
>>> it's a problem with this patch.
>>>
>>> docs/GL3.txt | 2 +-
>>> src/mapi/glapi/gen/es_EXT.xml | 22 +++++++++
>>> src/mesa/main/copyimage.c | 27 ++++++++++-
>>> src/mesa/main/extensions_table.h | 1 +
>>> src/mesa/main/mtypes.h | 1 +
>>> src/mesa/main/tests/dispatch_sanity.cpp | 3 ++
>>> src/mesa/main/textureview.c | 86 +++++++++++++++++++++++++++++++++
>>> src/mesa/state_tracker/st_extensions.c | 8 +++
>>> 8 files changed, 148 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/docs/GL3.txt b/docs/GL3.txt
>>> index 0957247..3c4db06 100644
>>> --- a/docs/GL3.txt
>>> +++ b/docs/GL3.txt
>>> @@ -241,7 +241,7 @@ GLES3.2, GLSL ES 3.2
>>> GL_KHR_debug DONE (all drivers)
>>> GL_KHR_robustness 90% done (the ARB variant)
>>> GL_KHR_texture_compression_astc_ldr DONE (i965/gen9+)
>>> - GL_OES_copy_image not started (based on GL_ARB_copy_image, which is done for some drivers)
>>> + GL_OES_copy_image DONE (core only)
>>> GL_OES_draw_buffers_indexed not started
>>> GL_OES_draw_elements_base_vertex DONE (all drivers)
>>> GL_OES_geometry_shader started (Marta)
>>> diff --git a/src/mapi/glapi/gen/es_EXT.xml b/src/mapi/glapi/gen/es_EXT.xml
>>> index fb0ef05..91e118f 100644
>>> --- a/src/mapi/glapi/gen/es_EXT.xml
>>> +++ b/src/mapi/glapi/gen/es_EXT.xml
>>> @@ -941,6 +941,28 @@
>>>
>>> </category>
>>>
>>> +<category name="GL_OES_copy_image" number="208">
>>> +
>>> + <function name="CopyImageSubDataOES" alias="CopyImageSubData" es2="3.0">
>>> + <param name="srcName" type="GLuint"/>
>>> + <param name="srcTarget" type="GLenum"/>
>>> + <param name="srcLevel" type="GLint"/>
>>> + <param name="srcX" type="GLint"/>
>>> + <param name="srcY" type="GLint"/>
>>> + <param name="srcZ" type="GLint"/>
>>> + <param name="dstName" type="GLuint"/>
>>> + <param name="dstTarget" type="GLenum"/>
>>> + <param name="dstLevel" type="GLint"/>
>>> + <param name="dstX" type="GLint"/>
>>> + <param name="dstY" type="GLint"/>
>>> + <param name="dstZ" type="GLint"/>
>>> + <param name="srcWidth" type="GLsizei"/>
>>> + <param name="srcHeight" type="GLsizei"/>
>>> + <param name="srcDepth" type="GLsizei"/>
>>> + </function>
>>> +
>>> +</category>
>>> +
>>> <!-- 175. GL_OES_geometry_shader -->
>>> <category name="GL_OES_geometry_shader" number="210">
>>> <enum name="GEOMETRY_SHADER_OES" value="0x8DD9"/>
>>> diff --git a/src/mesa/main/copyimage.c b/src/mesa/main/copyimage.c
>>> index d571d22..a0f1c69 100644
>>> --- a/src/mesa/main/copyimage.c
>>> +++ b/src/mesa/main/copyimage.c
>>> @@ -25,6 +25,7 @@
>>> * Jason Ekstrand <jason.ekstrand at intel.com>
>>> */
>>>
>>> +#include "context.h"
>>> #include "glheader.h"
>>> #include "errors.h"
>>> #include "enums.h"
>>> @@ -360,8 +361,32 @@ compressed_format_compatible(const struct gl_context *ctx,
>>> case GL_COMPRESSED_SIGNED_RED_RGTC1:
>>> compressedClass = BLOCK_CLASS_64_BITS;
>>> break;
>>> + case GL_COMPRESSED_RGBA8_ETC2_EAC:
>>> + case GL_COMPRESSED_SRGB8_ALPHA8_ETC2_EAC:
>>> + case GL_COMPRESSED_RG11_EAC:
>>> + case GL_COMPRESSED_SIGNED_RG11_EAC:
>>> + if (_mesa_is_gles(ctx))
>>> + compressedClass = BLOCK_CLASS_128_BITS;
>>> + else
>>> + return false;
>>> + break;
>>> + case GL_COMPRESSED_RGB8_ETC2:
>>> + case GL_COMPRESSED_SRGB8_ETC2:
>>> + case GL_COMPRESSED_R11_EAC:
>>> + case GL_COMPRESSED_SIGNED_R11_EAC:
>>> + case GL_COMPRESSED_RGB8_PUNCHTHROUGH_ALPHA1_ETC2:
>>> + case GL_COMPRESSED_SRGB8_PUNCHTHROUGH_ALPHA1_ETC2:
>>> + if (_mesa_is_gles(ctx))
>>> + compressedClass = BLOCK_CLASS_64_BITS;
>>> + else
>>> + return false;
>>> + break;
>>> default:
>>> - return false;
>>> + if (_mesa_is_gles(ctx) && _mesa_is_astc_format(compressedFormat))
>>> + compressedClass = BLOCK_CLASS_128_BITS;
>>> + else
>>> + return false;
>>> + break;
>>
>> I think we need to allow the ETC, ETC2, and ASTC formats on desktop when
>> the related extensions (GL_ARB_ES3_compatibility and
>> GL_KHR_texture_compression_astc_ldr) are supported. I think that's a
>> bug in the current implementation. It also leaks the "fake" ETC2
>> problem into desktop OpenGL. :(
>
> From ARB_texture_view:
>
> (13) Are interactions with ETC2/EAC compressed texture formats defined?
>
> RESOLVED: No. It is likely that these formats are emulated with
> uncompressed internal formats on older hardware, and the resulting
> complications make defining texture view classes for these formats too
> difficult for too little functionality.
>
> So I think this stuff can be ignored on desktop. Maybe for ASTC it
> makes sense, but I wholly agree with the assessment above about it
> being huge pain for little to no gain.
>
>>
>> But it would be a heaping helping of awesome sauce if either
>> GL_KHR_texture_compression_astc_ldr or GL_ARB_texture_view listed
>> interactions with the other spec. There are interactions lists in the
>> GL_OES_texture_view spec... do the changes here match that spec?
>
> You're just talking about the additional view classes right? I tried
> to get them right, but it's entirely possible that I messed it up. The
> rgb <-> srgb versions of the etc2 views worked fine, as one would
> expect. I didn't implement OES_texture_view because there are no dEQP
> tests for it. (Actually I did implement it... sorta, but I couldn't
> test it, so I didn't send it out.)
>
>>
>> Either way, I submitted a bug on the ARB/KHR spec.
>>
>> https://www.khronos.org/bugzilla/show_bug.cgi?id=1464
I believe this was resolved in the way that I expected (i.e. desktop
doesn't have to worry about this stuff). Any further comments? You can
find the latest (i.e. rebased... don't think I had any serious
changes) copy of this patch at
https://github.com/imirkin/mesa/commit/e884c7590b5d89a7c2b94cc177f9cb1dcdae9396.patch
as well as another patch which adds the EXT variant here (since that's
what dEQP wants):
https://github.com/imirkin/mesa/commit/7f5cc3d116a355823808f0598ad4738d60458aca.patch
I've never been able to run all of these to completion as I don't have
NVIDIA hardware that supports ETC2 [and stays up for more than 5
minutes before mysteriously hanging... looking at you, TK1 board], and
i965 has a horrid ARB_copy_image bug with cube + format conversion
copies (in the desktop version too) -- and I only recently got a SKL
in the first place [which has hw ETC2]. I'll check it out though... I
should be able to avoid that bug and double-check that ETC2 and ASTC
work as expected.
In the meanwhile, ping on the review :)
-ilia
More information about the mesa-dev
mailing list