Alyssa Rosenzweig <aly...@rosenzweig.io> writes:
>> Logic ops seem... challenging to emulate in the shader. That shader
>> would need the destination colors in the framebuffer storage format, and
>> I'm not sure that's always possible (maybe?).
>
> Alright, that's good to know.
>
> I will note that in Midgard, the native hardware ops are to load/store
> the actual framebuffer storage format. As seen later in the series, I
> lower load/store_output to a series of ops converting to/from float and
> the actual format. It's an open-question whether we want actual
> nir_intrinsics for this so the lowering happens in common NIR code and
> then we get fun logic ops and such.
>
>> It might also be fun to add support for GL_AMD_blend_minmax_factor and
>> GL_SGIX_blend_alpha_minmax. Looking at this lowering pass, it seems
>> like most of the work would be adding tests.
>
> Probably!
>
>> Having all of the lowering related to blending in one place seems like a
>> good idea.
>
> Probably, yes. Also since GLSL IR is, well.... ;)
>
>> > +/* These structs encapsulates the blend state such that it can be lowered
>> encapsulate
>> > + * cleanly */
>>
>> */ on its own line. There's at least one more instance of this below.
>
> Is there, uh, a thing I can stick in my vimrc to get this right?
>
>> Alas, case should be at the same indentation level as switch.
>
> Good to know, thank you.
>
>> Since factor is an enum, I think it's better to not have the default
>> case. If there's no default case, the compiler will issue a warning if
>> a new enum is added but not handled.
>>
>> Either way, the break is definitely not necessary.
>
> +1
>
>> { goes on it's own line.
>
> --Wait, that's a rule I actually follow, how'd I mess it up here? :P
>
>> Why? Without a vectorizer (or even with a vectorizer), it seems like
>> this will generate much worse code. I guess it's only a few
>> instructions once per shader, so it may not matter... but it's a little
>> surprising especially after going to extra effort to get the blend color
>> as a vector.
>
> Honestly? Since that's how vc4 (scalar) did it and for v1 of this series I was
> more eager to get something passing tests than particularly good. In my
> original implementation (before joining upstream), I did it like this
> and then used nir_opt_vectorize to get it back to something reasonable.
> That's one strategy still. The other option is to try to generate vector
> out of the gate, but that's hard to get right when RGB/alpha can be
> separate (but don't have to be), etc. The logic needed would approach
> just, well, having ALU vectorization... might as well merge the
> vectorize pass at that point...
>
> Suggestions welcome :)It does seem like we'll want vector some day for panfrost, but make tests work first and fix perf later (knowing that it's fixable) is fine with me.
signature.asc
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
