Re: [Mesa-dev] [PATCH 1/2] st/egl: Add support for EGL_NOK_swap_region

2011-12-13 Thread Chia-I Wu
On Fri, Dec 9, 2011 at 11:36 PM, Fredrik Höglund  wrote:
> Backends indicate that they support this extension by returning
> EGL_TRUE when native_display::get_param() is called with
> NATIVE_PARAM_PRESENT_REGION.
>
> native_present_control is extended to include the region that should
> be presented. When the whole surface is to be presented, this region
> will be a single rect containing the dimensions of the surface.
>
> Signed-off-by: Fredrik Höglund 
> ---
>  src/gallium/state_trackers/egl/common/egl_g3d.c    |    5 +++
>  .../state_trackers/egl/common/egl_g3d_api.c        |   31 ++-
>  src/gallium/state_trackers/egl/common/native.h     |   12 +++-
>  3 files changed, 45 insertions(+), 3 deletions(-)
>
> diff --git a/src/gallium/state_trackers/egl/common/egl_g3d.c 
> b/src/gallium/state_trackers/egl/common/egl_g3d.c
> index 182ce68..feebfaf 100644
> --- a/src/gallium/state_trackers/egl/common/egl_g3d.c
> +++ b/src/gallium/state_trackers/egl/common/egl_g3d.c
> @@ -606,6 +606,11 @@ egl_g3d_initialize(_EGLDriver *drv, _EGLDisplay *dpy)
>       dpy->Extensions.WL_bind_wayland_display = EGL_TRUE;
>  #endif
>
> +#ifdef EGL_NOK_swap_region
> +   if (gdpy->native->get_param(gdpy->native, NATIVE_PARAM_PRESENT_REGION))
> +      dpy->Extensions.NOK_swap_region = EGL_TRUE;
> +#endif
> +
Does EGL_NOK_swap_region require the contents of the back buffer to be
preserved?  If so, NATIVE_PARAM_PRESERVE_BUFFER needs to be checked
too.
>    if (egl_g3d_add_configs(drv, dpy, 1) == 1) {
>       _eglError(EGL_NOT_INITIALIZED, "eglInitialize(unable to add configs)");
>       goto fail;
> diff --git a/src/gallium/state_trackers/egl/common/egl_g3d_api.c 
> b/src/gallium/state_trackers/egl/common/egl_g3d_api.c
> index 911540e..e8f2abc 100644
> --- a/src/gallium/state_trackers/egl/common/egl_g3d_api.c
> +++ b/src/gallium/state_trackers/egl/common/egl_g3d_api.c
> @@ -546,7 +546,8 @@ egl_g3d_make_current(_EGLDriver *drv, _EGLDisplay *dpy,
>  }
>
>  static EGLBoolean
> -egl_g3d_swap_buffers(_EGLDriver *drv, _EGLDisplay *dpy, _EGLSurface *surf)
> +swap_buffers(_EGLDriver *drv, _EGLDisplay *dpy, _EGLSurface *surf,
> +             EGLint num_rects, const EGLint *rects, EGLBoolean preserve)
>  {
>    struct egl_g3d_surface *gsurf = egl_g3d_surface(surf);
>    _EGLContext *ctx = _eglGetCurrentContext();
> @@ -572,14 +573,36 @@ egl_g3d_swap_buffers(_EGLDriver *drv, _EGLDisplay *dpy, 
> _EGLSurface *surf)
>
>    memset(&ctrl, 0, sizeof(ctrl));
>    ctrl.natt = NATIVE_ATTACHMENT_BACK_LEFT;
> -   ctrl.preserve = (gsurf->base.SwapBehavior == EGL_BUFFER_PRESERVED);
> +   ctrl.preserve = preserve;
>    ctrl.swap_interval = gsurf->base.SwapInterval;
>    ctrl.premultiplied_alpha = (gsurf->base.VGAlphaFormat == 
> EGL_VG_ALPHA_FORMAT_PRE);
> +   ctrl.num_rects = num_rects;
> +   ctrl.rects = rects;
>
>    return gsurf->native->present(gsurf->native, &ctrl);
>  }
>
>  static EGLBoolean
> +egl_g3d_swap_buffers(_EGLDriver *drv, _EGLDisplay *dpy, _EGLSurface *surf)
> +{
> +   struct egl_g3d_surface *gsurf = egl_g3d_surface(surf);
> +   const EGLint rect[4] = { 0, 0, gsurf->base.Width, gsurf->base.Height };
> +
> +   return swap_buffers(drv, dpy, surf, 1, rect,
"return swap_buffers(drv, dpy, surf, 0, NULL, ...);" seems simpler here.
> +                       (gsurf->base.SwapBehavior == EGL_BUFFER_PRESERVED));
> +}
> +
> +#ifdef EGL_NOK_swap_region
> +static EGLBoolean
> +egl_g3d_swap_buffers_region(_EGLDriver *drv, _EGLDisplay *dpy, _EGLSurface 
> *surf,
> +                            EGLint num_rects, const EGLint *rects)
> +{
> +   /* Note: y=0=top */
> +   return swap_buffers(drv, dpy, surf, num_rects, rects, EGL_TRUE);
This assumes the native display supports buffer preservation.  If the
extension requires that, then NATIVE_PARAM_PRESERVE_BUFFER should be
checked.  If not, EGL_SWAP_BEHAVIOR should be honored.
> +}
> +#endif /* EGL_NOK_swap_region */
> +
> +static EGLBoolean
>  egl_g3d_copy_buffers(_EGLDriver *drv, _EGLDisplay *dpy, _EGLSurface *surf,
>                      EGLNativePixmapType target)
>  {
> @@ -867,4 +890,8 @@ egl_g3d_init_driver_api(_EGLDriver *drv)
>    drv->API.CreateScreenSurfaceMESA = egl_g3d_create_screen_surface;
>    drv->API.ShowScreenSurfaceMESA = egl_g3d_show_screen_surface;
>  #endif
> +
> +#ifdef EGL_NOK_swap_region
> +   drv->API.SwapBuffersRegionNOK = egl_g3d_swap_buffers_region;
> +#endif
>  }
> diff --git a/src/gallium/state_trackers/egl/common/native.h 
> b/src/gallium/state_trackers/egl/common/native.h
> index ee24c22..f1e067e 100644
> --- a/src/gallium/state_trackers/egl/common/native.h
> +++ b/src/gallium/state_trackers/egl/common/native.h
> @@ -81,7 +81,13 @@ enum native_param_type {
>     * EGL_ALPHA_SIZE.  EGL_VG_ALPHA_FORMAT attribute of a surface will affect
>     * how the surface is presented.
>     */
> -   NATIVE_PARAM_PREMULTIPLIED_ALPHA
> +   NATIVE_PARAM_PREMULTIPLIED_ALPHA,
> +
> +   /**
> +    * Return TRUE if native_surface::present supports presenting a partial
> +    * sur

Re: [Mesa-dev] [PATCH 1/3] egl: add EGL_NV_post_sub_buffer

2011-12-13 Thread Chia-I Wu
On Sun, Dec 11, 2011 at 1:56 AM, Fredrik Höglund  wrote:
> Signed-off-by: Fredrik Höglund 
> ---
>  include/EGL/eglext.h      |    9 +
>  src/egl/main/eglapi.c     |   24 
>  src/egl/main/eglapi.h     |    8 
>  src/egl/main/egldisplay.h |    2 ++
>  src/egl/main/eglmisc.c    |    2 ++
>  src/egl/main/eglsurface.c |    9 +
>  src/egl/main/eglsurface.h |    4 
>  7 files changed, 58 insertions(+), 0 deletions(-)
>
> diff --git a/include/EGL/eglext.h b/include/EGL/eglext.h
> index 9484b83..d03a24d 100644
> --- a/include/EGL/eglext.h
> +++ b/include/EGL/eglext.h
> @@ -144,6 +144,15 @@ typedef EGLImageKHR (EGLAPIENTRYP 
> PFNEGLCREATEDRMIMAGEMESA) (EGLDisplay dpy, con
>  typedef EGLBoolean (EGLAPIENTRYP PFNEGLEXPORTDRMIMAGEMESA) (EGLDisplay dpy, 
> EGLImageKHR image, EGLint *name, EGLint *handle, EGLint *stride);
>  #endif
>
> +#ifndef EGL_NV_post_sub_buffer
> +#define EGL_NV_post_sub_buffer 1
> +#define EGL_POST_SUB_BUFFER_SUPPORTED_NV       0x30BE
> +#ifdef EGL_EGLEXT_PROTOTYPES
> +EGLAPI EGLBoolean EGLAPIENTRY eglPostSubBufferNV(EGLDisplay dpy, EGLSurface 
> surface, EGLint x, EGLint y, EGLint width, EGLint height);
> +#endif /* EGL_EGLEXT_PROTOTYPES */
> +typedef EGLBoolean (EGLAPIENTRYP PFNEGLPOSTSUBBUFFERNVPROC) (EGLDisplay dpy, 
> EGLSurface surface, EGLint x, EGLint y, EGLint width, EGLint height);
> +#endif
> +
>  #ifndef EGL_WL_bind_wayland_display
>  #define EGL_WL_bind_wayland_display 1
>
> diff --git a/src/egl/main/eglapi.c b/src/egl/main/eglapi.c
> index 3cb1a5b..ff15476 100644
> --- a/src/egl/main/eglapi.c
> +++ b/src/egl/main/eglapi.c
> @@ -951,6 +951,9 @@ eglGetProcAddress(const char *procname)
>  #ifdef EGL_ANDROID_swap_rectangle
>       { "eglSetSwapRectangleANDROID", (_EGLProc) eglSetSwapRectangleANDROID },
>  #endif
> +#ifdef EGL_NV_post_sub_buffer
> +      { "eglPostSubBufferNV", (_EGLProc) eglPostSubBufferNV },
> +#endif
>       { NULL, NULL }
>    };
>    EGLint i;
> @@ -1590,3 +1593,24 @@ eglSetSwapRectangleANDROID(EGLDisplay dpy, EGLSurface 
> draw,
>    RETURN_EGL_EVAL(disp, ret);
>  }
>  #endif
> +
> +#ifdef EGL_NV_post_sub_buffer
> +EGLBoolean EGLAPIENTRY
> +eglPostSubBufferNV(EGLDisplay dpy, EGLSurface surface,
> +                   EGLint x, EGLint y, EGLint width, EGLint height)
> +{
> +   _EGLDisplay *disp = _eglLockDisplay(dpy);
> +   _EGLSurface *surf = _eglLookupSurface(surface, disp);
> +   _EGLDriver *drv;
> +   EGLBoolean ret;
> +
> +   _EGL_CHECK_SURFACE(disp, surf, EGL_FALSE, drv);
> +
> +   if (!disp->Extensions.NV_post_sub_buffer)
> +      RETURN_EGL_EVAL(disp, EGL_FALSE);
> +
> +   ret = drv->API.PostSubBufferNV(drv, disp, surf, x, y, width, height);
> +
> +   RETURN_EGL_EVAL(disp, ret);
> +}
> +#endif
> diff --git a/src/egl/main/eglapi.h b/src/egl/main/eglapi.h
> index 1e0aef6..f374273 100644
> --- a/src/egl/main/eglapi.h
> +++ b/src/egl/main/eglapi.h
> @@ -135,6 +135,10 @@ typedef EGLBoolean 
> (*UnbindWaylandDisplayWL_t)(_EGLDriver *drv, _EGLDisplay *dis
>  typedef EGLBoolean (*SetSwapRectangleANDROID_t)(_EGLDriver *drv, _EGLDisplay 
> *disp, _EGLSurface *draw, EGLint left, EGLint top, EGLint width, EGLint 
> height);
>  #endif
>
> +#ifdef EGL_NV_post_sub_buffer
> +typedef EGLBoolean (*PostSubBufferNV_t)(_EGLDriver *drv, _EGLDisplay *disp, 
> _EGLSurface *surface, EGLint x, EGLint y, EGLint width, EGLint height);
> +#endif
> +
>  /**
>  * The API dispatcher jumps through these functions
>  */
> @@ -218,6 +222,10 @@ struct _egl_api
>  #ifdef EGL_ANDROID_swap_rectangle
>    SetSwapRectangleANDROID_t SetSwapRectangleANDROID;
>  #endif
> +
> +#ifdef EGL_NV_post_sub_buffer
> +   PostSubBufferNV_t PostSubBufferNV;
> +#endif
>  };
>
>  #endif /* EGLAPI_INCLUDED */
> diff --git a/src/egl/main/egldisplay.h b/src/egl/main/egldisplay.h
> index 67a2e24..8f737df 100644
> --- a/src/egl/main/egldisplay.h
> +++ b/src/egl/main/egldisplay.h
> @@ -112,6 +112,8 @@ struct _egl_extensions
>
>    EGLBoolean ANDROID_image_native_buffer;
>    EGLBoolean ANDROID_swap_rectangle;
> +
> +   EGLBoolean NV_post_sub_buffer;
>  };
>
>
> diff --git a/src/egl/main/eglmisc.c b/src/egl/main/eglmisc.c
> index ab48bc6..30cd04e 100644
> --- a/src/egl/main/eglmisc.c
> +++ b/src/egl/main/eglmisc.c
> @@ -116,6 +116,8 @@ _eglUpdateExtensionsString(_EGLDisplay *dpy)
>
>    _EGL_CHECK_EXTENSION(ANDROID_image_native_buffer);
>    _EGL_CHECK_EXTENSION(ANDROID_swap_rectangle);
> +
> +   _EGL_CHECK_EXTENSION(NV_post_sub_buffer);
>  #undef _EGL_CHECK_EXTENSION
>  }
>
> diff --git a/src/egl/main/eglsurface.c b/src/egl/main/eglsurface.c
> index 3564ecd..92edff5 100644
> --- a/src/egl/main/eglsurface.c
> +++ b/src/egl/main/eglsurface.c
> @@ -323,6 +323,10 @@ _eglInitSurface(_EGLSurface *surf, _EGLDisplay *dpy, 
> EGLint type,
>    surf->VerticalResolution = EGL_UNKNOWN;
>    surf->AspectRatio = EGL_UNKNOWN;
>
> +#ifdef EGL_NV_post_sub_buffer
> +   surf->PostSubBufferSupportedNV = EGL_FALSE;
> +#endif
> +
I think this attribute should be set according to attrib_list.

Re: [Mesa-dev] [PATCH] Add mismatch check for glGetTexImage or it will return -1 and may lead to segment fault.

2011-12-13 Thread Brian Paul
On 12/12/2011 10:31 PM, jian.j.z...@intel.com wrote:
> From: Jian Zhao
> 
> ---
>   src/mesa/main/texgetimage.c |8 
>   1 files changed, 8 insertions(+), 0 deletions(-)
> 
> diff --git a/src/mesa/main/texgetimage.c b/src/mesa/main/texgetimage.c
> index ae0d51f..3f24187 100644
> --- a/src/mesa/main/texgetimage.c
> +++ b/src/mesa/main/texgetimage.c
> @@ -708,6 +708,14 @@ getteximage_error_check(struct gl_context *ctx, GLenum 
> target, GLint level,
> return GL_TRUE;
>  }
> 
> +   if (!_mesa_is_legal_format_and_type(ctx, format, type)) {
> +  /*GL_INVALID_OPERATION is generated by a format/type
> +   * mismatch (see the 1.2 spec page 94, sec 3.6.4.)
> +   */
> +  _mesa_error(ctx, GL_INVALID_OPERATION, "glGetTexImage(target)");
> +  return GL_TRUE;
> +   }
> +
>  baseFormat = _mesa_get_format_base_format(texImage->TexFormat);
> 
>  /* Make sure the requested image format is compatible with the

Reviewed-by: Brian Paul 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH] r600g: Fix tiling alignment to match docs

2011-12-13 Thread Simon Farnsworth
R600 and Evergreen have different tiling requirements. Fix Mesa to match the
documented requirements.

Signed-off-by: Simon Farnsworth 
---

This doesn't fix my problems with enabling macro tiling, but it does help
somewhat. I also need to fix tile shape in the kernel and alignment in the
DDX to avoid misrendering, at which point I see CP lockups instead.

I'm therefore sending this, the DDX patch, and the kernel patch out in order
to avoid getting stuck in a world where I have an 80% fix, someone else has
an 80% fix, and if only we talked, we'd have a 100% fix.

 src/gallium/drivers/r600/evergreen_state.c |2 +-
 src/gallium/drivers/r600/r600_texture.c|  140 
 2 files changed, 103 insertions(+), 39 deletions(-)

diff --git a/src/gallium/drivers/r600/evergreen_state.c 
b/src/gallium/drivers/r600/evergreen_state.c
index d0c02d5..d6b97da 100644
--- a/src/gallium/drivers/r600/evergreen_state.c
+++ b/src/gallium/drivers/r600/evergreen_state.c
@@ -1447,7 +1447,7 @@ static void evergreen_cb(struct r600_pipe_context *rctx, 
struct r600_pipe_state
offset >> 8, 0x, &rtex->resource, 
RADEON_USAGE_READWRITE);
r600_pipe_state_add_reg(rstate,
R_028C78_CB_COLOR0_DIM + cb * 0x3C,
-   0x0, 0x, NULL, 0);
+   S_028C78_WIDTH_MAX(surf->base.width) | 
S_028C78_HEIGHT_MAX(surf->base.height), 0x, NULL, 0);
r600_pipe_state_add_reg(rstate,
R_028C70_CB_COLOR0_INFO + cb * 0x3C,
color_info, 0x, &rtex->resource, 
RADEON_USAGE_READWRITE);
diff --git a/src/gallium/drivers/r600/r600_texture.c 
b/src/gallium/drivers/r600/r600_texture.c
index 6143133..16ffe23 100644
--- a/src/gallium/drivers/r600/r600_texture.c
+++ b/src/gallium/drivers/r600/r600_texture.c
@@ -88,23 +88,42 @@ static unsigned r600_get_block_alignment(struct pipe_screen 
*screen,
unsigned pixsize = util_format_get_blocksize(format);
int p_align;
 
-   switch(array_mode) {
-   case V_038000_ARRAY_2D_TILED_THIN1:
-   p_align = MAX2(rscreen->tiling_info.num_banks,
-  (((rscreen->tiling_info.group_bytes / 8) / 
pixsize) * rscreen->tiling_info.num_banks)) * 8;
-   /* further restrictions for scanout */
-   p_align = MAX2(rscreen->tiling_info.num_banks * 8, p_align);
-   break;
-   case V_038000_ARRAY_1D_TILED_THIN1:
-   p_align = MAX2(8, (rscreen->tiling_info.group_bytes / (8 * 
pixsize)));
-   /* further restrictions for scanout */
-   p_align = MAX2((rscreen->tiling_info.group_bytes / pixsize), 
p_align);
-   break;
-   case V_038000_ARRAY_LINEAR_ALIGNED:
-   case V_038000_ARRAY_LINEAR_GENERAL:
-   default:
-   p_align = MAX2(64, rscreen->tiling_info.group_bytes / pixsize);
-   break;
+   if (rscreen->chip_class >= EVERGREEN)
+   {
+   switch(array_mode) {
+   case V_038000_ARRAY_2D_TILED_THIN1:
+   /* Kernel uses bank width of 8, macro tile aspect of 1 
*/
+   p_align = rscreen->tiling_info.num_channels * 8 * 1 * 8;
+   break;
+   case V_038000_ARRAY_1D_TILED_THIN1:
+   p_align = MAX2(8, rscreen->tiling_info.group_bytes /
+  (8 * 1 * pixsize * 1));
+   break;
+   case V_038000_ARRAY_LINEAR_ALIGNED:
+   p_align = MAX2(64, rscreen->tiling_info.group_bytes / 
pixsize);
+   break;
+   case V_038000_ARRAY_LINEAR_GENERAL:
+   default:
+   p_align = 8;
+   }
+   }
+   else
+   {
+   switch(array_mode) {
+   case V_038000_ARRAY_2D_TILED_THIN1:
+   p_align = MAX2(rscreen->tiling_info.num_banks * 8,
+  ((rscreen->tiling_info.group_bytes / 8) 
/ pixsize) * rscreen->tiling_info.num_banks);
+   break;
+   case V_038000_ARRAY_1D_TILED_THIN1:
+   p_align = MAX2(8, (rscreen->tiling_info.group_bytes / 
(8 * pixsize)));
+   break;
+   case V_038000_ARRAY_LINEAR_ALIGNED:
+   p_align = MAX2(64, rscreen->tiling_info.group_bytes / 
pixsize);
+   break;
+   case V_038000_ARRAY_LINEAR_GENERAL:
+   default:
+   p_align = 1;
+   }
}
return p_align;
 }
@@ -115,16 +134,36 @@ static unsigned r600_get_height_alignment(struct 
pipe_screen *screen,
struct r600_screen* rscreen = (struct r600_screen *)screen;
int h_align;
 
-   switch (array_mode) {
-   case V_038000_ARRAY_2D

Re: [Mesa-dev] [PATCH] r600g: Fix tiling alignment to match docs

2011-12-13 Thread Simon Farnsworth
On Tuesday 13 December 2011, Simon Farnsworth  
wrote:
> R600 and Evergreen have different tiling requirements. Fix Mesa to match the
> documented requirements.
> 
> Signed-off-by: Simon Farnsworth 
> ---
> 
> This doesn't fix my problems with enabling macro tiling, but it does help
> somewhat. I also need to fix tile shape in the kernel and alignment in the
> DDX to avoid misrendering, at which point I see CP lockups instead.
> 
> I'm therefore sending this, the DDX patch, and the kernel patch out in order
> to avoid getting stuck in a world where I have an 80% fix, someone else has
> an 80% fix, and if only we talked, we'd have a 100% fix.
> 
Jerome has told me that he's got closer to fixing 2D tiling than I have -
I'm going to collaborate with him on fixing his remaining bugs rather than
continue with these patches.
-- 
Simon Farnsworth
Software Engineer
ONELAN Limited
http://www.onelan.com/
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] mesa: add const flags to skip MaxVarying and MaxUniform linker checks

2011-12-13 Thread Ian Romanick

On 12/10/2011 11:37 PM, Marek Olšák wrote:

On Sat, Dec 10, 2011 at 8:19 PM, Ian Romanick  wrote:


This may not be necessary in the short term.  I think r300 is
under-reporting it's capabilities.  See
https://bugs.freedesktop.org/show_bug.cgi?id=34201#c9


The problem is st/mesa is under-reporting its capabilities, because
when I was writing that code, I thought color varyings just don't
belong to the max varying limit. The color varyings shouldn't be
considered a generic varying resource (at least internally in Mesa),
because there may have different precision. They don't have to be
float when clamped, and they don't have to be exactly 32-bit float
when not clamped (r300 is an example of such a behavior).


Before fairly recent versions of OpenGL, there was no requirement that 
varyings be full single-precision.  I didn't think that r300 used full 
single-precision for anything.  Like I mentioned in the bug report, 
other drivers for that hardware report 40 varying floats.



I see this solution:

We don't need exact GL limits in gl_constants, only the limits which
make sense for drivers, like Gallium has. So we'd have something like
MaxGenericVaryings and glGet*(GL_MAX_VARYING_FLOATS) would return
MaxGenericVaryings*4+8. Then the linker wouldn't count the color
varyings in the number of used varying components. How does it sound?


Hardware is increasingly moving towards not having "special" varyings. 
All of Intel's i965-class hardware works that way, and I'm pretty sure 
NVIDIA hardware is the same way.  It seems like not counting the colors 
leads to a cascade of special-cases in each driver.


Imagine the case where someone writes a shader that uses 40 float 
varying components for non-builtin varyings.  Then what? 
GL_MAX_VARYING_FLOATS said it was ok, but the linker sees a different 
limit.  Then what?  If the link fails (either from the linker or the 
driver), the app developer will (rightfully!) report a bug or rage quit.


Assuming that all hardware can do unclamped colors, it seems like 
counting them as generic varyings is the right choice.


If there is some hardware that can't do unclamped colors, they shouldn't 
include them in the generic varying count, and we'll have to put 
something special in to (not) count them.

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH 0/2] Add support for clip distances in Gallium

2011-12-13 Thread Bryan Cain
These patches add support for clip distances in the Gallium interface and the
Mesa state tracker, respectively.  This should take care of gl_ClipDistance,
one of the few GLSL 1.30 features not yet implemented in Gallium.  If this is
merged, driver developers will need to add support to their drivers for the new
TGSI_SEMANTIC_CLIPDIST.

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH 1/2] gallium: add TGSI_SEMANTIC_CLIPDIST for clip distance

2011-12-13 Thread Bryan Cain
---
 src/gallium/auxiliary/tgsi/tgsi_dump.c |3 ++-
 src/gallium/include/pipe/p_shader_tokens.h |3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/gallium/auxiliary/tgsi/tgsi_dump.c 
b/src/gallium/auxiliary/tgsi/tgsi_dump.c
index e830aa5..bd299b0 100644
--- a/src/gallium/auxiliary/tgsi/tgsi_dump.c
+++ b/src/gallium/auxiliary/tgsi/tgsi_dump.c
@@ -129,7 +129,8 @@ static const char *semantic_names[] =
"PRIM_ID",
"INSTANCEID",
"VERTEXID",
-   "STENCIL"
+   "STENCIL",
+   "CLIPDIST"
 };
 
 static const char *immediate_type_names[] =
diff --git a/src/gallium/include/pipe/p_shader_tokens.h 
b/src/gallium/include/pipe/p_shader_tokens.h
index 10cfaf6..330e0ba 100644
--- a/src/gallium/include/pipe/p_shader_tokens.h
+++ b/src/gallium/include/pipe/p_shader_tokens.h
@@ -146,7 +146,8 @@ struct tgsi_declaration_dimension
 #define TGSI_SEMANTIC_INSTANCEID 10
 #define TGSI_SEMANTIC_VERTEXID   11
 #define TGSI_SEMANTIC_STENCIL12
-#define TGSI_SEMANTIC_COUNT  13 /**< number of semantic values */
+#define TGSI_SEMANTIC_CLIPDIST   13
+#define TGSI_SEMANTIC_COUNT  14 /**< number of semantic values */
 
 struct tgsi_declaration_semantic
 {
-- 
1.7.1

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH 2/2] st/mesa: add support for gl_ClipDistance

