[Cogl] [PATCH 1/3] texture-2d-slice: Fix the foreach_sub_texture_in_region implementation

[Robert Bragg]

This looks good to land to me:

Reviewed-by: Robert Bragg <robert at linux.intel.com>

thanks,
- Robert

On Tue, Dec 11, 2012 at 2:56 PM, Neil Roberts <neil at linux.intel.com> wrote:
> There were a few problems with the sub texture iterating code of
> sliced textures which were causing some conformance tests to fail when
> NPOT textures are disabled:
>
> • The spans are stored in un-normalized coordinates and the
>   coordinates passed to the foreach function are normalized. The
>   function was trying to un-normalize them before passing them to the
>   span iterator code but it was using the wrong factor which was
>   causing it to actually doubley normalize them.
>
> • The shim function to renormalize the coordinates before passing them
>   to the callback was renormalizing the sub-texture coordinates
>   instead of the virtual coordinates. The sub-texture coordinates are
>   already in the right scale for whatever is the underlying texture so
>   we don't need to touch them. Instead we need to normalize the
>   virtual coordinates because these are coming from the un-normalized
>   coordinates that we passed to the span iterating code.
>
> • The normalize factors passed to the span iterating were always 1.
>   The code uses this normalizing factor to round the incoming
>   coordinates to the nearest multiple of a full texture. It divides
>   the coordinates by the factor rather than multiplying so it looks
>   like we should be passing the virtual texture size here.
> ---
>  cogl/cogl-texture-2d-sliced.c     | 28 +++++++++++++++++-----------
>  tests/conform/test-conform-main.c |  4 ++--
>  2 files changed, 19 insertions(+), 13 deletions(-)
>
> diff --git a/cogl/cogl-texture-2d-sliced.c b/cogl/cogl-texture-2d-sliced.c
> index 6d1f530..96df692 100644
> --- a/cogl/cogl-texture-2d-sliced.c
> +++ b/cogl/cogl-texture-2d-sliced.c
> @@ -75,15 +75,18 @@ re_normalize_sub_texture_coords_cb (CoglTexture *sub_texture,
>                                      void *user_data)
>  {
>    ForeachData *data = user_data;
> +  /* The coordinates passed to the span iterating code were
> +   * un-normalized so we need to renormalize them before passing them
> +   * on */
>    float re_normalized_coords[4] =
>      {
> -      sub_texture_coords[0] * data->x_normalize_factor,
> -      sub_texture_coords[1] * data->y_normalize_factor,
> -      sub_texture_coords[2] * data->x_normalize_factor,
> -      sub_texture_coords[3] * data->y_normalize_factor
> +      meta_coords[0] * data->x_normalize_factor,
> +      meta_coords[1] * data->y_normalize_factor,
> +      meta_coords[2] * data->x_normalize_factor,
> +      meta_coords[3] * data->y_normalize_factor
>      };
>
> -  data->callback (sub_texture, re_normalized_coords, meta_coords,
> +  data->callback (sub_texture, sub_texture_coords, re_normalized_coords,
>                    data->user_data);
>  }
>
> @@ -115,19 +118,22 @@ _cogl_texture_2d_sliced_foreach_sub_texture_in_region (
>    data.x_normalize_factor = 1.0f / tex_2ds->width;
>    data.y_normalize_factor = 1.0f / tex_2ds->height;
>
> -  un_normalized_coords[0] = virtual_tx_1 * data.x_normalize_factor;
> -  un_normalized_coords[1] = virtual_ty_1 * data.y_normalize_factor;
> -  un_normalized_coords[2] = virtual_tx_2 * data.x_normalize_factor;
> -  un_normalized_coords[3] = virtual_ty_2 * data.y_normalize_factor;
> +  un_normalized_coords[0] = virtual_tx_1 * tex_2ds->width;
> +  un_normalized_coords[1] = virtual_ty_1 * tex_2ds->height;
> +  un_normalized_coords[2] = virtual_tx_2 * tex_2ds->width;
> +  un_normalized_coords[3] = virtual_ty_2 * tex_2ds->height;
>
> +  /* Note that the normalize factors passed here are the reciprocal of
> +   * the factors calculated above because the span iterating code
> +   * normalizes by dividing by the factor instead of multiplying */
>    _cogl_texture_spans_foreach_in_region (x_spans,
>                                           tex_2ds->slice_x_spans->len,
>                                           y_spans,
>                                           tex_2ds->slice_y_spans->len,
>                                           textures,
>                                           un_normalized_coords,
> -                                         1, /* x_normalize_factor */
> -                                         1, /* y_normalize_factor */
> +                                         tex_2ds->width,
> +                                         tex_2ds->height,
>                                           COGL_PIPELINE_WRAP_MODE_REPEAT,
>                                           COGL_PIPELINE_WRAP_MODE_REPEAT,
>                                           re_normalize_sub_texture_coords_cb,
> diff --git a/tests/conform/test-conform-main.c b/tests/conform/test-conform-main.c
> index c49ea58..5acf2e5 100644
> --- a/tests/conform/test-conform-main.c
> +++ b/tests/conform/test-conform-main.c
> @@ -63,7 +63,7 @@ main (int argc, char **argv)
>
>    ADD_TEST (test_sparse_pipeline, 0, 0);
>
> -  ADD_TEST (test_npot_texture, 0, TEST_REQUIREMENT_NPOT);
> +  ADD_TEST (test_npot_texture, 0, 0);
>    UNPORTED_TEST (test_multitexture);
>    UNPORTED_TEST (test_texture_mipmaps);
>    ADD_TEST (test_sub_texture, 0, 0);
> @@ -74,7 +74,7 @@ main (int argc, char **argv)
>    ADD_TEST (test_texture_3d, TEST_REQUIREMENT_TEXTURE_3D, 0);
>    ADD_TEST (test_wrap_modes, 0, 0);
>    UNPORTED_TEST (test_texture_pixmap_x11);
> -  ADD_TEST (test_texture_get_set_data, 0, TEST_REQUIREMENT_NPOT);
> +  ADD_TEST (test_texture_get_set_data, 0, 0);
>    ADD_TEST (test_texture_mipmap_get_set, 0, 0);
>    ADD_TEST (test_atlas_migration, 0, 0);
>    ADD_TEST (test_read_texture_formats, 0, 0);
> --
> 1.7.11.3.g3c3efa5
>
> _______________________________________________
> Cogl mailing list
> Cogl at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/cogl