Sean's Optimum Branch Merge

[seandepagnier]

Originally Posted by bdbcat

Sean...

On Mac, with texture compression, we see incomplete patches in quilt mode. No crashes. Pix attached.

On linux, skewed charts, quilt mode, OpenGL: some problem with clip region when zooming in, pix attached.


Dave

Dave,

Doesn't macos use a different region class? Maybe this is the cause?

I tried to make it possible to draw the world background chart in skewed mode (which should be possible) but has limitations when opengl is not used depending on rotation. For both cases, we can use git bisect to find where things went wrong.

I have a lingering diff in my optimum branch I never submitted because I can't remember the test cases where it matters, so wanted to avoid changes that are not needed, maybe this fixes it? If it does.. we should really fix things to avoid coordinate differences for opengl.

diff --git a/src/chartimg.cpp b/src/chartimg.cpp
index 1f47986..e3dbd3a 100644
--- a/src/chartimg.cpp
+++ b/src/chartimg.cpp
@@ -3084,26 +3084,41 @@ void ChartBaseBSB::GetValidCanvasRegion(const ViewPort& VPoint, OCPNRegion *pVal
       double raster_scale =  VPoint.view_scale_ppm / GetPPM();
 
       int rxl, rxr;
-      if(Rsrc.x < 0)
-            rxl = (int)(-Rsrc.x * raster_scale);
-      else
+      int ryb, ryt;
+
+      /* TODO: the original version works in non opengl mode,
+         the next one is needed for opengl.  This is needed for rotated
+         viewports.. without rotation.. either version is the same.
+         This should be modified so we can not make a distinction here
+         for opengl and use the same coordinates correct? */
+      extern bool g_bopengl;
+      if(!g_bopengl) {
+          if(Rsrc.x < 0)
+              rxl = (int)(-Rsrc.x * raster_scale);
+          else
             rxl = 0;
 
-      if(((Size_X - Rsrc.x) * raster_scale) < VPoint.pix_width)
-            rxr = (int)((Size_X - Rsrc.x) * raster_scale);
-      else
-            rxr = VPoint.pix_width;
+          if(((Size_X - Rsrc.x) * raster_scale) < VPoint.pix_width)
+              rxr = (int)((Size_X - Rsrc.x) * raster_scale);
+          else
+              rxr = VPoint.pix_width;
 
-      int ryb, ryt;
-      if(Rsrc.y < 0)
-            ryt = (int)(-Rsrc.y * raster_scale);
-      else
-            ryt = 0;
+          if(Rsrc.y < 0)
+              ryt = (int)(-Rsrc.y * raster_scale);
+          else
+              ryt = 0;
+          
+          if(((Size_Y - Rsrc.y) * raster_scale) < VPoint.pix_height)
+              ryb = (int)((Size_Y - Rsrc.y) * raster_scale);
+          else
+              ryb = VPoint.pix_height;
+      } else {
+          rxl = wxMax(-Rsrc.x * raster_scale, VPoint.rv_rect.x);
+          rxr = wxMin((Size_X - Rsrc.x) * raster_scale, VPoint.rv_rect.width + VPoint.rv_rect.x);
 
-      if(((Size_Y - Rsrc.y) * raster_scale) < VPoint.pix_height)
-            ryb = (int)((Size_Y - Rsrc.y) * raster_scale);
-      else
-            ryb = VPoint.pix_height;
+          ryt = wxMax(-Rsrc.y * raster_scale, VPoint.rv_rect.y);
+          ryb = wxMin((Size_Y - Rsrc.y) * raster_scale, VPoint.rv_rect.height + VPoint.rv_rect.y);
+      }
 
       pValidRegion->Clear();
       pValidRegion->Union(rxl, ryt, rxr - rxl, ryb - ryt);

[Hakan]

Originally Posted by boat_alexandra

........I'm not sure we need this. Maybe, by default texture compression caching should be disabled. You get a time estimation (not very accurate) once the operation starts. It is possible to click the skip button to abort. Also, it is possible to make the generation work without blocking the program, although it will slow the whole computer down a lot.

-Slow the computer down- Well, it's not possible to slow more then what's already done. I'm now on the eighth hours and another four is estimated all using 99% processing time. For a beta it's of course ok but we have to agree how to handle this before presenting it to a "normal" user. Now let's see if the process will let me post this reply

[Hakan]

Some extra mark symbols are colour inverted and not sized. They are oversized in a overview zoom scale. Maybe they are not built to be used in OpenGL?

[Hakan]

Win32, KAP-files and CM93
To complete Dave's post above. In some zoom levels the background chart is shown around one chart instead of surrounding KAPs or CM93. From the the red lines it's obvious there should have been other charts to view.

[bdbcat]

Sean...

The diff you supplied did the trick for OpenGL. And it turns out to work the same for DC mode as well, so I simplified the method to not care about display mode. The TODO is done, in github now.

Thanks
Dave

[bdbcat]

Sean, et al..

I changed the default for texture compression to "OFF" for now, so that we can test incrementally.

Dave

[ChuckSK]

Here are a few patches.

The first one deals with the arrow heads on selected and active routes.

diff --git a/src/Route.cpp b/src/Route.cpp
index 59fb801..9f4d6a1 100644
--- a/src/Route.cpp
+++ b/src/Route.cpp
@@ -481,11 +481,11 @@ void Route::DrawGL( ViewPort &VP )
     wxPoint rpt1, rpt2;
     while(node) {
         RoutePoint *prp = node->GetData();
-        node = node->GetNext();
         cc1->GetCanvasPointPix( prp->m_lat, prp->m_lon, &rpt2 );
         if(node != pRoutePointList->GetFirst())
             RenderSegmentArrowsGL( rpt1.x, rpt1.y, rpt2.x, rpt2.y, cc1->GetVP() );
         rpt1 = rpt2;
+        node = node->GetNext();
     }
 #endif
 }