2011-12-13 Thread Bryan Cain
---
 src/mesa/state_tracker/st_glsl_to_tgsi.cpp |1 +
 src/mesa/state_tracker/st_program.c|   18 ++
 2 files changed, 19 insertions(+), 0 deletions(-)

diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp 
b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
index 9ef65c8..d50176d 100644
--- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
+++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
@@ -5036,6 +5036,7 @@ st_link_shader(struct gl_context *ctx, struct 
gl_shader_program *prog)
   || progress;
 
  progress = lower_quadop_vector(ir, false) || progress;
+ progress = lower_clip_distance(ir) || progress;
 
  if (options->MaxIfDepth == 0)
 progress = lower_discard(ir) || progress;
diff --git a/src/mesa/state_tracker/st_program.c 
b/src/mesa/state_tracker/st_program.c
index 04d3ef6..73581dd 100644
--- a/src/mesa/state_tracker/st_program.c
+++ b/src/mesa/state_tracker/st_program.c
@@ -244,6 +244,14 @@ st_prepare_vertex_program(struct gl_context *ctx,
 stvp->output_semantic_name[slot] = TGSI_SEMANTIC_PSIZE;
 stvp->output_semantic_index[slot] = 0;
 break;
+ case VERT_RESULT_CLIP_DIST0:
+stvp->output_semantic_name[slot] = TGSI_SEMANTIC_CLIPDIST;
+stvp->output_semantic_index[slot] = 0;
+break;
+ case VERT_RESULT_CLIP_DIST1:
+stvp->output_semantic_name[slot] = TGSI_SEMANTIC_CLIPDIST;
+stvp->output_semantic_index[slot] = 1;
+break;
  case VERT_RESULT_EDGE:
 assert(0);
 break;
@@ -541,6 +549,16 @@ st_translate_fragment_program(struct st_context *st,
input_semantic_index[slot] = 0;
interpMode[slot] = TGSI_INTERPOLATE_CONSTANT;
break;
+case FRAG_ATTRIB_CLIP_DIST0:
+   input_semantic_name[slot] = TGSI_SEMANTIC_CLIPDIST;
+   input_semantic_index[slot] = 0;
+   interpMode[slot] = TGSI_INTERPOLATE_LINEAR;
+   break;
+case FRAG_ATTRIB_CLIP_DIST1:
+   input_semantic_name[slot] = TGSI_SEMANTIC_CLIPDIST;
+   input_semantic_index[slot] = 1;
+   interpMode[slot] = TGSI_INTERPOLATE_LINEAR;
+   break;
/* In most cases, there is nothing special about these
 * inputs, so adopt a convention to use the generic
 * semantic name and the mesa FRAG_ATTRIB_ number as the
-- 
1.7.1

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 0/7] Fix GLX files generated by Mesa for the server

2011-12-13 Thread Adam Jackson
On Thu, 2011-12-08 at 12:47 -0800, Ian Romanick wrote:
> There has been quite a bit of skew between what's in Mesa and what's
> needed in the xserver.  This patch series cleans that up.  Most of the
> changes are quite mundane and just make the code compile inside the
> xserver.  However, the changes in patch 6/7 modify the way the
> availability and use of backtrace is handled.
> 
> Once this series is reviewed and accepeted in Mesa, a short series
> that makes the newly generated code usable in the xserver will be
> posted to xorg-devel.
> 
> The two sequences together are prerequesites to a Mesa series and an
> xserver series that will implement GLX_ARB_create_context and
> GLX_ARB_create_context_profile.
> 
> Trees with these patches and the generated files can be found at:
> 
>   git://anongit.freedesktop.org/~idr/mesa glx-fixes
>   git://anongit.freedesktop.org/~idr/xserver glx-fixes
> 
> With these two branches, I observer no piglit regressions on Intel
> Ironlake hardware.  I have not tested other hardware, but I would
> expect any regressions to be hardware independent.

Reviewed-by: Adam Jackson 

- ajax


signature.asc
Description: This is a digitally signed message part
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] Bug#651370: libgl1-mesa-glx: need close on exec for dri device

2011-12-13 Thread Adam Jackson
On Sat, 2011-12-10 at 11:46 -0600, David Fries wrote:
> Set the close on exec flag when opening dri character devices, so they
> will be closed and free any resouces allocated in exec.

Reviewed-by: Adam Jackson 

- ajax


signature.asc
Description: This is a digitally signed message part
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH] st/mesa: expose conservative_depth if GLSL 1.3 is supported

2011-12-13 Thread Marek Olšák
It's not yet, but it can be enabled by the override environment variable.
---
 src/mesa/state_tracker/st_extensions.c |6 ++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/src/mesa/state_tracker/st_extensions.c 
b/src/mesa/state_tracker/st_extensions.c
index 457d5d6..227906b 100644
--- a/src/mesa/state_tracker/st_extensions.c
+++ b/src/mesa/state_tracker/st_extensions.c
@@ -256,6 +256,12 @@ void st_init_extensions(struct st_context *st)
ctx->Const.GLSLVersion = 120;
_mesa_override_glsl_version(st->ctx);
 
+   /* Extensions that only depend on the GLSL version:
+*/
+   if (ctx->Const.GLSLVersion >= 130) {
+  ctx->Extensions.ARB_conservative_depth = GL_TRUE;
+   }
+
/*
 * Extensions that are supported by all Gallium drivers:
 */
-- 
1.7.5.4

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH 0/2 v2] Add support for clip distances in Gallium

2011-12-13 Thread Bryan Cain
This is an updated version of the patch set I sent to the list a few hours
ago.  There is now a TGSI property called TGSI_PROPERTY_NUM_CLIP_DISTANCES
that drivers can use to determine how many of the 8 available clip distances
are actually used by a shader.

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH 1/2] gallium: add support for clip distances

2011-12-13 Thread Bryan Cain
---
 src/gallium/auxiliary/tgsi/tgsi_dump.c |6 --
 src/gallium/auxiliary/tgsi/tgsi_text.c |3 ++-
 src/gallium/auxiliary/tgsi/tgsi_ureg.c |   14 ++
 src/gallium/auxiliary/tgsi/tgsi_ureg.h |3 +++
 src/gallium/include/pipe/p_shader_tokens.h |6 --
 5 files changed, 27 insertions(+), 5 deletions(-)

diff --git a/src/gallium/auxiliary/tgsi/tgsi_dump.c 
b/src/gallium/auxiliary/tgsi/tgsi_dump.c
index e830aa5..4534731 100644
--- a/src/gallium/auxiliary/tgsi/tgsi_dump.c
+++ b/src/gallium/auxiliary/tgsi/tgsi_dump.c
@@ -129,7 +129,8 @@ static const char *semantic_names[] =
"PRIM_ID",
"INSTANCEID",
"VERTEXID",
-   "STENCIL"
+   "STENCIL",
+   "CLIPDIST"
 };
 
 static const char *immediate_type_names[] =
@@ -174,7 +175,8 @@ const char *tgsi_property_names[TGSI_PROPERTY_COUNT] =
"FS_COORD_ORIGIN",
"FS_COORD_PIXEL_CENTER",
"FS_COLOR0_WRITES_ALL_CBUFS",
-   "FS_DEPTH_LAYOUT"
+   "FS_DEPTH_LAYOUT",
+   "NUM_CLIP_DISTANCES"
 };
 
 static const char *tgsi_type_names[] =
diff --git a/src/gallium/auxiliary/tgsi/tgsi_text.c 
b/src/gallium/auxiliary/tgsi/tgsi_text.c
index eb9190c..f46ba19 100644
--- a/src/gallium/auxiliary/tgsi/tgsi_text.c
+++ b/src/gallium/auxiliary/tgsi/tgsi_text.c
@@ -1024,7 +1024,8 @@ static const char *semantic_names[TGSI_SEMANTIC_COUNT] =
"PRIM_ID",
"INSTANCEID",
"VERTEXID",
-   "STENCIL"
+   "STENCIL",
+   "CLIPDIST"
 };
 
 static const char *interpolate_names[TGSI_INTERPOLATE_COUNT] =
diff --git a/src/gallium/auxiliary/tgsi/tgsi_ureg.c 
b/src/gallium/auxiliary/tgsi/tgsi_ureg.c
index ee013a5..5966c8c 100644
--- a/src/gallium/auxiliary/tgsi/tgsi_ureg.c
+++ b/src/gallium/auxiliary/tgsi/tgsi_ureg.c
@@ -162,6 +162,7 @@ struct ureg_program
unsigned char property_fs_coord_pixel_center; /* = 
TGSI_FS_COORD_PIXEL_CENTER_* */
unsigned char property_fs_color0_writes_all_cbufs; /* = 
TGSI_FS_COLOR0_WRITES_ALL_CBUFS * */
unsigned char property_fs_depth_layout; /* TGSI_FS_DEPTH_LAYOUT */
+   unsigned char property_num_clip_distances;
 
unsigned nr_addrs;
unsigned nr_preds;
@@ -312,6 +313,13 @@ ureg_property_fs_depth_layout(struct ureg_program *ureg,
ureg->property_fs_depth_layout = fs_depth_layout;
 }
 
+void
+ureg_property_num_clip_distances(struct ureg_program *ureg,
+ unsigned num_clip_distances)
+{
+   ureg->property_num_clip_distances = num_clip_distances;
+}
+
 struct ureg_src
 ureg_DECL_fs_input_cyl_centroid(struct ureg_program *ureg,
unsigned semantic_name,
@@ -1404,6 +1412,12 @@ static void emit_decls( struct ureg_program *ureg )
 ureg->property_fs_depth_layout);
}
 
+   if (ureg->property_num_clip_distances) {
+  emit_property(ureg,
+TGSI_PROPERTY_NUM_CLIP_DISTANCES,
+ureg->property_num_clip_distances);
+   }
+
if (ureg->processor == TGSI_PROCESSOR_VERTEX) {
   for (i = 0; i < UREG_MAX_INPUT; i++) {
  if (ureg->vs_inputs[i/32] & (1 << (i%32))) {
diff --git a/src/gallium/auxiliary/tgsi/tgsi_ureg.h 
b/src/gallium/auxiliary/tgsi/tgsi_ureg.h
index a70d30f..f313a6f 100644
--- a/src/gallium/auxiliary/tgsi/tgsi_ureg.h
+++ b/src/gallium/auxiliary/tgsi/tgsi_ureg.h
@@ -161,6 +161,9 @@ void
 ureg_property_fs_depth_layout(struct ureg_program *ureg,
   unsigned fs_depth_layout);
 
+void
+ureg_property_num_clip_distances(struct ureg_program *ureg,
+ unsigned num_clip_distances);
 
 /***
  * Build shader declarations:
diff --git a/src/gallium/include/pipe/p_shader_tokens.h 
b/src/gallium/include/pipe/p_shader_tokens.h
index 10cfaf6..e5e6d46 100644
--- a/src/gallium/include/pipe/p_shader_tokens.h
+++ b/src/gallium/include/pipe/p_shader_tokens.h
@@ -146,7 +146,8 @@ struct tgsi_declaration_dimension
 #define TGSI_SEMANTIC_INSTANCEID 10
 #define TGSI_SEMANTIC_VERTEXID   11
 #define TGSI_SEMANTIC_STENCIL12
-#define TGSI_SEMANTIC_COUNT  13 /**< number of semantic values */
+#define TGSI_SEMANTIC_CLIPDIST   13
+#define TGSI_SEMANTIC_COUNT  14 /**< number of semantic values */
 
 struct tgsi_declaration_semantic
 {
@@ -189,7 +190,8 @@ union tgsi_immediate_data
 #define TGSI_PROPERTY_FS_COORD_PIXEL_CENTER  4
 #define TGSI_PROPERTY_FS_COLOR0_WRITES_ALL_CBUFS 5
 #define TGSI_PROPERTY_FS_DEPTH_LAYOUT6
-#define TGSI_PROPERTY_COUNT  7
+#define TGSI_PROPERTY_NUM_CLIP_DISTANCES 7
+#define TGSI_PROPERTY_COUNT  8
 
 struct tgsi_property {
unsigned Type : 4;  /**< TGSI_TOKEN_TYPE_PROPERTY */
-- 
1.7.1

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH 2/2] st/mesa: add support for gl_ClipDistance

2011-12-13 Thread Bryan Cain
---
 src/mesa/state_tracker/st_glsl_to_tgsi.cpp |   39 ++-
 src/mesa/state_tracker/st_program.c|   18 +
 2 files changed, 55 insertions(+), 2 deletions(-)

diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp 
b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
index 9ef65c8..5e54465 100644
--- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
+++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
@@ -304,6 +304,7 @@ public:
int samplers_used;
bool indirect_addr_temps;
bool indirect_addr_consts;
+   int num_clip_distances;

int glsl_version;
bool native_integers;
@@ -4738,6 +4739,9 @@ st_translate_program(
  t->samplers[i] = ureg_DECL_sampler(ureg, i);
   }
}
+   
+   /* Set the number of clip distances used in the shader. */
+   ureg_property_num_clip_distances(ureg, program->num_clip_distances);
 
/* Emit each instruction in turn:
 */
@@ -4797,7 +4801,8 @@ out:
 static struct gl_program *
 get_mesa_program(struct gl_context *ctx,
  struct gl_shader_program *shader_program,
-struct gl_shader *shader)
+ struct gl_shader *shader,
+ int num_clip_distances)
 {
glsl_to_tgsi_visitor* v = new glsl_to_tgsi_visitor();
struct gl_program *prog;
@@ -4842,6 +4847,7 @@ get_mesa_program(struct gl_context *ctx,
v->options = options;
v->glsl_version = ctx->Const.GLSLVersion;
v->native_integers = ctx->Const.NativeIntegers;
+   v->num_clip_distances = num_clip_distances;
 
_mesa_generate_parameters_list_for_uniforms(shader_program, shader,
   prog->Parameters);
@@ -4971,6 +4977,27 @@ get_mesa_program(struct gl_context *ctx,
return prog;
 }
 
+/**
+ * Searches through the IR for a declaration of gl_ClipDistance and returns the
+ * declared size of the gl_ClipDistance array.  Returns 0 if gl_ClipDistance is
+ * not declared in the IR.
+ */
+int get_clip_distance_size(exec_list *ir)
+{
+   foreach_iter (exec_list_iterator, iter, *ir) {
+  ir_instruction *inst = (ir_instruction *)iter.get();
+  ir_variable *var = inst->as_variable();
+  if (var == NULL) continue;
+  if (!strcmp(var->name, "gl_ClipDistance"))
+  {
+ fprintf(stderr, "gl_ClipDistance found with size %i\n", 
var->type->length);
+ return var->type->length;
+  }
+   }
+   
+   return 0;
+}
+
 extern "C" {
 
 struct gl_shader *
@@ -5009,6 +5036,7 @@ st_new_shader_program(struct gl_context *ctx, GLuint name)
 GLboolean
 st_link_shader(struct gl_context *ctx, struct gl_shader_program *prog)
 {
+   int num_clip_distances[MESA_SHADER_TYPES];
assert(prog->LinkStatus);
 
for (unsigned i = 0; i < MESA_SHADER_TYPES; i++) {
@@ -5020,6 +5048,11 @@ st_link_shader(struct gl_context *ctx, struct 
gl_shader_program *prog)
   const struct gl_shader_compiler_options *options =
 
&ctx->ShaderCompilerOptions[_mesa_shader_type_to_index(prog->_LinkedShaders[i]->Type)];
 
+  /* We have to determine the length of the gl_ClipDistance array before 
the
+   * array is lowered to two vec4s by lower_clip_distance().
+   */
+  num_clip_distances[i] = get_clip_distance_size(ir);
+
   do {
  progress = false;
 
@@ -5036,6 +5069,7 @@ st_link_shader(struct gl_context *ctx, struct 
gl_shader_program *prog)
   || progress;
 
  progress = lower_quadop_vector(ir, false) || progress;
+ progress = lower_clip_distance(ir) || progress;
 
  if (options->MaxIfDepth == 0)
 progress = lower_discard(ir) || progress;
@@ -5070,7 +5104,8 @@ st_link_shader(struct gl_context *ctx, struct 
gl_shader_program *prog)
   if (prog->_LinkedShaders[i] == NULL)
  continue;
 
-  linked_prog = get_mesa_program(ctx, prog, prog->_LinkedShaders[i]);
+  linked_prog = get_mesa_program(ctx, prog, prog->_LinkedShaders[i],
+ num_clip_distances[i]);
 
   if (linked_prog) {
 static const GLenum targets[] = {
diff --git a/src/mesa/state_tracker/st_program.c 
b/src/mesa/state_tracker/st_program.c
index 04d3ef6..73581dd 100644
--- a/src/mesa/state_tracker/st_program.c
+++ b/src/mesa/state_tracker/st_program.c
@@ -244,6 +244,14 @@ st_prepare_vertex_program(struct gl_context *ctx,
 stvp->output_semantic_name[slot] = TGSI_SEMANTIC_PSIZE;
 stvp->output_semantic_index[slot] = 0;
 break;
+ case VERT_RESULT_CLIP_DIST0:
+stvp->output_semantic_name[slot] = TGSI_SEMANTIC_CLIPDIST;
+stvp->output_semantic_index[slot] = 0;
+break;
+ case VERT_RESULT_CLIP_DIST1:
+stvp->output_semantic_name[slot] = TGSI_SEMANTIC_CLIPDIST;
+stvp->output_semantic_index[slot] = 1;
+break;
  case VERT_RESULT_EDGE:
 assert(0);
 break;
@@ -541,6 +549,16 @@ st_translate_fragment_program(struct st_context *st,
   

[Mesa-dev] [PATCH] Enable display list support for glClearBuffer functions

2011-12-13 Thread Anuj Phogat
Enabling display list support for glClearBuffer functions with minor fixes

Signed-off-by: Anuj Phogat 
---
Tested this patch with a newly developed piglit testcase 
(clearbuffer-display-list).
Please refer to piglit mailing list for testcase patch.

 src/mesa/main/dlist.c |   22 +++---
 1 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/src/mesa/main/dlist.c b/src/mesa/main/dlist.c
index e1acc80..b3edae0 100644
--- a/src/mesa/main/dlist.c
+++ b/src/mesa/main/dlist.c
@@ -1422,7 +1422,7 @@ save_ClearBufferiv(GLenum buffer, GLint drawbuffer, const 
GLint *value)
   }
}
if (ctx->ExecuteFlag) {
-  /*CALL_ClearBufferiv(ctx->Exec, (buffer, drawbuffer, value));*/
+  CALL_ClearBufferiv(ctx->Exec, (buffer, drawbuffer, value));
}
 }
 
@@ -1450,7 +1450,7 @@ save_ClearBufferuiv(GLenum buffer, GLint drawbuffer, 
const GLuint *value)
   }
}
if (ctx->ExecuteFlag) {
-  /*CALL_ClearBufferuiv(ctx->Exec, (buffer, drawbuffer, value));*/
+  CALL_ClearBufferuiv(ctx->Exec, (buffer, drawbuffer, value));
}
 }
 
@@ -1478,7 +1478,7 @@ save_ClearBufferfv(GLenum buffer, GLint drawbuffer, const 
GLfloat *value)
   }
}
if (ctx->ExecuteFlag) {
-  /*CALL_ClearBufferuiv(ctx->Exec, (buffer, drawbuffer, value));*/
+  CALL_ClearBufferfv(ctx->Exec, (buffer, drawbuffer, value));
}
 }
 
@@ -1498,7 +1498,7 @@ save_ClearBufferfi(GLenum buffer, GLint drawbuffer,
   n[4].i = stencil;
}
if (ctx->ExecuteFlag) {
-  /*CALL_ClearBufferfi(ctx->Exec, (buffer, drawbuffer, depth, stencil));*/
+  CALL_ClearBufferfi(ctx->Exec, (buffer, drawbuffer, depth, stencil));
}
 }
 
@@ -7545,36 +7545,36 @@ execute_list(struct gl_context *ctx, GLuint list)
 break;
  case OPCODE_CLEAR_BUFFER_IV:
 {
-   /*GLint value[4];
+   GLint value[4];
value[0] = n[3].i;
value[1] = n[4].i;
value[2] = n[5].i;
value[3] = n[6].i;
-   CALL_ClearBufferiv(ctx->Exec, (n[1].e, n[2].i, value));*/
+   CALL_ClearBufferiv(ctx->Exec, (n[1].e, n[2].i, value));
 }
 break;
  case OPCODE_CLEAR_BUFFER_UIV:
 {
-   /*GLuint value[4];
+   GLuint value[4];
value[0] = n[3].ui;
value[1] = n[4].ui;
value[2] = n[5].ui;
value[3] = n[6].ui;
-   CALL_ClearBufferiv(ctx->Exec, (n[1].e, n[2].i, value));*/
+   CALL_ClearBufferuiv(ctx->Exec, (n[1].e, n[2].i, value));
 }
 break;
  case OPCODE_CLEAR_BUFFER_FV:
 {
-   /*GLfloat value[4];
+   GLfloat value[4];
value[0] = n[3].f;
value[1] = n[4].f;
value[2] = n[5].f;
value[3] = n[6].f;
-   CALL_ClearBufferfv(ctx->Exec, (n[1].e, n[2].i, value));*/
+   CALL_ClearBufferfv(ctx->Exec, (n[1].e, n[2].i, value));
 }
 break;
  case OPCODE_CLEAR_BUFFER_FI:
-/*CALL_ClearBufferfi(ctx->Exec, (n[1].e, n[2].i, n[3].f, 
n[4].i));*/
+CALL_ClearBufferfi(ctx->Exec, (n[1].e, n[2].i, n[3].f, n[4].i));
 break;
  case OPCODE_CLEAR_COLOR:
 CALL_ClearColor(ctx->Exec, (n[1].f, n[2].f, n[3].f, n[4].f));
-- 
1.7.7.4

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 0/2 v2] Add support for clip distances in Gallium

2011-12-13 Thread Jose Fonseca


- Original Message -
> This is an updated version of the patch set I sent to the list a few
> hours
> ago.  


> There is now a TGSI property called
> TGSI_PROPERTY_NUM_CLIP_DISTANCES
> that drivers can use to determine how many of the 8 available clip
> distances
> are actually used by a shader.

Can't the info in TGSI_PROPERTY_NUM_CLIP_DISTANCES be easily derived from the 
shader, and queried through src/gallium/auxiliary/tgsi/tgsi_scan.h ?


Could you also elaborate on why TGSI_SEMANTIC_CLIPDIST is useful for the 
drivers? I personally don't have nothing against it, but just like to 
understand why it makes a difference.

Jose
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 0/2 v2] Add support for clip distances in Gallium

2011-12-13 Thread Bryan Cain
On 12/13/2011 02:11 PM, Jose Fonseca wrote:
> - Original Message -
>> This is an updated version of the patch set I sent to the list a few
>> hours
>> ago.  
>> There is now a TGSI property called
>> TGSI_PROPERTY_NUM_CLIP_DISTANCES
>> that drivers can use to determine how many of the 8 available clip
>> distances
>> are actually used by a shader.
> Can't the info in TGSI_PROPERTY_NUM_CLIP_DISTANCES be easily derived from the 
> shader, and queried through src/gallium/auxiliary/tgsi/tgsi_scan.h ?

No.  The clip distances can be indirectly addressed (there are up to 2
of them in vec4 form for a total of 8 floats), which makes it impossible
to determine which ones are used by analyzing the shader.

> Could you also elaborate on why TGSI_SEMANTIC_CLIPDIST is useful for the 
> drivers? I personally don't have nothing against it, but just like to 
> understand why it makes a difference.
>
> Jose

Unless my understanding of clip distances is wrong, the GPU uses clip
distances to decide whether a vertex should be clipped, and thus needs
to know which of the vertex shader outputs are clip distances.

Bryan


___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 0/2 v2] Add support for clip distances in Gallium

2011-12-13 Thread Christoph Bumiller
On 12/13/2011 09:11 PM, Jose Fonseca wrote:
> 
> 
> - Original Message -
>> This is an updated version of the patch set I sent to the list a few
>> hours
>> ago.  
> 
> 
>> There is now a TGSI property called
>> TGSI_PROPERTY_NUM_CLIP_DISTANCES
>> that drivers can use to determine how many of the 8 available clip
>> distances
>> are actually used by a shader.
> 
> Can't the info in TGSI_PROPERTY_NUM_CLIP_DISTANCES be easily derived from the 
> shader, and queried through src/gallium/auxiliary/tgsi/tgsi_scan.h ?
> 
> 
> Could you also elaborate on why TGSI_SEMANTIC_CLIPDIST is useful for the 
> drivers? I personally don't have nothing against it, but just like to 
> understand why it makes a difference.
> 

Why does TGSI_SEMANTIC_*POSITION* make a difference ?

Right, because the position values are consumed by the fixed function
rasterizer. So are the clip distances.

This is not about pipe_clip_state.ucp but about what this legacy cruft
has to be turned into if GL_CLIP_PLANEi is used instead of GLSL 1.3's
gl_ClipDistance[i].

The same mentality ("What's information useful for ?") cost me
TGSI_SEMANTIC_TEXCOORD and now I have to rely on a hack to make point
coordinate replacement work on nvc0
(http://cgit.freedesktop.org/mesa/mesa/tree/src/gallium/drivers/nvc0/nvc0_program.c#n29).

I'm sorry I'm a bit sensitive on the issue of dropping information at
the gallium threshold.

> Jose
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 0/2 v2] Add support for clip distances in Gallium

2011-12-13 Thread Christoph Bumiller
On 12/13/2011 09:29 PM, Christoph Bumiller wrote:
> On 12/13/2011 09:11 PM, Jose Fonseca wrote:
>>
>>
>> - Original Message -
>>> This is an updated version of the patch set I sent to the list a few
>>> hours
>>> ago.  
>>
>>
>>> There is now a TGSI property called
>>> TGSI_PROPERTY_NUM_CLIP_DISTANCES
>>> that drivers can use to determine how many of the 8 available clip
>>> distances
>>> are actually used by a shader.
>>
>> Can't the info in TGSI_PROPERTY_NUM_CLIP_DISTANCES be easily derived from 
>> the shader, and queried through src/gallium/auxiliary/tgsi/tgsi_scan.h ?
>>
>>
>> Could you also elaborate on why TGSI_SEMANTIC_CLIPDIST is useful for the 
>> drivers? I personally don't have nothing against it, but just like to 
>> understand why it makes a difference.
>>
> 
> Why does TGSI_SEMANTIC_*POSITION* make a difference ?
> 
> Right, because the position values are consumed by the fixed function
> rasterizer. So are the clip distances.
> 
> This is not about pipe_clip_state.ucp but about what this legacy cruft
> has to be turned into if GL_CLIP_PLANEi is used instead of GLSL 1.3's
> gl_ClipDistance[i].
> 

