kicad-developers team mailing list archive

[Maciej Sumiński <maciej.suminski@xxxxxxx>]

On 06/13/2016 01:58 AM, Collin Anderson wrote:
> This same behavior occurs on my OS X box as well.  It also has coordinate transformation issues if you try to lower the size of the window.  Here, rather than try and describe it, here is a gif (frame rate is low in the gif, it is not choppy in reality): https://metacollin.com/resize.gif 
> 
> I have been using OpenGL for years with limited success ( old but probably best demo that I may, in fact, not be full of crap ;)  https://www.youtube.com/watch?v=SGddIJBAEUk ) and I've been glancing at the code for anything obvious, and I did find one problem:
> 
> In the constructor for OPENGL_GAL:
> 
>   if( glMainContext == NULL )
>     {
>         glMainContext = GL_CONTEXT_MANAGER::Get().CreateCtx( this );
>         glPrivContext = glMainContext;
>         shader = new SHADER();
>     }
>     else
>     {
>         glPrivContext = GL_CONTEXT_MANAGER::Get().CreateCtx( this, glMainContext );
>     }
> 
> 
> This looks dangerous.  I might be wrong, KiCad's code base is ....non-trivial :).  So maybe there is no problem here, but if glMainContext is not NULL, is it guaranteed to be 'virgin' at this point? OpenGL contexts must be totally fresh when they are shared - you cannot make any opengl calls in any context until after you've created a shared context (if you are going to).  In other words, one would be expected to call the function that ultimately creates a new context that is shared with another one immediately after that other one has been created.  
> 
> i.e. 
> 
>  glMainContext = GL_CONTEXT_MANAGER::Get().CreateCtx( this ); // the main context
>  glPrivContext = GL_CONTEXT_MANAGER::Get().CreateCtx( this, glMainContext ); // Must create your shared context immediately after, before any OpenGL calls are made to the main context
> 
> Again, maybe that is exactly what will ultimately happen, I didn't get a chance to look at the higher level code to check.  Hopefully someone who knows what is going on can say for sure though. 
> 
> Anyway, I'll keep going through the code as time permits, at least while things are still broken OpenGLwise.  

Here is a patch that creates GL contexts in the way you recommend.
Please let me know if it solves the problem with OSX. I believe it will
change nothing, so probably the best way is to revert 6912 and look for
another solution to setting GL context when a window is not visible.

Regards,
Orson

=== modified file 'common/gal/opengl/opengl_gal.cpp'
--- old/common/gal/opengl/opengl_gal.cpp	2016-06-13 09:35:36.052999000 +0200
+++ new/common/gal/opengl/opengl_gal.cpp	2016-06-13 10:53:25.899765286 +0200
@@ -67,14 +67,10 @@ OPENGL_GAL::OPENGL_GAL( wxWindow* aParen
     if( glMainContext == NULL )
     {
         glMainContext = GL_CONTEXT_MANAGER::Get().CreateCtx( this );
-        glPrivContext = glMainContext;
         shader = new SHADER();
     }
-    else
-    {
-        glPrivContext = GL_CONTEXT_MANAGER::Get().CreateCtx( this, glMainContext );
-    }
 
+    glPrivContext = GL_CONTEXT_MANAGER::Get().CreateCtx( this, glMainContext );
     ++instanceCounter;
 
     // Check if OpenGL requirements are met
@@ -156,11 +152,7 @@ OPENGL_GAL::~OPENGL_GAL()
     delete overlayManager;
 
     GL_CONTEXT_MANAGER::Get().UnlockCtx( glPrivContext );
-
-    // If it was the main context, then it will be deleted
-    // when the last OpenGL GAL instance is destroyed (a few lines below)
-    if( glPrivContext != glMainContext )
-        GL_CONTEXT_MANAGER::Get().DestroyCtx( glPrivContext );
+    GL_CONTEXT_MANAGER::Get().DestroyCtx( glPrivContext );
 
     // Are we destroying the last GAL instance?
     if( instanceCounter == 0 )
@@ -268,6 +260,10 @@ void OPENGL_GAL::BeginDrawing()
         // Either load the font atlas to video memory, or simply bind it to a texture unit
         if( !isBitmapFontLoaded )
         {
+            // Load the texture in the main context
+            GL_CONTEXT_MANAGER::Get().UnlockCtx( glPrivContext );
+            GL_CONTEXT_MANAGER::Get().LockCtx( glMainContext );
+
             glActiveTexture( GL_TEXTURE0 + FONT_TEXTURE_UNIT );
             glGenTextures( 1, &fontTexture );
             glBindTexture( GL_TEXTURE_2D, fontTexture );
@@ -281,13 +277,15 @@ void OPENGL_GAL::BeginDrawing()
             glActiveTexture( GL_TEXTURE0 );
 
             isBitmapFontLoaded = true;
+
+            GL_CONTEXT_MANAGER::Get().UnlockCtx( glMainContext );
+            GL_CONTEXT_MANAGER::Get().LockCtx( glPrivContext );
+
         }
-        else
-        {
-            glActiveTexture( GL_TEXTURE0 + FONT_TEXTURE_UNIT );
-            glBindTexture( GL_TEXTURE_2D, fontTexture );
-            glActiveTexture( GL_TEXTURE0 );
-        }
+
+        glActiveTexture( GL_TEXTURE0 + FONT_TEXTURE_UNIT );
+        glBindTexture( GL_TEXTURE_2D, fontTexture );
+        glActiveTexture( GL_TEXTURE0 );
 
         // Set shader parameter
         GLint ufm_fontTexture = shader->AddParameter( "fontTexture" );
@@ -1491,7 +1489,7 @@ bool OPENGL_GAL::runTest()
     {
         wxDialog dlgtest( GetParent(), -1, wxT( "opengl test" ), wxPoint( 50, 50 ),
                         wxDLG_UNIT( GetParent(), wxSize( 50, 50 ) ) );
-        OPENGL_TEST* test = new OPENGL_TEST( &dlgtest, this, glPrivContext );
+        OPENGL_TEST* test = new OPENGL_TEST( &dlgtest, this, glMainContext );
 
         dlgtest.Raise();         // on Linux, on some windows managers (Unity for instance) this is needed to actually show the dialog
         dlgtest.ShowModal();

Attachment: signature.asc
Description: OpenPGP digital signature