On Friday, December 29, 2017 2:48:13 PM PST Eric Anholt wrote: > Kenneth Graunke <[email protected]> writes: > > > [ Unknown signature status ] > > On Thursday, December 28, 2017 5:56:18 PM PST Eric Anholt wrote: > >> For VC5, the shader needs to have the appropriate base type for the > >> variable in the render target write, and gallium's > >> FS_COLOR0_WRITES_ALL_CBUFS (used for glClearBufferiv) doesn't give you > >> that information. This pass lets the backend decide what types to explode > >> the gl_FragColor write out to. > >> > >> This would also be a prerequisite of moving some of VC5's render target > >> format packing into NIR as well. > > > > This type confusion seems a bit unfortunate. Both gl_FragColor and > > gl_FragData[i] are always float vec4s. But, we've used (abused?) it in > > the past for integer clear/blit paths, since it's the only way to splat > > out to all render targets. > > > > I suppose this must be an internally generated shader? Or, do you also > > need this for people writing to gl_FragColor but binding integer render > > targets? (If so, what about people declaring 'out vec4 foo' but binding > > an integer render target?) > > Yeah, it's gallium's internal glClearBufferiv() shader, translated using > TGSI-to-NIR. I had initially written this pass without the types > argument, but found that adding it got me a whole bunch more tests > working. I suppose we could move this into TTN and delay TTN until draw > time, to make sure that we always maintain proper types in the NIR, but > this was pretty easy. How about adding this to the top comment: > > * The pass's types argument lets the caller choose between vec4, uvec4, and > * ivec4 per render target. This is necessary for TGSI-to-NIR output, which > * emits vec4 gl_FragColor writes even for integer render targets (because > * TGSI doesn't know the types). > > If you use "out vec4 foo" but bind integer, you get "If the values > written by the fragment shader do not match the format(s) of the > corresponding color buffer(s), the result is undefined." (GL 4.3 spec)
Ah cool, I had forgotten that bit of the spec :) [snip] > How about: > > /* We only emit 4-component stores above. If you need to lower to fewer > * components based on render target format, you probably need to do > * that in a separate pass anyway to cover the non-gl_FragColor case. > */ > assert(glsl_get_components(types[i]) == 4); Both of these comments sound good to me - I wouldn't bother with anything more complicated. NIR shouldn't really care about types either...it's effectively just going to be bitcast. You've got my: Reviewed-by: Kenneth Graunke <[email protected]>
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ mesa-dev mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/mesa-dev
