On Tue, Mar 29, 2016 at 12:36 PM, Ilia Mirkin <[email protected]> wrote: > On Thu, Feb 18, 2016 at 1:24 PM, Ilia Mirkin <[email protected]> wrote: >> On Thu, Feb 18, 2016 at 1:14 PM, Ian Romanick <[email protected]> wrote: >>> On 02/15/2016 05:41 PM, Ilia Mirkin wrote: >>>> Signed-off-by: Ilia Mirkin <[email protected]> >>>> --- >>>> >>>> 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 <[email protected]> >>>> */ >>>> >>>> +#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 _______________________________________________ mesa-dev mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/mesa-dev
