On Wednesday, May 10, 2017 10:19:46 AM PDT Emil Velikov wrote: > On 10 May 2017 at 16:38, Jason Ekstrand <[email protected]> wrote: > > On Wed, May 10, 2017 at 8:26 AM, Emil Velikov <[email protected]> > > wrote: > >> > >> Hi Jason, > >> > >> Humble unrelated question. > >> > >> On 9 May 2017 at 18:00, Jason Ekstrand <[email protected]> wrote: > >> > >> > + if (isl_surf_usage_is_depth(info->usage)) { > >> > + if (info->format == ISL_FORMAT_R16_UNORM) { > >> > + return isl_extent3d(8, 4, 1); > >> > + } else { > >> > + return isl_extent3d(4, 4, 1); > >> > + } > >> > + } else if (isl_surf_usage_is_stencil(info->usage)) { > >> > + return isl_extent3d(8, 8, 1); > >> > + } else if (isl_format_is_compressed(info->format)) { > >> > + /* Compressed formats all have alignment equal to block size. */ > >> > + return isl_extent3d(1, 1, 1); > >> > + } > >> > >> I've seen a handful of constructs like the above.. Is there any reason > >> to keep the extra else/curly brackets? > >> Something like the following reads a bit easier yet admittedly I'm not > >> the person to set the coding style in isl. > > > > > > Does it? > > > Indeed it does, I would not have bothered otherwise ;-) > It's a subjective topic and my contributions here are, ahem, limited, > so feel do ignore me. > > Thanks > Emil
I see Jason's point here - the depth/stencil/compressed blocks are
kind of like cases in a switch. It's one of the three, and we're
choosing between them. I think the code reads fine as is here.
That said, I almost always prefer Emil's style, FWIW.
if (blah)
return foo;
return bar;
makes it obvious that something will be returned, as there's an
clear unconditional return at the top-level. I especially like this
style when "foo" is for a special case, and "bar" is the general case.
if (blah)
return foo;
else
return bar;
is a little less clear IMO - you have to recognize that the control flow
in the inner block will happen, because one of the two blocks will
always be hit.
But it's largely a matter of taste, so...*shrug*.
If I were writing this, I would probably handle the R16 case as:
if (info->format == ISL_FORMAT_R16_UNORM) {
return isl_extent3d(8, 4, 1);
return isl_extent3d(4, 4, 1);
but leave the rest as is. Totally up to you though, Jason.
--Ken
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ mesa-dev mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/mesa-dev