That wasn't clear. I meant that UCP could be replaced by
TGSI_SEMANTIC_CLIPDIST (which is what drivers for newer cards
effectively do internally), but older cards still support UCPs
separately so it best be kept alive as well.
If the shader writes TGSI_SEMANTIC_CLIPDIST, UCP state can be ignored.


> The same mentality ("What's information useful for ?") cost me
> TGSI_SEMANTIC_TEXCOORD and now I have to rely on a hack to make point
> coordinate replacement work on nvc0
> (http://cgit.freedesktop.org/mesa/mesa/tree/src/gallium/drivers/nvc0/nvc0_program.c#n29).
> 
> I'm sorry I'm a bit sensitive on the issue of dropping information at
> the gallium threshold.
> 
>> Jose
>> ___
>> mesa-dev mailing list
>> mesa-dev@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
> 

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 0/2 v2] Add support for clip distances in Gallium

2011-12-13 Thread Ian Romanick

On 12/13/2011 12:26 PM, Bryan Cain wrote:

On 12/13/2011 02:11 PM, Jose Fonseca wrote:

- Original Message -

This is an updated version of the patch set I sent to the list a few
hours
ago.
There is now a TGSI property called
TGSI_PROPERTY_NUM_CLIP_DISTANCES
that drivers can use to determine how many of the 8 available clip
distances
are actually used by a shader.

Can't the info in TGSI_PROPERTY_NUM_CLIP_DISTANCES be easily derived from the 
shader, and queried through src/gallium/auxiliary/tgsi/tgsi_scan.h ?


No.  The clip distances can be indirectly addressed (there are up to 2
of them in vec4 form for a total of 8 floats), which makes it impossible
to determine which ones are used by analyzing the shader.


The description is almost complete. :)  The issue is that the shader may 
declare


out float gl_ClipDistance[4];

the use non-constant addressing of the array.  The compiler knows that 
gl_ClipDistance has at most 4 elements, but post-hoc analysis would not 
be able to determine that.  Often the fixed-function hardware (see 
below) needs to know which clip distance values are actually written.



Could you also elaborate on why TGSI_SEMANTIC_CLIPDIST is useful for the 
drivers? I personally don't have nothing against it, but just like to 
understand why it makes a difference.

Jose


Unless my understanding of clip distances is wrong, the GPU uses clip
distances to decide whether a vertex should be clipped, and thus needs
to know which of the vertex shader outputs are clip distances.


Right.  Clip distance is generally written to a special register that 
some fixed-function hardware uses to do clipping.

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] Enable display list support for glClearBuffer functions

2011-12-13 Thread Ian Romanick

On 12/13/2011 12:01 PM, Anuj Phogat wrote:

Enabling display list support for glClearBuffer functions with minor fixes


There's also a #if 0 block in that file that should be removed.  This is 
a case where 'git blame' is useful.  doing 'git blame 
src/mesa/main/dlist.c' shows that all of those lines were added in 
commit 05fb922e.  Looking at commit 05fb922e shows the #if 0 block.



Signed-off-by: Anuj Phogat
---
Tested this patch with a newly developed piglit testcase 
(clearbuffer-display-list).
Please refer to piglit mailing list for testcase patch.

  src/mesa/main/dlist.c |   22 +++---
  1 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/src/mesa/main/dlist.c b/src/mesa/main/dlist.c
index e1acc80..b3edae0 100644
--- a/src/mesa/main/dlist.c
+++ b/src/mesa/main/dlist.c
@@ -1422,7 +1422,7 @@ save_ClearBufferiv(GLenum buffer, GLint drawbuffer, const 
GLint *value)
}
 }
 if (ctx->ExecuteFlag) {
-  /*CALL_ClearBufferiv(ctx->Exec, (buffer, drawbuffer, value));*/
+  CALL_ClearBufferiv(ctx->Exec, (buffer, drawbuffer, value));
 }
  }

@@ -1450,7 +1450,7 @@ save_ClearBufferuiv(GLenum buffer, GLint drawbuffer, 
const GLuint *value)
}
 }
 if (ctx->ExecuteFlag) {
-  /*CALL_ClearBufferuiv(ctx->Exec, (buffer, drawbuffer, value));*/
+  CALL_ClearBufferuiv(ctx->Exec, (buffer, drawbuffer, value));
 }
  }

@@ -1478,7 +1478,7 @@ save_ClearBufferfv(GLenum buffer, GLint drawbuffer, const 
GLfloat *value)
}
 }
 if (ctx->ExecuteFlag) {
-  /*CALL_ClearBufferuiv(ctx->Exec, (buffer, drawbuffer, value));*/
+  CALL_ClearBufferfv(ctx->Exec, (buffer, drawbuffer, value));
 }
  }

@@ -1498,7 +1498,7 @@ save_ClearBufferfi(GLenum buffer, GLint drawbuffer,
n[4].i = stencil;
 }
 if (ctx->ExecuteFlag) {
-  /*CALL_ClearBufferfi(ctx->Exec, (buffer, drawbuffer, depth, stencil));*/
+  CALL_ClearBufferfi(ctx->Exec, (buffer, drawbuffer, depth, stencil));
 }
  }

@@ -7545,36 +7545,36 @@ execute_list(struct gl_context *ctx, GLuint list)
  break;
   case OPCODE_CLEAR_BUFFER_IV:
  {
-   /*GLint value[4];
+   GLint value[4];
 value[0] = n[3].i;
 value[1] = n[4].i;
 value[2] = n[5].i;
 value[3] = n[6].i;
-   CALL_ClearBufferiv(ctx->Exec, (n[1].e, n[2].i, value));*/
+   CALL_ClearBufferiv(ctx->Exec, (n[1].e, n[2].i, value));
  }
  break;
   case OPCODE_CLEAR_BUFFER_UIV:
  {
-   /*GLuint value[4];
+   GLuint value[4];
 value[0] = n[3].ui;
 value[1] = n[4].ui;
 value[2] = n[5].ui;
 value[3] = n[6].ui;
-   CALL_ClearBufferiv(ctx->Exec, (n[1].e, n[2].i, value));*/
+   CALL_ClearBufferuiv(ctx->Exec, (n[1].e, n[2].i, value));
  }
  break;
   case OPCODE_CLEAR_BUFFER_FV:
  {
-   /*GLfloat value[4];
+   GLfloat value[4];
 value[0] = n[3].f;
 value[1] = n[4].f;
 value[2] = n[5].f;
 value[3] = n[6].f;
-   CALL_ClearBufferfv(ctx->Exec, (n[1].e, n[2].i, value));*/
+   CALL_ClearBufferfv(ctx->Exec, (n[1].e, n[2].i, value));
  }
  break;
   case OPCODE_CLEAR_BUFFER_FI:
-/*CALL_ClearBufferfi(ctx->Exec, (n[1].e, n[2].i, n[3].f, 
n[4].i));*/
+CALL_ClearBufferfi(ctx->Exec, (n[1].e, n[2].i, n[3].f, n[4].i));
  break;
   case OPCODE_CLEAR_COLOR:
  CALL_ClearColor(ctx->Exec, (n[1].f, n[2].f, n[3].f, n[4].f));


___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH 1/3] swrast: Don't do Z24S8 drawpixels fast-paths with Z32_X24S8 input data.