@@ -598,12 +598,12 @@ void Route::RenderSegmentArrowsGL( int xa, int ya, int xb, int yb, ViewPort &VP)
             / nom_arrow_size;
 
     float theta = atan2f( (float)yb - ya, (float)xb - xa );
-    theta -= PI / 2;
+    theta -= PI ;
 
     glPushMatrix();
-    glScalef(icon_scale_factor, icon_scale_factor, 1);
-    glRotatef(theta * 180/PI, 0, 0, 1);
     glTranslatef(xb, yb, 0);
+    glScalef(icon_scale_factor, icon_scale_factor, 1);
+    glRotatef(theta * 180/PI, 0, 0, 1);    
 
     glBegin(GL_POLYGON);
     for( int i = 0; i < 14; i+=2 )

This one deals with flashing displays when routes and AIS targets are rolled over.

diff --git a/src/chcanv.cpp b/src/chcanv.cpp
index 743590b..f2f4d8c 100644
--- a/src/chcanv.cpp
+++ b/src/chcanv.cpp
@@ -2471,7 +2471,7 @@ void ChartCanvas::OnRolloverPopupTimerEvent( wxTimerEvent& event )
                     m_RolloverPopupTimer.Start( 50, wxTIMER_ONE_SHOT );
                     m_pAISRolloverWin->IsActive( false );
                     m_AISRollover_MMSI = 0;
-                    Refresh();
+                    Refresh(false);
                     return;
                 }
 
@@ -2620,7 +2620,7 @@ void ChartCanvas::OnRolloverPopupTimerEvent( wxTimerEvent& event )
     }
 
     if( b_need_refresh )
-        Refresh();
+        Refresh(false);
 }

This fixes the disappearing waypoints on selected and active routes.

diff --git a/src/glChartCanvas.cpp b/src/glChartCanvas.cpp
index 0b83557..6358a52 100755
--- a/src/glChartCanvas.cpp
+++ b/src/glChartCanvas.cpp
@@ -1515,7 +1515,7 @@ void glChartCanvas::DrawAllRoutesAndWaypoints( ViewPort &vp, OCPNRegion &region
         for(wxRoutePointListNode *pnode = pWayPointMan->GetWaypointList()->GetFirst();
             pnode; pnode = pnode->GetNext() ) {
             RoutePoint *pWP = pnode->GetData();
-            if( pWP && !pWP->m_bIsInRoute && !pWP->m_bIsInTrack )
+            if( pWP /*&& !pWP->m_bIsInRoute && !pWP->m_bIsInTrack*/ )
                 pWP->DrawGL( vp, region );
         }
 }

