[1/4] software-isp: Center the GPU image crop

28462 diff mbox series

From: Ray Strode <rstrode@redhat.com>

The GPU debayer path scales the image to fill a requested output size
and crops any excess image area.

Its projection positions the scaled image against one side of the
output, so aspect-ratio changes and Bayer border removal shift the
visible scene.

This commit centers the projection on the output region in sensor
coordinates, distributing the crop across both sides.

Signed-off-by: Ray Strode <rstrode@redhat.com>
---
 src/libcamera/software_isp/debayer_egl.cpp | 23 +++++++++++++++++++---
 1 file changed, 20 insertions(+), 3 deletions(-)

On 15/10/2026 20:43, Ray Strode wrote:
> From: Ray Strode <rstrode@redhat.com>
> 
> The GPU debayer path scales the image to fill a requested output size
> and crops any excess image area.
> 
> Its projection positions the scaled image against one side of the
> output, so aspect-ratio changes and Bayer border removal shift the
> visible scene.
> 
> This commit centers the projection on the output region in sensor
> coordinates, distributing the crop across both sides.
> 
> Signed-off-by: Ray Strode <rstrode@redhat.com>
> ---
>   src/libcamera/software_isp/debayer_egl.cpp | 23 +++++++++++++++++++---
>   1 file changed, 20 insertions(+), 3 deletions(-)
> 
> diff --git a/src/libcamera/software_isp/debayer_egl.cpp b/src/libcamera/software_isp/debayer_egl.cpp
> index 300822f9..0f515c5e 100644
> --- a/src/libcamera/software_isp/debayer_egl.cpp
> +++ b/src/libcamera/software_isp/debayer_egl.cpp
> @@ -364,70 +364,87 @@ std::vector<PixelFormat> DebayerEGL::formats(PixelFormat inputFormat)
>   std::tuple<unsigned int, unsigned int>
>   DebayerEGL::strideAndFrameSize(const PixelFormat &outputFormat, const Size &size)
>   {
>   	/* Align stride to 256 bytes as a generic GPU memory access alignment */
>   	const PixelFormatInfo &info = PixelFormatInfo::info(outputFormat);
>   	return std::make_tuple(info.stride(size.width, 0, 256), info.frameSize(size, 256));
>   }
> 
>   uint32_t DebayerEGL::preferredInputStride(const PixelFormat &inputFormat, const Size &size)
>   {
>   	const PixelFormatInfo &info = PixelFormatInfo::info(inputFormat);
>   	return info.stride(size.width, 0, 256);
>   }
> 
>   void DebayerEGL::setShaderVariableValues(eGLImage &eglImageIn, const DebayerParams &params)
>   {
>   	/*
>   	 * Raw Bayer 8-bit, and packed raw Bayer 10-bit/12-bit formats
>   	 * are stored in a GL_LUMINANCE texture. The texture width is
>   	 * equal to the stride.
>   	 */
>   	GLfloat firstRed[] = { firstRed_x_, firstRed_y_ };
>   	GLfloat imgSize[] = { (GLfloat)width_,
>   			      (GLfloat)height_ };
>   	GLfloat Step[] = { static_cast<float>(bytesPerPixel_) / (inputConfig_.stride - 1),
>   			   1.0f / (height_ - 1) };
>   	GLfloat Stride = (GLfloat)width_ / (shaderStridePixels_ / bytesPerPixel_);
>   	/*
>   	 * Scale the output size from the native size the algorithm produces for
>   	 * the input size. Keep the aspect ratio and prefer cropping over black
> -	 * bars.
> +	 * bars - but crop from both sides, not from one.
> +	 *
> +	 * The viewport is the sensor frame while the buffer drawn into is the
> +	 * requested size, so the buffer covers only the corner of the viewport
> +	 * that starts at NDC -1 and ends where its own width reaches. Placing

NDC ?

> +	 * the quad at -(1 - scale) puts the texture's left edge exactly at
> +	 * that corner, so everything cropped is taken off one side: metered
> +	 * by registering a 640x480 stream against a 1920x1080 one of the same
> +	 * scene, the frame shown ran from x=48 rather than from the x=240
> +	 * that centring the 4:3 window in a 16:9 sensor calls for.
> +	 *
> +	 * Centre it on the region the buffer actually covers instead. That
> +	 * region spans NDC -1 to 2*out/viewport - 1, whose midpoint is
> +	 * out/viewport - 1, and putting the quad's centre there splits the
> +	 * crop evenly between the two edges at every size, the native one
> +	 * included - where it shares the eight columns of Bayer border
> +	 * instead of taking them all off the right.
>   	 */
>   	GLfloat scale = std::max((GLfloat)outputSize_.width / nativeOutputSize_.width,
>   				 (GLfloat)outputSize_.height / nativeOutputSize_.height);
> -	GLfloat trans = -(1.0f - scale);
> +	GLfloat transX = (GLfloat)outputSize_.width / width_ - 1.0f;
> +	GLfloat transY = (GLfloat)outputSize_.height / height_ - 1.0f;
>   	GLfloat projMatrix[] = {
>   		scale, 0, 0, 0,
>   		0, scale, 0, 0,
>   		0, 0, 1, 0,
> -		trans, trans, 0, 1
> +		transX, transY, 0, 1

I think this makes sense

>   	};
>   	/* Static const coordinates */
>   	static const GLfloat vcoordinates[4][2] = {
>   		{ -1.0f, -1.0f },
>   		{ -1.0f, +1.0f },
>   		{ +1.0f, +1.0f },
>   		{ +1.0f, -1.0f },
>   	};
>   	static const GLfloat tcoordinates[4][2] = {
>   		{ 0.0f, 0.0f },
>   		{ 0.0f, 1.0f },
>   		{ 1.0f, 1.0f },
>   		{ 1.0f, 0.0f },
>   	};
> 
>   	/* vertexIn - bayer_8.vert */
>   	glEnableVertexAttribArray(attributeVertex_);
>   	glVertexAttribPointer(attributeVertex_, 2, GL_FLOAT, GL_TRUE,
>   			      2 * sizeof(GLfloat), vcoordinates);
> 
>   	/* textureIn - bayer_8.vert */
>   	glEnableVertexAttribArray(attributeTexture_);
>   	glVertexAttribPointer(attributeTexture_, 2, GL_FLOAT, GL_TRUE,
>   			      2 * sizeof(GLfloat), tcoordinates);
> 
>   	/*
>   	 * Set the sampler2D to the respective texture unit for each texutre
>   	 * To simultaneously sample multiple textures we need to use multiple
>   	 * texture units
>   	 */
> --
> 2.56.0

Hi,

On Fri, Oct 16, 2026 at 5:46 AM Bryan O'Donoghue <bod.linux@nxsw.ie> wrote:
> > +      * The viewport is the sensor frame while the buffer drawn into is the
> > +      * requested size, so the buffer covers only the corner of the viewport
> > +      * that starts at NDC -1 and ends where its own width reaches. Placing
> NDC ?

Sorry, "normalized device coordinates" (-1.0, 1.0). Please see v2,
where I reworded things.

--Ray