[v3,30/39] libcamera: shaders: Extend bayer shaders to support swapping R and B on output
diff mbox series

Message ID 20251015012251.17508-31-bryan.odonoghue@linaro.org
State New
Headers show
Series
  • Add GLES 2.0 GPUISP to libcamera
Related show

Commit Message

Bryan O'Donoghue Oct. 15, 2025, 1:22 a.m. UTC
We can easily facilitate swapping R and B on output. Pivot on an
environment define for this purpose.

Reviewed-by: Milan Zamazal <mzamazal@redhat.com>
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 include/libcamera/internal/shaders/bayer_1x_packed.frag | 4 ++++
 include/libcamera/internal/shaders/bayer_unpacked.frag  | 6 +++++-
 2 files changed, 9 insertions(+), 1 deletion(-)

Comments

Kieran Bingham Oct. 15, 2025, 11:30 p.m. UTC | #1
Quoting Bryan O'Donoghue (2025-10-15 02:22:42)
> We can easily facilitate swapping R and B on output. Pivot on an
> environment define for this purpose.
> 

This looks like wizardry to me. There's not much there and it looks
reasonable for what is stated - but if I were to put an RB tag here I
think I'd feel like I was lying still ... I don't know what is supplying
the .bgr magic below.

Anyway - that won't stop me putting this though:

Acked-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

> Reviewed-by: Milan Zamazal <mzamazal@redhat.com>
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> ---
>  include/libcamera/internal/shaders/bayer_1x_packed.frag | 4 ++++
>  include/libcamera/internal/shaders/bayer_unpacked.frag  | 6 +++++-
>  2 files changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/include/libcamera/internal/shaders/bayer_1x_packed.frag b/include/libcamera/internal/shaders/bayer_1x_packed.frag
> index 90bd6457..c0632eb1 100644
> --- a/include/libcamera/internal/shaders/bayer_1x_packed.frag
> +++ b/include/libcamera/internal/shaders/bayer_1x_packed.frag
> @@ -268,5 +268,9 @@ void main(void)
>         rgb.b = texture2D(blue_param, vec2(rgb.b, 0.5)).b;
>  #endif
>  
> +#if defined (SWAP_BLUE)
> +       gl_FragColor = vec4(rgb.bgr, 1.0);
> +#else
>         gl_FragColor = vec4(rgb, 1.0);
> +#endif
>  }
> diff --git a/include/libcamera/internal/shaders/bayer_unpacked.frag b/include/libcamera/internal/shaders/bayer_unpacked.frag
> index 5955c2ea..74ce1509 100644
> --- a/include/libcamera/internal/shaders/bayer_unpacked.frag
> +++ b/include/libcamera/internal/shaders/bayer_unpacked.frag
> @@ -163,5 +163,9 @@ void main(void) {
>         rgb.b = texture2D(red_param, vec2(rgb.b, 0.5)).b;
>  #endif
>  
> -    gl_FragColor.rgb = rgb;
> +#if defined (SWAP_BLUE)
> +       gl_FragColor = vec4(rgb.bgr, 1.0);
> +#else
> +       gl_FragColor = vec4(rgb, 1.0);
> +#endif
>  }
> -- 
> 2.51.0
>
Barnabás Pőcze Oct. 16, 2025, 7:52 a.m. UTC | #2
Hi


2025. 10. 16. 1:30 keltezéssel, Kieran Bingham írta:
> Quoting Bryan O'Donoghue (2025-10-15 02:22:42)
>> We can easily facilitate swapping R and B on output. Pivot on an
>> environment define for this purpose.
>>
> 
> This looks like wizardry to me. There's not much there and it looks
> reasonable for what is stated - but if I were to put an RB tag here I
> think I'd feel like I was lying still ... I don't know what is supplying
> the .bgr magic below.

Maybe I misunderstand, but the `.bgr` part is explained at
https://registry.khronos.org/OpenGL/specs/gl/GLSLangSpec.4.60.html#vector-components

"""
The order of the components can be different to swizzle them, or replicated:

vec4 pos = vec4(1.0, 2.0, 3.0, 4.0);
vec4 swiz = pos.wzyx;   // swiz = (4.0, 3.0, 2.0, 1.0)
vec4 dup = pos.xxyy;    // dup = (1.0, 1.0, 2.0, 2.0)
"""


Regards,
Barnabás Pőcze

> 
> Anyway - that won't stop me putting this though:
> 
> Acked-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> 
>> Reviewed-by: Milan Zamazal <mzamazal@redhat.com>
>> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
>> ---
>>   include/libcamera/internal/shaders/bayer_1x_packed.frag | 4 ++++
>>   include/libcamera/internal/shaders/bayer_unpacked.frag  | 6 +++++-
>>   2 files changed, 9 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/libcamera/internal/shaders/bayer_1x_packed.frag b/include/libcamera/internal/shaders/bayer_1x_packed.frag
>> index 90bd6457..c0632eb1 100644
>> --- a/include/libcamera/internal/shaders/bayer_1x_packed.frag
>> +++ b/include/libcamera/internal/shaders/bayer_1x_packed.frag
>> @@ -268,5 +268,9 @@ void main(void)
>>          rgb.b = texture2D(blue_param, vec2(rgb.b, 0.5)).b;
>>   #endif
>>
>> +#if defined (SWAP_BLUE)
>> +       gl_FragColor = vec4(rgb.bgr, 1.0);
>> +#else
>>          gl_FragColor = vec4(rgb, 1.0);
>> +#endif
>>   }
>> diff --git a/include/libcamera/internal/shaders/bayer_unpacked.frag b/include/libcamera/internal/shaders/bayer_unpacked.frag
>> index 5955c2ea..74ce1509 100644
>> --- a/include/libcamera/internal/shaders/bayer_unpacked.frag
>> +++ b/include/libcamera/internal/shaders/bayer_unpacked.frag
>> @@ -163,5 +163,9 @@ void main(void) {
>>          rgb.b = texture2D(red_param, vec2(rgb.b, 0.5)).b;
>>   #endif
>>
>> -    gl_FragColor.rgb = rgb;
>> +#if defined (SWAP_BLUE)
>> +       gl_FragColor = vec4(rgb.bgr, 1.0);
>> +#else
>> +       gl_FragColor = vec4(rgb, 1.0);
>> +#endif
>>   }
>> --
>> 2.51.0
>>
Bryan O'Donoghue Oct. 16, 2025, 8:28 a.m. UTC | #3
On 16/10/2025 08:52, Barnabás Pőcze wrote:
> Hi
> 
> 
> 2025. 10. 16. 1:30 keltezéssel, Kieran Bingham írta:
>> Quoting Bryan O'Donoghue (2025-10-15 02:22:42)
>>> We can easily facilitate swapping R and B on output. Pivot on an
>>> environment define for this purpose.
>>>
>>
>> This looks like wizardry to me. There's not much there and it looks
>> reasonable for what is stated - but if I were to put an RB tag here I
>> think I'd feel like I was lying still ... I don't know what is supplying
>> the .bgr magic below.
> 
> Maybe I misunderstand, but the `.bgr` part is explained at
> https://registry.khronos.org/OpenGL/specs/gl/GLSLangSpec.4.60.html#vector-components
> 
> """
> The order of the components can be different to swizzle them, or replicated:
> 
> vec4 pos = vec4(1.0, 2.0, 3.0, 4.0);
> vec4 swiz = pos.wzyx;   // swiz = (4.0, 3.0, 2.0, 1.0)
> vec4 dup = pos.xxyy;    // dup = (1.0, 1.0, 2.0, 2.0)
> """
Yeah weirdly I think there is no cost in swizzling at all or no cost you 
can measure anyway.

I should spell it out in the commit log what the swizzle is and why it 
works.

---
bod

Patch
diff mbox series

diff --git a/include/libcamera/internal/shaders/bayer_1x_packed.frag b/include/libcamera/internal/shaders/bayer_1x_packed.frag
index 90bd6457..c0632eb1 100644
--- a/include/libcamera/internal/shaders/bayer_1x_packed.frag
+++ b/include/libcamera/internal/shaders/bayer_1x_packed.frag
@@ -268,5 +268,9 @@  void main(void)
 	rgb.b = texture2D(blue_param, vec2(rgb.b, 0.5)).b;
 #endif
 
+#if defined (SWAP_BLUE)
+	gl_FragColor = vec4(rgb.bgr, 1.0);
+#else
 	gl_FragColor = vec4(rgb, 1.0);
+#endif
 }
diff --git a/include/libcamera/internal/shaders/bayer_unpacked.frag b/include/libcamera/internal/shaders/bayer_unpacked.frag
index 5955c2ea..74ce1509 100644
--- a/include/libcamera/internal/shaders/bayer_unpacked.frag
+++ b/include/libcamera/internal/shaders/bayer_unpacked.frag
@@ -163,5 +163,9 @@  void main(void) {
 	rgb.b = texture2D(red_param, vec2(rgb.b, 0.5)).b;
 #endif
 
-    gl_FragColor.rgb = rgb;
+#if defined (SWAP_BLUE)
+	gl_FragColor = vec4(rgb.bgr, 1.0);
+#else
+	gl_FragColor = vec4(rgb, 1.0);
+#endif
 }