[Mesa-dev] Initial support for EXT_external_objects v4

Thu Jul 27 02:42:17 UTC 2017

On Wed, Jul 26, 2017 at 5:26 PM, Timothy Arceri <tarceri at itsqueeze.com>
wrote:

> On 27/07/17 04:53, Jason Ekstrand wrote:
>
>> On July 26, 2017 8:48:18 AM Samuel Pitoiset <samuel.pitoiset at gmail.com>
>> wrote:
>>
>> Hi guys,
>>>
>>> I didn't review the radeonsi and radv patches, but I have sent comments
>>> on other parts.
>>>
>>> More generally:
>>>
>>> - I wonder if some intermediate patches can break Mesa, would be nice to
>>> avoid that (especially for bisecting) Can you double check?
>>>
>>> - I don't see any piglit tests... or did I miss them? :)
>>>
>>
>> +1 on the need for tests.  I'll go even further and say that the obvious
>> minimal testing to exercise the extension is far from sufficient.  There
>> are a lot of complex ways in which two APIs can interact and we need to
>> ensure we get it right.  In particular, if we're going to claim that the
>> extension actually works properly we need:
>>
>> 1. Tests which use more than just GL.  In particular, we need to test
>> sharing between GL and Vulcan.
>>
>> 2. Test which exercise "complex" textures, i.e. textures with multiple
>> mip-levels, multiple array slices, 1D, 2D, 3D, etc.
>>
>> 3. Tests which test different texture formats.
>>
>> 4. Tests which test several different combinations of rendering,
>> clearing, glTexSubImage, etc. in one API and combinations of texturing,
>> glGetTexSubImage, etc. in the other API.
>>
>
5. Multisampling

> Preferably, a good set of combinations of the above.
>>
>> Also, the Vulcan bits need to pass validation.  I shouldn't have to say
>> this but it's important and an easy thing to forget so I thought it worth
>> mentioning.
>>
>
> Sure I agree on the need for tests but this isn't a tiny amount of work.

Yes, it is.  However, there are a lot of potentially very complex
interactions in this extension and it's impossible to get them right
without tests.  Claiming that it must work fine because SteamVR works is
about like claiming a C compiler works because it compiles hello world.
Yes, SteamVR is a complex app, but it's use of the extension is likely very
simple: only 2D non-array single-LOD images where rendering always hapens
in GL and texturing always happens in Vulkan.  If we want people to be able
to run arbitrary apps that use this extension, we need way more testing
than that.

> I don't necessarily think should block an initial implementation from
> landing.
>

That's a good question.  There's a part of me that's inclined to say we
should only block it on basic testing if any.  However...  I'm sure that as
the tests get written, we'll find bugs in the implementation and we may
want to sort those out before enabling it; certainly before 17.3.  Also, no
one likes writing piglit tests and I'm a bit concerned that if we land it
based on the promise of tests to be written in the future, they'll never
happen.  I don't want to be too onerous but I've seen this before (and been
the lazy guy who doesn't want to write tests) and I don't want to end up in
a place where we have an important and un-tested piece of functionality.

What I'm inclined to say is that I won't NAK it so long as we have at least
some Vulkan <-> GL sharing tests.  After all, radeon isn't my driver and
you can land untested (read broken) stuff if you want.  However, it won't
be turned on in i965 until that list is covered.  And, no, that doesn't
mean that I'm signing our team up to write the tests. :-)

> Do you have a suggestion for how we should implement testing between
> Vulkan and OpenGL? Can we leverage the crucible framework at all? Or should
> we just create some basic Vulkan framework in piglit from scratch? I only
> picked up this work yesterday so I haven't put too much thought into it.
>

i don't think we should try to "leverage" crucible directly.  However,
looking at crucible and liberally copying+pasting is probably a good idea.
Crucible does have some very nifty helpers in qonos.c which wrap various
Vulkan calls and make them a bit saner.  Also, it wouldn't hurt to grab the
image download code and the device enumeration code.  However, most of the
crucible framework is more for the multi-threaded test runner than for
actually making Vulkan easier.

--Jason

>
>> --Jason
>>
>> Thanks,
>>> Samuel.
>>>
>>> On 07/26/2017 01:46 PM, Timothy Arceri wrote:
>>>
>>>> Hi all,
>>>>
>>>> Andres is not around at the moment so as well are reviewing the
>>>> remaining patches I've rebased and added all Marek's suggestions.
>>>>
>>>> I've also made a few minor changes (see commit messages) and
>>>> reworked some of the patches to reduce code churn.
>>>>
>>>> I thought I'd send it out one last time to see if there was any
>>>> more feedback otherwise I'll probably push later in the week.
>>>>
>>>> Thanks,
>>>> Tim
>>>>
>>>> Series available at:
>>>>    https://github.com/tarceri/Mesa.git (memobj2)
>>>>
>>>> _______________________________________________
>>>> mesa-dev mailing list
>>>> mesa-dev at lists.freedesktop.org
>>>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>>>
>>>> _______________________________________________
>>> mesa-dev mailing list
>>> mesa-dev at lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>>
>>
>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170726/f9005105/attachment.html>