[Mesa-dev] [PATCH] mesa: add GL_OES_copy_image support
Ilia Mirkin
imirkin at alum.mit.edu
Tue Mar 29 19:17:54 UTC 2016
On Tue, Mar 29, 2016 at 12:36 PM, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
> 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
And FTR, all *copy_image*astc* and *copy_image*etc2* tests pass on SKL
with the above patches.
-ilia
More information about the mesa-dev
mailing list