vcl: opengl - Kill the last glErrors on common paths.

Re-work the makeSomeOpenGLContextCurrent logic into a shared function
with the existing AcquireContext logic in the SalOpenGLGraphics impl.
Use an OpenGLVCLContextZone placeholder to do both context and zone
management - to include destructors into the zone.

Fix a number of error cases around SalBitmaps where we did not have
a GL context associated when allocating, and/or freeing textures..

Don't drag a (potentially) slower context around in the
OpenGLSalBitmap when we're going to check / fetch a better VCL
context anyway.

Change-Id: Ibbb2358c47156cd078ad28b6aad4f03af36aaf23
Reviewed-on: https://gerrit.libreoffice.org/21127
Reviewed-by: Michael Meeks <michael.meeks@collabora.com>
Tested-by: Michael Meeks <michael.meeks@collabora.com>

4 files changed, 61 insertions, 89 deletions

diff --git a/vcl/opengl/gdiimpl.cxx b/vcl/opengl/gdiimpl.cxx
index 7cbf73fe9edd..96fe0288dc99 100644
--- a/vcl/opengl/gdiimpl.cxx
+++ b/vcl/opengl/gdiimpl.cxx
@@ -97,52 +97,22 @@ rtl::Reference<OpenGLContext> OpenGLSalGraphicsImpl::GetOpenGLContext()
     return mpContext;
 }
 
-rtl::Reference<OpenGLContext> OpenGLSalGraphicsImpl::GetDefaultContext()
-{
-    return ImplGetDefaultWindow()->GetGraphics()->GetOpenGLContext();
-}
-
 bool OpenGLSalGraphicsImpl::AcquireContext( bool bForceCreate )
 {
-    ImplSVData* pSVData = ImplGetSVData();
-
-    // We always prefer to bind our VirtualDevice / offscreen graphics
-    // to the current OpenGLContext - to avoid switching contexts.
-    if (mpContext.is() && OpenGLContext::hasCurrent() && !mpContext->isCurrent())
-    {
-        mpContext.clear();
-    }
+    mpContext = OpenGLContext::getVCLContext( false );
 
-    if( mpContext.is() )
+    if( !mpContext.is() && mpWindowContext.is() )
     {
-        // Check whether the context was reset underneath us.
-        if( mpContext->isInitialized() )
-            return true;
-        mpContext.clear();
-    }
-
-    // We don't care what context we have - but not switching context
-    // is rather useful from a performance perspective.
-    OpenGLContext *pContext = pSVData->maGDIData.mpLastContext;
-    while( pContext )
-    {
-        // check if this context can be used by this SalGraphicsImpl instance
-        if( UseContext( pContext ) )
-            break;
-        pContext = pContext->mpPrevContext;
-    }
-
-    if( mpContext.is() )
-        mpContext = pContext;
-    else if( mpWindowContext.is() )
         mpContext = mpWindowContext;
+    }
     else if( bForceCreate && !IsOffscreen() )
     {
         mpWindowContext = CreateWinContext();
         mpContext = mpWindowContext;
     }
+
     if( !mpContext.is() )
-        mpContext = GetDefaultContext();
+        mpContext = OpenGLContext::getVCLContext( true );
 
     return mpContext.is();
 }
@@ -2077,7 +2047,7 @@ void OpenGLSalGraphicsImpl::doFlush()
     VCL_GL_INFO( "Texture height " << maOffscreenTex.GetHeight() << " vs. window height " << GetHeight() );
 
     OpenGLProgram *pProgram =
-        mpWindowContext->UseProgram( "textureVertexShader", "textureFragmentShader", "" );
+        mpWindowContext->UseProgram( "textureVertexShader", "textureFragmentShader", "// flush shader\n" ); // flush helps profiling
     if( !pProgram )
         VCL_GL_INFO( "Can't compile simple copying shader !" );
     else
diff --git a/vcl/opengl/salbmp.cxx b/vcl/opengl/salbmp.cxx
index 31038c37ff0d..9e59e231c1ec 100644
--- a/vcl/opengl/salbmp.cxx
+++ b/vcl/opengl/salbmp.cxx
@@ -104,8 +104,7 @@ static vcl::DeleteOnDeinit< TextureAtlasVector > gTextureAtlases(new TextureAtla
 }
 
 OpenGLSalBitmap::OpenGLSalBitmap()
-: mpContext(nullptr)
-, mbDirtyTexture(true)
+: mbDirtyTexture(true)
 , mnBits(0)
 , mnBytesPerRow(0)
 , mnWidth(0)
@@ -124,7 +123,7 @@ OpenGLSalBitmap::~OpenGLSalBitmap()
 bool OpenGLSalBitmap::Create( const OpenGLTexture& rTex, long nX, long nY, long nWidth, long nHeight )
 {
     static const BitmapPalette aEmptyPalette;
-    OpenGLZone aZone;
+    OpenGLVCLContextZone aContextZone;
 
     Destroy();
     VCL_GL_INFO( "OpenGLSalBitmap::Create from FBO: ["
@@ -151,7 +150,7 @@ bool OpenGLSalBitmap::Create( const OpenGLTexture& rTex, long nX, long nY, long
 
 bool OpenGLSalBitmap::Create( const Size& rSize, sal_uInt16 nBits, const BitmapPalette& rBitmapPalette )
 {
-    OpenGLZone aZone;
+    OpenGLVCLContextZone aContextZone;
 
     Destroy();
     VCL_GL_INFO( "OpenGLSalBitmap::Create with size: " << rSize );
@@ -421,7 +420,6 @@ Size OpenGLSalBitmap::GetSize() const
 
 void OpenGLSalBitmap::ExecuteOperations()
 {
-    makeSomeOpenGLContextCurrent();
     while( !maPendingOps.empty() )
     {
         OpenGLSalBitmapOp* pOp = maPendingOps.front();
@@ -488,7 +486,7 @@ GLuint OpenGLSalBitmap::CreateTexture()
         }
     }
 
-    makeSomeOpenGLContextCurrent();
+    OpenGLVCLContextZone aContextZone;
 
     lclInstantiateTexture(maTexture, mnBufWidth, mnBufHeight, nFormat, nType, pData);
 
@@ -516,11 +514,12 @@ bool OpenGLSalBitmap::ReadTexture()
     if( pData == nullptr )
         return false;
 
+    OpenGLVCLContextZone aContextZone;
+
     if (mnBits == 8 || mnBits == 16 || mnBits == 24 || mnBits == 32)
     {
         determineTextureFormat(mnBits, nFormat, nType);
 
-        makeSomeOpenGLContextCurrent();
         maTexture.Read(nFormat, nType, pData);
         mnBufWidth = mnWidth;
         mnBufHeight = mnHeight;
@@ -529,7 +528,7 @@ bool OpenGLSalBitmap::ReadTexture()
     else if (mnBits == 1)
     {   // convert buffers from 24-bit RGB to 1-bit Mask
         std::vector<sal_uInt8> aBuffer(mnWidth * mnHeight * 3);
-        makeSomeOpenGLContextCurrent();
+
         sal_uInt8* pBuffer = aBuffer.data();
         determineTextureFormat(24, nFormat, nType);
         maTexture.Read(nFormat, nType, pBuffer);
@@ -585,6 +584,9 @@ bool OpenGLSalBitmap::calcChecksumGL(OpenGLTexture& rInputTexture, ChecksumType&
 {
     OUString FragShader("areaHashCRC64TFragmentShader");
 
+    OpenGLZone aZone;
+    rtl::Reference< OpenGLContext > xContext = OpenGLContext::getVCLContext();
+
     static vcl::DeleteOnDeinit<OpenGLTexture> gCRCTableTexture(
         new OpenGLTexture(512, 1, GL_RGBA, GL_UNSIGNED_BYTE,
                            vcl_get_crc64_table()));
@@ -595,7 +597,7 @@ bool OpenGLSalBitmap::calcChecksumGL(OpenGLTexture& rInputTexture, ChecksumType&
     int nWidth = rInputTexture.GetWidth();
     int nHeight = rInputTexture.GetHeight();
 
-    OpenGLProgram* pProgram = mpContext->UseProgram("textureVertexShader", FragShader);
+    OpenGLProgram* pProgram = xContext->UseProgram("textureVertexShader", FragShader);
     if (pProgram == nullptr)
         return false;
 
@@ -603,7 +605,7 @@ bool OpenGLSalBitmap::calcChecksumGL(OpenGLTexture& rInputTexture, ChecksumType&
     int nNewHeight = ceil( nHeight / 4.0 );
 
     OpenGLTexture aFirstPassTexture = OpenGLTexture(nNewWidth, nNewHeight);
-    OpenGLFramebuffer* pFramebuffer = mpContext->AcquireFramebuffer(aFirstPassTexture);
+    OpenGLFramebuffer* pFramebuffer = xContext->AcquireFramebuffer(aFirstPassTexture);
 
     pProgram->SetUniform1f( "xstep", 1.0 / mnWidth );
     pProgram->SetUniform1f( "ystep", 1.0 / mnHeight );
@@ -623,7 +625,7 @@ bool OpenGLSalBitmap::calcChecksumGL(OpenGLTexture& rInputTexture, ChecksumType&
     nWidth = aFirstPassTexture.GetWidth();
     nHeight = aFirstPassTexture.GetHeight();
 
-    pProgram = mpContext->UseProgram("textureVertexShader", FragShader);
+    pProgram = xContext->UseProgram("textureVertexShader", FragShader);
     if (pProgram == nullptr)
         return false;
 
@@ -631,7 +633,7 @@ bool OpenGLSalBitmap::calcChecksumGL(OpenGLTexture& rInputTexture, ChecksumType&
     nNewHeight = ceil( nHeight / 4.0 );
 
     OpenGLTexture aSecondPassTexture = OpenGLTexture(nNewWidth, nNewHeight);
-    pFramebuffer = mpContext->AcquireFramebuffer(aSecondPassTexture);
+    pFramebuffer = xContext->AcquireFramebuffer(aSecondPassTexture);
 
     pProgram->SetUniform1f( "xstep", 1.0 / mnWidth );
     pProgram->SetUniform1f( "ystep", 1.0 / mnHeight );
@@ -663,7 +665,7 @@ void OpenGLSalBitmap::updateChecksum() const
 
     OpenGLSalBitmap* pThis = const_cast<OpenGLSalBitmap*>(this);
 
-    if (!mpContext.is())
+    if (!mbDirtyTexture)
     {
         pThis->CreateTexture();
     }
@@ -687,25 +689,9 @@ rtl::Reference<OpenGLContext> OpenGLSalBitmap::GetBitmapContext()
     return ImplGetDefaultWindow()->GetGraphics()->GetOpenGLContext();
 }
 
-void OpenGLSalBitmap::makeSomeOpenGLContextCurrent()
-{
-    ImplSVData* pSVData = ImplGetSVData();
-
-    // TODO: make sure we can really use the last used context
-    OpenGLContext *pContext = pSVData->maGDIData.mpLastContext;
-    while( pContext && !pContext->isInitialized() )
-        pContext = pContext->mpPrevContext;
-    if( pContext )
-        mpContext = pContext;
-    else
-        mpContext = GetBitmapContext();
-    assert(mpContext.is() && "Couldn't get an OpenGL context");
-    mpContext->makeCurrent();
-}
-
 BitmapBuffer* OpenGLSalBitmap::AcquireBuffer( BitmapAccessMode nMode )
 {
-    OpenGLZone aZone;
+    OpenGLVCLContextZone aContextZone;
 
     if( nMode != BITMAP_INFO_ACCESS )
     {
@@ -816,7 +802,7 @@ BitmapBuffer* OpenGLSalBitmap::AcquireBuffer( BitmapAccessMode nMode )
 
 void OpenGLSalBitmap::ReleaseBuffer( BitmapBuffer* pBuffer, BitmapAccessMode nMode )
 {
-    OpenGLZone aZone;
+    OpenGLVCLContextZone aContextZone;
 
     if( nMode == BITMAP_WRITE_ACCESS )
     {
@@ -871,23 +857,22 @@ bool OpenGLSalBitmap::GetSystemData( BitmapSystemData& /*rData*/ )
 
 bool OpenGLSalBitmap::Replace( const Color& rSearchColor, const Color& rReplaceColor, sal_uLong nTol )
 {
-
     VCL_GL_INFO("::Replace");
 
     OpenGLZone aZone;
+    rtl::Reference< OpenGLContext > xContext = OpenGLContext::getVCLContext();
 
     OpenGLFramebuffer* pFramebuffer;
     OpenGLProgram* pProgram;
 
     GetTexture();
-    makeSomeOpenGLContextCurrent();
-    pProgram = mpContext->UseProgram( "textureVertexShader",
-                                      "replaceColorFragmentShader" );
+    pProgram = xContext->UseProgram( "textureVertexShader",
+                                     "replaceColorFragmentShader" );
     if( !pProgram )
         return false;
 
     OpenGLTexture aNewTex = OpenGLTexture( mnWidth, mnHeight );
-    pFramebuffer = mpContext->AcquireFramebuffer( aNewTex );
+    pFramebuffer = xContext->AcquireFramebuffer( aNewTex );
 
     pProgram->SetTexture( "sampler", maTexture );
     pProgram->SetColor( "search_color", rSearchColor );
diff --git a/vcl/opengl/scale.cxx b/vcl/opengl/scale.cxx
index 6fab40517b48..80bbdd6d521a 100644
--- a/vcl/opengl/scale.cxx
+++ b/vcl/opengl/scale.cxx
@@ -37,6 +37,7 @@ using vcl::BilinearKernel;
 using vcl::BoxKernel;
 
 bool OpenGLSalBitmap::ImplScaleFilter(
+    const rtl::Reference< OpenGLContext > &xContext,
     const double& rScaleX,
     const double& rScaleY,
     GLenum        nFilter )
@@ -47,13 +48,13 @@ bool OpenGLSalBitmap::ImplScaleFilter(
     int nNewWidth( mnWidth * rScaleX );
     int nNewHeight( mnHeight * rScaleY );
 
-    pProgram = mpContext->UseProgram( "textureVertexShader",
-                                      "textureFragmentShader" );
+    pProgram = xContext->UseProgram( "textureVertexShader",
+                                     "textureFragmentShader" );
     if( !pProgram )
         return false;
 
     OpenGLTexture aNewTex(nNewWidth, nNewHeight);
-    pFramebuffer = mpContext->AcquireFramebuffer( aNewTex );
+    pFramebuffer = xContext->AcquireFramebuffer( aNewTex );
 
     pProgram->SetTexture( "sampler", maTexture );
     nOldFilter = maTexture.GetFilter();
@@ -112,6 +113,7 @@ void OpenGLSalBitmap::ImplCreateKernel(
 }
 
 bool OpenGLSalBitmap::ImplScaleConvolution(
+    const rtl::Reference< OpenGLContext > &xContext,
     const double& rScaleX,
     const double& rScaleY,
     const Kernel& aKernel )
@@ -126,7 +128,7 @@ bool OpenGLSalBitmap::ImplScaleConvolution(
 
     // TODO Make sure the framebuffer is alright
 
-    pProgram = mpContext->UseProgram( "textureVertexShader",
+    pProgram = xContext->UseProgram( "textureVertexShader",
                                       "convolutionFragmentShader" );
     if( pProgram == nullptr )
         return false;
@@ -136,7 +138,7 @@ bool OpenGLSalBitmap::ImplScaleConvolution(
     {
         OpenGLTexture aScratchTex(nNewWidth, nNewHeight);
 
-        pFramebuffer = mpContext->AcquireFramebuffer( aScratchTex );
+        pFramebuffer = xContext->AcquireFramebuffer( aScratchTex );
 
         for( sal_uInt32 i = 0; i < 16; i++ )
         {
@@ -159,7 +161,7 @@ bool OpenGLSalBitmap::ImplScaleConvolution(
     {
         OpenGLTexture aScratchTex(nNewWidth, nNewHeight);
 
-        pFramebuffer = mpContext->AcquireFramebuffer( aScratchTex );
+        pFramebuffer = xContext->AcquireFramebuffer( aScratchTex );
 
         for( sal_uInt32 i = 0; i < 16; i++ )
         {
@@ -192,7 +194,8 @@ bool OpenGLSalBitmap::ImplScaleConvolution(
  the generic case needs to also consider that some source pixels contribute
  only partially to their resulting pixels (because of non-integer multiples).
 */
-bool OpenGLSalBitmap::ImplScaleArea( double rScaleX, double rScaleY )
+bool OpenGLSalBitmap::ImplScaleArea( const rtl::Reference< OpenGLContext > &xContext,
+                                     double rScaleX, double rScaleY )
 {
     int nNewWidth( mnWidth * rScaleX );
     int nNewHeight( mnHeight * rScaleY );
@@ -213,14 +216,14 @@ bool OpenGLSalBitmap::ImplScaleArea( double rScaleX, double rScaleY )
 
     // TODO Make sure the framebuffer is alright
 
-    OpenGLProgram* pProgram = mpContext->UseProgram( "textureVertexShader",
+    OpenGLProgram* pProgram = xContext->UseProgram( "textureVertexShader",
         fast ? OUString( "areaScaleFastFragmentShader" ) : OUString( "areaScaleFragmentShader" ));
     if( pProgram == nullptr )
         return false;
 
     OpenGLTexture aScratchTex(nNewWidth, nNewHeight);
 
-    OpenGLFramebuffer* pFramebuffer = mpContext->AcquireFramebuffer( aScratchTex );
+    OpenGLFramebuffer* pFramebuffer = xContext->AcquireFramebuffer( aScratchTex );
 
     // NOTE: This setup is also done in OpenGLSalGraphicsImpl::DrawTransformedTexture().
     if( fast )
@@ -263,31 +266,32 @@ bool OpenGLSalBitmap::ImplScale( const double& rScaleX, const double& rScaleY, B
     VCL_GL_INFO( "::ImplScale" );
 
     maUserBuffer.reset();
-    makeSomeOpenGLContextCurrent();
+    OpenGLVCLContextZone aContextZone;
+    rtl::Reference<OpenGLContext> xContext = OpenGLContext::getVCLContext();
 
     if( nScaleFlag == BmpScaleFlag::Fast )
     {
-        return ImplScaleFilter( rScaleX, rScaleY, GL_NEAREST );
+        return ImplScaleFilter( xContext, rScaleX, rScaleY, GL_NEAREST );
     }
     if( nScaleFlag == BmpScaleFlag::BiLinear )
     {
-        return ImplScaleFilter( rScaleX, rScaleY, GL_LINEAR );
+        return ImplScaleFilter( xContext, rScaleX, rScaleY, GL_LINEAR );
     }
     else if( nScaleFlag == BmpScaleFlag::Super || nScaleFlag == BmpScaleFlag::Default )
     {
         const Lanczos3Kernel aKernel;
 
-        return ImplScaleConvolution( rScaleX, rScaleY, aKernel );
+        return ImplScaleConvolution( xContext, rScaleX, rScaleY, aKernel );
     }
     else if( nScaleFlag == BmpScaleFlag::BestQuality && rScaleX <= 1 && rScaleY <= 1 )
     { // Use are scaling for best quality, but only if downscaling.
-        return ImplScaleArea( rScaleX, rScaleY );
+        return ImplScaleArea( xContext, rScaleX, rScaleY );
     }
     else if( nScaleFlag == BmpScaleFlag::Lanczos || nScaleFlag == BmpScaleFlag::BestQuality  )
     {
         const Lanczos3Kernel aKernel;
 
-        return ImplScaleConvolution( rScaleX, rScaleY, aKernel );
+        return ImplScaleConvolution( xContext, rScaleX, rScaleY, aKernel );
     }
 
     SAL_WARN( "vcl.opengl", "Invalid flag for scaling operation" );
@@ -296,7 +300,7 @@ bool OpenGLSalBitmap::ImplScale( const double& rScaleX, const double& rScaleY, B
 
 bool OpenGLSalBitmap::Scale( const double& rScaleX, const double& rScaleY, BmpScaleFlag nScaleFlag )
 {
-    OpenGLZone aZone;
+    OpenGLVCLContextZone aContextZone;
 
     VCL_GL_INFO("::Scale " << int(nScaleFlag)
              << " from " << mnWidth << "x" << mnHeight
@@ -309,7 +313,6 @@ bool OpenGLSalBitmap::Scale( const double& rScaleX, const double& rScaleY, BmpSc
         nScaleFlag == BmpScaleFlag::Default ||
         nScaleFlag == BmpScaleFlag::BestQuality )
     {
-        makeSomeOpenGLContextCurrent();
         ImplScale( rScaleX, rScaleY, nScaleFlag );
         return true;
     }
diff --git a/vcl/opengl/texture.cxx b/vcl/opengl/texture.cxx
index 83aa3d44325e..303b8b8f19d6 100644
--- a/vcl/opengl/texture.cxx
+++ b/vcl/opengl/texture.cxx
@@ -28,6 +28,7 @@
 
 #include "opengl/framebuffer.hxx"
 #include "opengl/texture.hxx"
+#include "opengl/zone.hxx"
 
 // texture with allocated size
 ImplOpenGLTexture::ImplOpenGLTexture( int nWidth, int nHeight, bool bAllocate ) :
@@ -38,6 +39,8 @@ ImplOpenGLTexture::ImplOpenGLTexture( int nWidth, int nHeight, bool bAllocate )
     mnOptStencil( 0 ),
     mnFreeSlots(-1)
 {
+    OpenGLVCLContextZone aContextZone;
+
     glGenTextures( 1, &mnTexture );
     CHECK_GL_ERROR();
     glBindTexture( GL_TEXTURE_2D, mnTexture );
@@ -71,6 +74,8 @@ ImplOpenGLTexture::ImplOpenGLTexture( int nX, int nY, int nWidth, int nHeight )
     mnOptStencil( 0 ),
     mnFreeSlots(-1)
 {
+    OpenGLVCLContextZone aContextZone;
+
     // FIXME We need the window height here
     // nY = GetHeight() - nHeight - nY;
 
@@ -104,6 +109,8 @@ ImplOpenGLTexture::ImplOpenGLTexture( int nWidth, int nHeight, int nFormat, int
     mnOptStencil( 0 ),
     mnFreeSlots(-1)
 {
+    OpenGLVCLContextZone aContextZone;
+
     if( !mnTexture )
     {
         glGenTextures( 1, &mnTexture );
@@ -152,13 +159,18 @@ ImplOpenGLTexture::~ImplOpenGLTexture()
     VCL_GL_INFO( "~OpenGLTexture " << mnTexture );
     if( mnTexture != 0 )
     {
+        OpenGLVCLContextZone aContextZone;
+
         // FIXME: this is really not optimal performance-wise.
 
         // Check we have been correctly un-bound from all framebuffers.
         ImplSVData* pSVData = ImplGetSVData();
         rtl::Reference<OpenGLContext> pContext = pSVData->maGDIData.mpLastContext;
         if( pContext.is() )
+        {
+            pContext->makeCurrent();
             pContext->UnbindTextureFromFramebuffers( mnTexture );
+        }
 
         if( mnOptStencil != 0 )
             glDeleteRenderbuffers( 1, &mnOptStencil );
@@ -426,6 +438,8 @@ void OpenGLTexture::Read( GLenum nFormat, GLenum nType, sal_uInt8* pData )
         return;
     }
 
+    OpenGLVCLContextZone aContextZone;
+
     VCL_GL_INFO( "Reading texture " << Id() << " " << GetWidth() << "x" << GetHeight() );
 
     if( GetWidth() == mpImpl->mnWidth && GetHeight() == mpImpl->mnHeight )