2011-12-13 Thread Eric Anholt
The cool part was that in the "fbo-depthstencil -drawpixels
GL_DEPTH24_STENCIL8 32F_24_8_REV" testcase, the shifting happened to
end up with a value awfully close to the expected value, except for
every other pixel being 0 (the stencil value, shifted away to
nothing).
---
 src/mesa/swrast/s_drawpix.c |6 --
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/mesa/swrast/s_drawpix.c b/src/mesa/swrast/s_drawpix.c
index b6c4337..7259881 100644
--- a/src/mesa/swrast/s_drawpix.c
+++ b/src/mesa/swrast/s_drawpix.c
@@ -625,7 +625,8 @@ draw_depth_stencil_pixels(struct gl_context *ctx, GLint x, 
GLint y,
   GL_DEPTH_STENCIL_EXT, type, i, 0);
 
  if (ctx->Depth.Mask) {
-if (!scaleOrBias && ctx->DrawBuffer->Visual.depthBits == 24) {
+if (!scaleOrBias && ctx->DrawBuffer->Visual.depthBits == 24 &&
+   type == GL_UNSIGNED_INT_24_8) {
/* fast path 24-bit zbuffer */
GLuint zValues[MAX_WIDTH];
GLint j;
@@ -639,7 +640,8 @@ draw_depth_stencil_pixels(struct gl_context *ctx, GLint x, 
GLint y,
else
   depthRb->PutRow(ctx, depthRb, width, x, y + i, zValues,NULL);
 }
-else if (!scaleOrBias && ctx->DrawBuffer->Visual.depthBits == 16) {
+else if (!scaleOrBias && ctx->DrawBuffer->Visual.depthBits == 16 &&
+type == GL_UNSIGNED_INT_24_8) {
/* fast path 16-bit zbuffer */
GLushort zValues[MAX_WIDTH];
GLint j;
-- 
1.7.7.3

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH 3/3] swrast: Convert the simple glFramebufferBlit path to MapRenderbuffer.

2011-12-13 Thread Eric Anholt
There is one significant functional change here: the simple blit now
only handles exact matches of formats.  From the
ARB_framebuffer_object spec:

"If the color formats of the read and draw framebuffers do not
 match, and  includes COLOR_BUFFER_BIT, pixel groups are
 converted to match the destination format as in CopyPixels"

The previous code would generally do this, unless the RB DataTypes
didn't match, in which case it assertion failed.  Rather than adding
special-case format conversion code to the simple path, fall back to
the slow paths which will need to handle format conversion as well.
---
 src/mesa/swrast/s_blit.c |  188 +++--
 1 files changed, 96 insertions(+), 92 deletions(-)

diff --git a/src/mesa/swrast/s_blit.c b/src/mesa/swrast/s_blit.c
index 803ad2e..6f10c3a 100644
--- a/src/mesa/swrast/s_blit.c
+++ b/src/mesa/swrast/s_blit.c
@@ -454,7 +454,7 @@ blit_linear(struct gl_context *ctx,
  * Simple case:  Blit color, depth or stencil with no scaling or flipping.
  * XXX we could easily support vertical flipping here.
  */
-static void
+static GLboolean
 simple_blit(struct gl_context *ctx,
 GLint srcX0, GLint srcY0, GLint srcX1, GLint srcY1,
 GLint dstX0, GLint dstY0, GLint dstX1, GLint dstY1,
@@ -463,9 +463,10 @@ simple_blit(struct gl_context *ctx,
struct gl_renderbuffer *readRb, *drawRb;
const GLint width = srcX1 - srcX0;
const GLint height = srcY1 - srcY0;
-   GLint row, srcY, dstY, yStep;
-   GLint comps, bytesPerRow;
-   void *rowBuffer;
+   GLint row;
+   GLint bytesPerRow, srcStride, dstStride;
+   void *temp = NULL;
+   GLubyte *srcMap, *dstMap;
 
/* only one buffer */
ASSERT(_mesa_bitcount(buffer) == 1);
@@ -478,85 +479,93 @@ simple_blit(struct gl_context *ctx,
ASSERT(srcX1 - srcX0 == dstX1 - dstX0);
ASSERT(srcY1 - srcY0 == dstY1 - dstY0);
 
-   /* From the GL_ARB_framebuffer_object spec:
-*
-* "If the source and destination buffers are identical, and the source
-*  and destination rectangles overlap, the result of the blit operation
-*  is undefined."
-*
-* However, we provide the expected result anyway by flipping the order of
-* the memcpy of rows.
-*/
-   if (srcY0 > dstY0) {
-  /* src above dst: copy bottom-to-top */
-  yStep = 1;
-  srcY = srcY0;
-  dstY = dstY0;
-   }
-   else {
-  /* src below dst: copy top-to-bottom */
-  yStep = -1;
-  srcY = srcY1 - 1;
-  dstY = dstY1 - 1;
-   }
-
switch (buffer) {
case GL_COLOR_BUFFER_BIT:
   readRb = ctx->ReadBuffer->_ColorReadBuffer;
   drawRb = ctx->DrawBuffer->_ColorDrawBuffers[0];
-  comps = 4;
   break;
case GL_DEPTH_BUFFER_BIT:
-  readRb = ctx->ReadBuffer->_DepthBuffer;
-  drawRb = ctx->DrawBuffer->_DepthBuffer;
-  comps = 1;
+  readRb = ctx->ReadBuffer->Attachment[BUFFER_DEPTH].Renderbuffer;
+  drawRb = ctx->DrawBuffer->Attachment[BUFFER_DEPTH].Renderbuffer;
   break;
case GL_STENCIL_BUFFER_BIT:
-  readRb = ctx->ReadBuffer->_StencilBuffer;
-  drawRb = ctx->DrawBuffer->_StencilBuffer;
-  comps = 1;
+  readRb = ctx->ReadBuffer->Attachment[BUFFER_STENCIL].Renderbuffer;
+  drawRb = ctx->DrawBuffer->Attachment[BUFFER_STENCIL].Renderbuffer;
   break;
default:
   _mesa_problem(ctx, "unexpected buffer in simple_blit()");
-  return;
+  return GL_TRUE;
}
 
-   ASSERT(readRb->DataType == drawRb->DataType);
+   if (readRb->Format != drawRb->Format)
+  return GL_FALSE;
 
-   /* compute bytes per row */
-   switch (readRb->DataType) {
-   case GL_UNSIGNED_BYTE:
-  bytesPerRow = comps * width * sizeof(GLubyte);
-  break;
-   case GL_UNSIGNED_SHORT:
-  bytesPerRow = comps * width * sizeof(GLushort);
-  break;
-   case GL_UNSIGNED_INT:
-  bytesPerRow = comps * width * sizeof(GLuint);
-  break;
-   case GL_FLOAT:
-  bytesPerRow = comps * width * sizeof(GLfloat);
-  break;
-   default:
-  _mesa_problem(ctx, "unexpected buffer type in simple_blit");
-  return;
+   /* This path does direct memcpy of the data, and if the buffers are
+* depth/stencil, it would end up stomping over the other one of depth or
+* stencil even though it wasn't asked to.
+*/
+   if (_mesa_is_format_packed_depth_stencil(readRb->Format))
+  return GL_FALSE;
+
+   bytesPerRow = _mesa_get_format_bytes(readRb->Format) * width;
+
+   ctx->Driver.MapRenderbuffer(ctx, readRb, srcX0, srcY0, width, height,
+  GL_MAP_READ_BIT, &srcMap, &srcStride);
+   if (!srcMap) {
+  _mesa_error(ctx, GL_OUT_OF_MEMORY, "glBlitFrameBufferEXT");
+  return GL_TRUE;
+   }
+
+   /* From the GL_ARB_framebuffer_object spec:
+*
+* "If the source and destination buffers are identical, and the source
+*  and destination rectangles overlap, the result of the blit operation
+*  is undefined."
+*
+* However, we pr

[Mesa-dev] [PATCH 2/3] swrast: Add a note about overlapping support for framebuffer blit.

2011-12-13 Thread Eric Anholt
---
 src/mesa/swrast/s_blit.c |   10 +-
 1 files changed, 9 insertions(+), 1 deletions(-)

diff --git a/src/mesa/swrast/s_blit.c b/src/mesa/swrast/s_blit.c
index 2817ec1..803ad2e 100644
--- a/src/mesa/swrast/s_blit.c
+++ b/src/mesa/swrast/s_blit.c
@@ -478,7 +478,15 @@ simple_blit(struct gl_context *ctx,
ASSERT(srcX1 - srcX0 == dstX1 - dstX0);
ASSERT(srcY1 - srcY0 == dstY1 - dstY0);
 
-   /* determine if copy should be bottom-to-top or top-to-bottom */
+   /* From the GL_ARB_framebuffer_object spec:
+*
+* "If the source and destination buffers are identical, and the source
+*  and destination rectangles overlap, the result of the blit operation
+*  is undefined."
+*
+* However, we provide the expected result anyway by flipping the order of
+* the memcpy of rows.
+*/
if (srcY0 > dstY0) {
   /* src above dst: copy bottom-to-top */
   yStep = 1;
-- 
1.7.7.3

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 0/2 v2] Add support for clip distances in Gallium

2011-12-13 Thread Jose Fonseca


- Original Message -
> On 12/13/2011 09:11 PM, Jose Fonseca wrote:
> > 
> > 
> > - Original Message -
> >> This is an updated version of the patch set I sent to the list a
> >> few
> >> hours
> >> ago.
> > 
> > 
> >> There is now a TGSI property called
> >> TGSI_PROPERTY_NUM_CLIP_DISTANCES
> >> that drivers can use to determine how many of the 8 available clip
> >> distances
> >> are actually used by a shader.
> > 
> > Can't the info in TGSI_PROPERTY_NUM_CLIP_DISTANCES be easily
> > derived from the shader, and queried through
> > src/gallium/auxiliary/tgsi/tgsi_scan.h ?
> > 
> > 
> > Could you also elaborate on why TGSI_SEMANTIC_CLIPDIST is useful
> > for the drivers? I personally don't have nothing against it, but
> > just like to understand why it makes a difference.
> > 
> 
> Why does TGSI_SEMANTIC_*POSITION* make a difference ?
> 
> Right, because the position values are consumed by the fixed function
> rasterizer. So are the clip distances.
> 
> This is not about pipe_clip_state.ucp but about what this legacy
> cruft
> has to be turned into if GL_CLIP_PLANEi is used instead of GLSL 1.3's
> gl_ClipDistance[i].

I'm just surprised because I thought there was no more fixed function clipping.

Can you give an example of such legacy cruft?

> The same mentality ("What's information useful for ?") cost me
> TGSI_SEMANTIC_TEXCOORD and now I have to rely on a hack to make point
> coordinate replacement work on nvc0
> (http://cgit.freedesktop.org/mesa/mesa/tree/src/gallium/drivers/nvc0/nvc0_program.c#n29).
>
> I'm sorry I'm a bit sensitive on the issue of dropping information at
> the gallium threshold.

Please don't vent it on me.  As I explicitly said, I don't oppose (I imagined 
there was a good reason), and I'm asking for my own education.

And these are questions that always need to be asked.  Or even better, should 
be answered from the start -- commit messages in general, and interface changes 
in particular, should not just summarize what its being changed, but also _why_ 
it changed. The patch series included no pipe driver changes, so it's not 
possible even to induce from them.


Concerning TGSI_SEMANTIC_TEXCOORD I can't remember/comment if/what/why it was 
decided like that.  Please start a new thread and a link to any previous thread 
if you wanna revisit it.

I'm sorry the choices done in gallium cause you so much grief. But dropping 
information is a trade off between simple interface and a rich interface, and 
not inherently bad or good. The only question is have the right trade offs been 
made. And there's always room for improvement.


Jose
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 06/14] i965/vs: Implement vec4_visitor::generate_tex().

2011-12-13 Thread Eric Anholt
On Thu,  8 Dec 2011 17:07:57 -0800, Kenneth Graunke  
wrote:
> +   if (inst->header_present) {
> +  /* Set up an implied move from g0 to the MRF. */
> +  src = retype(brw_vec8_grf(0, 0), BRW_REGISTER_TYPE_UW);
> +   }

It doesn't seem like UW could be the right answer here -- I would think
UD is the right format, if it has to be changed at all (though it looks
like gen6_resolve_implied_move retypes anyway, so this retype is irrelevant).

Actually, now that we don't do ALT_MODE any more, I think we wouldn't
even need to retype away from float in most cases.


pgplvb2MriygI.pgp
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 1/2] gallium: add TGSI_SEMANTIC_CLIPDIST for clip distance

2011-12-13 Thread Brian Paul
On Tue, Dec 13, 2011 at 9:59 AM, Bryan Cain  wrote:
> ---
>  src/gallium/auxiliary/tgsi/tgsi_dump.c     |    3 ++-
>  src/gallium/include/pipe/p_shader_tokens.h |    3 ++-
>  2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/src/gallium/auxiliary/tgsi/tgsi_dump.c 
> b/src/gallium/auxiliary/tgsi/tgsi_dump.c
> index e830aa5..bd299b0 100644
> --- a/src/gallium/auxiliary/tgsi/tgsi_dump.c
> +++ b/src/gallium/auxiliary/tgsi/tgsi_dump.c
> @@ -129,7 +129,8 @@ static const char *semantic_names[] =
>    "PRIM_ID",
>    "INSTANCEID",
>    "VERTEXID",
> -   "STENCIL"
> +   "STENCIL",
> +   "CLIPDIST"
>  };
>
>  static const char *immediate_type_names[] =
> diff --git a/src/gallium/include/pipe/p_shader_tokens.h 
> b/src/gallium/include/pipe/p_shader_tokens.h
> index 10cfaf6..330e0ba 100644
> --- a/src/gallium/include/pipe/p_shader_tokens.h
> +++ b/src/gallium/include/pipe/p_shader_tokens.h
> @@ -146,7 +146,8 @@ struct tgsi_declaration_dimension
>  #define TGSI_SEMANTIC_INSTANCEID 10
>  #define TGSI_SEMANTIC_VERTEXID   11
>  #define TGSI_SEMANTIC_STENCIL    12
> -#define TGSI_SEMANTIC_COUNT      13 /**< number of semantic values */
> +#define TGSI_SEMANTIC_CLIPDIST   13
> +#define TGSI_SEMANTIC_COUNT      14 /**< number of semantic values */
>
>  struct tgsi_declaration_semantic
>  {

Reviewed-by: Brian Paul 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 0/2 v2] Add support for clip distances in Gallium

2011-12-13 Thread Jose Fonseca


- Original Message -
> On 12/13/2011 12:26 PM, Bryan Cain wrote:
> > On 12/13/2011 02:11 PM, Jose Fonseca wrote:
> >> - Original Message -
> >>> This is an updated version of the patch set I sent to the list a
> >>> few
> >>> hours
> >>> ago.
> >>> There is now a TGSI property called
> >>> TGSI_PROPERTY_NUM_CLIP_DISTANCES
> >>> that drivers can use to determine how many of the 8 available
> >>> clip
> >>> distances
> >>> are actually used by a shader.
> >> Can't the info in TGSI_PROPERTY_NUM_CLIP_DISTANCES be easily
> >> derived from the shader, and queried through
> >> src/gallium/auxiliary/tgsi/tgsi_scan.h ?
> >
> > No.  The clip distances can be indirectly addressed (there are up
> > to 2
> > of them in vec4 form for a total of 8 floats), which makes it
> > impossible
> > to determine which ones are used by analyzing the shader.
> 
> The description is almost complete. :)  The issue is that the shader
> may
> declare
> 
> out float gl_ClipDistance[4];
>
> the use non-constant addressing of the array.  The compiler knows
> that
> gl_ClipDistance has at most 4 elements, but post-hoc analysis would
> not
> be able to determine that.  Often the fixed-function hardware (see
> below) needs to know which clip distance values are actually written.

But don't all the clip distances written by the shader need to be declared?

E.g.:

DCL OUT[0], CLIPDIST[0]
DCL OUT[1], CLIPDIST[1]
DCL OUT[2], CLIPDIST[2]
DCL OUT[3], CLIPDIST[3]

therefore a trivial analysis of the declarations convey that?

> >> Could you also elaborate on why TGSI_SEMANTIC_CLIPDIST is useful
> >> for the drivers? I personally don't have nothing against it, but
> >> just like to understand why it makes a difference.
> >>
> >> Jose
> >
> > Unless my understanding of clip distances is wrong, the GPU uses
> > clip
> > distances to decide whether a vertex should be clipped, and thus
> > needs
> > to know which of the vertex shader outputs are clip distances.
> 
> Right.  Clip distance is generally written to a special register that
> some fixed-function hardware uses to do clipping.

Right. Don't know what I was thinking...

Jose
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 0/2 v2] Add support for clip distances in Gallium

2011-12-13 Thread Jose Fonseca


- Original Message -
> 
> 
> - Original Message -
> > On 12/13/2011 09:11 PM, Jose Fonseca wrote:
> > > 
> > > 
> > > - Original Message -
> > >> This is an updated version of the patch set I sent to the list a
> > >> few
> > >> hours
> > >> ago.
> > > 
> > > 
> > >> There is now a TGSI property called
> > >> TGSI_PROPERTY_NUM_CLIP_DISTANCES
> > >> that drivers can use to determine how many of the 8 available
> > >> clip
> > >> distances
> > >> are actually used by a shader.
> > > 
> > > Can't the info in TGSI_PROPERTY_NUM_CLIP_DISTANCES be easily
> > > derived from the shader, and queried through
> > > src/gallium/auxiliary/tgsi/tgsi_scan.h ?
> > > 
> > > 
> > > Could you also elaborate on why TGSI_SEMANTIC_CLIPDIST is useful
> > > for the drivers? I personally don't have nothing against it, but
> > > just like to understand why it makes a difference.
> > > 
> > 
> > Why does TGSI_SEMANTIC_*POSITION* make a difference ?
> > 
> > Right, because the position values are consumed by the fixed
> > function
> > rasterizer. So are the clip distances.
> > 
> > This is not about pipe_clip_state.ucp but about what this legacy
> > cruft
> > has to be turned into if GL_CLIP_PLANEi is used instead of GLSL
> > 1.3's
> > gl_ClipDistance[i].
> 
> I'm just surprised because I thought there was no more fixed function
> clipping.

Don't know what I was thinking. Please ignore.

> Can you give an example of such legacy cruft?

Jose
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 0/2 v2] Add support for clip distances in Gallium

2011-12-13 Thread Bryan Cain
On 12/13/2011 03:09 PM, Jose Fonseca wrote:
>
> - Original Message -
>> On 12/13/2011 12:26 PM, Bryan Cain wrote:
>>> On 12/13/2011 02:11 PM, Jose Fonseca wrote:
 - Original Message -
> This is an updated version of the patch set I sent to the list a
> few
> hours
> ago.
> There is now a TGSI property called
> TGSI_PROPERTY_NUM_CLIP_DISTANCES
> that drivers can use to determine how many of the 8 available
> clip
> distances
> are actually used by a shader.
 Can't the info in TGSI_PROPERTY_NUM_CLIP_DISTANCES be easily
 derived from the shader, and queried through
 src/gallium/auxiliary/tgsi/tgsi_scan.h ?
>>> No.  The clip distances can be indirectly addressed (there are up
>>> to 2
>>> of them in vec4 form for a total of 8 floats), which makes it
>>> impossible
>>> to determine which ones are used by analyzing the shader.
>> The description is almost complete. :)  The issue is that the shader
>> may
>> declare
>>
>> out float gl_ClipDistance[4];
>>
>> the use non-constant addressing of the array.  The compiler knows
>> that
>> gl_ClipDistance has at most 4 elements, but post-hoc analysis would
>> not
>> be able to determine that.  Often the fixed-function hardware (see
>> below) needs to know which clip distance values are actually written.
> But don't all the clip distances written by the shader need to be declared?
>
> E.g.:
>
> DCL OUT[0], CLIPDIST[0]
> DCL OUT[1], CLIPDIST[1]
> DCL OUT[2], CLIPDIST[2]
> DCL OUT[3], CLIPDIST[3]
>
> therefore a trivial analysis of the declarations convey that?

No.  Clip distance is an array of up to 8 floats in GLSL, but it's
represented in the hardware as 2 vec4s.  You can tell by analyzing the
declarations whether there are more than 4 clip distances in use, but
not which components the shader writes to. 
TGSI_PROPERTY_NUM_CLIP_DISTANCES is the number of components in use, not
the number of full vectors.

Bryan

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 07/14] i965/vs: Implement vec4_visitor::visit(ir_texture *).

2011-12-13 Thread Eric Anholt
On Thu,  8 Dec 2011 17:07:58 -0800, Kenneth Graunke  
wrote:
> This translates the GLSL compiler's IR into vec4_instruction IR,
> generating code to load coordinates, LOD info, shadow comparitors, and
> so on into the appropriate message registers.
> 
> It turns out that the SIMD4x2 parameters are identical on Gen 5-7, and
> the Gen4 code is similar enough that, unlike in the FS, it's easy enough
> to support all generations in a single function.
> 
> Signed-off-by: Kenneth Graunke 
> ---
>  src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp |  114 
> ++--
>  1 files changed, 107 insertions(+), 7 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp 
> b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
> index 853c3ee..85490bb 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp

> +   vec4_instruction *inst;
> +   switch (ir->op) {
> +   case ir_tex:

Isn't this one invalid for vertex shaders, too?

> +   case ir_txl:
> +  inst = new(mem_ctx) vec4_instruction(this, SHADER_OPCODE_TXL);
> +  break;
> +   case ir_txd:
> +  inst = new(mem_ctx) vec4_instruction(this, SHADER_OPCODE_TXD);
> +  break;
> +   case ir_txf:
> +  inst = new(mem_ctx) vec4_instruction(this, SHADER_OPCODE_TXF);
> +  break;
> +   case ir_txs:
> +  inst = new(mem_ctx) vec4_instruction(this, SHADER_OPCODE_TXS);
> +  break;
> +   case ir_txb:
> +  assert(!"TXB is not valid for vertex shaders.");
> +   }

> +  /* Load the LOD info */
> +  if (ir->op == ir_txl) {
> +  ir->lod_info.lod->accept(this);
> +  int mrf, writemask;
> +  if (intel->gen >= 5) {
> + mrf = param_base + 1;
> + writemask = ir->shadow_comparitor ? WRITEMASK_Y : WRITEMASK_X;
> + inst->mlen++;
> +  } else /* intel->gen == 4 */ {
> + mrf = param_base;
> + writemask = WRITEMASK_Z;
> +  }
> +  emit(MOV(dst_reg(MRF, mrf, ir->lod_info.lod->type, writemask),
> +   this->result));

I'd move the accept down next to the MOV.  I'm always afraid of the
hidden return data from accept getting trashed before it gets consumed.
I wish the visitor stuff let us do the sensible thing and produce a
return value.



pgpQn39oTYCht.pgp
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 1/3] swrast: Don't do Z24S8 drawpixels fast-paths with Z32_X24S8 input data.

2011-12-13 Thread Brian Paul
On Tue, Dec 13, 2011 at 1:53 PM, Eric Anholt  wrote:
> The cool part was that in the "fbo-depthstencil -drawpixels
> GL_DEPTH24_STENCIL8 32F_24_8_REV" testcase, the shifting happened to
> end up with a value awfully close to the expected value, except for
> every other pixel being 0 (the stencil value, shifted away to
> nothing).
> ---
>  src/mesa/swrast/s_drawpix.c |    6 --
>  1 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/src/mesa/swrast/s_drawpix.c b/src/mesa/swrast/s_drawpix.c
> index b6c4337..7259881 100644
> --- a/src/mesa/swrast/s_drawpix.c
> +++ b/src/mesa/swrast/s_drawpix.c
> @@ -625,7 +625,8 @@ draw_depth_stencil_pixels(struct gl_context *ctx, GLint 
> x, GLint y,
>                                   GL_DEPTH_STENCIL_EXT, type, i, 0);
>
>          if (ctx->Depth.Mask) {
> -            if (!scaleOrBias && ctx->DrawBuffer->Visual.depthBits == 24) {
> +            if (!scaleOrBias && ctx->DrawBuffer->Visual.depthBits == 24 &&
> +               type == GL_UNSIGNED_INT_24_8) {
>                /* fast path 24-bit zbuffer */
>                GLuint zValues[MAX_WIDTH];
>                GLint j;
> @@ -639,7 +640,8 @@ draw_depth_stencil_pixels(struct gl_context *ctx, GLint 
> x, GLint y,
>                else
>                   depthRb->PutRow(ctx, depthRb, width, x, y + i, 
> zValues,NULL);
>             }
> -            else if (!scaleOrBias && ctx->DrawBuffer->Visual.depthBits == 
> 16) {
> +            else if (!scaleOrBias && ctx->DrawBuffer->Visual.depthBits == 16 
> &&
> +                    type == GL_UNSIGNED_INT_24_8) {
>                /* fast path 16-bit zbuffer */
>                GLushort zValues[MAX_WIDTH];
>                GLint j;

Reviewed-by: Brian Paul 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 2/3] swrast: Add a note about overlapping support for framebuffer blit.

2011-12-13 Thread Brian Paul
On Tue, Dec 13, 2011 at 1:53 PM, Eric Anholt  wrote:
> ---
>  src/mesa/swrast/s_blit.c |   10 +-
>  1 files changed, 9 insertions(+), 1 deletions(-)
>
> diff --git a/src/mesa/swrast/s_blit.c b/src/mesa/swrast/s_blit.c
> index 2817ec1..803ad2e 100644
> --- a/src/mesa/swrast/s_blit.c
> +++ b/src/mesa/swrast/s_blit.c
> @@ -478,7 +478,15 @@ simple_blit(struct gl_context *ctx,
>    ASSERT(srcX1 - srcX0 == dstX1 - dstX0);
>    ASSERT(srcY1 - srcY0 == dstY1 - dstY0);
>
> -   /* determine if copy should be bottom-to-top or top-to-bottom */
> +   /* From the GL_ARB_framebuffer_object spec:
> +    *
> +    *     "If the source and destination buffers are identical, and the 
> source
> +    *      and destination rectangles overlap, the result of the blit 
> operation
> +    *      is undefined."
> +    *
> +    * However, we provide the expected result anyway by flipping the order of
> +    * the memcpy of rows.
> +    */
>    if (srcY0 > dstY0) {
>       /* src above dst: copy bottom-to-top */
>       yStep = 1;


Reviewed-by: Brian Paul 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 1/2] gallium: add TGSI_SEMANTIC_CLIPDIST for clip distance

2011-12-13 Thread Bryan Cain
On 12/13/2011 03:07 PM, Brian Paul wrote:
> On Tue, Dec 13, 2011 at 9:59 AM, Bryan Cain  wrote:
>> ---
>>  src/gallium/auxiliary/tgsi/tgsi_dump.c |3 ++-
>>  src/gallium/include/pipe/p_shader_tokens.h |3 ++-
>>  2 files changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/gallium/auxiliary/tgsi/tgsi_dump.c 
>> b/src/gallium/auxiliary/tgsi/tgsi_dump.c
>> index e830aa5..bd299b0 100644
>> --- a/src/gallium/auxiliary/tgsi/tgsi_dump.c
>> +++ b/src/gallium/auxiliary/tgsi/tgsi_dump.c
>> @@ -129,7 +129,8 @@ static const char *semantic_names[] =
>>"PRIM_ID",
>>"INSTANCEID",
>>"VERTEXID",
>> -   "STENCIL"
>> +   "STENCIL",
>> +   "CLIPDIST"
>>  };
>>
>>  static const char *immediate_type_names[] =
>> diff --git a/src/gallium/include/pipe/p_shader_tokens.h 
>> b/src/gallium/include/pipe/p_shader_tokens.h
>> index 10cfaf6..330e0ba 100644
>> --- a/src/gallium/include/pipe/p_shader_tokens.h
>> +++ b/src/gallium/include/pipe/p_shader_tokens.h
>> @@ -146,7 +146,8 @@ struct tgsi_declaration_dimension
>>  #define TGSI_SEMANTIC_INSTANCEID 10
>>  #define TGSI_SEMANTIC_VERTEXID   11
>>  #define TGSI_SEMANTIC_STENCIL12
>> -#define TGSI_SEMANTIC_COUNT  13 /**< number of semantic values */
>> +#define TGSI_SEMANTIC_CLIPDIST   13
>> +#define TGSI_SEMANTIC_COUNT  14 /**< number of semantic values */
>>
>>  struct tgsi_declaration_semantic
>>  {
> Reviewed-by: Brian Paul http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 0/2 v2] Add support for clip distances in Gallium

2011-12-13 Thread Christoph Bumiller
On 12/13/2011 09:58 PM, Jose Fonseca wrote:
> 
> 
> - Original Message -
>> On 12/13/2011 09:11 PM, Jose Fonseca wrote:
>>>
>>>
>>> - Original Message -
 This is an updated version of the patch set I sent to the list a
 few
 hours
 ago.
>>>
>>>
 There is now a TGSI property called
 TGSI_PROPERTY_NUM_CLIP_DISTANCES
 that drivers can use to determine how many of the 8 available clip
 distances
 are actually used by a shader.
>>>
>>> Can't the info in TGSI_PROPERTY_NUM_CLIP_DISTANCES be easily
>>> derived from the shader, and queried through
>>> src/gallium/auxiliary/tgsi/tgsi_scan.h ?
>>>
>>>
>>> Could you also elaborate on why TGSI_SEMANTIC_CLIPDIST is useful
>>> for the drivers? I personally don't have nothing against it, but
>>> just like to understand why it makes a difference.
>>>
>>
>> Why does TGSI_SEMANTIC_*POSITION* make a difference ?
>>
>> Right, because the position values are consumed by the fixed function
>> rasterizer. So are the clip distances.
>>
>> This is not about pipe_clip_state.ucp but about what this legacy
>> cruft
>> has to be turned into if GL_CLIP_PLANEi is used instead of GLSL 1.3's
>> gl_ClipDistance[i].
> 
> I'm just surprised because I thought there was no more fixed function 
> clipping.
> 

nvfx and r300, and I think even r600, are probably happy to have UCPs in
the interface and can make direct use of them, see nvfx_ucp_validate in
nvfx_state_emit.c or r300_set_clip_state in r300_state.c

nv50 and nvc0 have to use the same shader outputs for both legacy
glClipPlanes and gl_ClipDistance[].

On nv50, there is shader-external state that designates which outputs
are clip distances, on nvc0 they have to be written to special output
slots/addresses so the semantic information is important to be able to
assign slots at compile time (the GPU was specially designed to make
this possible and to support ARB_separate_shader_objects nicely).

In D3D10/11, there is a system value semantic for clip distances and
cull distances (cull distance is a different clipping mode and consumes
the same slots as clip distances).
See
http://msdn.microsoft.com/en-us/library/windows/desktop/bb509647%28v=vs.85%29.aspx.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 13/14] i965/vs: Implement EXT_texture_swizzle support for VS texturing.

2011-12-13 Thread Eric Anholt
On Thu,  8 Dec 2011 17:08:04 -0800, Kenneth Graunke  
wrote:
> Signed-off-by: Kenneth Graunke 
> +   if (one_mask) {
> +  swizzled_result.writemask = one_mask;
> +  emit(MOV(swizzled_result, src_reg(1.0f)));
> +   }
>  }

I think this would be wrong for SWIZZLE_ONE of integer textures.


pgpNAonxY7Alw.pgp
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] i965 vertex shader texturing support

2011-12-13 Thread Eric Anholt
On Thu,  8 Dec 2011 17:07:51 -0800, Kenneth Graunke  
wrote:
> This series adds vertex shader texturing support to the i965 driver, as
> required by OpenGL 3.0.  Only two things remain: textureGrad() on shadow
> samplers (which isn't supported in hardware), and coordinate clamping to
> support proper filtering at texture borders.
> 
> While I added code for texturing on all generations, I'm only enabling it
> on Gen6-7 for now due to lack of testing: my texelFetch and textureSize
> tests fail to run on Gen4-5 due to unrelated interpolation bugs.
> 
> Thanks in advance for the review!

Review sent out, patches with no reply are

Reviewed-by: Eric Anholt 


pgpR3UQCheW6w.pgp
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 01/11] mesa: implement DrawTransformFeedback from ARB_transform_feedback2

2011-12-13 Thread Christoph Bumiller
On 12/12/2011 05:37 PM, Jose Fonseca wrote:
> - Original Message -
>> On 12/12/2011 02:49 PM, Jose Fonseca wrote:
>>> - Original Message -
 From: Marek Olšák 

 It's like DrawArrays, but the count is taken from a transform
 feedback
 object.

 This removes DrawTransformFeedback from dd_function_table and adds
 the same
 function to GLvertexformat (with the function parameters matching
 GL).

 The vbo_draw_func callback has a new parameter
 "struct gl_transform_feedback_object *tfb_vertcount".

 The rest of the code just validates states and forwards the
 transform
 feedback object into vbo_draw_func.
>>>
>>> I ventured reviewing this too, and I have a question: the
>>> unfinished code in master had comments saying
>>>
>>>/*
>>> * Get number of vertices in obj's feedback buffer.
>>> * Call ctx->Exec.DrawArrays(mode, 0, count);
>>> */
>>>
>>> which seems simpler than passing the new tfb_vertcount parameter
>>> everywhere, just for the sake of obtaining count, so what's the
>>> rationale for doing this differently here?
>>>
>>
>> The count is "contained" in the TFB object (through its association
>> with
>> a pipe_stream_output_target which tracks the amount of vertices
>> written
>> to it) and is not supposed to be read back by the CPU because that
>> would
>> mean waiting.
>>
>> The driver will let the GPU write the count into the command buffer,
>> so
>> it only needs to sync command processing with TFB.
> 
> I see. Makes sense.
> 
> I have no more objections against the series FWIW.
> 

Thank you very much for reviewing the changes.

If no one comes along with objections/improvements Marek or I will push
the series in a few days then.

> It would be nice to fix softpipe/llvmpipe though, but IIUC they could not 
> possibly have worked with Mesa before, and I don't think anybody is relying 
> on that softpipe/llvmpipe functionality w/ other state trackers at this 
> point, so I suppose it could wait.
> 
> Jose
> 

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 3/3] swrast: Convert the simple glFramebufferBlit path to MapRenderbuffer.

2011-12-13 Thread Brian Paul
On Tue, Dec 13, 2011 at 1:53 PM, Eric Anholt  wrote:
> There is one significant functional change here: the simple blit now
> only handles exact matches of formats.  From the
> ARB_framebuffer_object spec:
>
>    "If the color formats of the read and draw framebuffers do not
>     match, and  includes COLOR_BUFFER_BIT, pixel groups are
>     converted to match the destination format as in CopyPixels"
>
> The previous code would generally do this, unless the RB DataTypes
> didn't match, in which case it assertion failed.  Rather than adding
> special-case format conversion code to the simple path, fall back to
> the slow paths which will need to handle format conversion as well.
> ---
>  src/mesa/swrast/s_blit.c |  188 +++--
>  1 files changed, 96 insertions(+), 92 deletions(-)
>
> diff --git a/src/mesa/swrast/s_blit.c b/src/mesa/swrast/s_blit.c
> index 803ad2e..6f10c3a 100644
> --- a/src/mesa/swrast/s_blit.c
> +++ b/src/mesa/swrast/s_blit.c
> @@ -454,7 +454,7 @@ blit_linear(struct gl_context *ctx,
>  * Simple case:  Blit color, depth or stencil with no scaling or flipping.
>  * XXX we could easily support vertical flipping here.
>  */
> -static void
> +static GLboolean
>  simple_blit(struct gl_context *ctx,
>             GLint srcX0, GLint srcY0, GLint srcX1, GLint srcY1,
>             GLint dstX0, GLint dstY0, GLint dstX1, GLint dstY1,
> @@ -463,9 +463,10 @@ simple_blit(struct gl_context *ctx,
>    struct gl_renderbuffer *readRb, *drawRb;
>    const GLint width = srcX1 - srcX0;
>    const GLint height = srcY1 - srcY0;
> -   GLint row, srcY, dstY, yStep;
> -   GLint comps, bytesPerRow;
> -   void *rowBuffer;
> +   GLint row;
> +   GLint bytesPerRow, srcStride, dstStride;
> +   void *temp = NULL;
> +   GLubyte *srcMap, *dstMap;
>
>    /* only one buffer */
>    ASSERT(_mesa_bitcount(buffer) == 1);
> @@ -478,85 +479,93 @@ simple_blit(struct gl_context *ctx,
>    ASSERT(srcX1 - srcX0 == dstX1 - dstX0);
>    ASSERT(srcY1 - srcY0 == dstY1 - dstY0);
>
> -   /* From the GL_ARB_framebuffer_object spec:
> -    *
> -    *     "If the source and destination buffers are identical, and the 
> source
> -    *      and destination rectangles overlap, the result of the blit 
> operation
> -    *      is undefined."
> -    *
> -    * However, we provide the expected result anyway by flipping the order of
> -    * the memcpy of rows.
> -    */
> -   if (srcY0 > dstY0) {
> -      /* src above dst: copy bottom-to-top */
> -      yStep = 1;
> -      srcY = srcY0;
> -      dstY = dstY0;
> -   }
> -   else {
> -      /* src below dst: copy top-to-bottom */
> -      yStep = -1;
> -      srcY = srcY1 - 1;
> -      dstY = dstY1 - 1;
> -   }
> -
>    switch (buffer) {
>    case GL_COLOR_BUFFER_BIT:
>       readRb = ctx->ReadBuffer->_ColorReadBuffer;
>       drawRb = ctx->DrawBuffer->_ColorDrawBuffers[0];
> -      comps = 4;
>       break;
>    case GL_DEPTH_BUFFER_BIT:
> -      readRb = ctx->ReadBuffer->_DepthBuffer;
> -      drawRb = ctx->DrawBuffer->_DepthBuffer;
> -      comps = 1;
> +      readRb = ctx->ReadBuffer->Attachment[BUFFER_DEPTH].Renderbuffer;
> +      drawRb = ctx->DrawBuffer->Attachment[BUFFER_DEPTH].Renderbuffer;
>       break;
>    case GL_STENCIL_BUFFER_BIT:
> -      readRb = ctx->ReadBuffer->_StencilBuffer;
> -      drawRb = ctx->DrawBuffer->_StencilBuffer;
> -      comps = 1;
> +      readRb = ctx->ReadBuffer->Attachment[BUFFER_STENCIL].Renderbuffer;
> +      drawRb = ctx->DrawBuffer->Attachment[BUFFER_STENCIL].Renderbuffer;
>       break;
>    default:
>       _mesa_problem(ctx, "unexpected buffer in simple_blit()");
> -      return;
> +      return GL_TRUE;
>    }
>
> -   ASSERT(readRb->DataType == drawRb->DataType);
> +   if (readRb->Format != drawRb->Format)
> +      return GL_FALSE;
>
> -   /* compute bytes per row */
> -   switch (readRb->DataType) {
> -   case GL_UNSIGNED_BYTE:
> -      bytesPerRow = comps * width * sizeof(GLubyte);
> -      break;
> -   case GL_UNSIGNED_SHORT:
> -      bytesPerRow = comps * width * sizeof(GLushort);
> -      break;
> -   case GL_UNSIGNED_INT:
> -      bytesPerRow = comps * width * sizeof(GLuint);
> -      break;
> -   case GL_FLOAT:
> -      bytesPerRow = comps * width * sizeof(GLfloat);
> -      break;
> -   default:
> -      _mesa_problem(ctx, "unexpected buffer type in simple_blit");
> -      return;
> +   /* This path does direct memcpy of the data, and if the buffers are
> +    * depth/stencil, it would end up stomping over the other one of depth or
> +    * stencil even though it wasn't asked to.
> +    */
> +   if (_mesa_is_format_packed_depth_stencil(readRb->Format))
> +      return GL_FALSE;
> +
> +   bytesPerRow = _mesa_get_format_bytes(readRb->Format) * width;
> +
> +   ctx->Driver.MapRenderbuffer(ctx, readRb, srcX0, srcY0, width, height,
> +                              GL_MAP_READ_BIT, &srcMap, &srcStride);
> +   if (!srcMap) {
> +      _mesa_error(ctx, GL_OUT_OF_MEMORY, "glBlitFrameBufferEXT");
> +      return GL_TRUE

Re: [Mesa-dev] [PATCH 0/2 v2] Add support for clip distances in Gallium

2011-12-13 Thread Jose Fonseca


- Original Message -
> On 12/13/2011 03:09 PM, Jose Fonseca wrote:
> >
> > - Original Message -
> >> On 12/13/2011 12:26 PM, Bryan Cain wrote:
> >>> On 12/13/2011 02:11 PM, Jose Fonseca wrote:
>  - Original Message -
> > This is an updated version of the patch set I sent to the list
> > a
> > few
> > hours
> > ago.
> > There is now a TGSI property called
> > TGSI_PROPERTY_NUM_CLIP_DISTANCES
> > that drivers can use to determine how many of the 8 available
> > clip
> > distances
> > are actually used by a shader.
>  Can't the info in TGSI_PROPERTY_NUM_CLIP_DISTANCES be easily
>  derived from the shader, and queried through
>  src/gallium/auxiliary/tgsi/tgsi_scan.h ?
> >>> No.  The clip distances can be indirectly addressed (there are up
> >>> to 2
> >>> of them in vec4 form for a total of 8 floats), which makes it
> >>> impossible
> >>> to determine which ones are used by analyzing the shader.
> >> The description is almost complete. :)  The issue is that the
> >> shader
> >> may
> >> declare
> >>
> >> out float gl_ClipDistance[4];
> >>
> >> the use non-constant addressing of the array.  The compiler knows
> >> that
> >> gl_ClipDistance has at most 4 elements, but post-hoc analysis
> >> would
> >> not
> >> be able to determine that.  Often the fixed-function hardware (see
> >> below) needs to know which clip distance values are actually
> >> written.
> > But don't all the clip distances written by the shader need to be
> > declared?
> >
> > E.g.:
> >
> > DCL OUT[0], CLIPDIST[0]
> > DCL OUT[1], CLIPDIST[1]
> > DCL OUT[2], CLIPDIST[2]
> > DCL OUT[3], CLIPDIST[3]
> >
> > therefore a trivial analysis of the declarations convey that?
> 
> No.  Clip distance is an array of up to 8 floats in GLSL, but it's
> represented in the hardware as 2 vec4s.  You can tell by analyzing
> the
> declarations whether there are more than 4 clip distances in use, but
> not which components the shader writes to.
> TGSI_PROPERTY_NUM_CLIP_DISTANCES is the number of components in use,
> not
> the number of full vectors.

Lets imagine 

  out float gl_ClipDistance[6];

Each a clip distance is a scalar float.

Either all hardware represents the 8 clip distances as two 4 vectors, and we do:

  DCL OUT[0].xywz, CLIPDIST[0]
  DCL OUT[1].xy, CLIPDIST[1]

using the full range of struct tgsi_declaration::UsageMask [1] or we represent 
them as as scalars:

  DCL OUT[0].x, CLIPDIST[0]
  DCL OUT[1].x, CLIPDIST[1]
  DCL OUT[2].x, CLIPDIST[2]
  DCL OUT[3].x, CLIPDIST[3]
  DCL OUT[4].x, CLIPDIST[4]
  DCL OUT[5].x, CLIPDIST[5]

If indirect addressing is allowed as I read bore, then maybe the later is 
better.

I confess my ignorance about clipping and maybe I'm being dense, but I still 
don't see the need for this TGSI_PROPERTY_NUM_CLIP_DISTANCES.  Could you please 
draft an example TGSI shader showing this property (or just paste one generated 
with your change)?  I think that would help a lot.


Jose


[1] I don't know if tgsi_dump pays much attention to  
tgsi_declaration::UsageMask, but it does exist.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 0/2 v2] Add support for clip distances in Gallium

2011-12-13 Thread Bryan Cain
On 12/13/2011 03:25 PM, Jose Fonseca wrote:
>
> - Original Message -
>> On 12/13/2011 03:09 PM, Jose Fonseca wrote:
>>> - Original Message -
 On 12/13/2011 12:26 PM, Bryan Cain wrote:
> On 12/13/2011 02:11 PM, Jose Fonseca wrote:
>> - Original Message -
>>> This is an updated version of the patch set I sent to the list
>>> a
>>> few
>>> hours
>>> ago.
>>> There is now a TGSI property called
>>> TGSI_PROPERTY_NUM_CLIP_DISTANCES
>>> that drivers can use to determine how many of the 8 available
>>> clip
>>> distances
>>> are actually used by a shader.
>> Can't the info in TGSI_PROPERTY_NUM_CLIP_DISTANCES be easily
>> derived from the shader, and queried through
>> src/gallium/auxiliary/tgsi/tgsi_scan.h ?
> No.  The clip distances can be indirectly addressed (there are up
> to 2
> of them in vec4 form for a total of 8 floats), which makes it
> impossible
> to determine which ones are used by analyzing the shader.
 The description is almost complete. :)  The issue is that the
 shader
 may
 declare

 out float gl_ClipDistance[4];

 the use non-constant addressing of the array.  The compiler knows
 that
 gl_ClipDistance has at most 4 elements, but post-hoc analysis
 would
 not
 be able to determine that.  Often the fixed-function hardware (see
 below) needs to know which clip distance values are actually
 written.
>>> But don't all the clip distances written by the shader need to be
>>> declared?
>>>
>>> E.g.:
>>>
>>> DCL OUT[0], CLIPDIST[0]
>>> DCL OUT[1], CLIPDIST[1]
>>> DCL OUT[2], CLIPDIST[2]
>>> DCL OUT[3], CLIPDIST[3]
>>>
>>> therefore a trivial analysis of the declarations convey that?
>> No.  Clip distance is an array of up to 8 floats in GLSL, but it's
>> represented in the hardware as 2 vec4s.  You can tell by analyzing
>> the
>> declarations whether there are more than 4 clip distances in use, but
>> not which components the shader writes to.
>> TGSI_PROPERTY_NUM_CLIP_DISTANCES is the number of components in use,
>> not
>> the number of full vectors.
> Lets imagine 
>
>   out float gl_ClipDistance[6];
>
> Each a clip distance is a scalar float.
>
> Either all hardware represents the 8 clip distances as two 4 vectors, and we 
> do:
>
>   DCL OUT[0].xywz, CLIPDIST[0]
>   DCL OUT[1].xy, CLIPDIST[1]
>
> using the full range of struct tgsi_declaration::UsageMask [1] or we 
> represent them as as scalars:
>
>   DCL OUT[0].x, CLIPDIST[0]
>   DCL OUT[1].x, CLIPDIST[1]
>   DCL OUT[2].x, CLIPDIST[2]
>   DCL OUT[3].x, CLIPDIST[3]
>   DCL OUT[4].x, CLIPDIST[4]
>   DCL OUT[5].x, CLIPDIST[5]
>
> If indirect addressing is allowed as I read bore, then maybe the later is 
> better.
>
> I confess my ignorance about clipping and maybe I'm being dense, but I still 
> don't see the need for this TGSI_PROPERTY_NUM_CLIP_DISTANCES.  Could you 
> please draft an example TGSI shader showing this property (or just paste one 
> generated with your change)?  I think that would help a lot.
>
>
> Jose
>
>
> [1] I don't know if tgsi_dump pays much attention to  
> tgsi_declaration::UsageMask, but it does exist.

UsageMask might work, but before that can be considered a viable
solution, someone will need to make it possible to actually declare it
from ureg.  As it is, ureg is hardcoded to set UsageMask to xyzw no
matter what on all declared inputs and outputs.

Bryan

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 0/2 v2] Add support for clip distances in Gallium

2011-12-13 Thread Jose Fonseca
- Original Message -
> On 12/13/2011 03:25 PM, Jose Fonseca wrote:
> >
> > - Original Message -
> >> On 12/13/2011 03:09 PM, Jose Fonseca wrote:
> >>> - Original Message -
>  On 12/13/2011 12:26 PM, Bryan Cain wrote:
> > On 12/13/2011 02:11 PM, Jose Fonseca wrote:
> >> - Original Message -
> >>> This is an updated version of the patch set I sent to the
> >>> list
> >>> a
> >>> few
> >>> hours
> >>> ago.
> >>> There is now a TGSI property called
> >>> TGSI_PROPERTY_NUM_CLIP_DISTANCES
> >>> that drivers can use to determine how many of the 8 available
> >>> clip
> >>> distances
> >>> are actually used by a shader.
> >> Can't the info in TGSI_PROPERTY_NUM_CLIP_DISTANCES be easily
> >> derived from the shader, and queried through
> >> src/gallium/auxiliary/tgsi/tgsi_scan.h ?
> > No.  The clip distances can be indirectly addressed (there are
> > up
> > to 2
> > of them in vec4 form for a total of 8 floats), which makes it
> > impossible
> > to determine which ones are used by analyzing the shader.
>  The description is almost complete. :)  The issue is that the
>  shader
>  may
>  declare
> 
>  out float gl_ClipDistance[4];
> 
>  the use non-constant addressing of the array.  The compiler
>  knows
>  that
>  gl_ClipDistance has at most 4 elements, but post-hoc analysis
>  would
>  not
>  be able to determine that.  Often the fixed-function hardware
>  (see
>  below) needs to know which clip distance values are actually
>  written.
> >>> But don't all the clip distances written by the shader need to be
> >>> declared?
> >>>
> >>> E.g.:
> >>>
> >>> DCL OUT[0], CLIPDIST[0]
> >>> DCL OUT[1], CLIPDIST[1]
> >>> DCL OUT[2], CLIPDIST[2]
> >>> DCL OUT[3], CLIPDIST[3]
> >>>
> >>> therefore a trivial analysis of the declarations convey that?
> >> No.  Clip distance is an array of up to 8 floats in GLSL, but it's
> >> represented in the hardware as 2 vec4s.  You can tell by analyzing
> >> the
> >> declarations whether there are more than 4 clip distances in use,
> >> but
> >> not which components the shader writes to.
> >> TGSI_PROPERTY_NUM_CLIP_DISTANCES is the number of components in
> >> use,
> >> not
> >> the number of full vectors.
> > Lets imagine
> >
> >   out float gl_ClipDistance[6];
> >
> > Each a clip distance is a scalar float.
> >
> > Either all hardware represents the 8 clip distances as two 4
> > vectors, and we do:
> >
> >   DCL OUT[0].xywz, CLIPDIST[0]
> >   DCL OUT[1].xy, CLIPDIST[1]
> >
> > using the full range of struct tgsi_declaration::UsageMask [1] or
> > we represent them as as scalars:
> >
> >   DCL OUT[0].x, CLIPDIST[0]
> >   DCL OUT[1].x, CLIPDIST[1]
> >   DCL OUT[2].x, CLIPDIST[2]
> >   DCL OUT[3].x, CLIPDIST[3]
> >   DCL OUT[4].x, CLIPDIST[4]
> >   DCL OUT[5].x, CLIPDIST[5]
> >
> > If indirect addressing is allowed as I read bore, then maybe the
> > later is better.
> >
> > I confess my ignorance about clipping and maybe I'm being dense,
> > but I still don't see the need for this
> > TGSI_PROPERTY_NUM_CLIP_DISTANCES.  Could you please draft an
> > example TGSI shader showing this property (or just paste one
> > generated with your change)?  I think that would help a lot.
> >
> >
> > Jose
> >
> >
> > [1] I don't know if tgsi_dump pays much attention to
> >  tgsi_declaration::UsageMask, but it does exist.
> 
> UsageMask might work, but before that can be considered a viable
> solution, someone will need to make it possible to actually declare
> it
> from ureg.  As it is, ureg is hardcoded to set UsageMask to xyzw no
> matter what on all declared inputs and outputs.

ureg automatically fills the UsageMask from the destionation register masks, 
since it easy to determine from the opcodes.

Which leads me to my second point, if indirect addressing of CLIPDIST is 
allowed, then we can't really pack the clip distance as 4-elem vectors in TGSI: 
not only the syntax would be very weird, but it would create havoc on all 
tgsi-translating code that makes decisions based on indirect addressing of 
registers.

That is, 

  float gl_ClipDistance[6];

  gl_ClipDistance[i] = foo;

would become

   DCL OUT[0].x, CLIPDIST[0]
   DCL OUT[1].x, CLIPDIST[1]
   DCL OUT[2].x, CLIPDIST[2]
   DCL OUT[3].x, CLIPDIST[3]
   DCL OUT[4].x, CLIPDIST[4]
   DCL OUT[5].x, CLIPDIST[5]
   MOV OUT[ADDR[0].x].x, foo

and the info from TGSI_PROPERTY_NUM_CLIP_DISTANCES can be obtained by walking 
the declaration (which can/should be done only once in tgsi_scan).

But this just doesn't look like it would ever work:

   DCL OUT[0].xyzw, CLIPDIST[0]
   DCL OUT[1].xy  , CLIPDIST[1]
   MOV OUT[ADDR[0].x]., foo

Jose
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 0/2 v2] Add support for clip distances in Gallium

2011-12-13 Thread Bryan Cain
On 12/13/2011 03:48 PM, Jose Fonseca wrote:
> - Original Message -
>> On 12/13/2011 03:25 PM, Jose Fonseca wrote:
>>> - Original Message -
 On 12/13/2011 03:09 PM, Jose Fonseca wrote:
> - Original Message -
>> On 12/13/2011 12:26 PM, Bryan Cain wrote:
>>> On 12/13/2011 02:11 PM, Jose Fonseca wrote:
 - Original Message -
> This is an updated version of the patch set I sent to the
> list
> a
> few
> hours
> ago.
> There is now a TGSI property called
> TGSI_PROPERTY_NUM_CLIP_DISTANCES
> that drivers can use to determine how many of the 8 available
> clip
> distances
> are actually used by a shader.
 Can't the info in TGSI_PROPERTY_NUM_CLIP_DISTANCES be easily
 derived from the shader, and queried through
 src/gallium/auxiliary/tgsi/tgsi_scan.h ?
>>> No.  The clip distances can be indirectly addressed (there are
>>> up
>>> to 2
>>> of them in vec4 form for a total of 8 floats), which makes it
>>> impossible
>>> to determine which ones are used by analyzing the shader.
>> The description is almost complete. :)  The issue is that the
>> shader
>> may
>> declare
>>
>> out float gl_ClipDistance[4];
>>
>> the use non-constant addressing of the array.  The compiler
>> knows
>> that
>> gl_ClipDistance has at most 4 elements, but post-hoc analysis
>> would
>> not
>> be able to determine that.  Often the fixed-function hardware
>> (see
>> below) needs to know which clip distance values are actually
>> written.
> But don't all the clip distances written by the shader need to be
> declared?
>
> E.g.:
>
> DCL OUT[0], CLIPDIST[0]
> DCL OUT[1], CLIPDIST[1]
> DCL OUT[2], CLIPDIST[2]
> DCL OUT[3], CLIPDIST[3]
>
> therefore a trivial analysis of the declarations convey that?
 No.  Clip distance is an array of up to 8 floats in GLSL, but it's
 represented in the hardware as 2 vec4s.  You can tell by analyzing
 the
 declarations whether there are more than 4 clip distances in use,
 but
 not which components the shader writes to.
 TGSI_PROPERTY_NUM_CLIP_DISTANCES is the number of components in
 use,
 not
 the number of full vectors.
>>> Lets imagine
>>>
>>>   out float gl_ClipDistance[6];
>>>
>>> Each a clip distance is a scalar float.
>>>
>>> Either all hardware represents the 8 clip distances as two 4
>>> vectors, and we do:
>>>
>>>   DCL OUT[0].xywz, CLIPDIST[0]
>>>   DCL OUT[1].xy, CLIPDIST[1]
>>>
>>> using the full range of struct tgsi_declaration::UsageMask [1] or
>>> we represent them as as scalars:
>>>
>>>   DCL OUT[0].x, CLIPDIST[0]
>>>   DCL OUT[1].x, CLIPDIST[1]
>>>   DCL OUT[2].x, CLIPDIST[2]
>>>   DCL OUT[3].x, CLIPDIST[3]
>>>   DCL OUT[4].x, CLIPDIST[4]
>>>   DCL OUT[5].x, CLIPDIST[5]
>>>
>>> If indirect addressing is allowed as I read bore, then maybe the
>>> later is better.
>>>
>>> I confess my ignorance about clipping and maybe I'm being dense,
>>> but I still don't see the need for this
>>> TGSI_PROPERTY_NUM_CLIP_DISTANCES.  Could you please draft an
>>> example TGSI shader showing this property (or just paste one
>>> generated with your change)?  I think that would help a lot.
>>>
>>>
>>> Jose
>>>
>>>
>>> [1] I don't know if tgsi_dump pays much attention to
>>>  tgsi_declaration::UsageMask, but it does exist.
>> UsageMask might work, but before that can be considered a viable
>> solution, someone will need to make it possible to actually declare
>> it
>> from ureg.  As it is, ureg is hardcoded to set UsageMask to xyzw no
>> matter what on all declared inputs and outputs.
> ureg automatically fills the UsageMask from the destionation register masks, 
> since it easy to determine from the opcodes.
>
> Which leads me to my second point, if indirect addressing of CLIPDIST is 
> allowed, then we can't really pack the clip distance as 4-elem vectors in 
> TGSI: not only the syntax would be very weird, but it would create havoc on 
> all tgsi-translating code that makes decisions based on indirect addressing 
> of registers.
>
> That is, 
>
>   float gl_ClipDistance[6];
>
>   gl_ClipDistance[i] = foo;
>
> would become
>
>DCL OUT[0].x, CLIPDIST[0]
>DCL OUT[1].x, CLIPDIST[1]
>DCL OUT[2].x, CLIPDIST[2]
>DCL OUT[3].x, CLIPDIST[3]
>DCL OUT[4].x, CLIPDIST[4]
>DCL OUT[5].x, CLIPDIST[5]
>MOV OUT[ADDR[0].x].x, foo
>
> and the info from TGSI_PROPERTY_NUM_CLIP_DISTANCES can be obtained by walking 
> the declaration (which can/should be done only once in tgsi_scan).
>
> But this just doesn't look like it would ever work:
>
>DCL OUT[0].xyzw, CLIPDIST[0]
>DCL OUT[1].xy  , CLIPDIST[1]
>MOV OUT[ADDR[0].x]., foo
>
> Jose

If ureg automatically fills the UsageMask from the accessed components,
it's probably a better solution than the property.

A

Re: [Mesa-dev] [PATCH 0/2 v2] Add support for clip distances in Gallium

2011-12-13 Thread Bryan Cain
On 12/13/2011 03:48 PM, Jose Fonseca wrote:
> - Original Message -
>> On 12/13/2011 03:25 PM, Jose Fonseca wrote:
>>> - Original Message -
 On 12/13/2011 03:09 PM, Jose Fonseca wrote:
> - Original Message -
>> On 12/13/2011 12:26 PM, Bryan Cain wrote:
>>> On 12/13/2011 02:11 PM, Jose Fonseca wrote:
 - Original Message -
> This is an updated version of the patch set I sent to the
> list
> a
> few
> hours
> ago.
> There is now a TGSI property called
> TGSI_PROPERTY_NUM_CLIP_DISTANCES
> that drivers can use to determine how many of the 8 available
> clip
> distances
> are actually used by a shader.
 Can't the info in TGSI_PROPERTY_NUM_CLIP_DISTANCES be easily
 derived from the shader, and queried through
 src/gallium/auxiliary/tgsi/tgsi_scan.h ?
>>> No.  The clip distances can be indirectly addressed (there are
>>> up
>>> to 2
>>> of them in vec4 form for a total of 8 floats), which makes it
>>> impossible
>>> to determine which ones are used by analyzing the shader.
>> The description is almost complete. :)  The issue is that the
>> shader
>> may
>> declare
>>
>> out float gl_ClipDistance[4];
>>
>> the use non-constant addressing of the array.  The compiler
>> knows
>> that
>> gl_ClipDistance has at most 4 elements, but post-hoc analysis
>> would
>> not
>> be able to determine that.  Often the fixed-function hardware
>> (see
>> below) needs to know which clip distance values are actually
>> written.
> But don't all the clip distances written by the shader need to be
> declared?
>
> E.g.:
>
> DCL OUT[0], CLIPDIST[0]
> DCL OUT[1], CLIPDIST[1]
> DCL OUT[2], CLIPDIST[2]
> DCL OUT[3], CLIPDIST[3]
>
> therefore a trivial analysis of the declarations convey that?
 No.  Clip distance is an array of up to 8 floats in GLSL, but it's
 represented in the hardware as 2 vec4s.  You can tell by analyzing
 the
 declarations whether there are more than 4 clip distances in use,
 but
 not which components the shader writes to.
 TGSI_PROPERTY_NUM_CLIP_DISTANCES is the number of components in
 use,
 not
 the number of full vectors.
>>> Lets imagine
>>>
>>>   out float gl_ClipDistance[6];
>>>
>>> Each a clip distance is a scalar float.
>>>
>>> Either all hardware represents the 8 clip distances as two 4
>>> vectors, and we do:
>>>
>>>   DCL OUT[0].xywz, CLIPDIST[0]
>>>   DCL OUT[1].xy, CLIPDIST[1]
>>>
>>> using the full range of struct tgsi_declaration::UsageMask [1] or
>>> we represent them as as scalars:
>>>
>>>   DCL OUT[0].x, CLIPDIST[0]
>>>   DCL OUT[1].x, CLIPDIST[1]
>>>   DCL OUT[2].x, CLIPDIST[2]
>>>   DCL OUT[3].x, CLIPDIST[3]
>>>   DCL OUT[4].x, CLIPDIST[4]
>>>   DCL OUT[5].x, CLIPDIST[5]
>>>
>>> If indirect addressing is allowed as I read bore, then maybe the
>>> later is better.
>>>
>>> I confess my ignorance about clipping and maybe I'm being dense,
>>> but I still don't see the need for this
>>> TGSI_PROPERTY_NUM_CLIP_DISTANCES.  Could you please draft an
>>> example TGSI shader showing this property (or just paste one
>>> generated with your change)?  I think that would help a lot.
>>>
>>>
>>> Jose
>>>
>>>
>>> [1] I don't know if tgsi_dump pays much attention to
>>>  tgsi_declaration::UsageMask, but it does exist.
>> UsageMask might work, but before that can be considered a viable
>> solution, someone will need to make it possible to actually declare
>> it
>> from ureg.  As it is, ureg is hardcoded to set UsageMask to xyzw no
>> matter what on all declared inputs and outputs.
> ureg automatically fills the UsageMask from the destionation register masks, 
> since it easy to determine from the opcodes.

Wait, where does it do that?  When I search through tgsi_ureg.c for
"UsageMask", all it shows are assignments of TGSI_WRITEMASK_XYZW to the
UsageMask property.

Bryan

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 0/2 v2] Add support for clip distances in Gallium

2011-12-13 Thread Jose Fonseca


- Original Message -
> On 12/13/2011 03:48 PM, Jose Fonseca wrote:
> > - Original Message -
> >> On 12/13/2011 03:25 PM, Jose Fonseca wrote:
> >>> - Original Message -
>  On 12/13/2011 03:09 PM, Jose Fonseca wrote:
> > - Original Message -
> >> On 12/13/2011 12:26 PM, Bryan Cain wrote:
> >>> On 12/13/2011 02:11 PM, Jose Fonseca wrote:
>  - Original Message -
> > This is an updated version of the patch set I sent to the
> > list
> > a
> > few
> > hours
> > ago.
> > There is now a TGSI property called
> > TGSI_PROPERTY_NUM_CLIP_DISTANCES
> > that drivers can use to determine how many of the 8
> > available
> > clip
> > distances
> > are actually used by a shader.
>  Can't the info in TGSI_PROPERTY_NUM_CLIP_DISTANCES be easily
>  derived from the shader, and queried through
>  src/gallium/auxiliary/tgsi/tgsi_scan.h ?
> >>> No.  The clip distances can be indirectly addressed (there
> >>> are
> >>> up
> >>> to 2
> >>> of them in vec4 form for a total of 8 floats), which makes it
> >>> impossible
> >>> to determine which ones are used by analyzing the shader.
> >> The description is almost complete. :)  The issue is that the
> >> shader
> >> may
> >> declare
> >>
> >> out float gl_ClipDistance[4];
> >>
> >> the use non-constant addressing of the array.  The compiler
> >> knows
> >> that
> >> gl_ClipDistance has at most 4 elements, but post-hoc analysis
> >> would
> >> not
> >> be able to determine that.  Often the fixed-function hardware
> >> (see
> >> below) needs to know which clip distance values are actually
> >> written.
> > But don't all the clip distances written by the shader need to
> > be
> > declared?
> >
> > E.g.:
> >
> > DCL OUT[0], CLIPDIST[0]
> > DCL OUT[1], CLIPDIST[1]
> > DCL OUT[2], CLIPDIST[2]
> > DCL OUT[3], CLIPDIST[3]
> >
> > therefore a trivial analysis of the declarations convey that?
>  No.  Clip distance is an array of up to 8 floats in GLSL, but
>  it's
>  represented in the hardware as 2 vec4s.  You can tell by
>  analyzing
>  the
>  declarations whether there are more than 4 clip distances in
>  use,
>  but
>  not which components the shader writes to.
>  TGSI_PROPERTY_NUM_CLIP_DISTANCES is the number of components in
>  use,
>  not
>  the number of full vectors.
> >>> Lets imagine
> >>>
> >>>   out float gl_ClipDistance[6];
> >>>
> >>> Each a clip distance is a scalar float.
> >>>
> >>> Either all hardware represents the 8 clip distances as two 4
> >>> vectors, and we do:
> >>>
> >>>   DCL OUT[0].xywz, CLIPDIST[0]
> >>>   DCL OUT[1].xy, CLIPDIST[1]
> >>>
> >>> using the full range of struct tgsi_declaration::UsageMask [1] or
> >>> we represent them as as scalars:
> >>>
> >>>   DCL OUT[0].x, CLIPDIST[0]
> >>>   DCL OUT[1].x, CLIPDIST[1]
> >>>   DCL OUT[2].x, CLIPDIST[2]
> >>>   DCL OUT[3].x, CLIPDIST[3]
> >>>   DCL OUT[4].x, CLIPDIST[4]
> >>>   DCL OUT[5].x, CLIPDIST[5]
> >>>
> >>> If indirect addressing is allowed as I read bore, then maybe the
> >>> later is better.
> >>>
> >>> I confess my ignorance about clipping and maybe I'm being dense,
> >>> but I still don't see the need for this
> >>> TGSI_PROPERTY_NUM_CLIP_DISTANCES.  Could you please draft an
> >>> example TGSI shader showing this property (or just paste one
> >>> generated with your change)?  I think that would help a lot.
> >>>
> >>>
> >>> Jose
> >>>
> >>>
> >>> [1] I don't know if tgsi_dump pays much attention to
> >>>  tgsi_declaration::UsageMask, but it does exist.
> >> UsageMask might work, but before that can be considered a viable
> >> solution, someone will need to make it possible to actually
> >> declare
> >> it
> >> from ureg.  As it is, ureg is hardcoded to set UsageMask to xyzw
> >> no
> >> matter what on all declared inputs and outputs.
> > ureg automatically fills the UsageMask from the destionation
> > register masks, since it easy to determine from the opcodes.
> 
> Wait, where does it do that?  When I search through tgsi_ureg.c for
> "UsageMask", all it shows are assignments of TGSI_WRITEMASK_XYZW to
> the
> UsageMask property.

ah. I may be lying. But I'm pretty sure I wrote such code somewhere, sometime. 
Let me dig it.

Jose
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 0/2 v2] Add support for clip distances in Gallium

2011-12-13 Thread Jose Fonseca


- Original Message -
> On 12/13/2011 03:48 PM, Jose Fonseca wrote:
> > - Original Message -
> >> On 12/13/2011 03:25 PM, Jose Fonseca wrote:
> >>> - Original Message -
>  On 12/13/2011 03:09 PM, Jose Fonseca wrote:
> > - Original Message -
> >> On 12/13/2011 12:26 PM, Bryan Cain wrote:
> >>> On 12/13/2011 02:11 PM, Jose Fonseca wrote:
>  - Original Message -
> > This is an updated version of the patch set I sent to the
> > list
> > a
> > few
> > hours
> > ago.
> > There is now a TGSI property called
> > TGSI_PROPERTY_NUM_CLIP_DISTANCES
> > that drivers can use to determine how many of the 8
> > available
> > clip
> > distances
> > are actually used by a shader.
>  Can't the info in TGSI_PROPERTY_NUM_CLIP_DISTANCES be easily
>  derived from the shader, and queried through
>  src/gallium/auxiliary/tgsi/tgsi_scan.h ?
> >>> No.  The clip distances can be indirectly addressed (there
> >>> are
> >>> up
> >>> to 2
> >>> of them in vec4 form for a total of 8 floats), which makes it
> >>> impossible
> >>> to determine which ones are used by analyzing the shader.
> >> The description is almost complete. :)  The issue is that the
> >> shader
> >> may
> >> declare
> >>
> >> out float gl_ClipDistance[4];
> >>
> >> the use non-constant addressing of the array.  The compiler
> >> knows
> >> that
> >> gl_ClipDistance has at most 4 elements, but post-hoc analysis
> >> would
> >> not
> >> be able to determine that.  Often the fixed-function hardware
> >> (see
> >> below) needs to know which clip distance values are actually
> >> written.
> > But don't all the clip distances written by the shader need to
> > be
> > declared?
> >
> > E.g.:
> >
> > DCL OUT[0], CLIPDIST[0]
> > DCL OUT[1], CLIPDIST[1]
> > DCL OUT[2], CLIPDIST[2]
> > DCL OUT[3], CLIPDIST[3]
> >
> > therefore a trivial analysis of the declarations convey that?
>  No.  Clip distance is an array of up to 8 floats in GLSL, but
>  it's
>  represented in the hardware as 2 vec4s.  You can tell by
>  analyzing
>  the
>  declarations whether there are more than 4 clip distances in
>  use,
>  but
>  not which components the shader writes to.
>  TGSI_PROPERTY_NUM_CLIP_DISTANCES is the number of components in
>  use,
>  not
>  the number of full vectors.
> >>> Lets imagine
> >>>
> >>>   out float gl_ClipDistance[6];
> >>>
> >>> Each a clip distance is a scalar float.
> >>>
> >>> Either all hardware represents the 8 clip distances as two 4
> >>> vectors, and we do:
> >>>
> >>>   DCL OUT[0].xywz, CLIPDIST[0]
> >>>   DCL OUT[1].xy, CLIPDIST[1]
> >>>
> >>> using the full range of struct tgsi_declaration::UsageMask [1] or
> >>> we represent them as as scalars:
> >>>
> >>>   DCL OUT[0].x, CLIPDIST[0]
> >>>   DCL OUT[1].x, CLIPDIST[1]
> >>>   DCL OUT[2].x, CLIPDIST[2]
> >>>   DCL OUT[3].x, CLIPDIST[3]
> >>>   DCL OUT[4].x, CLIPDIST[4]
> >>>   DCL OUT[5].x, CLIPDIST[5]
> >>>
> >>> If indirect addressing is allowed as I read bore, then maybe the
> >>> later is better.
> >>>
> >>> I confess my ignorance about clipping and maybe I'm being dense,
> >>> but I still don't see the need for this
> >>> TGSI_PROPERTY_NUM_CLIP_DISTANCES.  Could you please draft an
> >>> example TGSI shader showing this property (or just paste one
> >>> generated with your change)?  I think that would help a lot.
> >>>
> >>>
> >>> Jose
> >>>
> >>>
> >>> [1] I don't know if tgsi_dump pays much attention to
> >>>  tgsi_declaration::UsageMask, but it does exist.
> >> UsageMask might work, but before that can be considered a viable
> >> solution, someone will need to make it possible to actually
> >> declare
> >> it
> >> from ureg.  As it is, ureg is hardcoded to set UsageMask to xyzw
> >> no
> >> matter what on all declared inputs and outputs.
> > ureg automatically fills the UsageMask from the destionation
> > register masks, since it easy to determine from the opcodes.
> >
> > Which leads me to my second point, if indirect addressing of
> > CLIPDIST is allowed, then we can't really pack the clip distance
> > as 4-elem vectors in TGSI: not only the syntax would be very
> > weird, but it would create havoc on all tgsi-translating code that
> > makes decisions based on indirect addressing of registers.
> >
> > That is,
> >
> >   float gl_ClipDistance[6];
> >
> >   gl_ClipDistance[i] = foo;
> >
> > would become
> >
> >DCL OUT[0].x, CLIPDIST[0]
> >DCL OUT[1].x, CLIPDIST[1]
> >DCL OUT[2].x, CLIPDIST[2]
> >DCL OUT[3].x, CLIPDIST[3]
> >DCL OUT[4].x, CLIPDIST[4]
> >DCL OUT[5].x, CLIPDIST[5]
> >MOV OUT[ADDR[0].x].x, foo
> >
> > and the info from TGSI_PROPERTY_NUM_CLIP_DISTANCES can be obtained
> > by walking the

