On Tue, Aug 23, 2016 at 12:18 AM, Francisco Jerez <curroje...@riseup.net> wrote: > Ilia Mirkin <imir...@alum.mit.edu> writes: > >> On Tue, Aug 23, 2016 at 12:05 AM, Francisco Jerez <curroje...@riseup.net> >> wrote: >>> Ilia Mirkin <imir...@alum.mit.edu> writes: >>> >>>> On Mon, Aug 22, 2016 at 10:55 PM, Francisco Jerez <curroje...@riseup.net> >>>> wrote: >>>>> Ilia Mirkin <imir...@alum.mit.edu> writes: >>>>> >>>>>> On Mon, Aug 22, 2016 at 9:59 PM, Francisco Jerez <curroje...@riseup.net> >>>>>> wrote: >>>>>>> gl_SecondaryFragColorEXT should have the same location as gl_FragColor >>>>>>> for the secondary fragment color to be replicated to all fragment >>>>>>> outputs. The incorrect location of gl_SecondaryFragColorEXT would >>>>>>> cause the linker to mark both FRAG_RESULT_COLOR and FRAG_RESULT_DATA0 >>>>>>> as being written to, which isn't allowed by the spec and would >>>>>>> ultimately lead to an assertion failure in >>>>>>> fs_visitor::emit_fb_writes() on my i965-fb-fetch branch. >>>>>> >>>>>> My recollection was that it didn't work with COLOR for "stupid" >>>>>> reasons. Can you confirm that >>>>>> bin/arb_blend_func_extended-fbo-extended-blend-pattern_gles2 -auto >>>>>> passes with this patch? >>>>>> >>>>> Yes, it does, in fact >>>>> arb_blend_func_extended-fbo-extended-blend-pattern_gles2 hits the i965 >>>>> assertion failure I mentioned above unless this patch is applied. >>>> >>>> This causes the test in question to fail on nouveau... the TGSI shader >>>> generated starts with >>>> >>>> FRAG >>>> PROPERTY FS_COLOR0_WRITES_ALL_CBUFS 1 >>>> DCL IN[0], POSITION, LINEAR >>>> DCL OUT[0], SAMPLEMASK >>> >>> Heh, this smells a lot like the bug fixed in PATCH 1, but somewhere in >>> the mesa state tracker. st_glsl_to_tgsi.cpp:2422 does: >>> >>> | entry = new(mem_ctx) variable_storage(var, >>> | PROGRAM_OUTPUT, >>> | var->data.location >>> | + var->data.index); >>> >>> which is obviously bogus, e.g. for var->data.location == >>> FRAG_RESULT_COLOR and var->data.index == 1 you get >>> FRAG_RESULT_SAMPLE_MASK which explains the sample mask declaration >>> above. >> >> Right, because having FRAG_RESULT_COLOR and index != 0 was never >> possible prior to this. That might be why Ryan stuck it into >> FRAG_RESULT_DATA0 [I may have been the one to suggest that]. > > Heh, so I guess that's the "stupid" reason you were referring to, > working around this mesa state tracker bug in the GLSL front-end.
Right. Or another way of looking at it, FRAG_RESULT_COLOR + index != 0 is illegal (as it would, among other things, imply multi-rt support for dual-source blending), and the former code was making the GLSL fe not emit the illegal combination. Whichever way you look at it, breaking st/mesa isn't a great option. -ilia _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev