[Mesa-dev] [PATCH] mesa: add GL_UNSIGNED_INT_24_8 to _mesa_pack_depth_span

[Kenneth Graunke]

On Friday, October 02, 2015 05:04:21 PM Ilia Mirkin wrote:
> On Fri, Oct 2, 2015 at 4:47 PM, Ian Romanick <idr at freedesktop.org> wrote:
> > On 10/01/2015 12:15 PM, Ilia Mirkin wrote:
> >> On Thu, Oct 1, 2015 at 3:12 PM, Ian Romanick <idr at freedesktop.org> wrote:
> >>> I'm just
> >>> wondering because Mesa doesn't support that extension.  How is this even
> >>> being hit?
> >>
> >> See 81d2fd91a90 (mesa: add NV_read_{depth,stencil,depth_stencil} extensions)
> >
> > Okay, that's weird.  I must have had some old branch checked out at the
> > time.  I did 'grep -r NV_read_depth src/', and it came back empty.  Now
> > I just get to be irritated that we enabled THREE extensions for which we
> > have ZERO tests... and least one is clearly completely broken. :(
> >
> > I guess now I at least have something concrete to point to then next
> > time I object to enabling an ES extension that "just" allows some
> > desktop functionality. ;)
> 
> I believe Rob tested at least some of it with qapitrace[1], as
> otherwise there was no way to get access to the data in a
> renderbuffer, which can be quite useful for debugging. Not all of us
> have your level of familiarity with how GL works, but how will we
> learn without making some mistakes? :)
> 
> No matter how many tests we might have, they'll always leave
> *something* out. The fact that there are no tests at all for this ext
> isn't great, of course. But there are also no functional tests for
> {ARB,AMD}_conservative_depth and probably a number of others.
> 
>   -ilia

I think we can all agree that enabling a feature with 0 tests is bad
practice.  Just because people have done it before doesn't make it wise.
Or in a positive spin: our commitment to Piglit is one of the reasons
we have such high quality drivers.  It's worth it, even if it can be a
bunch of unglamorous and annoying work.

I'm OK with not adding comprehensive tests for ES extensions that port
over GL functionality, but at least touch testing the feature is
valuable.

Also...GL_ARB_conservative_depth is a bit different.  It provides a hint
which the driver can use to speed up some operations - but there's no
new feature, or observable behavior.  Drivers can completely ignore it
and still have a valid, correct implementation of the extension.  So
it's a bit tricky to test (but we could at least look for breakage...)

In contrast, GL_NV_read_depth_stencil turns a GL call that used to be an
error into a functioning read of depth data, which seems straightforward
to test.

--Ken
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20151002/a34e8312/attachment.sig>