Re: [Mesa-dev] [PATCH 0/2 v2] Add support for clip distances in Gallium

2011-12-13 Thread Christoph Bumiller
On 12/13/2011 10:48 PM, Jose Fonseca wrote:
> - Original Message -
>> On 12/13/2011 03:25 PM, Jose Fonseca wrote:
>>>
>>> - Original Message -
 On 12/13/2011 03:09 PM, Jose Fonseca wrote:
> - Original Message -
>> On 12/13/2011 12:26 PM, Bryan Cain wrote:
>>> On 12/13/2011 02:11 PM, Jose Fonseca wrote:
 - Original Message -
> This is an updated version of the patch set I sent to the
> list
> a
> few
> hours
> ago.
> There is now a TGSI property called
> TGSI_PROPERTY_NUM_CLIP_DISTANCES
> that drivers can use to determine how many of the 8 available
> clip
> distances
> are actually used by a shader.
 Can't the info in TGSI_PROPERTY_NUM_CLIP_DISTANCES be easily
 derived from the shader, and queried through
 src/gallium/auxiliary/tgsi/tgsi_scan.h ?
>>> No.  The clip distances can be indirectly addressed (there are
>>> up
>>> to 2
>>> of them in vec4 form for a total of 8 floats), which makes it
>>> impossible
>>> to determine which ones are used by analyzing the shader.
>> The description is almost complete. :)  The issue is that the
>> shader
>> may
>> declare
>>
>> out float gl_ClipDistance[4];
>>
>> the use non-constant addressing of the array.  The compiler
>> knows
>> that
>> gl_ClipDistance has at most 4 elements, but post-hoc analysis
>> would
>> not
>> be able to determine that.  Often the fixed-function hardware
>> (see
>> below) needs to know which clip distance values are actually
>> written.
> But don't all the clip distances written by the shader need to be
> declared?
>
> E.g.:
>
> DCL OUT[0], CLIPDIST[0]
> DCL OUT[1], CLIPDIST[1]
> DCL OUT[2], CLIPDIST[2]
> DCL OUT[3], CLIPDIST[3]
>
> therefore a trivial analysis of the declarations convey that?
 No.  Clip distance is an array of up to 8 floats in GLSL, but it's
 represented in the hardware as 2 vec4s.  You can tell by analyzing
 the
 declarations whether there are more than 4 clip distances in use,
 but
 not which components the shader writes to.
 TGSI_PROPERTY_NUM_CLIP_DISTANCES is the number of components in
 use,
 not
 the number of full vectors.
>>> Lets imagine
>>>
>>>   out float gl_ClipDistance[6];
>>>
>>> Each a clip distance is a scalar float.
>>>
>>> Either all hardware represents the 8 clip distances as two 4
>>> vectors, and we do:
>>>
>>>   DCL OUT[0].xywz, CLIPDIST[0]
>>>   DCL OUT[1].xy, CLIPDIST[1]
>>>
>>> using the full range of struct tgsi_declaration::UsageMask [1] or
>>> we represent them as as scalars:
>>>
>>>   DCL OUT[0].x, CLIPDIST[0]
>>>   DCL OUT[1].x, CLIPDIST[1]
>>>   DCL OUT[2].x, CLIPDIST[2]
>>>   DCL OUT[3].x, CLIPDIST[3]
>>>   DCL OUT[4].x, CLIPDIST[4]
>>>   DCL OUT[5].x, CLIPDIST[5]
>>>
>>> If indirect addressing is allowed as I read bore, then maybe the
>>> later is better.
>>>
>>> I confess my ignorance about clipping and maybe I'm being dense,
>>> but I still don't see the need for this
>>> TGSI_PROPERTY_NUM_CLIP_DISTANCES.  Could you please draft an
>>> example TGSI shader showing this property (or just paste one
>>> generated with your change)?  I think that would help a lot.
>>>
>>>
>>> Jose
>>>
>>>
>>> [1] I don't know if tgsi_dump pays much attention to
>>>  tgsi_declaration::UsageMask, but it does exist.
>>
>> UsageMask might work, but before that can be considered a viable
>> solution, someone will need to make it possible to actually declare
>> it
>> from ureg.  As it is, ureg is hardcoded to set UsageMask to xyzw no
>> matter what on all declared inputs and outputs.
> 
> ureg automatically fills the UsageMask from the destionation register masks, 
> since it easy to determine from the opcodes.
> 
> Which leads me to my second point, if indirect addressing of CLIPDIST is 
> allowed, then we can't really pack the clip distance as 4-elem vectors in 
> TGSI: not only the syntax would be very weird, but it would create havoc on 
> all tgsi-translating code that makes decisions based on indirect addressing 
> of registers.
> 
> That is, 
> 
>   float gl_ClipDistance[6];
> 
>   gl_ClipDistance[i] = foo;
> 
> would become
> 
>DCL OUT[0].x, CLIPDIST[0]
>DCL OUT[1].x, CLIPDIST[1]
>DCL OUT[2].x, CLIPDIST[2]
>DCL OUT[3].x, CLIPDIST[3]
>DCL OUT[4].x, CLIPDIST[4]
>DCL OUT[5].x, CLIPDIST[5]
>MOV OUT[ADDR[0].x].x, foo
> 

This cannot work properly yet.

For instance, the clip distance slots in my hardware's output memory
space are packed, i.e. consuming NUM_CLIP_DISTANCES * 4 bytes.
(This cannot be changed, except by spilling outputs to high latency
memory and moving them all at the end, which is very undesirable.)

The MOV OUT[ADDR[0].x].x, however, has no way to know whether to scale
ADDR[0].x by 4 or by 16 bytes (as it would for arrays of

Re: [Mesa-dev] [PATCH 0/2 v2] Add support for clip distances in Gallium

2011-12-13 Thread Jose Fonseca
- Original Message -
> 
> 
> - Original Message -
> > On 12/13/2011 03:48 PM, Jose Fonseca wrote:
> > > - Original Message -
> > >> On 12/13/2011 03:25 PM, Jose Fonseca wrote:
> > >>> - Original Message -
> >  On 12/13/2011 03:09 PM, Jose Fonseca wrote:
> > > - Original Message -
> > >> On 12/13/2011 12:26 PM, Bryan Cain wrote:
> > >>> On 12/13/2011 02:11 PM, Jose Fonseca wrote:
> >  - Original Message -
> > > This is an updated version of the patch set I sent to the
> > > list
> > > a
> > > few
> > > hours
> > > ago.
> > > There is now a TGSI property called
> > > TGSI_PROPERTY_NUM_CLIP_DISTANCES
> > > that drivers can use to determine how many of the 8
> > > available
> > > clip
> > > distances
> > > are actually used by a shader.
> >  Can't the info in TGSI_PROPERTY_NUM_CLIP_DISTANCES be
> >  easily
> >  derived from the shader, and queried through
> >  src/gallium/auxiliary/tgsi/tgsi_scan.h ?
> > >>> No.  The clip distances can be indirectly addressed (there
> > >>> are
> > >>> up
> > >>> to 2
> > >>> of them in vec4 form for a total of 8 floats), which makes
> > >>> it
> > >>> impossible
> > >>> to determine which ones are used by analyzing the shader.
> > >> The description is almost complete. :)  The issue is that
> > >> the
> > >> shader
> > >> may
> > >> declare
> > >>
> > >> out float gl_ClipDistance[4];
> > >>
> > >> the use non-constant addressing of the array.  The compiler
> > >> knows
> > >> that
> > >> gl_ClipDistance has at most 4 elements, but post-hoc
> > >> analysis
> > >> would
> > >> not
> > >> be able to determine that.  Often the fixed-function
> > >> hardware
> > >> (see
> > >> below) needs to know which clip distance values are actually
> > >> written.
> > > But don't all the clip distances written by the shader need
> > > to
> > > be
> > > declared?
> > >
> > > E.g.:
> > >
> > > DCL OUT[0], CLIPDIST[0]
> > > DCL OUT[1], CLIPDIST[1]
> > > DCL OUT[2], CLIPDIST[2]
> > > DCL OUT[3], CLIPDIST[3]
> > >
> > > therefore a trivial analysis of the declarations convey that?
> >  No.  Clip distance is an array of up to 8 floats in GLSL, but
> >  it's
> >  represented in the hardware as 2 vec4s.  You can tell by
> >  analyzing
> >  the
> >  declarations whether there are more than 4 clip distances in
> >  use,
> >  but
> >  not which components the shader writes to.
> >  TGSI_PROPERTY_NUM_CLIP_DISTANCES is the number of components
> >  in
> >  use,
> >  not
> >  the number of full vectors.
> > >>> Lets imagine
> > >>>
> > >>>   out float gl_ClipDistance[6];
> > >>>
> > >>> Each a clip distance is a scalar float.
> > >>>
> > >>> Either all hardware represents the 8 clip distances as two 4
> > >>> vectors, and we do:
> > >>>
> > >>>   DCL OUT[0].xywz, CLIPDIST[0]
> > >>>   DCL OUT[1].xy, CLIPDIST[1]
> > >>>
> > >>> using the full range of struct tgsi_declaration::UsageMask [1]
> > >>> or
> > >>> we represent them as as scalars:
> > >>>
> > >>>   DCL OUT[0].x, CLIPDIST[0]
> > >>>   DCL OUT[1].x, CLIPDIST[1]
> > >>>   DCL OUT[2].x, CLIPDIST[2]
> > >>>   DCL OUT[3].x, CLIPDIST[3]
> > >>>   DCL OUT[4].x, CLIPDIST[4]
> > >>>   DCL OUT[5].x, CLIPDIST[5]
> > >>>
> > >>> If indirect addressing is allowed as I read bore, then maybe
> > >>> the
> > >>> later is better.
> > >>>
> > >>> I confess my ignorance about clipping and maybe I'm being
> > >>> dense,
> > >>> but I still don't see the need for this
> > >>> TGSI_PROPERTY_NUM_CLIP_DISTANCES.  Could you please draft an
> > >>> example TGSI shader showing this property (or just paste one
> > >>> generated with your change)?  I think that would help a lot.
> > >>>
> > >>>
> > >>> Jose
> > >>>
> > >>>
> > >>> [1] I don't know if tgsi_dump pays much attention to
> > >>>  tgsi_declaration::UsageMask, but it does exist.
> > >> UsageMask might work, but before that can be considered a viable
> > >> solution, someone will need to make it possible to actually
> > >> declare
> > >> it
> > >> from ureg.  As it is, ureg is hardcoded to set UsageMask to xyzw
> > >> no
> > >> matter what on all declared inputs and outputs.
> > > ureg automatically fills the UsageMask from the destionation
> > > register masks, since it easy to determine from the opcodes.
> > 
> > Wait, where does it do that?  When I search through tgsi_ureg.c for
> > "UsageMask", all it shows are assignments of TGSI_WRITEMASK_XYZW to
> > the
> > UsageMask property.
> 
> ah. I may be lying. But I'm pretty sure I wrote such code somewhere,
> sometime. Let me dig it.

I was lying.  I wrote tgsi_util_get_inst_usage_mask() in 
src/gallium/auxiliary/tgsi/tgsi_util.c , but it only analy

Re: [Mesa-dev] [PATCH] Enable display list support for glClearBuffer functions

2011-12-13 Thread Anuj Phogat
On Tue 13 Dec 2011 12:49:28 PM PST, Ian Romanick wrote:
> On 12/13/2011 12:01 PM, Anuj Phogat wrote:
>> Enabling display list support for glClearBuffer functions with minor
>> fixes
>
> There's also a #if 0 block in that file that should be removed.  This
> is a case where 'git blame' is useful.  doing 'git blame
> src/mesa/main/dlist.c' shows that all of those lines were added in
> commit 05fb922e.  Looking at commit 05fb922e shows the #if 0 block.
The #if 0 block is already removed in some previous commit. 
 code snippet from src/mesa/main/dlist.c, line 10293-10297: 
   /* GL 3.0 */
   SET_ClearBufferiv(table, save_ClearBufferiv);
   SET_ClearBufferuiv(table, save_ClearBufferuiv);
   SET_ClearBufferfv(table, save_ClearBufferfv);
   SET_ClearBufferfi(table, save_ClearBufferfi);


___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 0/2 v2] Add support for clip distances in Gallium

2011-12-13 Thread Bryan Cain
On 12/13/2011 04:22 PM, Jose Fonseca wrote:
> - Original Message -
>>
>> - Original Message -
>>> On 12/13/2011 03:48 PM, Jose Fonseca wrote:
 - Original Message -
> On 12/13/2011 03:25 PM, Jose Fonseca wrote:
>> - Original Message -
>>> On 12/13/2011 03:09 PM, Jose Fonseca wrote:
 - Original Message -
> On 12/13/2011 12:26 PM, Bryan Cain wrote:
>> On 12/13/2011 02:11 PM, Jose Fonseca wrote:
>>> - Original Message -
 This is an updated version of the patch set I sent to the
 list
 a
 few
 hours
 ago.
 There is now a TGSI property called
 TGSI_PROPERTY_NUM_CLIP_DISTANCES
 that drivers can use to determine how many of the 8
 available
 clip
 distances
 are actually used by a shader.
>>> Can't the info in TGSI_PROPERTY_NUM_CLIP_DISTANCES be
>>> easily
>>> derived from the shader, and queried through
>>> src/gallium/auxiliary/tgsi/tgsi_scan.h ?
>> No.  The clip distances can be indirectly addressed (there
>> are
>> up
>> to 2
>> of them in vec4 form for a total of 8 floats), which makes
>> it
>> impossible
>> to determine which ones are used by analyzing the shader.
> The description is almost complete. :)  The issue is that
> the
> shader
> may
> declare
>
> out float gl_ClipDistance[4];
>
> the use non-constant addressing of the array.  The compiler
> knows
> that
> gl_ClipDistance has at most 4 elements, but post-hoc
> analysis
> would
> not
> be able to determine that.  Often the fixed-function
> hardware
> (see
> below) needs to know which clip distance values are actually
> written.
 But don't all the clip distances written by the shader need
 to
 be
 declared?

 E.g.:

 DCL OUT[0], CLIPDIST[0]
 DCL OUT[1], CLIPDIST[1]
 DCL OUT[2], CLIPDIST[2]
 DCL OUT[3], CLIPDIST[3]

 therefore a trivial analysis of the declarations convey that?
>>> No.  Clip distance is an array of up to 8 floats in GLSL, but
>>> it's
>>> represented in the hardware as 2 vec4s.  You can tell by
>>> analyzing
>>> the
>>> declarations whether there are more than 4 clip distances in
>>> use,
>>> but
>>> not which components the shader writes to.
>>> TGSI_PROPERTY_NUM_CLIP_DISTANCES is the number of components
>>> in
>>> use,
>>> not
>>> the number of full vectors.
>> Lets imagine
>>
>>   out float gl_ClipDistance[6];
>>
>> Each a clip distance is a scalar float.
>>
>> Either all hardware represents the 8 clip distances as two 4
>> vectors, and we do:
>>
>>   DCL OUT[0].xywz, CLIPDIST[0]
>>   DCL OUT[1].xy, CLIPDIST[1]
>>
>> using the full range of struct tgsi_declaration::UsageMask [1]
>> or
>> we represent them as as scalars:
>>
>>   DCL OUT[0].x, CLIPDIST[0]
>>   DCL OUT[1].x, CLIPDIST[1]
>>   DCL OUT[2].x, CLIPDIST[2]
>>   DCL OUT[3].x, CLIPDIST[3]
>>   DCL OUT[4].x, CLIPDIST[4]
>>   DCL OUT[5].x, CLIPDIST[5]
>>
>> If indirect addressing is allowed as I read bore, then maybe
>> the
>> later is better.
>>
>> I confess my ignorance about clipping and maybe I'm being
>> dense,
>> but I still don't see the need for this
>> TGSI_PROPERTY_NUM_CLIP_DISTANCES.  Could you please draft an
>> example TGSI shader showing this property (or just paste one
>> generated with your change)?  I think that would help a lot.
>>
>>
>> Jose
>>
>>
>> [1] I don't know if tgsi_dump pays much attention to
>>  tgsi_declaration::UsageMask, but it does exist.
> UsageMask might work, but before that can be considered a viable
> solution, someone will need to make it possible to actually
> declare
> it
> from ureg.  As it is, ureg is hardcoded to set UsageMask to xyzw
> no
> matter what on all declared inputs and outputs.
 ureg automatically fills the UsageMask from the destionation
 register masks, since it easy to determine from the opcodes.
>>> Wait, where does it do that?  When I search through tgsi_ureg.c for
>>> "UsageMask", all it shows are assignments of TGSI_WRITEMASK_XYZW to
>>> the
>>> UsageMask property.
>> ah. I may be lying. But I'm pretty sure I wrote such code somewhere,
>> sometime. Let me dig it.
> I was lying.  I wrote tgsi_util_get_inst_usage_mask() in 
> src/gallium/auxiliary/tgsi/tgsi_util.c , but it only analyses which registers 
> are _read_, and never got hooked into ureg anyway.
>
> I don't wa

Re: [Mesa-dev] [PATCH 0/2 v2] Add support for clip distances in Gallium

2011-12-13 Thread Alex Deucher
On Tue, Dec 13, 2011 at 4:18 PM, Christoph Bumiller
 wrote:
> On 12/13/2011 09:58 PM, Jose Fonseca wrote:
>>
>>
>> - Original Message -
>>> On 12/13/2011 09:11 PM, Jose Fonseca wrote:


 - Original Message -
> This is an updated version of the patch set I sent to the list a
> few
> hours
> ago.


> There is now a TGSI property called
> TGSI_PROPERTY_NUM_CLIP_DISTANCES
> that drivers can use to determine how many of the 8 available clip
> distances
> are actually used by a shader.

 Can't the info in TGSI_PROPERTY_NUM_CLIP_DISTANCES be easily
 derived from the shader, and queried through
 src/gallium/auxiliary/tgsi/tgsi_scan.h ?


 Could you also elaborate on why TGSI_SEMANTIC_CLIPDIST is useful
 for the drivers? I personally don't have nothing against it, but
 just like to understand why it makes a difference.

>>>
>>> Why does TGSI_SEMANTIC_*POSITION* make a difference ?
>>>
>>> Right, because the position values are consumed by the fixed function
>>> rasterizer. So are the clip distances.
>>>
>>> This is not about pipe_clip_state.ucp but about what this legacy
>>> cruft
>>> has to be turned into if GL_CLIP_PLANEi is used instead of GLSL 1.3's
>>> gl_ClipDistance[i].
>>
>> I'm just surprised because I thought there was no more fixed function 
>> clipping.
>>
>
> nvfx and r300, and I think even r600, are probably happy to have UCPs in
> the interface and can make direct use of them, see nvfx_ucp_validate in
> nvfx_state_emit.c or r300_set_clip_state in r300_state.c

R3xx-cayman all support legacy user clip planes (see
r600_set_clip_state() and evergreen_set_clip_state()).  r6xx-cayman
also support shader exported clip/cull distance.  Clip/cull distance
is exported from the vertex shader (as 1 or 2 vec4s) via special
export locations (similar to position, point size, edge flag, rt
index, viewport index, and kill flag).  See the comment starting with
"Special export handling in shaders" in r600_shader.c.

Alex

>
> nv50 and nvc0 have to use the same shader outputs for both legacy
> glClipPlanes and gl_ClipDistance[].
>
> On nv50, there is shader-external state that designates which outputs
> are clip distances, on nvc0 they have to be written to special output
> slots/addresses so the semantic information is important to be able to
> assign slots at compile time (the GPU was specially designed to make
> this possible and to support ARB_separate_shader_objects nicely).
>
> In D3D10/11, there is a system value semantic for clip distances and
> cull distances (cull distance is a different clipping mode and consumes
> the same slots as clip distances).
> See
> http://msdn.microsoft.com/en-us/library/windows/desktop/bb509647%28v=vs.85%29.aspx.
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 0/2 v2] Add support for clip distances in Gallium

2011-12-13 Thread Marek Olšák
On Tue, Dec 13, 2011 at 11:22 PM, Jose Fonseca  wrote:
> Another approach would be just to add the property, and kill output mask. Two 
> ways of doing the same is what I'd like to avoid. I'll need a day (it's late 
> here) to think about this and see how output mask is being actually used in 
> the code.

I think it would be better to axe the UsageMask (unless there's a need
to declare an output with the mask _Y_W for example) and add a
NumComponents field. The interpolation of varyings would be less
computionally-expensive if we knew the exact number of used components
and the number were less than 4 (depends on hw).

Marek
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 0/2 v2] Add support for clip distances in Gallium

2011-12-13 Thread Christoph Bumiller
On 12/14/2011 12:24 AM, Marek Olšák wrote:
> On Tue, Dec 13, 2011 at 11:22 PM, Jose Fonseca  wrote:
>> Another approach would be just to add the property, and kill output mask. 
>> Two ways of doing the same is what I'd like to avoid. I'll need a day (it's 
>> late here) to think about this and see how output mask is being actually 
>> used in the code.
> 
> I think it would be better to axe the UsageMask (unless there's a need
> to declare an output with the mask _Y_W for example) and add a
> NumComponents field. The interpolation of varyings would be less
> computionally-expensive if we knew the exact number of used components
> and the number were less than 4 (depends on hw).

I'd even save slots if a user only used, for example, gl_TexCoord[0].xw.
You cannot make it .xy internally because of point coordinate replacement.

If you want NumComponents, just do last_bit_set(UsageMask) + 1.

> 
> Marek
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH 0/8] i965 gen6: Initial implementation of transform feedback.

2011-12-13 Thread Paul Berry
This is a follow up to "i965 gen6: Pass-through GS program for
future use by transform feedback".  This patch series implements
basic transform feedback functionality in i965 Gen6 (Sandy
Bridge).

Patches 1/8 and 2/8 lay some additional core mesa groundwork for
transform feedback, by having the linker record the stride for each
transform feedback buffer (it seems silly to do this in
driver-specific code), and by fixing a minor bug in
transformfeedback.c.

Patches 3/8 and 4/8 do some trivial i965 groundwork, fixing a flaw in
emitting IF instructions that was never exercised before, and moving
the VUE map to a place where it can be accessed while compiling the
geometry shader.

Patch 5/8 adds actual transform feedback capability, and patch 6/8
advertises support for the EXT_transform_feedback extension.  Since
there are a bunch of corner cases that aren't handled yet, I'm only
advertising support for the extension when the environment variable
MESA_GL_VERSION_OVERRIDE=3.0 is set.  I'll make it unconditional once
transform feedback support is finished.

Patches 7/8 and 8/8 add some fun pipeline flushing to ensure that if
the output of transform feedback is used in subsequent draw
operations, the subsequent draw operations won't start before
transform feedback completes.

Stuff that remains to implement after this patch series:
- Reset the streamed vertex buffer index to 0 on BeginTransformFeedback().
- Handle overflow of transform feedback buffers.
- Rasterizer discard.
- PRIMITIVES_GENERATED and TRANSFORM_FEEDBACK_PRIMITIVES_WRITTEN queries.

[PATCH 1/8] mesa: Record transform feedback stride in linker output.
[PATCH 2/8] mesa: Fix off-by-one error in transform feedback size check.
[PATCH 3/8] i965 gen6+: Use 1-wide null operands for IF instructions
[PATCH 4/8] i965 gs: Move vue_map to brw_gs_compile.
[PATCH 5/8] i965 gen6: Initial implementation of transform feedback.
[PATCH 6/8] i965 gen6: Turn on transform feedback extension.
[PATCH 7/8] i965 gen6+: Make intel_batchbuffer_emit_mi_flush() actually flush.
[PATCH 8/8] i965: Flush pipeline on EndTransformFeedback.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH 1/8] mesa: Record transform feedback stride in linker output.

2011-12-13 Thread Paul Berry
This patch adds the field gl_transform_feedback_info::BufferStride,
which records the total number of components (per vertex) that
transform feedback is being instructed to store in each buffer.  The
i965 gen6 back-end needs this information in order to set up binding
tables, and it seems better to have the linker provide this
information rather than force the back-end to recompute it.
---
 src/glsl/linker.cpp|4 +++-
 src/mesa/main/mtypes.h |5 +
 2 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
index b8a7126..5eb2a20 100644
--- a/src/glsl/linker.cpp
+++ b/src/glsl/linker.cpp
@@ -1599,6 +1599,7 @@ tfeedback_decl::store(struct gl_shader_program *prog,
   info->Outputs[info->NumOutputs].NumComponents = this->vector_elements;
   info->Outputs[info->NumOutputs].OutputBuffer = buffer;
   ++info->NumOutputs;
+  info->BufferStride[buffer] += this->vector_elements;
}
return true;
 }
@@ -1863,7 +1864,8 @@ store_tfeedback_info(struct gl_context *ctx, struct 
gl_shader_program *prog,
  tfeedback_decl *tfeedback_decls)
 {
unsigned total_tfeedback_components = 0;
-   prog->LinkedTransformFeedback.NumOutputs = 0;
+   memset(&prog->LinkedTransformFeedback, 0,
+  sizeof(prog->LinkedTransformFeedback));
for (unsigned i = 0; i < num_tfeedback_decls; ++i) {
   unsigned buffer =
  prog->TransformFeedback.BufferMode == GL_SEPARATE_ATTRIBS ? i : 0;
diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
index 1934349..d4c600a 100644
--- a/src/mesa/main/mtypes.h
+++ b/src/mesa/main/mtypes.h
@@ -1822,6 +1822,11 @@ struct gl_transform_feedback_info {
   unsigned OutputBuffer;
   unsigned NumComponents;
} Outputs[MAX_PROGRAM_OUTPUTS];
+
+   /**
+* Number of components stored in each buffer.
+*/
+   unsigned BufferStride[MAX_FEEDBACK_ATTRIBS];
 };
 
 /**
-- 
1.7.6.4

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH 2/8] mesa: Fix off-by-one error in transform feedback size check.

2011-12-13 Thread Paul Berry
In _mesa_BindBufferRange(), we need to verify that the offset and size
specified by the client do not exceed the size of the underlying
buffer.  We were accidentally doing this check using ">=" rather than
">", so we were generating a bogus error if the client specified an
offset and size that fit exactly in the underlying buffer.
---
 src/mesa/main/transformfeedback.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/src/mesa/main/transformfeedback.c 
b/src/mesa/main/transformfeedback.c
index 799245d..78ca64d 100644
--- a/src/mesa/main/transformfeedback.c
+++ b/src/mesa/main/transformfeedback.c
@@ -486,7 +486,7 @@ _mesa_BindBufferRange(GLenum target, GLuint index,
   return;
}
 
-   if (offset + size >= bufObj->Size) {
+   if (offset + size > bufObj->Size) {
   _mesa_error(ctx, GL_INVALID_VALUE,
   "glBindBufferRange(offset + size %d > buffer size %d)",
  (int) (offset + size), (int) (bufObj->Size));
-- 
1.7.6.4

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH 3/8] i965 gen6+: Use 1-wide null operands for IF instructions

2011-12-13 Thread Paul Berry
The Sandy Bridge PRM, volume 4, part 2, section 5.3.10 ("5.3.10
Register Region Restrictions") contains the following restriction on
the execution size and operand width of instructions:

   "3. ExecSize must be equal to or greater than Width."

When emitting an IF instruction in single program flow mode on Gen6+,
we use an ExecSize of 1, therefore the Width of each operand must also
be 1.

The operands of an IF instruction are not actually used for their
normal purpose on Gen6+ (which is probably the reason this wasn't
causing a GPU hang), but nonetheless it seems prudent to follow this
rule.  This patch unconditionally uses 1-wide null operands for Gen6+
IF instructions, rather than the standard null operand, which is 8
components wide.
---
 src/mesa/drivers/dri/i965/brw_eu_emit.c |8 
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_eu_emit.c 
b/src/mesa/drivers/dri/i965/brw_eu_emit.c
index a46a81b..d48753c 100644
--- a/src/mesa/drivers/dri/i965/brw_eu_emit.c
+++ b/src/mesa/drivers/dri/i965/brw_eu_emit.c
@@ -941,11 +941,11 @@ brw_IF(struct brw_compile *p, GLuint execute_size)
} else if (intel->gen == 6) {
   brw_set_dest(p, insn, brw_imm_w(0));
   insn->bits1.branch_gen6.jump_count = 0;
-  brw_set_src0(p, insn, retype(brw_null_reg(), BRW_REGISTER_TYPE_D));
-  brw_set_src1(p, insn, retype(brw_null_reg(), BRW_REGISTER_TYPE_D));
+  brw_set_src0(p, insn, vec1(retype(brw_null_reg(), BRW_REGISTER_TYPE_D)));
+  brw_set_src1(p, insn, vec1(retype(brw_null_reg(), BRW_REGISTER_TYPE_D)));
} else {
-  brw_set_dest(p, insn, retype(brw_null_reg(), BRW_REGISTER_TYPE_D));
-  brw_set_src0(p, insn, retype(brw_null_reg(), BRW_REGISTER_TYPE_D));
+  brw_set_dest(p, insn, vec1(retype(brw_null_reg(), BRW_REGISTER_TYPE_D)));
+  brw_set_src0(p, insn, vec1(retype(brw_null_reg(), BRW_REGISTER_TYPE_D)));
   brw_set_src1(p, insn, brw_imm_ud(0));
   insn->bits3.break_cont.jip = 0;
   insn->bits3.break_cont.uip = 0;
-- 
1.7.6.4

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH 4/8] i965 gs: Move vue_map to brw_gs_compile.

2011-12-13 Thread Paul Berry
This patch stores the geometry shader VUE map from a local variable in
compile_gs_prog() to a field in the brw_gs_compile struct, so that it
will be available while compiling the geometry shader.  This is
necessary in order to support transform feedback on Gen6, because the
Gen6 geometry shader code that supports transform feedback needs to be
able to inspect the VUE map in order to find the correct vertex data
to output.
---
 src/mesa/drivers/dri/i965/brw_gs.c |5 ++---
 src/mesa/drivers/dri/i965/brw_gs.h |2 ++
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_gs.c 
b/src/mesa/drivers/dri/i965/brw_gs.c
index 69ffa19..f5d5898 100644
--- a/src/mesa/drivers/dri/i965/brw_gs.c
+++ b/src/mesa/drivers/dri/i965/brw_gs.c
@@ -57,9 +57,8 @@ static void compile_gs_prog( struct brw_context *brw,

c.key = *key;
/* The geometry shader needs to access the entire VUE. */
-   struct brw_vue_map vue_map;
-   brw_compute_vue_map(&vue_map, intel, c.key.userclip_active, c.key.attrs);
-   c.nr_regs = (vue_map.num_slots + 1)/2;
+   brw_compute_vue_map(&c.vue_map, intel, c.key.userclip_active, c.key.attrs);
+   c.nr_regs = (c.vue_map.num_slots + 1)/2;
 
mem_ctx = NULL;

diff --git a/src/mesa/drivers/dri/i965/brw_gs.h 
b/src/mesa/drivers/dri/i965/brw_gs.h
index abcb0b2..ecab3ef 100644
--- a/src/mesa/drivers/dri/i965/brw_gs.h
+++ b/src/mesa/drivers/dri/i965/brw_gs.h
@@ -66,6 +66,8 @@ struct brw_gs_compile {
 
/* Number of registers used to store vertex data */
GLuint nr_regs;
+
+   struct brw_vue_map vue_map;
 };
 
 #define ATTR_SIZE  (4*4)
-- 
1.7.6.4

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH 5/8] i965 gen6: Initial implementation of transform feedback.

2011-12-13 Thread Paul Berry
This patch adds basic transform feedback capability for Gen6 hardware.
This consists of several related pieces of functionality:

(1) In gen6_sol.c, we set up binding table entries for use by
transform feedback.  We use one binding table entry per transform
feedback varying (this allows us to avoid doing pointer arithmetic in
the shader, since we can set up the binding table entries with the
appropriate offsets and surface pitches to place each varying at the
correct address).

(2) In brw_context.c, we advertise the hardware capabilities, which
are as follows:

   MAX_TRANSFORM_FEEDBACK_INTERLEAVED_COMPONENTS 64
   MAX_TRANSFORM_FEEDBACK_SEPARATE_ATTRIBS4
   MAX_TRANSFORM_FEEDBACK_SEPARATE_COMPONENTS16

OpenGL 3.0 requires these values to be at least 64, 4, and 4,
respectively.  The reason we advertise a larger value than required
for MAX_TRANSFORM_FEEDBACK_SEPARATE_COMPONENTS is that we have already
set aside 64 binding table entries, so we might as well make them all
available in both separate attribs and interleaved modes.

(3) We set aside a single SVBI ("streamed vertex buffer index") for
use by transform feedback.  The hardware supports four independent
SVBI's, but we only need one, since vertices are added to all
transform feedback buffers at the same rate.  Note: at the moment this
index is reset to 0 only when the driver is initialized.  It needs to
be reset to 0 whenever BeginTransformFeedback() is called, and
otherwise preserved.

(4) In brw_gs_emit.c and brw_gs.c, we modify the geometry shader
program to output transform feedback data as a side effect.

(5) In gen6_gs_state.c, we configure the geometry shader stage to
handle the SVBI pointer correctly.
---
 src/mesa/drivers/dri/i965/Makefile.sources|1 +
 src/mesa/drivers/dri/i965/brw_context.c   |   20 
 src/mesa/drivers/dri/i965/brw_context.h   |   42 -
 src/mesa/drivers/dri/i965/brw_defines.h   |6 +
 src/mesa/drivers/dri/i965/brw_eu.h|7 ++
 src/mesa/drivers/dri/i965/brw_eu_emit.c   |   39 +++
 src/mesa/drivers/dri/i965/brw_gs.c|   26 +-
 src/mesa/drivers/dri/i965/brw_gs.h|   20 
 src/mesa/drivers/dri/i965/brw_gs_emit.c   |  112 +++--
 src/mesa/drivers/dri/i965/brw_misc_state.c|2 +-
 src/mesa/drivers/dri/i965/brw_state.h |1 +
 src/mesa/drivers/dri/i965/brw_state_upload.c  |1 +
 src/mesa/drivers/dri/i965/brw_wm_surface_state.c  |   61 +++
 src/mesa/drivers/dri/i965/gen6_gs_state.c |9 ++-
 src/mesa/drivers/dri/i965/gen6_sol.c  |  102 +++
 src/mesa/drivers/dri/i965/gen7_wm_surface_state.c |1 +
 src/mesa/drivers/dri/intel/intel_context.h|7 ++
 17 files changed, 445 insertions(+), 12 deletions(-)
 create mode 100644 src/mesa/drivers/dri/i965/gen6_sol.c

diff --git a/src/mesa/drivers/dri/i965/Makefile.sources 
b/src/mesa/drivers/dri/i965/Makefile.sources
index cd6a8f4..e50f9c3 100644
--- a/src/mesa/drivers/dri/i965/Makefile.sources
+++ b/src/mesa/drivers/dri/i965/Makefile.sources
@@ -93,6 +93,7 @@ i965_C_SOURCES := \
gen6_sampler_state.c \
gen6_scissor_state.c \
gen6_sf_state.c \
+gen6_sol.c \
gen6_urb.c \
gen6_viewport_state.c \
gen6_vs_state.c \
diff --git a/src/mesa/drivers/dri/i965/brw_context.c 
b/src/mesa/drivers/dri/i965/brw_context.c
index faa02bf..eb68bf4 100644
--- a/src/mesa/drivers/dri/i965/brw_context.c
+++ b/src/mesa/drivers/dri/i965/brw_context.c
@@ -178,6 +178,26 @@ brwCreateContext(int api,

ctx->Const.MaxTextureMaxAnisotropy = 16.0;
 
+   /* Hardware only supports a limited number of transform feedback buffers.
+* So we need to override the Mesa default (which is based only on software
+* limits).
+*/
+   ctx->Const.MaxTransformFeedbackSeparateAttribs = BRW_MAX_SOL_BUFFERS;
+
+   /* On Gen6, in the worst case, we use up one binding table entry per
+* transform feedback component (see comments above the definition of
+* BRW_MAX_SOL_BINDINGS, in brw_context.h), so we need to advertise a value
+* for MAX_TRANSFORM_FEEDBACK_INTERLEAVED_COMPONENTS equal to
+* BRW_MAX_SOL_BINDINGS.
+*
+* In "separate components" mode, we need to divide this value by
+* BRW_MAX_SOL_BUFFERS, so that the total number of binding table entries
+* used up by all buffers will not exceed BRW_MAX_SOL_BINDINGS.
+*/
+   ctx->Const.MaxTransformFeedbackInterleavedComponents = BRW_MAX_SOL_BINDINGS;
+   ctx->Const.MaxTransformFeedbackSeparateComponents =
+  BRW_MAX_SOL_BINDINGS / BRW_MAX_SOL_BUFFERS;
+
/* if conformance mode is set, swrast can handle any size AA point */
ctx->Const.MaxPointSizeAA = 255.0;
 
diff --git a/src/mesa/drivers/dri/i965/brw_context.h 
b/src/mesa/drivers/dri/i965/brw_context.h
index 70a45c7..da1de02 100644
--- a/src/mesa/drivers/dri/i965/brw_context.h
+++ b/src/mesa/d

[Mesa-dev] [PATCH 6/8] i965 gen6: Turn on transform feedback extension.

2011-12-13 Thread Paul Berry
This patch advertises support for EXT_transform_feedback on Intel Gen6
and higher.

Since transform feedback support is not completely finished yet, for
now we only advertise support for it when MESA_GL_VERSION_OVERRIDE is
3.0 or greater (since transform feedback is required by GL version
3.0).
---
 src/mesa/drivers/dri/intel/intel_extensions.c |7 +--
 1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/src/mesa/drivers/dri/intel/intel_extensions.c 
b/src/mesa/drivers/dri/intel/intel_extensions.c
index 681f5f2..41ae29e 100644
--- a/src/mesa/drivers/dri/intel/intel_extensions.c
+++ b/src/mesa/drivers/dri/intel/intel_extensions.c
@@ -98,10 +98,13 @@ intelInitExtensions(struct gl_context *ctx)
ctx->Extensions.OES_EGL_image = true;
 #endif
 
-   if (intel->gen >= 6)
+   if (intel->gen >= 6) {
   ctx->Const.GLSLVersion = 130;
-   else
+  if (override_version >= 30)
+ ctx->Extensions.EXT_transform_feedback = true;
+   } else {
   ctx->Const.GLSLVersion = 120;
+   }
_mesa_override_glsl_version(ctx);
 
if (intel->gen >= 5)
-- 
1.7.6.4

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH 7/8] i965 gen6+: Make intel_batchbuffer_emit_mi_flush() actually flush.

2011-12-13 Thread Paul Berry
Previous to this patch, the function intel_batchbuffer_emit_mi_flush()
was a bit of a misnomer.  On Gen4+, when not using the blit engine, it
didn't actually flush the pipeline--it simply generated a
_3DSTATE_PIPE_CONTROL command with the necessary bits set to flush GPU
caches.  This was usually sufficient, since in most situations where
intel_batchbuffer_emit_mi_flush() wass called, all we really care
about was ensuring cache coherency.

However, with the advent of OpenGL 3.0, there are two cases in which
data output by one stage of the pipeline might be consumed, in a later
draw operation, by an earlier stage of the pipeline:

(a) When using textures in the vertex shader.

(b) When using drawing with a vertex buffer that was previously
generated using transform feedback.

This patch addresses case (a) by changing
intel_batchbuffer_emit_mi_flush() so that on Gen6+, it sets the
PIPE_CONTROL_CS_STALL bit (this forces the pipeline to actually
flush).  (Case (b) will be addressed by the next patch in the series).

This is not an ideal solution--in a perfect world, the driver would
have some buffer dependency tracking so that we would only have to
flush the pipeline in the two cases above.  Until that dependency
tracking is implemented, however, it seems prudent to have
intel_batchbuffer_emit_mi_flush() actually flush the pipeline, so that
we get correct rendering, at the expense of a (hopefully small)
performance hit.

The change is only applied to Gen6+, since at the moment only Gen6+
supports the OpenGL 3.0 features that make a full pipeline flush
necessary.
---
 src/mesa/drivers/dri/intel/intel_batchbuffer.c |3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/src/mesa/drivers/dri/intel/intel_batchbuffer.c 
b/src/mesa/drivers/dri/intel/intel_batchbuffer.c
index 6991db8..4ff098a 100644
--- a/src/mesa/drivers/dri/intel/intel_batchbuffer.c
+++ b/src/mesa/drivers/dri/intel/intel_batchbuffer.c
@@ -461,7 +461,8 @@ intel_batchbuffer_emit_mi_flush(struct intel_context *intel)
   PIPE_CONTROL_WRITE_FLUSH |
   PIPE_CONTROL_DEPTH_CACHE_FLUSH |
   PIPE_CONTROL_TC_FLUSH |
-  PIPE_CONTROL_NO_WRITE);
+  PIPE_CONTROL_NO_WRITE |
+   PIPE_CONTROL_CS_STALL);
 OUT_BATCH(0); /* write address */
 OUT_BATCH(0); /* write data */
 ADVANCE_BATCH();
-- 
1.7.6.4

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH 8/8] i965: Flush pipeline on EndTransformFeedback.

2011-12-13 Thread Paul Berry
A common use case for transform feedback is to perform one draw
operation that writes transform feedback output to a buffer, followed
by a second draw operation that consumes that buffer as vertex input.
Since vertex input is consumed at an earlier pipeline stage than
writing transform feedback output, we need to flush the pipeline to
ensure that the transform feedback output is completely written before
the data is consumed.

In an ideal world, we would do some dependency tracking, so that we
would only flush the pipeline if the next draw call was about to
consume data generated by a previous draw call in the same batch.
However, since we don't have that sort of dependency tracking
infrastructure right now, we just unconditionally flush the buffer
every time glEndTransformFeedback() is called.  This will cause a
performance hit compared to the ideal case (since we will sometimes
flush the pipeline unnecessarily), but fortunately the performance hit
will be confined to circumstances where transform feedback is in use.
---
 src/mesa/drivers/dri/i965/brw_context.c |8 ++--
 src/mesa/drivers/dri/i965/brw_context.h |5 +
 src/mesa/drivers/dri/i965/gen6_sol.c|   16 
 3 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_context.c 
b/src/mesa/drivers/dri/i965/brw_context.c
index eb68bf4..f7b88c3 100644
--- a/src/mesa/drivers/dri/i965/brw_context.c
+++ b/src/mesa/drivers/dri/i965/brw_context.c
@@ -52,6 +52,7 @@
 
 #include "tnl/t_pipeline.h"
 #include "glsl/ralloc.h"
+#include "intel_batchbuffer.h"
 
 /***
  * Mesa's Driver Functions
@@ -109,7 +110,8 @@ static void brwPrepareExecBegin(struct gl_context *ctx)
}
 }
 
-static void brwInitDriverFunctions( struct dd_function_table *functions )
+static void brwInitDriverFunctions(struct intel_context *intel,
+   struct dd_function_table *functions)
 {
intelInitDriverFunctions( functions );
 
@@ -117,6 +119,8 @@ static void brwInitDriverFunctions( struct 
dd_function_table *functions )
brw_init_queryobj_functions(functions);
 
functions->PrepareExecBegin = brwPrepareExecBegin;
+   if (intel->gen == 6)
+  functions->EndTransformFeedback = gen6_end_transform_feedback;
 }
 
 bool
@@ -136,7 +140,7 @@ brwCreateContext(int api,
   return false;
}
 
-   brwInitDriverFunctions( &functions );
+   brwInitDriverFunctions(intel, &functions);
 
if (!intelInitContext( intel, api, mesaVis, driContextPriv,
  sharedContextPrivate, &functions )) {
diff --git a/src/mesa/drivers/dri/i965/brw_context.h 
b/src/mesa/drivers/dri/i965/brw_context.h
index da1de02..fa6c883 100644
--- a/src/mesa/drivers/dri/i965/brw_context.h
+++ b/src/mesa/drivers/dri/i965/brw_context.h
@@ -1071,6 +1071,11 @@ void brw_init_surface_formats(struct brw_context *brw);
 bool
 brw_fprog_uses_noperspective(const struct gl_fragment_program *fprog);
 
+/* gen6_sol.c */
+void
+gen6_end_transform_feedback(struct gl_context *ctx,
+struct gl_transform_feedback_object *obj);
+
 
 
 /*==
diff --git a/src/mesa/drivers/dri/i965/gen6_sol.c 
b/src/mesa/drivers/dri/i965/gen6_sol.c
index af372c1..53e3325 100644
--- a/src/mesa/drivers/dri/i965/gen6_sol.c
+++ b/src/mesa/drivers/dri/i965/gen6_sol.c
@@ -28,6 +28,7 @@
 
 #include "brw_context.h"
 #include "intel_buffer_objects.h"
+#include "intel_batchbuffer.h"
 #include "brw_defines.h"
 
 static void
@@ -100,3 +101,18 @@ const struct brw_tracked_state gen6_sol_surface = {
},
.emit = brw_update_sol_surfaces,
 };
+
+void
+gen6_end_transform_feedback(struct gl_context *ctx,
+struct gl_transform_feedback_object *obj)
+{
+   /* After EndTransformFeedback, it's likely that the client program will try
+* to draw using the contents of the transform feedback buffer as vertex
+* input.  In order for this to work, we need to flush the data through at
+* least the GS stage of the pipeline, and flush out the render cache.  For
+* simplicity, just do a full flush.
+*/
+   struct brw_context *brw = brw_context(ctx);
+   struct intel_context *intel = &brw->intel;
+   intel_batchbuffer_emit_mi_flush(intel);
+}
-- 
1.7.6.4

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 1/8] mesa: Record transform feedback stride in linker output.

2011-12-13 Thread Brian Paul
On Tue, Dec 13, 2011 at 4:35 PM, Paul Berry  wrote:
> This patch adds the field gl_transform_feedback_info::BufferStride,
> which records the total number of components (per vertex) that
> transform feedback is being instructed to store in each buffer.  The
> i965 gen6 back-end needs this information in order to set up binding
> tables, and it seems better to have the linker provide this
> information rather than force the back-end to recompute it.
> ---
>  src/glsl/linker.cpp    |    4 +++-
>  src/mesa/main/mtypes.h |    5 +
>  2 files changed, 8 insertions(+), 1 deletions(-)
>
> diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
> index b8a7126..5eb2a20 100644
> --- a/src/glsl/linker.cpp
> +++ b/src/glsl/linker.cpp
> @@ -1599,6 +1599,7 @@ tfeedback_decl::store(struct gl_shader_program *prog,
>       info->Outputs[info->NumOutputs].NumComponents = this->vector_elements;
>       info->Outputs[info->NumOutputs].OutputBuffer = buffer;
>       ++info->NumOutputs;
> +      info->BufferStride[buffer] += this->vector_elements;
>    }
>    return true;
>  }
> @@ -1863,7 +1864,8 @@ store_tfeedback_info(struct gl_context *ctx, struct 
> gl_shader_program *prog,
>                      tfeedback_decl *tfeedback_decls)
>  {
>    unsigned total_tfeedback_components = 0;
> -   prog->LinkedTransformFeedback.NumOutputs = 0;
> +   memset(&prog->LinkedTransformFeedback, 0,
> +          sizeof(prog->LinkedTransformFeedback));
>    for (unsigned i = 0; i < num_tfeedback_decls; ++i) {
>       unsigned buffer =
>          prog->TransformFeedback.BufferMode == GL_SEPARATE_ATTRIBS ? i : 0;
> diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
> index 1934349..d4c600a 100644
> --- a/src/mesa/main/mtypes.h
> +++ b/src/mesa/main/mtypes.h
> @@ -1822,6 +1822,11 @@ struct gl_transform_feedback_info {
>       unsigned OutputBuffer;
>       unsigned NumComponents;
>    } Outputs[MAX_PROGRAM_OUTPUTS];
> +
> +   /**
> +    * Number of components stored in each buffer.
> +    */
> +   unsigned BufferStride[MAX_FEEDBACK_ATTRIBS];
>  };

Minor nit: I'd use a single line comment there: /** Number of
components... */ to be consistent with other comments.

The rest looks OK to me otherwise.  Reviewed-by: Brian Paul
  But if you have any doubts, another one of the
GLSL people should probably review the rest too.

-Brian
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 07/14] i965/vs: Implement vec4_visitor::visit(ir_texture *).

