render/wlr_texture: clamp texture coordinates to edge by default

[quantum5]

Clamping texture coordinates prevents OpenGL from blending the left and
right edge (or top and bottom edge) when scaling textures with GL_LINEAR
filtering. This prevents visual artifacts like swaywm/sway#5809.

Per discussion on IRC, this behaviour is made default. Compositors that want
the wrapping behaviour (e.g. for tiled patterns) can override this by doing:

struct wlr_gles2_texture_attribs attribs;
wlr_gles2_texture_get_attribs(texture, &attribs);

glBindTexture(attribs.target, attribs.tex);
glTexParameteri(attribs.target, GL_TEXTURE_WRAP_S, GL_REPEAT);
glTexParameteri(attribs.target, GL_TEXTURE_WRAP_T, GL_REPEAT);
glBindTexture(attribs.target, 0);

---

Breaking change: The GLES2 renderer no longer wraps textures with GL_REPEAT. If you want to repeat a texture, manually change wlroots' default via glTexParameteri.

[ammen99]

Where do we need tiling btw?

[quantum5]

@emersion mentioned on IRC that some compositors use GL_REPEAT to render borders. So this behaviour is being used.

As a result, I am somewhat reluctant to simply change the default to be GL_CLAMP_TO_EDGE, since it will be a breaking change, and won't generate a compile-time error.

[emersion]

As I mentioned on IRC, this sounds like a work around rather than a proper fix. We shouldn't query texels outside of the texture's geometry in the first place.

[quantum5]

I don't believe we are querying texels outside of the texture's geometry. This clamping behaviour is required to avoid wraparound at texture coordinate 1.0 as long as scaling happens.

For example (in 1D for simplicity), if texture is 32 pixels wide, the polygon is 64 pixels wide, and we set texture coordinate 0.0 at the left end of the polygon and 1.0 at the right end of the polygon. Then, at polygon pixel 63, it will blend between texture pixel 31 and 32 (which is the same as 0) due to the wraparound instead of just using the value for texture pixel 31.

We do use texture coordinate 1.0 for cursors, as they are rendered with wlr_render_texture_with_matrix, which creates a box as follows:

	struct wlr_fbox box = {
		.x = 0,
		.y = 0,
		.width = texture->width,
		.height = texture->height,
	};

In gles2_render_subtexture_with_matrix, the texture coordinates are computed as follows:

	const GLfloat x1 = box->x / wlr_texture->width;
	const GLfloat y1 = box->y / wlr_texture->height;
	const GLfloat x2 = (box->x + box->width) / wlr_texture->width;
	const GLfloat y2 = (box->y + box->height) / wlr_texture->height;
	const GLfloat texcoord[] = {
		x2, y1, // top right
		x1, y1, // top left
		x2, y2, // bottom right
		x1, y2, // bottom left
	};

Since x == 0 and box->width == wlr_texture->width, x2 will be 1.0, and similarly for y2.

[emersion]

Texture wrapping only happens when the texture coordinates are outside of the [0; 1] range.

[quantum5]

When using bilinear filtering to scale up a texture, that is not the case. Even if the polygon only has texture coordinates in [0.0, 1.0], one of the closest pixel neighbours of pixels on the rightmost (or bottommost) edge may be beyond 1.0, causing wrapping.

[Xyene]

Texture wrapping only happens when the texture coordinates are outside of the [0; 1] range.

I believe this is the case, but that sampling outside the [0; 1] range happens even without doing anything "wrong", so long as you're using glTexParameteri(..., GL_TEXTURE_(MIN|MAX)_FILTER, GL_LINEAR); and drawing on a polygon with a texture smaller than the polygon (i.e. scaling with not-GL_NEAREST is required). For every textured pixel drawn with GL_LINEAR, the color is the weighted average of the four texture elements that are closest to its center, with wraparound unless GL_CLAMP_TO_EDGE is specified (GL_REPEAT is the default).

As a quick reference, Mesa's software renderer has separate branches for handling GL_LINEAR + GL_REPEAT samplers:

https://gitlab.freedesktop.org/mesa/mesa/-/blob/master/src/mesa/swrast/s_texfilter.c#L140-194

/**
 * Used for GL_REPEAT wrap mode.  Using A % B doesn't produce the
 * right results for A<0.  Casting to A to be unsigned only works if B
 * is a power of two.  Adding a bias to A (which is a multiple of B)
 * avoids the problems with A < 0 (for reasonable A) without using a
 * conditional.
 */
#define REMAINDER(A, B) (((A) + (B) * 1024) % (B))


/**
 * Used to compute texel locations for linear sampling.
 * Input:
 *    wrapMode = GL_REPEAT, GL_CLAMP, GL_CLAMP_TO_EDGE, GL_CLAMP_TO_BORDER
 *    s = texcoord in [0,1]
 *    size = width (or height or depth) of texture
 * Output:
 *    i0, i1 = returns two nearest texel indexes
 *    weight = returns blend factor between texels
 */
static void
linear_texel_locations(GLenum wrapMode,
                       const struct gl_texture_image *img,
                       GLint size, GLfloat s,
                       GLint *i0, GLint *i1, GLfloat *weight)
{
   const struct swrast_texture_image *swImg = swrast_texture_image_const(img);
   GLfloat u;
   switch (wrapMode) {
   case GL_REPEAT:
      u = s * size - 0.5F;
      if (swImg->_IsPowerOfTwo) {
         *i0 = util_ifloor(u) & (size - 1);
         *i1 = (*i0 + 1) & (size - 1);
      }
      else {
         *i0 = REMAINDER(util_ifloor(u), size);
         *i1 = REMAINDER(*i0 + 1, size);
      }
      break;
   case GL_CLAMP_TO_EDGE:
      if (s <= 0.0F)
         u = 0.0F;
      else if (s >= 1.0F)
         u = (GLfloat) size;
      else
         u = s * size;
      u -= 0.5F;
      *i0 = util_ifloor(u);
      *i1 = *i0 + 1;
      if (*i0 < 0)
         *i0 = 0;
      if (*i1 >= (GLint) size)
         *i1 = size - 1;
      break;

(linear_texel_locations, through linear_texcoord is what texture2D(tex, texCoord) eventually ends up delegating to. The blending of all four texture elements is in the same file, in sample_2d_linear.)

[quantum5]

@emersion gentle ping. 😃

[quantum5]

Per discussion on IRC, the clamping behaviour is now applied to all textures by default. Compositors that want the old behaviour can override it through the existing mechanism, e.g. sway setting GL_TEXTURE_MAG_FILTER.

[Xyene]

(@emersion: I can't access IRC atm, but I got the discussion out-of-band and this LGTM)

[emersion]

Thanks for the patch, sorry for the delay!