[Cogl] [PATCH] Allow npot Texture2D creation with only basic npot support

[Robert Bragg]

Thanks for the suggestions; I folded them into my patch and pushed to master.

regards,
- Robert

On Tue, Apr 24, 2012 at 6:39 PM, Neil Roberts <neil at linux.intel.com> wrote:
> Robert Bragg <robert at sixbynine.org> writes:
>
>> @@ -402,7 +411,9 @@ cogl_texture_new_from_bitmap (CoglBitmap *bitmap,
>>      return COGL_TEXTURE (atlas_tex);
>>
>>    /* If that doesn't work try a fast path 2D texture */
>> -  if ((tex_2d = cogl_texture_2d_new_from_bitmap (bitmap,
>> +  if (cogl_has_feature (ctx, COGL_FEATURE_ID_TEXTURE_NPOT_BASIC) &&
>> +      cogl_has_feature (ctx, COGL_FEATURE_ID_TEXTURE_NPOT_MIPMAP) &&
>> +      (tex_2d = cogl_texture_2d_new_from_bitmap (bitmap,
>>                                                   internal_format,
>>                                                   NULL)))
>
> Should this have a check for whether the size of the texture is already
> a power of two like cogl_texture_new_from_size has?
>
> I think to make it fallback to software repeating we also need to modify
> the _cogl_texture_2d_transform_quad_coords_to_gl function. Maybe you
> could squash something like this in?
>
> diff --git a/cogl/cogl-texture-2d.c b/cogl/cogl-texture-2d.c
> index e7df732..9fcb38f 100644
> --- a/cogl/cogl-texture-2d.c
> +++ b/cogl/cogl-texture-2d.c
> @@ -659,15 +659,17 @@ _cogl_texture_2d_transform_quad_coords_to_gl (CoglTexture *tex,
>   /* The texture coordinates map directly so we don't need to do
>      anything other than check for repeats */
>
> -  CoglBool need_repeat = FALSE;
>   int i;
>
>   for (i = 0; i < 4; i++)
>     if (coords[i] < 0.0f || coords[i] > 1.0f)
> -      need_repeat = TRUE;
> +      /* Repeat is needed */
> +      return (_cogl_texture_2d_can_hardware_repeat (tex) ?
> +              COGL_TRANSFORM_HARDWARE_REPEAT :
> +              COGL_TRANSFORM_SOFTWARE_REPEAT);
>
> -  return (need_repeat ? COGL_TRANSFORM_HARDWARE_REPEAT
> -          : COGL_TRANSFORM_NO_REPEAT);
> +  /* No repeat is needed */
> +  return COGL_TRANSFORM_NO_REPEAT;
>  }
>
>  static CoglBool
>
> Otherwise looks good to me.
>
> Reviewed-by: Neil Roberts <neil at linux.intel.com>
>
> Regards,
> - Neil