On Tue, Oct 20, 2015 at 02:52:29PM -0700, Matt Turner wrote: > On Tue, Oct 20, 2015 at 2:29 PM, Ben Widawsky > <benjamin.widaw...@intel.com> wrote: > > Gen9 adds the ability to write out a stencil value, so we need to expand the > > virtual payload by one. Abstracting this now makes that change easier to > > read. > > > > I was admittedly confused early on about some of the hardcoding. If people > > believe the resulting code is inferior, I am not super attached to the > > patch. > > > > Cc: Francisco Jerez <curroje...@riseup.net> > > Signed-off-by: Ben Widawsky <b...@bwidawsk.net> > > --- > > src/mesa/drivers/dri/i965/brw_defines.h | 18 ++++++++++-------- > > src/mesa/drivers/dri/i965/brw_fs.cpp | 21 +++++++++++---------- > > 2 files changed, 21 insertions(+), 18 deletions(-) > > > > diff --git a/src/mesa/drivers/dri/i965/brw_defines.h > > b/src/mesa/drivers/dri/i965/brw_defines.h > > index 7a5ee1b..e06c9d6 100644 > > --- a/src/mesa/drivers/dri/i965/brw_defines.h > > +++ b/src/mesa/drivers/dri/i965/brw_defines.h > > @@ -912,14 +912,6 @@ enum opcode { > > /** > > * Same as FS_OPCODE_FB_WRITE but expects its arguments separately as > > * individual sources instead of as a single payload blob: > > - * > > - * Source 0: [required] Color 0. > > - * Source 1: [optional] Color 1 (for dual source blend messages). > > - * Source 2: [optional] Src0 Alpha. > > - * Source 3: [optional] Source Depth (gl_FragDepth) > > - * Source 4: [optional (gen4-5)] Destination Depth passthrough from > > thread > > - * Source 5: [optional] Sample Mask (gl_SampleMask). > > - * Source 6: [required] Number of color components (as a UD immediate). > > */ > > FS_OPCODE_FB_WRITE_LOGICAL, > > > > @@ -1318,6 +1310,16 @@ enum brw_urb_write_flags { > > BRW_URB_WRITE_ALLOCATE | BRW_URB_WRITE_COMPLETE, > > }; > > > > +enum fb_write_logical_args { > > + FB_WRITE_COLOR0 = 0, /* REQUIRED */ > > + FB_WRITE_COLOR1 = 1, /* for dual source blend messages */ > > + FB_WRITE_SRC0_ALPHA = 2, > > + FB_WRITE_SRC_DEPTH = 3, /* gl_FragDepth */ > > + FB_WRITE_DST_DEPTH = 4, /* GEN4-5: passthrough from thread */ > > + FB_WRITE_OMASK = 5, /* Sample Mask (gl_SampleMask) */ > > + FB_WRITE_COMPONENTS = 6, /* REQUIRED */ > > Do we gain anything by assigning values explicitly?
Just code readability. As a noob coming into the code, seeing a random "6" or "4" in places was strange and it took a bit to figure out where to get the sensible value from. Is there any specific opposition toward doing this, or some reason it wasn't done in the first place? I honestly don't care too much... _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev