[Mesa-dev] [PATCH 08/12] nir: Add a simple nir_lower_wpos_center() pass for Vulkan drivers.
nir_lower_wpos_ytransform() is great for OpenGL, which allows applications to choose whether their coordinate system's origin is upper left/lower left, and whether the pixel center should be on integer/half-integer boundaries. Vulkan, however, has much simpler requirements: the pixel center is always half-integer, and the origin is always upper left. No coordinate transform is needed - we just need to add <0.5, 0.5>. This means that we can avoid using (and setting up) a uniform. I thought about adding more options to nir_lower_wpos_ytransform(), but making a new pass that never even touched uniforms seemed simpler. Signed-off-by: Kenneth Graunke --- src/compiler/Makefile.sources| 1 + src/compiler/nir/nir.h | 1 + src/compiler/nir/nir_lower_wpos_center.c | 107 +++ 3 files changed, 109 insertions(+) create mode 100644 src/compiler/nir/nir_lower_wpos_center.c diff --git a/src/compiler/Makefile.sources b/src/compiler/Makefile.sources index 97f9eb4..b8f2b49 100644 --- a/src/compiler/Makefile.sources +++ b/src/compiler/Makefile.sources @@ -212,6 +212,7 @@ NIR_FILES = \ nir/nir_lower_vars_to_ssa.c \ nir/nir_lower_var_copies.c \ nir/nir_lower_vec_to_movs.c \ + nir/nir_lower_wpos_center.c \ nir/nir_lower_wpos_ytransform.c \ nir/nir_metadata.c \ nir/nir_move_vec_src_uses_to_dest.c \ diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h index a21a7bd..78913d3 100644 --- a/src/compiler/nir/nir.h +++ b/src/compiler/nir/nir.h @@ -2407,6 +2407,7 @@ typedef struct nir_lower_wpos_ytransform_options { bool nir_lower_wpos_ytransform(nir_shader *shader, const nir_lower_wpos_ytransform_options *options); +bool nir_lower_wpos_center(nir_shader *shader); typedef struct nir_lower_drawpixels_options { int texcoord_state_tokens[5]; diff --git a/src/compiler/nir/nir_lower_wpos_center.c b/src/compiler/nir/nir_lower_wpos_center.c new file mode 100644 index 000..d66109d --- /dev/null +++ b/src/compiler/nir/nir_lower_wpos_center.c @@ -0,0 +1,107 @@ +/* + * Copyright © 2015 Red Hat + * Copyright © 2016 Intel Corporation + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice (including the next + * paragraph) shall be included in all copies or substantial portions of the + * Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS + * IN THE SOFTWARE. + */ + +#include "nir.h" +#include "nir_builder.h" +#include "program/prog_instruction.h" + +/** + * This pass adds <0.5, 0.5> to all uses of gl_FragCoord. + * + * Run before nir_lower_io(). + * + * For a more full featured pass, consider using nir_lower_wpos_ytransform(), + * which can handle pixel center integer / half integer, and origin lower + * left / upper left transformations. + * + * This simple pass is primarily intended for use by Vulkan drivers on + * hardware which provides an integer pixel center. Vulkan mandates that + * the pixel center must be half-integer, and also that the coordinate + * system's origin must be upper left. This means that there's no need + * for a uniform - we can always just add a constant. + */ + +static void +add_half_to_fragcoord(nir_builder *b, nir_intrinsic_instr *intr) +{ + nir_ssa_def *wpos = &intr->dest.ssa; + + assert(intr->dest.is_ssa); + + b->cursor = nir_after_instr(&intr->instr); + + wpos = nir_fadd(b, wpos, nir_imm_vec4(b, 0.5f, 0.5f, 0.0f, 0.0f)); + + nir_ssa_def_rewrite_uses_after(&intr->dest.ssa, nir_src_for_ssa(wpos), + &intr->instr); +} + +static bool +lower_wpos_center_block(nir_builder *b, nir_block *block) +{ + bool progress = false; + + nir_foreach_instr_safe(instr, block) { + if (instr->type == nir_instr_type_intrinsic) { + nir_intrinsic_instr *intr = nir_instr_as_intrinsic(instr); + if (intr->intrinsic == nir_intrinsic_load_var) { +nir_deref_var *dvar = intr->variables[0]; +nir_variable *var = dvar->var; + +if (var->data.mode == nir_var_shader_in && +var->d
Re: [Mesa-dev] [PATCH 08/12] nir: Add a simple nir_lower_wpos_center() pass for Vulkan drivers.
On Wed, May 18, 2016 at 3:00 PM, Kenneth Graunke wrote: > nir_lower_wpos_ytransform() is great for OpenGL, which allows > applications to choose whether their coordinate system's origin is > upper left/lower left, and whether the pixel center should be on > integer/half-integer boundaries. > > Vulkan, however, has much simpler requirements: the pixel center > is always half-integer, and the origin is always upper left. No > coordinate transform is needed - we just need to add <0.5, 0.5>. > This means that we can avoid using (and setting up) a uniform. > > I thought about adding more options to nir_lower_wpos_ytransform(), > but making a new pass that never even touched uniforms seemed simpler. > > Signed-off-by: Kenneth Graunke > --- > src/compiler/Makefile.sources| 1 + > src/compiler/nir/nir.h | 1 + > src/compiler/nir/nir_lower_wpos_center.c | 107 > +++ > 3 files changed, 109 insertions(+) > create mode 100644 src/compiler/nir/nir_lower_wpos_center.c > > diff --git a/src/compiler/Makefile.sources b/src/compiler/Makefile.sources > index 97f9eb4..b8f2b49 100644 > --- a/src/compiler/Makefile.sources > +++ b/src/compiler/Makefile.sources > @@ -212,6 +212,7 @@ NIR_FILES = \ > nir/nir_lower_vars_to_ssa.c \ > nir/nir_lower_var_copies.c \ > nir/nir_lower_vec_to_movs.c \ > + nir/nir_lower_wpos_center.c \ > nir/nir_lower_wpos_ytransform.c \ > nir/nir_metadata.c \ > nir/nir_move_vec_src_uses_to_dest.c \ > diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h > index a21a7bd..78913d3 100644 > --- a/src/compiler/nir/nir.h > +++ b/src/compiler/nir/nir.h > @@ -2407,6 +2407,7 @@ typedef struct nir_lower_wpos_ytransform_options { > > bool nir_lower_wpos_ytransform(nir_shader *shader, > const nir_lower_wpos_ytransform_options > *options); > +bool nir_lower_wpos_center(nir_shader *shader); > > typedef struct nir_lower_drawpixels_options { > int texcoord_state_tokens[5]; > diff --git a/src/compiler/nir/nir_lower_wpos_center.c > b/src/compiler/nir/nir_lower_wpos_center.c > new file mode 100644 > index 000..d66109d > --- /dev/null > +++ b/src/compiler/nir/nir_lower_wpos_center.c > @@ -0,0 +1,107 @@ > +/* > + * Copyright © 2015 Red Hat > + * Copyright © 2016 Intel Corporation > + * > + * Permission is hereby granted, free of charge, to any person obtaining a > + * copy of this software and associated documentation files (the "Software"), > + * to deal in the Software without restriction, including without limitation > + * the rights to use, copy, modify, merge, publish, distribute, sublicense, > + * and/or sell copies of the Software, and to permit persons to whom the > + * Software is furnished to do so, subject to the following conditions: > + * > + * The above copyright notice and this permission notice (including the next > + * paragraph) shall be included in all copies or substantial portions of the > + * Software. > + * > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER > DEALINGS > + * IN THE SOFTWARE. > + */ > + > +#include "nir.h" > +#include "nir_builder.h" > +#include "program/prog_instruction.h" > + > +/** > + * This pass adds <0.5, 0.5> to all uses of gl_FragCoord. > + * > + * Run before nir_lower_io(). > + * > + * For a more full featured pass, consider using nir_lower_wpos_ytransform(), > + * which can handle pixel center integer / half integer, and origin lower > + * left / upper left transformations. > + * > + * This simple pass is primarily intended for use by Vulkan drivers on > + * hardware which provides an integer pixel center. Vulkan mandates that > + * the pixel center must be half-integer, and also that the coordinate > + * system's origin must be upper left. This means that there's no need > + * for a uniform - we can always just add a constant. > + */ > + > +static void > +add_half_to_fragcoord(nir_builder *b, nir_intrinsic_instr *intr) > +{ > + nir_ssa_def *wpos = &intr->dest.ssa; > + > + assert(intr->dest.is_ssa); > + > + b->cursor = nir_after_instr(&intr->instr); > + > + wpos = nir_fadd(b, wpos, nir_imm_vec4(b, 0.5f, 0.5f, 0.0f, 0.0f)); > + > + nir_ssa_def_rewrite_uses_after(&intr->dest.ssa, nir_src_for_ssa(wpos), > + &intr->instr); > +} > + > +static bool > +lower_wpos_center_block(nir_builder *b, nir_block *block) > +{ > + bool progress = false; > + > + nir_foreach_instr_safe(instr, block) { Does this need to be _safe? If it does, why (for my own education)? __
Re: [Mesa-dev] [PATCH 08/12] nir: Add a simple nir_lower_wpos_center() pass for Vulkan drivers.
On Thursday, May 19, 2016 1:21:16 PM PDT Matt Turner wrote: > On Wed, May 18, 2016 at 3:00 PM, Kenneth Graunke wrote: > > nir_lower_wpos_ytransform() is great for OpenGL, which allows > > applications to choose whether their coordinate system's origin is > > upper left/lower left, and whether the pixel center should be on > > integer/half-integer boundaries. > > > > Vulkan, however, has much simpler requirements: the pixel center > > is always half-integer, and the origin is always upper left. No > > coordinate transform is needed - we just need to add <0.5, 0.5>. > > This means that we can avoid using (and setting up) a uniform. > > > > I thought about adding more options to nir_lower_wpos_ytransform(), > > but making a new pass that never even touched uniforms seemed simpler. > > > > Signed-off-by: Kenneth Graunke > > --- > > src/compiler/Makefile.sources| 1 + > > src/compiler/nir/nir.h | 1 + > > src/compiler/nir/nir_lower_wpos_center.c | 107 ++ + > > 3 files changed, 109 insertions(+) > > create mode 100644 src/compiler/nir/nir_lower_wpos_center.c > > > > diff --git a/src/compiler/Makefile.sources b/src/compiler/Makefile.sources > > index 97f9eb4..b8f2b49 100644 > > --- a/src/compiler/Makefile.sources > > +++ b/src/compiler/Makefile.sources > > @@ -212,6 +212,7 @@ NIR_FILES = \ > > nir/nir_lower_vars_to_ssa.c \ > > nir/nir_lower_var_copies.c \ > > nir/nir_lower_vec_to_movs.c \ > > + nir/nir_lower_wpos_center.c \ > > nir/nir_lower_wpos_ytransform.c \ > > nir/nir_metadata.c \ > > nir/nir_move_vec_src_uses_to_dest.c \ > > diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h > > index a21a7bd..78913d3 100644 > > --- a/src/compiler/nir/nir.h > > +++ b/src/compiler/nir/nir.h > > @@ -2407,6 +2407,7 @@ typedef struct nir_lower_wpos_ytransform_options { > > > > bool nir_lower_wpos_ytransform(nir_shader *shader, > > const nir_lower_wpos_ytransform_options *options); > > +bool nir_lower_wpos_center(nir_shader *shader); > > > > typedef struct nir_lower_drawpixels_options { > > int texcoord_state_tokens[5]; > > diff --git a/src/compiler/nir/nir_lower_wpos_center.c b/src/compiler/nir/ nir_lower_wpos_center.c > > new file mode 100644 > > index 000..d66109d > > --- /dev/null > > +++ b/src/compiler/nir/nir_lower_wpos_center.c > > @@ -0,0 +1,107 @@ > > +/* > > + * Copyright © 2015 Red Hat > > + * Copyright © 2016 Intel Corporation > > + * > > + * Permission is hereby granted, free of charge, to any person obtaining a > > + * copy of this software and associated documentation files (the "Software"), > > + * to deal in the Software without restriction, including without limitation > > + * the rights to use, copy, modify, merge, publish, distribute, sublicense, > > + * and/or sell copies of the Software, and to permit persons to whom the > > + * Software is furnished to do so, subject to the following conditions: > > + * > > + * The above copyright notice and this permission notice (including the next > > + * paragraph) shall be included in all copies or substantial portions of the > > + * Software. > > + * > > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR > > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, > > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL > > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER > > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING > > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS > > + * IN THE SOFTWARE. > > + */ > > + > > +#include "nir.h" > > +#include "nir_builder.h" > > +#include "program/prog_instruction.h" > > + > > +/** > > + * This pass adds <0.5, 0.5> to all uses of gl_FragCoord. > > + * > > + * Run before nir_lower_io(). > > + * > > + * For a more full featured pass, consider using nir_lower_wpos_ytransform(), > > + * which can handle pixel center integer / half integer, and origin lower > > + * left / upper left transformations. > > + * > > + * This simple pass is primarily intended for use by Vulkan drivers on > > + * hardware which provides an integer pixel center. Vulkan mandates that > > + * the pixel center must be half-integer, and also that the coordinate > > + * system's origin must be upper left. This means that there's no need > > + * for a uniform - we can always just add a constant. > > + */ > > + > > +static void > > +add_half_to_fragcoord(nir_builder *b, nir_intrinsic_instr *intr) > > +{ > > + nir_ssa_def *wpos = &intr->dest.ssa; > > + > > + assert(intr->dest.is_ssa); > > + > > + b->cursor = nir_after_instr(&intr->instr); > > + > > + wpos = nir_fadd(b, wpos, nir_imm_vec4(b, 0.5f, 0.5f, 0.0f, 0.0f)); > > + > > + nir_ssa_def_rewrite_uses_after(&intr->dest.ssa, nir_src_for_ssa(wpos), > > +