Chuck

[ChuckSK]

There is a problem using textures for chart symbols.

When ChartSymbols::LoadRasterFileForColorTable is called in the start-up process, g_texture_rectangle_format has not yet been set. This generates a warning message in the log file--Warning: unable to use texture for chart symbols. So, in s52plib::RenderRasterSymbol a texture is not found.

If using OpenGL is de-selected and then re_selected, g_texture_rectangle_format is set before ChartSymbols::LoadRasterFileForColorTable is called and all is well.

Chuck

[seandepagnier]

Originally Posted by Hakan

Win32, KAP-files and CM93
To complete Dave's post above. In some zoom levels the background chart is shown around one chart instead of surrounding KAPs or CM93. From the the red lines it's obvious there should have been other charts to view.

Can you verify that this problem didn't exist before? If so, could you use git-bisect to determine which commit broke things?

Does anyone else experience this?

Originally Posted by ChuckSK

There is a problem using textures for chart symbols.

When ChartSymbols::LoadRasterFileForColorTable is called in the start-up process, g_texture_rectangle_format has not yet been set. This generates a warning message in the log file--Warning: unable to use texture for chart symbols. So, in s52plib::RenderRasterSymbol a texture is not found.

If using OpenGL is de-selected and then re_selected, g_texture_rectangle_format is set before ChartSymbols::LoadRasterFileForColorTable is called and all is well.

Chuck

This is because Dave added setting the color scheme to vector charts earlier in the program (needed for the s63 charts) which now happens before the glChartCanvas is initialized... This needs to happen a bit later somehow.

I also found a crash if your disk is completely full when you exit the program.. and it's not obvious what the cause is.

Originally Posted by ChuckSK

Here are a few patches.

The first one deals with the arrow heads on selected and active routes.

diff --git a/src/Route.cpp b/src/Route.cpp
index 59fb801..9f4d6a1 100644
--- a/src/Route.cpp
+++ b/src/Route.cpp
@@ -481,11 +481,11 @@ void Route::DrawGL( ViewPort &VP )
     wxPoint rpt1, rpt2;
     while(node) {
         RoutePoint *prp = node->GetData();
-        node = node->GetNext();
         cc1->GetCanvasPointPix( prp->m_lat, prp->m_lon, &rpt2 );
         if(node != pRoutePointList->GetFirst())
             RenderSegmentArrowsGL( rpt1.x, rpt1.y, rpt2.x, rpt2.y, cc1->GetVP() );
         rpt1 = rpt2;
+        node = node->GetNext();
     }
 #endif
 }
@@ -598,12 +598,12 @@ void Route::RenderSegmentArrowsGL( int xa, int ya, int xb, int yb, ViewPort &VP)
             / nom_arrow_size;
 
     float theta = atan2f( (float)yb - ya, (float)xb - xa );
-    theta -= PI / 2;
+    theta -= PI ;
 
     glPushMatrix();
-    glScalef(icon_scale_factor, icon_scale_factor, 1);
-    glRotatef(theta * 180/PI, 0, 0, 1);
     glTranslatef(xb, yb, 0);
+    glScalef(icon_scale_factor, icon_scale_factor, 1);
+    glRotatef(theta * 180/PI, 0, 0, 1);    
 
     glBegin(GL_POLYGON);
     for( int i = 0; i < 14; i+=2 )

[/code]

This fixes the disappearing waypoints on selected and active routes.

diff --git a/src/glChartCanvas.cpp b/src/glChartCanvas.cpp
index 0b83557..6358a52 100755
--- a/src/glChartCanvas.cpp
+++ b/src/glChartCanvas.cpp
@@ -1515,7 +1515,7 @@ void glChartCanvas::DrawAllRoutesAndWaypoints( ViewPort &vp, OCPNRegion &region
         for(wxRoutePointListNode *pnode = pWayPointMan->GetWaypointList()->GetFirst();
             pnode; pnode = pnode->GetNext() ) {
             RoutePoint *pWP = pnode->GetData();
-            if( pWP && !pWP->m_bIsInRoute && !pWP->m_bIsInTrack )
+            if( pWP /*&& !pWP->m_bIsInRoute && !pWP->m_bIsInTrack*/ )
                 pWP->DrawGL( vp, region );
         }
 }

Good, catch.. I don't know what I was thinking. This code actually meant to be used for all routes, not just selected ones.

This one deals with flashing displays when routes and AIS targets are rolled over.

diff --git a/src/chcanv.cpp b/src/chcanv.cpp
index 743590b..f2f4d8c 100644
--- a/src/chcanv.cpp
+++ b/src/chcanv.cpp
@@ -2471,7 +2471,7 @@ void ChartCanvas::OnRolloverPopupTimerEvent( wxTimerEvent& event )
                     m_RolloverPopupTimer.Start( 50, wxTIMER_ONE_SHOT );
                     m_pAISRolloverWin->IsActive( false );
                     m_AISRollover_MMSI = 0;
-                    Refresh();
+                    Refresh(false);
                     return;
                 }
 
@@ -2620,7 +2620,7 @@ void ChartCanvas::OnRolloverPopupTimerEvent( wxTimerEvent& event )
     }
 
     if( b_need_refresh )
-        Refresh();
+        Refresh(false);
 }

Chuck

This is not specific to the optimum branch is it? In any case I don't get a flashing display, and I think this should be re-implemented to not use a rollover window in opengl mode to do more correct alpha blending without needing to grab pixels...

[bdbcat]

Sean/Chuck...

I changed again the initialization sequence for the S52Plib raster symbol table load. Should be OK now. There is a nagging circle condition here, leading to unnecessary module dependencies. IMHO, it would be cleaner if the OpenGL setup code could be run long before the first actual glChartCanvas::OnPaint() method, and sometime before the PlugIns are loaded, as well.

Dave

[ChuckSK]

Sean

The screen still flashes during AIS target rollover. Route rollover is fixed.

Chuck

[cagney]

Originally Posted by ChuckSK

Sean

The screen still flashes during AIS target rollover. Route rollover is fixed.

Chuck

No problems with rollover on Linux.
After yesterdays or today’s updates, a routes way-points are still visible after after hiding the route in the route manager.
Thomas

[bdbcat]

Hello testers...

A couple of changes committed to improve usability of routes/wps and routemanagerdialog, especially if OpenGL Frame Buffer Objects are enabled.

Please keep the test reports coming, no matter how arcane. There is a lot of regression ground to cover here.

Enjoy
Dave

[bdbcat]

Chuck, et al...

Latest commit corrects MSW OpenGL flashing on rollover and Refresh().

Thanks
Dave

[seandepagnier]

For windows users who want to do some testing... I have built installers for windows here:

https://sourceforge.net/projects/ope..._experimental/

Keep in mind this is using mingw compiler (cross compiled under linux) so it could potentially have "other" problems. I tried to build a 64bit binary and it always seems to crash when starting (I think there are several windows-specific bugs for 64bits) It will use sse2 instructions though, and should be faster than the visual studio build for several reasons. Keep in mind I'm using wine windows, not microsoft windows for testing.


Another thing.. I noticed in the code:

    //    On MSW, force the entire process to run on one CPU core only                              
    //    This resolves some difficulty with wxThread syncronization

Dave or anyone, do you know why we have this? I changed it from all windows to visual studio only (so the above download doesn't do this on windows) as this has serious negative implications for users with smp systems. For example, cpu hungry operations like generating the compressed cache or weather routing both take advantage of multiple threads, but this undermines it.


There are _several_ really annoying bugs related to the region class which caused many crashes only when optimizations were enabled... wxWidgets uses different region classes for each platform which seems strange to me because it should be a portable feature. In any case, the windows (when compiled with -O) (and probably mac) versions appear to exhibit undefined behavior in certain circumstances. I fixed all of this by just using the builtin gdk version which is already in opencpn and probably more efficient anyway. It had a few features left out which would explain issues on macosx.