Re: [Mesa-dev] [PATCH V5 1/2] i965/blorp: Add bilinear filtering of samples for multisample scaled blits

2013-07-01 Thread Paul Berry 
On 28 June 2013 16:59, Anuj Phogat anuj.pho...@gmail.com wrote:

 Current implementation of ext_framebuffer_multisample_blit_scaled in
 i965/blorp uses nearest filtering for multisample scaled blits. Using
 nearest filtering produces blocky artifacts and negates the benefits
 of MSAA. That is the reason why extension was not enabled on i965.

 This patch implements the bilinear filtering of samples in blorp engine.
 Images generated with this patch are free from blocky artifacts and show
 big improvement in visual quality.

 Observed no piglit and gles3 regressions.

 V3:
 - Algorithm used for filtering assumes a rectangular grid of samples
   roughly corresponding to sample locations.
 - Test the boundary conditions on the edges of texture.

 V4:
 - Clip texcoords and use conditional MOVs.
 - Send texture dimensions as push constants.
 - Remove the optimization in case of scaled multisample blits.

 V5:
 - Move mcs_fetch() inside the 'for' loop after computing pixel coordinates.

 Signed-off-by: Anuj Phogat anuj.pho...@gmail.com
 ---

  src/mesa/drivers/dri/i965/brw_blorp.h|  16 ++
  src/mesa/drivers/dri/i965/brw_blorp_blit.cpp | 278
 +--
  2 files changed, 273 insertions(+), 21 deletions(-)

 diff --git a/src/mesa/drivers/dri/i965/brw_blorp.h
 b/src/mesa/drivers/dri/i965/brw_blorp.h
 index ffc27cc..9277d09 100644
 --- a/src/mesa/drivers/dri/i965/brw_blorp.h
 +++ b/src/mesa/drivers/dri/i965/brw_blorp.h
 @@ -178,8 +178,15 @@ struct brw_blorp_wm_push_constants
 uint32_t dst_x1;
 uint32_t dst_y0;
 uint32_t dst_y1;
 +   /* Top right coordinates of the rectangular sample grid used for
 +* multisample scaled blitting.
 +*/
 +   float sample_grid_x1;
 +   float sample_grid_y1;
 brw_blorp_coord_transform_params x_transform;
 brw_blorp_coord_transform_params y_transform;
 +   /* Pad out to an integral number of registers */
 +   uint32_t pad[6];
  };

  /* Every 32 bytes of push constant data constitutes one GEN register. */
 @@ -319,6 +326,15 @@ struct brw_blorp_blit_prog_key
  * than one sample per pixel.
  */
 bool persample_msaa_dispatch;
 +
 +   /* True for scaled blitting. */
 +   bool blit_scaled;
 +
 +   /* Scale factors between the pixel grid and the grid of samples. We're
 +* using grid of samples for bilinear filetring in multisample scaled
 blits.
 +*/
 +   float x_scale;
 +   float y_scale;
  };

  class brw_blorp_blit_params : public brw_blorp_params
 diff --git a/src/mesa/drivers/dri/i965/brw_blorp_blit.cpp
 b/src/mesa/drivers/dri/i965/brw_blorp_blit.cpp
 index 8694128..d39bae1 100644
 --- a/src/mesa/drivers/dri/i965/brw_blorp_blit.cpp
 +++ b/src/mesa/drivers/dri/i965/brw_blorp_blit.cpp
 @@ -622,7 +622,8 @@ private:
 void kill_if_outside_dst_rect();
 void translate_dst_to_src();
 void single_to_blend();
 -   void manual_blend(unsigned num_samples);
 +   void manual_blend_average(unsigned num_samples);
 +   void manual_blend_bilinear(unsigned num_samples);
 void sample(struct brw_reg dst);
 void texel_fetch(struct brw_reg dst);
 void mcs_fetch();
 @@ -651,6 +652,11 @@ private:
 struct brw_reg dst_x1;
 struct brw_reg dst_y0;
 struct brw_reg dst_y1;
 +   /* Top right coordinates of the rectangular sample grid used for
 +* multisample scaled blitting.
 +*/
 +   struct brw_reg sample_grid_x1;
 +   struct brw_reg sample_grid_y1;
 struct {
struct brw_reg multiplier;
struct brw_reg offset;
 @@ -676,6 +682,16 @@ private:
  */
 struct brw_reg y_coords[2];

 +   /* X, Y coordinates of the pixel from which we need to fetch the
 specific
 +*  sample. These are used for multisample scaled blitting.
 +*/
 +   struct brw_reg x_sample_coords;
 +   struct brw_reg y_sample_coords;
 +
 +   /* Fractional parts of the x and y coordinates, used as bilinear
 interpolation coefficients */
 +   struct brw_reg x_frac;
 +   struct brw_reg y_frac;
 +
 /* Which element of x_coords and y_coords is currently in use.
  */
 int xy_coord_index;
 @@ -814,15 +830,17 @@ brw_blorp_blit_program::compile(struct brw_context
 *brw,
  * that we want to texture from.  Exception: if we are blending, then
 S is
  * irrelevant, because we are going to fetch all samples.
  */
 -   if (key-blend) {
 +   if (key-blend  !key-blit_scaled) {
if (brw-intel.gen == 6) {
   /* Gen6 hardware an automatically blend using the SAMPLE message
 */
   single_to_blend();
   sample(texture_data[0]);
} else {
   /* Gen7+ hardware doesn't automaticaly blend. */
 - manual_blend(key-src_samples);
 + manual_blend_average(key-src_samples);
}
 +   } else if(key-blend  key-blit_scaled) {
 +  manual_blend_bilinear(key-src_samples);
 } else {
/* We aren't blending, which means we just want to fetch a single
 sample
 * from the source surface.  The address that we want to fetch from
 is
 @@ -872,18

[Mesa-dev] [PATCH V5 1/2] i965/blorp: Add bilinear filtering of samples for multisample scaled blits

2013-06-28 Thread Anuj Phogat 
Current implementation of ext_framebuffer_multisample_blit_scaled in
i965/blorp uses nearest filtering for multisample scaled blits. Using
nearest filtering produces blocky artifacts and negates the benefits
of MSAA. That is the reason why extension was not enabled on i965.

This patch implements the bilinear filtering of samples in blorp engine.
Images generated with this patch are free from blocky artifacts and show
big improvement in visual quality.

Observed no piglit and gles3 regressions.

V3:
- Algorithm used for filtering assumes a rectangular grid of samples
  roughly corresponding to sample locations.
- Test the boundary conditions on the edges of texture.

V4:
- Clip texcoords and use conditional MOVs.
- Send texture dimensions as push constants.
- Remove the optimization in case of scaled multisample blits.

V5:
- Move mcs_fetch() inside the 'for' loop after computing pixel coordinates.

Signed-off-by: Anuj Phogat anuj.pho...@gmail.com
---

 src/mesa/drivers/dri/i965/brw_blorp.h|  16 ++
 src/mesa/drivers/dri/i965/brw_blorp_blit.cpp | 278 +--
 2 files changed, 273 insertions(+), 21 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_blorp.h 
b/src/mesa/drivers/dri/i965/brw_blorp.h
index ffc27cc..9277d09 100644
--- a/src/mesa/drivers/dri/i965/brw_blorp.h
+++ b/src/mesa/drivers/dri/i965/brw_blorp.h
@@ -178,8 +178,15 @@ struct brw_blorp_wm_push_constants
uint32_t dst_x1;
uint32_t dst_y0;
uint32_t dst_y1;
+   /* Top right coordinates of the rectangular sample grid used for
+* multisample scaled blitting.
+*/
+   float sample_grid_x1;
+   float sample_grid_y1;
brw_blorp_coord_transform_params x_transform;
brw_blorp_coord_transform_params y_transform;
+   /* Pad out to an integral number of registers */
+   uint32_t pad[6];
 };
 
 /* Every 32 bytes of push constant data constitutes one GEN register. */
@@ -319,6 +326,15 @@ struct brw_blorp_blit_prog_key
 * than one sample per pixel.
 */
bool persample_msaa_dispatch;
+
+   /* True for scaled blitting. */
+   bool blit_scaled;
+
+   /* Scale factors between the pixel grid and the grid of samples. We're
+* using grid of samples for bilinear filetring in multisample scaled blits.
+*/
+   float x_scale;
+   float y_scale;
 };
 
 class brw_blorp_blit_params : public brw_blorp_params
diff --git a/src/mesa/drivers/dri/i965/brw_blorp_blit.cpp 
b/src/mesa/drivers/dri/i965/brw_blorp_blit.cpp
index 8694128..d39bae1 100644
--- a/src/mesa/drivers/dri/i965/brw_blorp_blit.cpp
+++ b/src/mesa/drivers/dri/i965/brw_blorp_blit.cpp
@@ -622,7 +622,8 @@ private:
void kill_if_outside_dst_rect();
void translate_dst_to_src();
void single_to_blend();
-   void manual_blend(unsigned num_samples);
+   void manual_blend_average(unsigned num_samples);
+   void manual_blend_bilinear(unsigned num_samples);
void sample(struct brw_reg dst);
void texel_fetch(struct brw_reg dst);
void mcs_fetch();
@@ -651,6 +652,11 @@ private:
struct brw_reg dst_x1;
struct brw_reg dst_y0;
struct brw_reg dst_y1;
+   /* Top right coordinates of the rectangular sample grid used for
+* multisample scaled blitting.
+*/
+   struct brw_reg sample_grid_x1;
+   struct brw_reg sample_grid_y1;
struct {
   struct brw_reg multiplier;
   struct brw_reg offset;
@@ -676,6 +682,16 @@ private:
 */
struct brw_reg y_coords[2];
 
+   /* X, Y coordinates of the pixel from which we need to fetch the specific
+*  sample. These are used for multisample scaled blitting.
+*/
+   struct brw_reg x_sample_coords;
+   struct brw_reg y_sample_coords;
+
+   /* Fractional parts of the x and y coordinates, used as bilinear 
interpolation coefficients */
+   struct brw_reg x_frac;
+   struct brw_reg y_frac;
+
/* Which element of x_coords and y_coords is currently in use.
 */
int xy_coord_index;
@@ -814,15 +830,17 @@ brw_blorp_blit_program::compile(struct brw_context *brw,
 * that we want to texture from.  Exception: if we are blending, then S is
 * irrelevant, because we are going to fetch all samples.
 */
-   if (key-blend) {
+   if (key-blend  !key-blit_scaled) {
   if (brw-intel.gen == 6) {
  /* Gen6 hardware an automatically blend using the SAMPLE message */
  single_to_blend();
  sample(texture_data[0]);
   } else {
  /* Gen7+ hardware doesn't automaticaly blend. */
- manual_blend(key-src_samples);
+ manual_blend_average(key-src_samples);
   }
+   } else if(key-blend  key-blit_scaled) {
+  manual_blend_bilinear(key-src_samples);
} else {
   /* We aren't blending, which means we just want to fetch a single sample
* from the source surface.  The address that we want to fetch from is
@@ -872,18 +890,21 @@ void
 brw_blorp_blit_program::alloc_push_const_regs(int base_reg)
 {
 #define CONST_LOC(name) offsetof(brw_blorp_wm_push_constants, name)
-#define ALLOC_REG(name) \
+#define

Re: [Mesa-dev] [PATCH V5 1/2] i965/blorp: Add bilinear filtering of samples for multisample scaled blits

2013-06-28 Thread Chris Forbes 
Anuj,

Yeah -- multisample textures on Gen7 are currently UMS for color. If
you wanted to enable support for CMS, it should be reasonably
straightforward, but requires some tweaks in the shader backend.

This looks like a really nice quality improvement -- for the series:

Acked-by: Chris Forbes chr...@ijw.co.nz
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev