| Message ID | 20201103155025.5948-3-laurent.pinchart@ideasonboard.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Hi Laurent, Thanks for your patch! On 03.11.2020 18:50, Laurent Pinchart wrote: > When ViewFinderGL::setFormat() is called, the fragment shader is deleted > and recreated for the new format. This results in unnecessary shader > recompilation if only the size is changed and the pixel format remains > the same. Keep the existing shader in that case. > > The null test for fragmentShader_ can be removed, as if the shader > program is linked, the fragment shader is guaranteed to exist. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Reviewed-by: Andrey Konovalov <andrey.konovalov@linaro.org> Thanks, Andrey > --- > src/qcam/viewfinder_gl.cpp | 16 ++++++++++------ > 1 file changed, 10 insertions(+), 6 deletions(-) > > diff --git a/src/qcam/viewfinder_gl.cpp b/src/qcam/viewfinder_gl.cpp > index 03a576ba89d2..c07292523504 100644 > --- a/src/qcam/viewfinder_gl.cpp > +++ b/src/qcam/viewfinder_gl.cpp > @@ -53,19 +53,23 @@ const QList<libcamera::PixelFormat> &ViewFinderGL::nativeFormats() const > int ViewFinderGL::setFormat(const libcamera::PixelFormat &format, > const QSize &size) > { > - /* If the fragment is created remove it and create a new one. */ > - if (fragmentShader_) { > + if (format != format_) { > + /* > + * If the fragment already exists, remove it and create a new > + * one for the new format. > + */ > if (shaderProgram_.isLinked()) { > shaderProgram_.release(); > shaderProgram_.removeShader(fragmentShader_.get()); > fragmentShader_.reset(); > } > + > + if (!selectFormat(format)) > + return -1; > + > + format_ = format; > } > > - if (!selectFormat(format)) > - return -1; > - > - format_ = format; > size_ = size; > > updateGeometry(); >
Hi Laurent, On 03/11/2020 15:50, Laurent Pinchart wrote: > When ViewFinderGL::setFormat() is called, the fragment shader is deleted > and recreated for the new format. This results in unnecessary shader > recompilation if only the size is changed and the pixel format remains > the same. Keep the existing shader in that case. > > The null test for fragmentShader_ can be removed, as if the shader > program is linked, the fragment shader is guaranteed to exist. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > --- > src/qcam/viewfinder_gl.cpp | 16 ++++++++++------ > 1 file changed, 10 insertions(+), 6 deletions(-) > > diff --git a/src/qcam/viewfinder_gl.cpp b/src/qcam/viewfinder_gl.cpp > index 03a576ba89d2..c07292523504 100644 > --- a/src/qcam/viewfinder_gl.cpp > +++ b/src/qcam/viewfinder_gl.cpp > @@ -53,19 +53,23 @@ const QList<libcamera::PixelFormat> &ViewFinderGL::nativeFormats() const > int ViewFinderGL::setFormat(const libcamera::PixelFormat &format, > const QSize &size) > { > - /* If the fragment is created remove it and create a new one. */ > - if (fragmentShader_) { > + if (format != format_) { > + /* > + * If the fragment already exists, remove it and create a new > + * one for the new format. > + */ > if (shaderProgram_.isLinked()) { > shaderProgram_.release(); > shaderProgram_.removeShader(fragmentShader_.get()); > fragmentShader_.reset(); > } > + > + if (!selectFormat(format)) > + return -1; > + > + format_ = format; > } > > - if (!selectFormat(format)) > - return -1; > - > - format_ = format; > size_ = size; > > updateGeometry(); >
Hi Laurent, Thanks for your work. On 2020-11-03 17:50:20 +0200, Laurent Pinchart wrote: > When ViewFinderGL::setFormat() is called, the fragment shader is deleted > and recreated for the new format. This results in unnecessary shader > recompilation if only the size is changed and the pixel format remains > the same. Keep the existing shader in that case. > > The null test for fragmentShader_ can be removed, as if the shader > program is linked, the fragment shader is guaranteed to exist. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > --- > src/qcam/viewfinder_gl.cpp | 16 ++++++++++------ > 1 file changed, 10 insertions(+), 6 deletions(-) > > diff --git a/src/qcam/viewfinder_gl.cpp b/src/qcam/viewfinder_gl.cpp > index 03a576ba89d2..c07292523504 100644 > --- a/src/qcam/viewfinder_gl.cpp > +++ b/src/qcam/viewfinder_gl.cpp > @@ -53,19 +53,23 @@ const QList<libcamera::PixelFormat> &ViewFinderGL::nativeFormats() const > int ViewFinderGL::setFormat(const libcamera::PixelFormat &format, > const QSize &size) > { > - /* If the fragment is created remove it and create a new one. */ > - if (fragmentShader_) { > + if (format != format_) { > + /* > + * If the fragment already exists, remove it and create a new > + * one for the new format. > + */ > if (shaderProgram_.isLinked()) { > shaderProgram_.release(); > shaderProgram_.removeShader(fragmentShader_.get()); > fragmentShader_.reset(); > } > + > + if (!selectFormat(format)) > + return -1; > + > + format_ = format; > } > > - if (!selectFormat(format)) > - return -1; > - > - format_ = format; > size_ = size; > > updateGeometry(); > -- > Regards, > > Laurent Pinchart > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
diff --git a/src/qcam/viewfinder_gl.cpp b/src/qcam/viewfinder_gl.cpp index 03a576ba89d2..c07292523504 100644 --- a/src/qcam/viewfinder_gl.cpp +++ b/src/qcam/viewfinder_gl.cpp @@ -53,19 +53,23 @@ const QList<libcamera::PixelFormat> &ViewFinderGL::nativeFormats() const int ViewFinderGL::setFormat(const libcamera::PixelFormat &format, const QSize &size) { - /* If the fragment is created remove it and create a new one. */ - if (fragmentShader_) { + if (format != format_) { + /* + * If the fragment already exists, remove it and create a new + * one for the new format. + */ if (shaderProgram_.isLinked()) { shaderProgram_.release(); shaderProgram_.removeShader(fragmentShader_.get()); fragmentShader_.reset(); } + + if (!selectFormat(format)) + return -1; + + format_ = format; } - if (!selectFormat(format)) - return -1; - - format_ = format; size_ = size; updateGeometry();
When ViewFinderGL::setFormat() is called, the fragment shader is deleted and recreated for the new format. This results in unnecessary shader recompilation if only the size is changed and the pixel format remains the same. Keep the existing shader in that case. The null test for fragmentShader_ can be removed, as if the shader program is linked, the fragment shader is guaranteed to exist. Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- src/qcam/viewfinder_gl.cpp | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-)