| Message ID | 20260706110120.2509826-6-bryan.odonoghue@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
Hi Bryan, thanks for the updated series. On 06.07.26 13:01, Bryan O'Donoghue wrote: > When operating from a texture cache on dma-buf inputs we will no longer > create new textures nor attach those textures for dma-buf handles we have > already encountered. > > This means we will use the texture id associated with a given texture unit > to switch between one texture and another. The pages associated with the > texture will have been populated with new data by the CSI2 receiver. All we > will do is say to the GPU "reuse this texture id" aka zero-copy. > > However we must also activate and bind that texture for each loop. This > cost is small but necessary for zero-copy. > > Reviewed-by: Robert Mader <robert.mader@collabora.com> > Reviewed-by: Milan Zamazal <mzamazal@redhat.com> > Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> > --- > include/libcamera/internal/egl.h | 1 + > src/libcamera/egl.cpp | 24 +++++++++++++++++++----- > 2 files changed, 20 insertions(+), 5 deletions(-) > > diff --git a/include/libcamera/internal/egl.h b/include/libcamera/internal/egl.h > index 9b4261c55..1e31d490b 100644 > --- a/include/libcamera/internal/egl.h > +++ b/include/libcamera/internal/egl.h > @@ -112,6 +112,7 @@ public: > void createOutputTexture2D(eGLImage &eglImage); > > int attachTextureToFBO(eGLImage &eglImage); > + void activateBindTexture(eGLImage &eglImage); > > void pushEnv(std::vector<std::string> &shaderEnv, const char *str); > void makeCurrent(); > diff --git a/src/libcamera/egl.cpp b/src/libcamera/egl.cpp > index b91f9096a..fae907881 100644 > --- a/src/libcamera/egl.cpp > +++ b/src/libcamera/egl.cpp > @@ -138,6 +138,23 @@ int eGL::attachTextureToFBO(eGLImage &eglImage) > return ret; > } > > +/** > + * \brief Activate a texture unit and bind a texture to that unit > + * \param[in,out] eglImage EGL image containing data related to unit and texture id > + * > + * When we create a texture we will bind a texture unit and texture id so > + * we can set filters. For the case where a texture already exists though > + * we need to activate and bind an existing texture. This helper function > + * facilitates both cases. > + * > + */ > +void eGL::activateBindTexture(eGLImage &eglImage) > +{ > + // Bind texture unit and texture > + glActiveTexture(eglImage.texture_unit_); > + glBindTexture(GL_TEXTURE_2D, eglImage.texture_); > +} > + > /** > * \brief Create a DMA-BUF backed 2D texture > * \param[in,out] eglImage EGL image to associate with the DMA-BUF > @@ -197,9 +214,7 @@ int eGL::createDMABufTexture2D(eGLImage &eglImage, int fd, bool output) > return -ENODEV; > } > > - // Bind texture unit and texture > - glActiveTexture(eglImage.texture_unit_); > - glBindTexture(GL_TEXTURE_2D, eglImage.texture_); > + activateBindTexture(eglImage); > > // Generate texture with filter semantics > glEGLImageTargetTexture2DOES(GL_TEXTURE_2D, image); > @@ -270,8 +285,7 @@ void eGL::createTexture2D(eGLImage &eglImage, void *data) > { > ASSERT(tid_ == Thread::currentId()); > > - glActiveTexture(eglImage.texture_unit_); > - glBindTexture(GL_TEXTURE_2D, eglImage.texture_); > + activateBindTexture(eglImage); > > // Generate texture, bind, associate image to texture, configure, unbind > glTexImage2D(GL_TEXTURE_2D, 0, eglImage.format_, eglImage.width_, eglImage.height_, 0, eglImage.format_, GL_UNSIGNED_BYTE, data); This still misses using `activateBindTexture()` in `updateTexture2D()` - which is why I again suggest to move this commit *before* "libcamera: egl: Add updateTexture2D" ;)
On 06/07/2026 12:13, Robert Mader wrote: > Hi Bryan, thanks for the updated series. > > On 06.07.26 13:01, Bryan O'Donoghue wrote: >> When operating from a texture cache on dma-buf inputs we will no longer >> create new textures nor attach those textures for dma-buf handles we have >> already encountered. >> >> This means we will use the texture id associated with a given texture >> unit >> to switch between one texture and another. The pages associated with the >> texture will have been populated with new data by the CSI2 receiver. >> All we >> will do is say to the GPU "reuse this texture id" aka zero-copy. >> >> However we must also activate and bind that texture for each loop. This >> cost is small but necessary for zero-copy. >> >> Reviewed-by: Robert Mader <robert.mader@collabora.com> >> Reviewed-by: Milan Zamazal <mzamazal@redhat.com> >> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> >> --- >> include/libcamera/internal/egl.h | 1 + >> src/libcamera/egl.cpp | 24 +++++++++++++++++++----- >> 2 files changed, 20 insertions(+), 5 deletions(-) >> >> diff --git a/include/libcamera/internal/egl.h b/include/libcamera/ >> internal/egl.h >> index 9b4261c55..1e31d490b 100644 >> --- a/include/libcamera/internal/egl.h >> +++ b/include/libcamera/internal/egl.h >> @@ -112,6 +112,7 @@ public: >> void createOutputTexture2D(eGLImage &eglImage); >> int attachTextureToFBO(eGLImage &eglImage); >> + void activateBindTexture(eGLImage &eglImage); >> void pushEnv(std::vector<std::string> &shaderEnv, const char *str); >> void makeCurrent(); >> diff --git a/src/libcamera/egl.cpp b/src/libcamera/egl.cpp >> index b91f9096a..fae907881 100644 >> --- a/src/libcamera/egl.cpp >> +++ b/src/libcamera/egl.cpp >> @@ -138,6 +138,23 @@ int eGL::attachTextureToFBO(eGLImage &eglImage) >> return ret; >> } >> +/** >> + * \brief Activate a texture unit and bind a texture to that unit >> + * \param[in,out] eglImage EGL image containing data related to unit >> and texture id >> + * >> + * When we create a texture we will bind a texture unit and texture >> id so >> + * we can set filters. For the case where a texture already exists >> though >> + * we need to activate and bind an existing texture. This helper >> function >> + * facilitates both cases. >> + * >> + */ >> +void eGL::activateBindTexture(eGLImage &eglImage) >> +{ >> + // Bind texture unit and texture >> + glActiveTexture(eglImage.texture_unit_); >> + glBindTexture(GL_TEXTURE_2D, eglImage.texture_); >> +} >> + >> /** >> * \brief Create a DMA-BUF backed 2D texture >> * \param[in,out] eglImage EGL image to associate with the DMA-BUF >> @@ -197,9 +214,7 @@ int eGL::createDMABufTexture2D(eGLImage &eglImage, >> int fd, bool output) >> return -ENODEV; >> } >> - // Bind texture unit and texture >> - glActiveTexture(eglImage.texture_unit_); >> - glBindTexture(GL_TEXTURE_2D, eglImage.texture_); >> + activateBindTexture(eglImage); >> // Generate texture with filter semantics >> glEGLImageTargetTexture2DOES(GL_TEXTURE_2D, image); >> @@ -270,8 +285,7 @@ void eGL::createTexture2D(eGLImage &eglImage, void >> *data) >> { >> ASSERT(tid_ == Thread::currentId()); >> - glActiveTexture(eglImage.texture_unit_); >> - glBindTexture(GL_TEXTURE_2D, eglImage.texture_); >> + activateBindTexture(eglImage); >> // Generate texture, bind, associate image to texture, >> configure, unbind >> glTexImage2D(GL_TEXTURE_2D, 0, eglImage.format_, >> eglImage.width_, eglImage.height_, 0, eglImage.format_, >> GL_UNSIGNED_BYTE, data); > This still misses using `activateBindTexture()` in `updateTexture2D()` - > which is why I again suggest to move this commit *before* "libcamera: > egl: Add updateTexture2D" ;) > its worse than that, we are doing activateBindTexture in debayer_egl.cpp and then activate/Bind here again manually. --- bod
On 06/07/2026 12:18, Bryan O'Donoghue wrote: >>> configure, unbind >>> glTexImage2D(GL_TEXTURE_2D, 0, eglImage.format_, >>> eglImage.width_, eglImage.height_, 0, eglImage.format_, >>> GL_UNSIGNED_BYTE, data); >> This still misses using `activateBindTexture()` in `updateTexture2D()` - >> which is why I again suggest to move this commit*before* "libcamera: >> egl: Add updateTexture2D" 😉 >> > its worse than that, we are doing activateBindTexture in debayer_egl.cpp > and then activate/Bind here again manually. > > --- > bod I'm wrong about that, you do need to activate texture on dma import mode each time. --- bod
diff --git a/include/libcamera/internal/egl.h b/include/libcamera/internal/egl.h index 9b4261c55..1e31d490b 100644 --- a/include/libcamera/internal/egl.h +++ b/include/libcamera/internal/egl.h @@ -112,6 +112,7 @@ public: void createOutputTexture2D(eGLImage &eglImage); int attachTextureToFBO(eGLImage &eglImage); + void activateBindTexture(eGLImage &eglImage); void pushEnv(std::vector<std::string> &shaderEnv, const char *str); void makeCurrent(); diff --git a/src/libcamera/egl.cpp b/src/libcamera/egl.cpp index b91f9096a..fae907881 100644 --- a/src/libcamera/egl.cpp +++ b/src/libcamera/egl.cpp @@ -138,6 +138,23 @@ int eGL::attachTextureToFBO(eGLImage &eglImage) return ret; } +/** + * \brief Activate a texture unit and bind a texture to that unit + * \param[in,out] eglImage EGL image containing data related to unit and texture id + * + * When we create a texture we will bind a texture unit and texture id so + * we can set filters. For the case where a texture already exists though + * we need to activate and bind an existing texture. This helper function + * facilitates both cases. + * + */ +void eGL::activateBindTexture(eGLImage &eglImage) +{ + // Bind texture unit and texture + glActiveTexture(eglImage.texture_unit_); + glBindTexture(GL_TEXTURE_2D, eglImage.texture_); +} + /** * \brief Create a DMA-BUF backed 2D texture * \param[in,out] eglImage EGL image to associate with the DMA-BUF @@ -197,9 +214,7 @@ int eGL::createDMABufTexture2D(eGLImage &eglImage, int fd, bool output) return -ENODEV; } - // Bind texture unit and texture - glActiveTexture(eglImage.texture_unit_); - glBindTexture(GL_TEXTURE_2D, eglImage.texture_); + activateBindTexture(eglImage); // Generate texture with filter semantics glEGLImageTargetTexture2DOES(GL_TEXTURE_2D, image); @@ -270,8 +285,7 @@ void eGL::createTexture2D(eGLImage &eglImage, void *data) { ASSERT(tid_ == Thread::currentId()); - glActiveTexture(eglImage.texture_unit_); - glBindTexture(GL_TEXTURE_2D, eglImage.texture_); + activateBindTexture(eglImage); // Generate texture, bind, associate image to texture, configure, unbind glTexImage2D(GL_TEXTURE_2D, 0, eglImage.format_, eglImage.width_, eglImage.height_, 0, eglImage.format_, GL_UNSIGNED_BYTE, data);