Tobias Klausmann
2015-May-24 19:56 UTC
[Nouveau] [RFC PATCH 00/11] Implement ARB_cull_distance
On 24.05.2015 21:36, Ilia Mirkin wrote:> On Sun, May 24, 2015 at 3:30 PM, Tobias Klausmann > <tobias.johannes.klausmann at mni.thm.de> wrote: >> >> On 24.05.2015 20:25, Ilia Mirkin wrote: >>> I'm having a bit of trouble tracing through this. What happens if I >>> have a shader that just does: >>> >>> gl_ClipDistance[0] = 1; >>> gl_CullDistance[0] = 1; >>> >>> what does the resulting TGSI look like? (Assuming that clip plane 0 is >>> enabled.) What about the generated nvc0 code (for the vertex shader)? >> >> (hack up a patch for this, run it without DRI_PRIME=1, see i pass and forget >> to check it again) >> yeah those are equal, sorry for wasting your time on this :/ > Not a waste at all... let's ignore any shortcomings of your patches > for a second, and think it through -- what do you want the TGSI to > look like? I'm not even sure. > > Do you want to have a separate 2x CLIPDIST and 2x CULLDIST and let the > driver worry about figuring out the max clip dist used and sticking > the cull dists above it? Or do you want to work it out at a lower > level where they share a single CLIPORCULLDIST semantic and get a > separate e.g. shader property that gives them the mask? > > I don't know how other hardware works, but nv50/nvc0 hw has 8 > clip_or_cull distances, and a mask that selects whether each is a clip > or a cull distance. But perhaps other hw has them totally separate, > dunno.With my limited experience about other hardware i'd go with seperate clip/cull and let the drivers figure out the right way to place them. That gives us the freedom to have it the way nv50/nvc0 works and other ways, like seperated clip/cull_distances if needed. Maybe we should just consider nouveau and radeon and decide by the hw of these often used drivers. Marek any comments on how the various radeons work?> >> >>> On Sun, May 24, 2015 at 1:57 PM, Tobias Klausmann >>> <tobias.johannes.klausmann at mni.thm.de> wrote: >>>> This patch series adds the needed support for this extension to the >>>> various >>>> parts of mesa to finally enable it for nvc0. >>>> >>>> Dave Airlie (1): >>>> glsl: lower cull_distance into cull_distance_mesa >>>> >>>> Tobias Klausmann (10): >>>> glapi: add GL_ARB_cull_distance >>>> mesa/main: add support for GL_ARB_cull_distance >>>> mesa/prog: Add varyings for arb_cull_distance >>>> mesa/st: add support for GL_ARB_cull_distance >>>> glsl: Add a helper to see if an array was unsize in the shader >>>> glsl: Add arb_cull_distance support >>>> i965: rename UsesClipDistanceOut to UsesClipCullDistanceOut >>>> gallium: add support for arb_cull_distance >>>> nouveau/codegen: sort in galliums cull_distance semantic into the >>>> drivers bitmask >>>> nouveau/nvc0: implement cull_distance as a special form of clip >>>> distance >>>> >>>> docs/GL3.txt | 2 +- >>>> docs/relnotes/10.7.0.html | 4 +- >>>> src/gallium/auxiliary/cso_cache/cso_context.c | 3 + >>>> src/gallium/drivers/freedreno/freedreno_screen.c | 1 + >>>> src/gallium/drivers/i915/i915_screen.c | 1 + >>>> src/gallium/drivers/ilo/ilo_screen.c | 1 + >>>> src/gallium/drivers/llvmpipe/lp_screen.c | 2 + >>>> .../drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp | 5 + >>>> src/gallium/drivers/nouveau/nv30/nv30_screen.c | 1 + >>>> src/gallium/drivers/nouveau/nv50/nv50_screen.c | 1 + >>>> src/gallium/drivers/nouveau/nvc0/nvc0_program.c | 6 +- >>>> src/gallium/drivers/nouveau/nvc0/nvc0_program.h | 1 + >>>> src/gallium/drivers/nouveau/nvc0/nvc0_screen.c | 1 + >>>> .../drivers/nouveau/nvc0/nvc0_state_validate.c | 1 + >>>> src/gallium/drivers/r300/r300_screen.c | 1 + >>>> src/gallium/drivers/r600/r600_pipe.c | 1 + >>>> src/gallium/drivers/radeonsi/si_pipe.c | 1 + >>>> src/gallium/drivers/softpipe/sp_screen.c | 2 + >>>> src/gallium/drivers/svga/svga_screen.c | 1 + >>>> src/gallium/drivers/vc4/vc4_screen.c | 1 + >>>> src/gallium/include/pipe/p_defines.h | 1 + >>>> src/glsl/Makefile.sources | 1 + >>>> src/glsl/ast_to_hir.cpp | 14 + >>>> src/glsl/builtin_variables.cpp | 13 +- >>>> src/glsl/glcpp/glcpp-parse.y | 3 + >>>> src/glsl/glsl_parser_extras.cpp | 1 + >>>> src/glsl/glsl_parser_extras.h | 3 + >>>> src/glsl/glsl_types.cpp | 8 +- >>>> src/glsl/glsl_types.h | 10 +- >>>> src/glsl/ir_optimization.h | 1 + >>>> src/glsl/link_varyings.cpp | 17 +- >>>> src/glsl/link_varyings.h | 3 +- >>>> src/glsl/linker.cpp | 124 +++-- >>>> src/glsl/lower_cull_distance.cpp | 549 >>>> +++++++++++++++++++++ >>>> src/glsl/standalone_scaffolding.cpp | 1 + >>>> src/glsl/tests/varyings_test.cpp | 27 + >>>> src/mapi/glapi/gen/gl_API.xml | 7 +- >>>> src/mesa/drivers/dri/i965/brw_fs_visitor.cpp | 2 +- >>>> src/mesa/drivers/dri/i965/brw_gs.c | 2 +- >>>> src/mesa/drivers/dri/i965/brw_vec4.cpp | 2 +- >>>> src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp | 2 +- >>>> src/mesa/drivers/dri/i965/brw_vs.c | 2 +- >>>> src/mesa/main/extensions.c | 1 + >>>> src/mesa/main/get.c | 26 + >>>> src/mesa/main/get_hash_params.py | 4 + >>>> src/mesa/main/mtypes.h | 22 +- >>>> src/mesa/main/shaderapi.c | 4 +- >>>> src/mesa/main/tests/enum_strings.cpp | 2 + >>>> src/mesa/program/prog_print.c | 4 + >>>> src/mesa/state_tracker/st_extensions.c | 4 + >>>> src/mesa/state_tracker/st_program.c | 34 ++ >>>> 51 files changed, 859 insertions(+), 72 deletions(-) >>>> create mode 100644 src/glsl/lower_cull_distance.cpp >>>> >>>> -- >>>> 2.4.1 >>>> >>>> _______________________________________________ >>>> Nouveau mailing list >>>> Nouveau at lists.freedesktop.org >>>> http://lists.freedesktop.org/mailman/listinfo/nouveau >>
It's the same on Radeon. There are 2x ClipOrCullDistance output vectors and a mask saying it should clip or cull or do nothing. Marek On Sun, May 24, 2015 at 9:56 PM, Tobias Klausmann <tobias.johannes.klausmann at mni.thm.de> wrote:> > > On 24.05.2015 21:36, Ilia Mirkin wrote: >> >> On Sun, May 24, 2015 at 3:30 PM, Tobias Klausmann >> <tobias.johannes.klausmann at mni.thm.de> wrote: >>> >>> >>> On 24.05.2015 20:25, Ilia Mirkin wrote: >>>> >>>> I'm having a bit of trouble tracing through this. What happens if I >>>> have a shader that just does: >>>> >>>> gl_ClipDistance[0] = 1; >>>> gl_CullDistance[0] = 1; >>>> >>>> what does the resulting TGSI look like? (Assuming that clip plane 0 is >>>> enabled.) What about the generated nvc0 code (for the vertex shader)? >>> >>> >>> (hack up a patch for this, run it without DRI_PRIME=1, see i pass and >>> forget >>> to check it again) >>> yeah those are equal, sorry for wasting your time on this :/ >> >> Not a waste at all... let's ignore any shortcomings of your patches >> for a second, and think it through -- what do you want the TGSI to >> look like? I'm not even sure. >> >> Do you want to have a separate 2x CLIPDIST and 2x CULLDIST and let the >> driver worry about figuring out the max clip dist used and sticking >> the cull dists above it? Or do you want to work it out at a lower >> level where they share a single CLIPORCULLDIST semantic and get a >> separate e.g. shader property that gives them the mask? >> >> I don't know how other hardware works, but nv50/nvc0 hw has 8 >> clip_or_cull distances, and a mask that selects whether each is a clip >> or a cull distance. But perhaps other hw has them totally separate, >> dunno. > > > With my limited experience about other hardware i'd go with seperate > clip/cull and let the drivers figure out the right way to place them. That > gives us the freedom to have it the way nv50/nvc0 works and other ways, like > seperated clip/cull_distances if needed. Maybe we should just consider > nouveau and radeon and decide by the hw of these often used drivers. > > Marek any comments on how the various radeons work? > > >> >>> >>>> On Sun, May 24, 2015 at 1:57 PM, Tobias Klausmann >>>> <tobias.johannes.klausmann at mni.thm.de> wrote: >>>>> >>>>> This patch series adds the needed support for this extension to the >>>>> various >>>>> parts of mesa to finally enable it for nvc0. >>>>> >>>>> Dave Airlie (1): >>>>> glsl: lower cull_distance into cull_distance_mesa >>>>> >>>>> Tobias Klausmann (10): >>>>> glapi: add GL_ARB_cull_distance >>>>> mesa/main: add support for GL_ARB_cull_distance >>>>> mesa/prog: Add varyings for arb_cull_distance >>>>> mesa/st: add support for GL_ARB_cull_distance >>>>> glsl: Add a helper to see if an array was unsize in the shader >>>>> glsl: Add arb_cull_distance support >>>>> i965: rename UsesClipDistanceOut to UsesClipCullDistanceOut >>>>> gallium: add support for arb_cull_distance >>>>> nouveau/codegen: sort in galliums cull_distance semantic into the >>>>> drivers bitmask >>>>> nouveau/nvc0: implement cull_distance as a special form of clip >>>>> distance >>>>> >>>>> docs/GL3.txt | 2 +- >>>>> docs/relnotes/10.7.0.html | 4 +- >>>>> src/gallium/auxiliary/cso_cache/cso_context.c | 3 + >>>>> src/gallium/drivers/freedreno/freedreno_screen.c | 1 + >>>>> src/gallium/drivers/i915/i915_screen.c | 1 + >>>>> src/gallium/drivers/ilo/ilo_screen.c | 1 + >>>>> src/gallium/drivers/llvmpipe/lp_screen.c | 2 + >>>>> .../drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp | 5 + >>>>> src/gallium/drivers/nouveau/nv30/nv30_screen.c | 1 + >>>>> src/gallium/drivers/nouveau/nv50/nv50_screen.c | 1 + >>>>> src/gallium/drivers/nouveau/nvc0/nvc0_program.c | 6 +- >>>>> src/gallium/drivers/nouveau/nvc0/nvc0_program.h | 1 + >>>>> src/gallium/drivers/nouveau/nvc0/nvc0_screen.c | 1 + >>>>> .../drivers/nouveau/nvc0/nvc0_state_validate.c | 1 + >>>>> src/gallium/drivers/r300/r300_screen.c | 1 + >>>>> src/gallium/drivers/r600/r600_pipe.c | 1 + >>>>> src/gallium/drivers/radeonsi/si_pipe.c | 1 + >>>>> src/gallium/drivers/softpipe/sp_screen.c | 2 + >>>>> src/gallium/drivers/svga/svga_screen.c | 1 + >>>>> src/gallium/drivers/vc4/vc4_screen.c | 1 + >>>>> src/gallium/include/pipe/p_defines.h | 1 + >>>>> src/glsl/Makefile.sources | 1 + >>>>> src/glsl/ast_to_hir.cpp | 14 + >>>>> src/glsl/builtin_variables.cpp | 13 +- >>>>> src/glsl/glcpp/glcpp-parse.y | 3 + >>>>> src/glsl/glsl_parser_extras.cpp | 1 + >>>>> src/glsl/glsl_parser_extras.h | 3 + >>>>> src/glsl/glsl_types.cpp | 8 +- >>>>> src/glsl/glsl_types.h | 10 +- >>>>> src/glsl/ir_optimization.h | 1 + >>>>> src/glsl/link_varyings.cpp | 17 +- >>>>> src/glsl/link_varyings.h | 3 +- >>>>> src/glsl/linker.cpp | 124 +++-- >>>>> src/glsl/lower_cull_distance.cpp | 549 >>>>> +++++++++++++++++++++ >>>>> src/glsl/standalone_scaffolding.cpp | 1 + >>>>> src/glsl/tests/varyings_test.cpp | 27 + >>>>> src/mapi/glapi/gen/gl_API.xml | 7 +- >>>>> src/mesa/drivers/dri/i965/brw_fs_visitor.cpp | 2 +- >>>>> src/mesa/drivers/dri/i965/brw_gs.c | 2 +- >>>>> src/mesa/drivers/dri/i965/brw_vec4.cpp | 2 +- >>>>> src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp | 2 +- >>>>> src/mesa/drivers/dri/i965/brw_vs.c | 2 +- >>>>> src/mesa/main/extensions.c | 1 + >>>>> src/mesa/main/get.c | 26 + >>>>> src/mesa/main/get_hash_params.py | 4 + >>>>> src/mesa/main/mtypes.h | 22 +- >>>>> src/mesa/main/shaderapi.c | 4 +- >>>>> src/mesa/main/tests/enum_strings.cpp | 2 + >>>>> src/mesa/program/prog_print.c | 4 + >>>>> src/mesa/state_tracker/st_extensions.c | 4 + >>>>> src/mesa/state_tracker/st_program.c | 34 ++ >>>>> 51 files changed, 859 insertions(+), 72 deletions(-) >>>>> create mode 100644 src/glsl/lower_cull_distance.cpp >>>>> >>>>> -- >>>>> 2.4.1 >>>>> >>>>> _______________________________________________ >>>>> Nouveau mailing list >>>>> Nouveau at lists.freedesktop.org >>>>> http://lists.freedesktop.org/mailman/listinfo/nouveau >>> >>> >
On 25 May 2015 at 08:11, Marek Olšák <maraeo at gmail.com> wrote:> It's the same on Radeon. There are 2x ClipOrCullDistance output > vectors and a mask saying it should clip or cull or do nothing. > > Marek >My thinking was gallium should have a single semantic and a mask in the shader definition maybe. though it doesn't solve the does nvidia do the right thing with cull[0] and clip[0], and what is the right thing. Dave.