On Wed, Jan 11, 2017 at 5:32 PM, Marek Olšák <mar...@gmail.com> wrote: > On Wed, Jan 11, 2017 at 4:33 PM, Erik Faye-Lund <kusmab...@gmail.com> wrote: >> On Wed, Jan 11, 2017 at 4:14 PM, Nicolai Hähnle <nhaeh...@gmail.com> wrote: >>> On 11.01.2017 13:17, Marek Olšák wrote: >>>> >>>> On Tue, Jan 10, 2017 at 6:48 PM, Jason Ekstrand <ja...@jlekstrand.net> >>>> wrote: >>>>> >>>>> I'll be honest, I'm not a fan... Given that D3D10 has one defined >>>>> behavior, >>>>> D3D9 has another, and GL doesn't specify, I don't really think we should >>>>> be >>>>> making a global change to all drivers to do the D3D9 behavior just to fix >>>>> one app. Sure, other apps probably have the same bug, but are we going >>>>> to >>>>> have apps that expect the D3D10 behavior that we've now explicitly made >>>>> not >>>>> work? >>>>> >>>>> If we're going to hack around an app bug, I would really rather see it >>>>> behind a driconf option rather than a global change to driver behavior. >>>>> Even better, it'd be cool if we could see the app get fixed. (Yes, I know >>>>> that's not likely). >>>> >>>> >>>> I think we are not in a position to refuse this workaround, or put >>>> more precisely, to have a different behavior from everybody else. By >>>> "we", I mean i965, radeonsi, svga. All closed drivers use abs. Many >>>> Mesa drivers also use abs internally (r300, r600, nv30, nv50/nvc0). >>>> This is not really a workaround for a specific application, even >>>> though it's strongly motivated by that. It's a fix to align the few >>>> remaining drivers with all others. >>>> >>>> We talked with the publisher about this a very long time ago. While I >>>> don't remember the details (Nicolai?), I think they refused to fix it >>>> because radeonsi appeared to be the only driver not doing abs. >>> >>> >>> If I remember correctly, it wasn't so much a refusal as a lack of >>> follow-through. They even had an option in their framework to add the >>> abs(...) when translating shaders, but somehow didn't turn it on >>> unconditionally for some reason... >> >> VP even says so here: >> https://github.com/virtual-programming/specops-linux/issues/20 >> >> They recommend against patching mesa to do abs, though. > > We should still patch Mesa to align the behavior with closed drivers > and gallium drivers like r600g and nouveau. In other words, it's too > late to tell us not to patch Mesa, because r600g and nouveau have been > "patched" since the beginning. > > We only need to decide whether we should do it in the GLSL compiler or > radeonsi, i.e. whether we should exclude i965 and svga.
Since it's undefined in GLSL, there's known apps that depend on it, and this patch only changes the GLSL compiler, I personally think this approach is totally sane as well. Patching at a lower level makes it difficult (but not necessarily impossible) to implement different semantics if such should arise somehow (GL_ARB_gl_spirv + SPIRV extensions perhaps?). _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