2011-12-13 Thread Kenneth Graunke
On 12/13/2011 01:12 PM, Eric Anholt wrote:
> On Thu,  8 Dec 2011 17:07:58 -0800, Kenneth Graunke  
> wrote:
>> This translates the GLSL compiler's IR into vec4_instruction IR,
>> generating code to load coordinates, LOD info, shadow comparitors, and
>> so on into the appropriate message registers.
>>
>> It turns out that the SIMD4x2 parameters are identical on Gen 5-7, and
>> the Gen4 code is similar enough that, unlike in the FS, it's easy enough
>> to support all generations in a single function.
>>
>> Signed-off-by: Kenneth Graunke 
>> ---
>>  src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp |  114 
>> ++--
>>  1 files changed, 107 insertions(+), 7 deletions(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp 
>> b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
>> index 853c3ee..85490bb 100644
>> --- a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
> 
>> +   vec4_instruction *inst;
>> +   switch (ir->op) {
>> +   case ir_tex:
> 
> Isn't this one invalid for vertex shaders, too?

Nope.  From the GLSL 1.30 spec, page 80 (86 in the PDF):
"The implicit level of detail is selected as follows: For a texture that
is not mip-mapped, the texture is used directly.  If it is mip-mapped
and running in a fragment shader, the LOD computed by the implementation
is used to do the texture lookup.  If it is mip-mapped and running on
the vertex shader, then the base texture is used."

So ir_tex is equivalent to ir_txl with lod 0.

>> +   case ir_txl:
>> +  inst = new(mem_ctx) vec4_instruction(this, SHADER_OPCODE_TXL);
>> +  break;
>> +   case ir_txd:
>> +  inst = new(mem_ctx) vec4_instruction(this, SHADER_OPCODE_TXD);
>> +  break;
>> +   case ir_txf:
>> +  inst = new(mem_ctx) vec4_instruction(this, SHADER_OPCODE_TXF);
>> +  break;
>> +   case ir_txs:
>> +  inst = new(mem_ctx) vec4_instruction(this, SHADER_OPCODE_TXS);
>> +  break;
>> +   case ir_txb:
>> +  assert(!"TXB is not valid for vertex shaders.");
>> +   }
> 
>> +  /* Load the LOD info */
>> +  if (ir->op == ir_txl) {
>> + ir->lod_info.lod->accept(this);
>> + int mrf, writemask;
>> + if (intel->gen >= 5) {
>> +mrf = param_base + 1;
>> +writemask = ir->shadow_comparitor ? WRITEMASK_Y : WRITEMASK_X;
>> +inst->mlen++;
>> + } else /* intel->gen == 4 */ {
>> +mrf = param_base;
>> +writemask = WRITEMASK_Z;
>> + }
>> + emit(MOV(dst_reg(MRF, mrf, ir->lod_info.lod->type, writemask),
>> +  this->result));
> 
> I'd move the accept down next to the MOV.  I'm always afraid of the
> hidden return data from accept getting trashed before it gets consumed.
> I wish the visitor stuff let us do the sensible thing and produce a
> return value.

I totally agree - they used to be right next to each other before I
added the Gen4 code.  I'll move it.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 2/8] mesa: Fix off-by-one error in transform feedback size check.

2011-12-13 Thread Brian Paul
On Tue, Dec 13, 2011 at 4:35 PM, Paul Berry  wrote:
> In _mesa_BindBufferRange(), we need to verify that the offset and size
> specified by the client do not exceed the size of the underlying
> buffer.  We were accidentally doing this check using ">=" rather than
> ">", so we were generating a bogus error if the client specified an
> offset and size that fit exactly in the underlying buffer.
> ---
>  src/mesa/main/transformfeedback.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/src/mesa/main/transformfeedback.c 
> b/src/mesa/main/transformfeedback.c
> index 799245d..78ca64d 100644
> --- a/src/mesa/main/transformfeedback.c
> +++ b/src/mesa/main/transformfeedback.c
> @@ -486,7 +486,7 @@ _mesa_BindBufferRange(GLenum target, GLuint index,
>       return;
>    }
>
> -   if (offset + size >= bufObj->Size) {
> +   if (offset + size > bufObj->Size) {
>       _mesa_error(ctx, GL_INVALID_VALUE,
>                   "glBindBufferRange(offset + size %d > buffer size %d)",
>                  (int) (offset + size), (int) (bufObj->Size));

Reviewed-by: Brian Paul 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 0/2 v2] Add support for clip distances in Gallium

2011-12-13 Thread Ian Romanick

On 12/13/2011 01:25 PM, Jose Fonseca wrote:



- Original Message -

On 12/13/2011 03:09 PM, Jose Fonseca wrote:


- Original Message -

On 12/13/2011 12:26 PM, Bryan Cain wrote:

On 12/13/2011 02:11 PM, Jose Fonseca wrote:

- Original Message -

This is an updated version of the patch set I sent to the list
a
few
hours
ago.
There is now a TGSI property called
TGSI_PROPERTY_NUM_CLIP_DISTANCES
that drivers can use to determine how many of the 8 available
clip
distances
are actually used by a shader.

Can't the info in TGSI_PROPERTY_NUM_CLIP_DISTANCES be easily
derived from the shader, and queried through
src/gallium/auxiliary/tgsi/tgsi_scan.h ?

No.  The clip distances can be indirectly addressed (there are up
to 2
of them in vec4 form for a total of 8 floats), which makes it
impossible
to determine which ones are used by analyzing the shader.

The description is almost complete. :)  The issue is that the
shader
may
declare

out float gl_ClipDistance[4];

the use non-constant addressing of the array.  The compiler knows
that
gl_ClipDistance has at most 4 elements, but post-hoc analysis
would
not
be able to determine that.  Often the fixed-function hardware (see
below) needs to know which clip distance values are actually
written.

But don't all the clip distances written by the shader need to be
declared?

E.g.:

DCL OUT[0], CLIPDIST[0]
DCL OUT[1], CLIPDIST[1]
DCL OUT[2], CLIPDIST[2]
DCL OUT[3], CLIPDIST[3]

therefore a trivial analysis of the declarations convey that?


No.  Clip distance is an array of up to 8 floats in GLSL, but it's
represented in the hardware as 2 vec4s.  You can tell by analyzing
the
declarations whether there are more than 4 clip distances in use, but
not which components the shader writes to.
TGSI_PROPERTY_NUM_CLIP_DISTANCES is the number of components in use,
not
the number of full vectors.


Lets imagine

   out float gl_ClipDistance[6];

Each a clip distance is a scalar float.

Either all hardware represents the 8 clip distances as two 4 vectors, and we do:

   DCL OUT[0].xywz, CLIPDIST[0]
   DCL OUT[1].xy, CLIPDIST[1]

using the full range of struct tgsi_declaration::UsageMask [1] or we represent 
them as as scalars:

   DCL OUT[0].x, CLIPDIST[0]
   DCL OUT[1].x, CLIPDIST[1]
   DCL OUT[2].x, CLIPDIST[2]
   DCL OUT[3].x, CLIPDIST[3]
   DCL OUT[4].x, CLIPDIST[4]
   DCL OUT[5].x, CLIPDIST[5]

If indirect addressing is allowed as I read bore, then maybe the later is 
better.


As far as I'm aware, all hardware represents it as the former, and we 
have a lowering pass to fix-up the float[] accesses to be vec4[] accesses.

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 0/2 v2] Add support for clip distances in Gallium

2011-12-13 Thread Ian Romanick

On 12/13/2011 02:12 PM, Jose Fonseca wrote:



- Original Message -

On 12/13/2011 03:48 PM, Jose Fonseca wrote:

- Original Message -

On 12/13/2011 03:25 PM, Jose Fonseca wrote:

- Original Message -

On 12/13/2011 03:09 PM, Jose Fonseca wrote:

- Original Message -

On 12/13/2011 12:26 PM, Bryan Cain wrote:

On 12/13/2011 02:11 PM, Jose Fonseca wrote:

- Original Message -

This is an updated version of the patch set I sent to the
list
a
few
hours
ago.
There is now a TGSI property called
TGSI_PROPERTY_NUM_CLIP_DISTANCES
that drivers can use to determine how many of the 8
available
clip
distances
are actually used by a shader.

Can't the info in TGSI_PROPERTY_NUM_CLIP_DISTANCES be easily
derived from the shader, and queried through
src/gallium/auxiliary/tgsi/tgsi_scan.h ?

No.  The clip distances can be indirectly addressed (there
are
up
to 2
of them in vec4 form for a total of 8 floats), which makes it
impossible
to determine which ones are used by analyzing the shader.

The description is almost complete. :)  The issue is that the
shader
may
declare

out float gl_ClipDistance[4];

the use non-constant addressing of the array.  The compiler
knows
that
gl_ClipDistance has at most 4 elements, but post-hoc analysis
would
not
be able to determine that.  Often the fixed-function hardware
(see
below) needs to know which clip distance values are actually
written.

But don't all the clip distances written by the shader need to
be
declared?

E.g.:

DCL OUT[0], CLIPDIST[0]
DCL OUT[1], CLIPDIST[1]
DCL OUT[2], CLIPDIST[2]
DCL OUT[3], CLIPDIST[3]

therefore a trivial analysis of the declarations convey that?

No.  Clip distance is an array of up to 8 floats in GLSL, but
it's
represented in the hardware as 2 vec4s.  You can tell by
analyzing
the
declarations whether there are more than 4 clip distances in
use,
but
not which components the shader writes to.
TGSI_PROPERTY_NUM_CLIP_DISTANCES is the number of components in
use,
not
the number of full vectors.

Lets imagine

   out float gl_ClipDistance[6];

Each a clip distance is a scalar float.

Either all hardware represents the 8 clip distances as two 4
vectors, and we do:

   DCL OUT[0].xywz, CLIPDIST[0]
   DCL OUT[1].xy, CLIPDIST[1]

using the full range of struct tgsi_declaration::UsageMask [1] or
we represent them as as scalars:

   DCL OUT[0].x, CLIPDIST[0]
   DCL OUT[1].x, CLIPDIST[1]
   DCL OUT[2].x, CLIPDIST[2]
   DCL OUT[3].x, CLIPDIST[3]
   DCL OUT[4].x, CLIPDIST[4]
   DCL OUT[5].x, CLIPDIST[5]

If indirect addressing is allowed as I read bore, then maybe the
later is better.

I confess my ignorance about clipping and maybe I'm being dense,
but I still don't see the need for this
TGSI_PROPERTY_NUM_CLIP_DISTANCES.  Could you please draft an
example TGSI shader showing this property (or just paste one
generated with your change)?  I think that would help a lot.


Jose


[1] I don't know if tgsi_dump pays much attention to
  tgsi_declaration::UsageMask, but it does exist.

UsageMask might work, but before that can be considered a viable
solution, someone will need to make it possible to actually
declare
it
from ureg.  As it is, ureg is hardcoded to set UsageMask to xyzw
no
matter what on all declared inputs and outputs.

ureg automatically fills the UsageMask from the destionation
register masks, since it easy to determine from the opcodes.

Which leads me to my second point, if indirect addressing of
CLIPDIST is allowed, then we can't really pack the clip distance
as 4-elem vectors in TGSI: not only the syntax would be very
weird, but it would create havoc on all tgsi-translating code that
makes decisions based on indirect addressing of registers.

That is,

   float gl_ClipDistance[6];

   gl_ClipDistance[i] = foo;

would become

DCL OUT[0].x, CLIPDIST[0]
DCL OUT[1].x, CLIPDIST[1]
DCL OUT[2].x, CLIPDIST[2]
DCL OUT[3].x, CLIPDIST[3]
DCL OUT[4].x, CLIPDIST[4]
DCL OUT[5].x, CLIPDIST[5]
MOV OUT[ADDR[0].x].x, foo

and the info from TGSI_PROPERTY_NUM_CLIP_DISTANCES can be obtained
by walking the declaration (which can/should be done only once in
tgsi_scan).

But this just doesn't look like it would ever work:

DCL OUT[0].xyzw, CLIPDIST[0]
DCL OUT[1].xy  , CLIPDIST[1]
MOV OUT[ADDR[0].x]., foo

Jose


If ureg automatically fills the UsageMask from the accessed
components,
it's probably a better solution than the property.

About the indirect addressing of components: the GLSL compiler lowers
indirect addressing of the gl_ClipDistance array to indirect
addressing
of the 2 vec4s, combined with conditional moves to the different
components.  Which is okay, because although indirect addressing of
gl_ClipDistance is allowed by the GLSL specification, meaning have to
support it, it's not something that's actually useful in practical
situations.


It sounds a bit complicated -- the lowered indirect addressing code will 
certainly result in inefficient code for software rendering (e.g, draw

[Mesa-dev] [PATCH 1/2] mesa, gallium: add a cap for GPUs without unified color+generic varying slots

2011-12-13 Thread Marek Olšák
---
 src/gallium/drivers/r300/r300_screen.c |3 ++-
 src/gallium/include/pipe/p_defines.h   |3 ++-
 src/glsl/linker.cpp|6 ++
 src/mesa/main/mtypes.h |9 +
 src/mesa/state_tracker/st_extensions.c |3 +++
 5 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/src/gallium/drivers/r300/r300_screen.c 
b/src/gallium/drivers/r300/r300_screen.c
index 0bae065..a761939 100644
--- a/src/gallium/drivers/r300/r300_screen.c
+++ b/src/gallium/drivers/r300/r300_screen.c
@@ -102,6 +102,7 @@ static int r300_get_param(struct pipe_screen* pscreen, enum 
pipe_cap param)
 case PIPE_CAP_TEXTURE_BARRIER:
 case PIPE_CAP_TGSI_CAN_COMPACT_VARYINGS:
 case PIPE_CAP_TGSI_CAN_COMPACT_CONSTANTS:
+case PIPE_CAP_SEPARATE_COLOR_VARYINGS:
 return 1;
 
 /* r300 cannot do swizzling of compressed textures. Supported 
otherwise. */
@@ -182,7 +183,7 @@ static int r300_get_shader_param(struct pipe_screen 
*pscreen, unsigned shader, e
  * R500 has the ability to turn 3rd and 4th color into
  * additional texcoords but there is no two-sided color
  * selection then. However the facing bit can be used instead. */
-return 10;
+return 8;
 case PIPE_SHADER_CAP_MAX_CONSTS:
 return is_r500 ? 256 : 32;
 case PIPE_SHADER_CAP_MAX_CONST_BUFFERS:
diff --git a/src/gallium/include/pipe/p_defines.h 
b/src/gallium/include/pipe/p_defines.h
index 30f1d7f..5229c5f 100644
--- a/src/gallium/include/pipe/p_defines.h
+++ b/src/gallium/include/pipe/p_defines.h
@@ -467,7 +467,8 @@ enum pipe_cap {
PIPE_CAP_CONDITIONAL_RENDER = 52,
PIPE_CAP_TEXTURE_BARRIER = 53,
PIPE_CAP_TGSI_CAN_COMPACT_VARYINGS = 54, /* temporary */
-   PIPE_CAP_TGSI_CAN_COMPACT_CONSTANTS = 55 /* temporary */
+   PIPE_CAP_TGSI_CAN_COMPACT_CONSTANTS = 55, /* temporary */
+   PIPE_CAP_SEPARATE_COLOR_VARYINGS = 56
 };
 
 /**
diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
index b8a7126..e9298bb 100644
--- a/src/glsl/linker.cpp
+++ b/src/glsl/linker.cpp
@@ -1805,6 +1805,12 @@ assign_varying_locations(struct gl_context *ctx,
  */
 var->mode = ir_var_auto;
  } else {
+if (ctx->Const.GLSLSeparateColorVaryings &&
+(var->location == FRAG_ATTRIB_COL0 ||
+ var->location == FRAG_ATTRIB_COL1)) {
+   continue;
+}
+
 /* The packing rules are used for vertex shader inputs are also
  * used for fragment shader inputs.
  */
diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
index 1934349..9e9ad83 100644
--- a/src/mesa/main/mtypes.h
+++ b/src/mesa/main/mtypes.h
@@ -2839,6 +2839,15 @@ struct gl_constants
 */
GLboolean GLSLSkipStrictMaxVaryingLimitCheck;
GLboolean GLSLSkipStrictMaxUniformLimitCheck;
+
+   /**
+* Whether the color varyings do not share varying slots with generic
+* varyings. In such a case, the driver must not include the color
+* varyings in the maximum number of varyings limit. In return,
+* the GLSL linker will not count the color varyings to the number of
+* used varying components.
+*/
+   GLboolean GLSLSeparateColorVaryings;
 };
 
 
diff --git a/src/mesa/state_tracker/st_extensions.c 
b/src/mesa/state_tracker/st_extensions.c
index 457d5d6..7a7919f 100644
--- a/src/mesa/state_tracker/st_extensions.c
+++ b/src/mesa/state_tracker/st_extensions.c
@@ -228,6 +228,9 @@ void st_init_limits(struct st_context *st)
 
c->GLSLSkipStrictMaxVaryingLimitCheck =
   screen->get_param(screen, PIPE_CAP_TGSI_CAN_COMPACT_VARYINGS);
+
+   c->GLSLSeparateColorVaryings =
+  screen->get_param(screen, PIPE_CAP_SEPARATE_COLOR_VARYINGS);
 }
 
 
-- 
1.7.4.1

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH 2/2] glsl: don't include system values in varyings usage

2011-12-13 Thread Marek Olšák
---
 src/glsl/linker.cpp |7 +++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
index e9298bb..1086ef7 100644
--- a/src/glsl/linker.cpp
+++ b/src/glsl/linker.cpp
@@ -1811,6 +1811,13 @@ assign_varying_locations(struct gl_context *ctx,
continue;
 }
 
+/* System values are not varyings per-se.
+ * NOTE: gl_PointCoord does occupy a varying slot. */
+if (var->location == FRAG_ATTRIB_WPOS ||
+var->location == FRAG_ATTRIB_FACE) {
+   continue;
+}
+
 /* The packing rules are used for vertex shader inputs are also
  * used for fragment shader inputs.
  */
-- 
1.7.4.1

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH] meta: rework dest image allocation in mipmap generation code

2011-12-13 Thread Brian Paul
From: Brian Paul 

This fixes two things:
1. If the texture object was created with glTexStorage2D, the call
   to _mesa_TexImage2D() would generate INVALID_OPERATION since the
   texture is marked as immutable.
2. _mesa_TexImage2D() always frees any existing texture image memory
   before allocating new memory.  That's inefficient since the existing
   image is usually the right size already.  Now we only make the call
   when necessary.

v2: use _mesa_TexImage() in prepare_dest_image() to make sure side-effects
of changing a texture image are observed (like FBO completeness).
---
 src/mesa/drivers/common/meta.c |  129 +---
 1 files changed, 94 insertions(+), 35 deletions(-)

diff --git a/src/mesa/drivers/common/meta.c b/src/mesa/drivers/common/meta.c
index 259041f..976640b 100644
--- a/src/mesa/drivers/common/meta.c
+++ b/src/mesa/drivers/common/meta.c
@@ -2789,6 +2789,90 @@ setup_texture_coords(GLenum faceTarget,
 
 
 /**
+ * Helper function for mipmap generation.
+ * Make sure the specified destination mipmap level is the right size/format
+ * for mipmap generation.  If not, (re) allocate it.
+ * \return GL_TRUE if successful, GL_FALSE if mipmap generation should stop
+ */
+static GLboolean
+prepare_dest_image(struct gl_context *ctx,
+   struct gl_texture_object *texObj, GLuint level,
+   GLsizei width, GLsizei height, GLsizei depth,
+   GLsizei border, GLenum intFormat, gl_format format)
+{
+   const GLuint numFaces = texObj->Target == GL_TEXTURE_CUBE_MAP ? 6 : 1;
+   GLuint face;
+
+   if (texObj->Immutable) {
+  /* The texture was created with glTexStorage() so the number/size of
+   * mipmap levels is fixed and the storage for all images is already
+   * allocated.
+   */
+  if (!texObj->Image[0][level]) {
+ /* No more levels to create - we're done */
+ return GL_FALSE;
+  }
+  else {
+ /* Nothing to do - the texture memory must have already been
+  * allocated to the right size so we're all set.
+  */
+ return GL_TRUE;
+  }
+   }
+
+   for (face = 0; face < numFaces; face++) {
+  struct gl_texture_image *dstImage;
+  GLenum target;
+
+  if (numFaces == 1)
+ target = texObj->Target;
+  else
+ target = GL_TEXTURE_CUBE_MAP_POSITIVE_X + face;
+
+  dstImage = _mesa_get_tex_image(ctx, texObj, target, level);
+  if (!dstImage) {
+ /* out of memory */
+ return GL_FALSE;
+  }
+
+  if (dstImage->Width != width ||
+  dstImage->Height != height ||
+  dstImage->Depth != depth ||
+  dstImage->Border != border ||
+  dstImage->InternalFormat != intFormat ||
+  dstImage->TexFormat != format) {
+ /* need to (re)allocate image */
+ switch (target) {
+ case GL_TEXTURE_1D:
+_mesa_TexImage1D(target, level, intFormat, width, border,
+ GL_RGBA, GL_UNSIGNED_BYTE, NULL);
+break;
+ case GL_TEXTURE_2D:
+ case GL_TEXTURE_CUBE_MAP_POSITIVE_X:
+ case GL_TEXTURE_CUBE_MAP_NEGATIVE_X:
+ case GL_TEXTURE_CUBE_MAP_POSITIVE_Y:
+ case GL_TEXTURE_CUBE_MAP_NEGATIVE_Y:
+ case GL_TEXTURE_CUBE_MAP_POSITIVE_Z:
+ case GL_TEXTURE_CUBE_MAP_NEGATIVE_Z:
+_mesa_TexImage2D(target, level, intFormat, width, height, border,
+ GL_RGBA, GL_UNSIGNED_BYTE, NULL);
+break;
+ case GL_TEXTURE_3D:
+_mesa_TexImage3D(target, level, intFormat, width, height, depth,
+ border, GL_RGBA, GL_UNSIGNED_BYTE, NULL);
+break;
+ default:
+_mesa_problem(ctx, "bad texture target in prepare_dest_image()");
+return GL_FALSE;
+ }
+  }
+   }
+
+   return GL_TRUE;
+}
+
+
+/**
  * Called via ctx->Driver.GenerateMipmap()
  * Note: We don't yet support 3D textures, 1D/2D array textures or texture
  * borders.
@@ -2943,43 +3027,18 @@ _mesa_meta_GenerateMipmap(struct gl_context *ctx, 
GLenum target,
  break;
   }
 
-  /* Set MaxLevel large enough to hold the new level when we allocate it  
*/
+  /* Allocate storage for the destination mipmap image(s) */
+
+  /* Set MaxLevel large enough to hold the new level when we allocate it */
   _mesa_TexParameteri(target, GL_TEXTURE_MAX_LEVEL, dstLevel);
 
-  /* Create empty dest image */
-  if (target == GL_TEXTURE_1D) {
- _mesa_TexImage1D(target, dstLevel, srcImage->InternalFormat,
-  dstWidth, border,
-  GL_RGBA, GL_UNSIGNED_BYTE, NULL);
-  }
-  else if (target == GL_TEXTURE_3D) {
- _mesa_TexImage3D(target, dstLevel, srcImage->InternalFormat,
-  dstWidth, dstHeight, dstDepth, border,
-  GL_RGBA, GL_UNSIGNED_BYTE, NULL);
-  }
-  else {
- 

Re: [Mesa-dev] [PATCH 0/2 v2] Add support for clip distances in Gallium

2011-12-13 Thread Christoph Bumiller
On 12/14/2011 12:58 AM, Ian Romanick wrote:
> On 12/13/2011 01:25 PM, Jose Fonseca wrote:
>>
>>
>> - Original Message -
>>> On 12/13/2011 03:09 PM, Jose Fonseca wrote:

 - Original Message -
> On 12/13/2011 12:26 PM, Bryan Cain wrote:
>> On 12/13/2011 02:11 PM, Jose Fonseca wrote:
>>> - Original Message -
 This is an updated version of the patch set I sent to the list
 a
 few
 hours
 ago.
 There is now a TGSI property called
 TGSI_PROPERTY_NUM_CLIP_DISTANCES
 that drivers can use to determine how many of the 8 available
 clip
 distances
 are actually used by a shader.
>>> Can't the info in TGSI_PROPERTY_NUM_CLIP_DISTANCES be easily
>>> derived from the shader, and queried through
>>> src/gallium/auxiliary/tgsi/tgsi_scan.h ?
>> No.  The clip distances can be indirectly addressed (there are up
>> to 2
>> of them in vec4 form for a total of 8 floats), which makes it
>> impossible
>> to determine which ones are used by analyzing the shader.
> The description is almost complete. :)  The issue is that the
> shader
> may
> declare
>
> out float gl_ClipDistance[4];
>
> the use non-constant addressing of the array.  The compiler knows
> that
> gl_ClipDistance has at most 4 elements, but post-hoc analysis
> would
> not
> be able to determine that.  Often the fixed-function hardware (see
> below) needs to know which clip distance values are actually
> written.
 But don't all the clip distances written by the shader need to be
 declared?

 E.g.:

 DCL OUT[0], CLIPDIST[0]
 DCL OUT[1], CLIPDIST[1]
 DCL OUT[2], CLIPDIST[2]
 DCL OUT[3], CLIPDIST[3]

 therefore a trivial analysis of the declarations convey that?
>>>
>>> No.  Clip distance is an array of up to 8 floats in GLSL, but it's
>>> represented in the hardware as 2 vec4s.  You can tell by analyzing
>>> the
>>> declarations whether there are more than 4 clip distances in use, but
>>> not which components the shader writes to.
>>> TGSI_PROPERTY_NUM_CLIP_DISTANCES is the number of components in use,
>>> not
>>> the number of full vectors.
>>
>> Lets imagine
>>
>>out float gl_ClipDistance[6];
>>
>> Each a clip distance is a scalar float.
>>
>> Either all hardware represents the 8 clip distances as two 4 vectors,
>> and we do:
>>
>>DCL OUT[0].xywz, CLIPDIST[0]
>>DCL OUT[1].xy, CLIPDIST[1]
>>
>> using the full range of struct tgsi_declaration::UsageMask [1] or we
>> represent them as as scalars:
>>
>>DCL OUT[0].x, CLIPDIST[0]
>>DCL OUT[1].x, CLIPDIST[1]
>>DCL OUT[2].x, CLIPDIST[2]
>>DCL OUT[3].x, CLIPDIST[3]
>>DCL OUT[4].x, CLIPDIST[4]
>>DCL OUT[5].x, CLIPDIST[5]
>>
>> If indirect addressing is allowed as I read bore, then maybe the later
>> is better.
> 
> As far as I'm aware, all hardware represents it as the former, and we
> have a lowering pass to fix-up the float[] accesses to be vec4[] accesses.

GeForce8+ = scalar architecture, no vectors, addresses are byte based,
can access individual components just fine.

Something like:

gl_ClipDistance[i - 12] = some_value;

DCL OUT[0].xyzw, POSITION
DCL OUT[1-8].x, CLIPDIST[0-7]

MOV OUT<1>[ADDR[0].x - 12].x, TEMP[0].
*  **

*   - tgsi_dimension.Index specifying the base address by referencing a
declaration
**  - tgsi_src_register.Index

is the only way I see to make this work nicely on all hardware.

(This is also needed if OUT[i] and OUT[i + 1] cannot be assigned to
contiguous hardware resources because of semantic.)

For constrained hardware the driver can build the clunky

c := ADDR[0].x % 4
i := ADDR[0].x / 4
IF [c == 0]
  MOV OUT[i].x, TEMP[0].
ELSE
IF [c == 1]
  MOV OUT[i].y, TEMP[0].
ELSE
IF [c == 2]
  MOV OUT[i].z, TEMP[0].
ELSE
  MOV OUT[i].w, TEMP[0].
ENDIF

itself.

> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev