From 6ee563ad55202e158617e7a82b58bb0e28d9d313 Mon Sep 17 00:00:00 2001 From: Robert Osfield Date: Sat, 21 Dec 2002 19:41:27 +0000 Subject: [PATCH] Updates the terrapage to try and make it thread "safer" when used with the OSG. --- src/osgPlugins/txp/TrPageArchive.cpp | 10 +- src/osgPlugins/txp/TrPageParser.cpp | 3 +- src/osgPlugins/txp/trPagePageManager.cpp | 501 ++++++++++++----------- src/osgPlugins/txp/trPagePageManager.h | 7 + 4 files changed, 265 insertions(+), 256 deletions(-) diff --git a/src/osgPlugins/txp/TrPageArchive.cpp b/src/osgPlugins/txp/TrPageArchive.cpp index cbfa08c40..a6ebace4d 100644 --- a/src/osgPlugins/txp/TrPageArchive.cpp +++ b/src/osgPlugins/txp/TrPageArchive.cpp @@ -164,17 +164,11 @@ void TrPageArchive::LoadMaterials() } else if( mode == trpgTexture::Local ) { - ref_ptr osg_texture = GetLocalTexture(image_helper,0, tex); - osg_texture->ref(); - m_textures[i] = osg_texture; - // delete [] data; + m_textures[i] = GetLocalTexture(image_helper,0, tex); } else if( mode == trpgTexture::Template ) { - ref_ptr osg_texture = GetLocalTexture(image_helper,0, tex); - if (osg_texture.valid()) osg_texture->ref(); - m_textures[i] = osg_texture; - // delete [] data; + m_textures[i] = GetLocalTexture(image_helper,0, tex); } else { diff --git a/src/osgPlugins/txp/TrPageParser.cpp b/src/osgPlugins/txp/TrPageParser.cpp index c8f16a304..0d026fb83 100644 --- a/src/osgPlugins/txp/TrPageParser.cpp +++ b/src/osgPlugins/txp/TrPageParser.cpp @@ -173,8 +173,7 @@ Texture2D* txp::GetLocalTexture(trpgrImageHelper& image_helper, trpgLocalMateria image->setMipmapData(mipmaps); } -// delete [] data; -// image->unref(); + // AAAARRRGH! TOOK ME 2 DAYS TO FIGURE IT OUT // EVERY IMAGE HAS TO HAVE UNIQUE NAME FOR OPTIMIZER NOT TO OPTIMIZE IT OFF // diff --git a/src/osgPlugins/txp/trPagePageManager.cpp b/src/osgPlugins/txp/trPagePageManager.cpp index f192383d0..552dd6f40 100644 --- a/src/osgPlugins/txp/trPagePageManager.cpp +++ b/src/osgPlugins/txp/trPagePageManager.cpp @@ -28,85 +28,85 @@ using namespace osg; OSGPageManager::OSGPageManager(TrPageArchive *in_arch,trpgPageManager *in_pageManage) { - archive = in_arch; - pageManage = in_pageManage; + archive = in_arch; + pageManage = in_pageManage; - if (!in_arch) - throw 1; + if (!in_arch) + throw 1; - if (pageManage) - pageManageOurs = false; - else { - pageManage = new trpgPageManager(); - pageManage->SetPageDistFactor(1.2); - pageManage->Init(archive); - pageManageOurs = true; - } + if (pageManage) + pageManageOurs = false; + else { + pageManage = new trpgPageManager(); + pageManage->SetPageDistFactor(1.2); + pageManage->Init(archive); + pageManageOurs = true; + } - // Location info we'll need later - const trpgHeader *head = archive->GetHeader(); - trpg2dPoint sw,ne; - head->GetExtents(sw,ne); - originX = sw.x; - originY = sw.y; + // Location info we'll need later + const trpgHeader *head = archive->GetHeader(); + trpg2dPoint sw,ne; + head->GetExtents(sw,ne); + originX = sw.x; + originY = sw.y; - threadMode = ThreadNone; + threadMode = ThreadNone; } OSGPageManager::~OSGPageManager() { - if (threadMode != ThreadNone) - EndThread(); - if (pageManageOurs) - delete pageManage; - pageManage = NULL; + if (threadMode != ThreadNone) + EndThread(); + if (pageManageOurs) + delete pageManage; + pageManage = NULL; } /* Update - Bring the paging up to date based on the given location. - The location is in TerraPage's coordinate system, but must - still be adjusted to the SW corner. + Bring the paging up to date based on the given location. + The location is in TerraPage's coordinate system, but must + still be adjusted to the SW corner. */ bool OSGPageManager::UpdateNoThread(osg::Group *rootNode,double locX,double locY,int numTile) { - // Adjust to TerraPage coordinates - double lx = locX-originX; - double ly = locY-originY; + // Adjust to TerraPage coordinates + double lx = locX-originX; + double ly = locY-originY; - /* Do that paging thing: - - Tell the manager the new location - - Iterate over the unloads - - Iterate over the loads - */ - trpg2dPoint loc; - loc.x = lx; - loc.y = ly; - if (pageManage->SetLocation(loc)) { -// printf("Location (%f,%f) resulted in changes.",loc.x,loc.y); -// trpgFilePrintBuffer printBuf(stdout); -// pageManage->Print(printBuf); - } + /* Do that paging thing: + - Tell the manager the new location + - Iterate over the unloads + - Iterate over the loads + */ + trpg2dPoint loc; + loc.x = lx; + loc.y = ly; + if (pageManage->SetLocation(loc)) { +// printf("Location (%f,%f) resulted in changes.",loc.x,loc.y); +// trpgFilePrintBuffer printBuf(stdout); +// pageManage->Print(printBuf); + } - // Do the unloads - trpgManagedTile *tile=NULL; - while ((tile = pageManage->GetNextUnload())) { - archive->UnLoadTile(pageManage,tile); - pageManage->AckUnload(); - }; + // Do the unloads + trpgManagedTile *tile=NULL; + while ((tile = pageManage->GetNextUnload())) { + archive->UnLoadTile(pageManage,tile); + pageManage->AckUnload(); + }; - // Decide how many loads to do per frame - int loadCount=0; + // Decide how many loads to do per frame + int loadCount=0; - // Do the loads - while ((tile = pageManage->GetNextLoad())) { - archive->LoadTile(rootNode,pageManage,tile); - pageManage->AckLoad(); - loadCount++; - if (numTile > 0 && loadCount >= numTile) - break; - }; + // Do the loads + while ((tile = pageManage->GetNextLoad())) { + archive->LoadTile(rootNode,pageManage,tile); + pageManage->AckLoad(); + loadCount++; + if (numTile > 0 && loadCount >= numTile) + break; + }; - return true; + return true; } // Mutex lock function @@ -114,9 +114,9 @@ bool OSGPageManager::UpdateNoThread(osg::Group *rootNode,double locX,double locY void osgLockMutex(ThreadMutex &mtx) { #if defined (_WIN32) - WaitForSingleObject( mtx, INFINITE); + WaitForSingleObject( mtx, INFINITE); #else - pthread_mutex_lock( &mtx ); + pthread_mutex_lock( &mtx ); #endif } @@ -125,9 +125,9 @@ void osgLockMutex(ThreadMutex &mtx) void osgUnLockMutex(ThreadMutex &mtx) { #if defined (_WIN32) - ReleaseMutex(mtx); + ReleaseMutex(mtx); #else - pthread_mutex_unlock( &mtx ); + pthread_mutex_unlock( &mtx ); #endif } @@ -159,8 +159,8 @@ void osgSetEvent(ThreadEvent &ev) #if defined (_WIN32) DWORD WINAPI ThreadFunc( LPVOID lpParam ) { - OSGPageManager *myPager = (OSGPageManager *)lpParam; - myPager->ThreadLoop(); + OSGPageManager *myPager = (OSGPageManager *)lpParam; + myPager->ThreadLoop(); return 0; } @@ -175,56 +175,56 @@ void *ThreadFunc( void *data ) #endif /* Start Thread - This initialized - --- Main Thread --- + This initialized + --- Main Thread --- */ bool OSGPageManager::StartThread(ThreadMode mode,ThreadID &newThread) { - positionValid = false; + positionValid = false; #if defined(_WIN32) - // Create the event we'll use to wake up the pager thread when the location changes - locationChangeEvent = CreateEvent(NULL,false,false,"Location Change Event"); + // Create the event we'll use to wake up the pager thread when the location changes + locationChangeEvent = CreateEvent(NULL,false,false,"Location Change Event"); - // Create the mutexes we'll need later - changeListMutex = CreateMutex(NULL,false,"Change List Mutex"); - locationMutex = CreateMutex(NULL,false,"Location Mutex"); + // Create the mutexes we'll need later + changeListMutex = CreateMutex(NULL,false,"Change List Mutex"); + locationMutex = CreateMutex(NULL,false,"Location Mutex"); - { - // Create the thread - DWORD dwThreadId=0; - LPVOID dwThrdParam = (LPVOID) this; + { + // Create the thread + DWORD dwThreadId=0; + LPVOID dwThrdParam = (LPVOID) this; - newThread = CreateThread( - NULL, // default security attributes - 0, // use default stack size - ThreadFunc, // thread function - dwThrdParam, // argument to thread function - 0, // use default creation flags - &dwThreadId); // returns the thread identifier - } + newThread = CreateThread( + NULL, // default security attributes + 0, // use default stack size + ThreadFunc, // thread function + dwThrdParam, // argument to thread function + 0, // use default creation flags + &dwThreadId); // returns the thread identifier + } - // Note: Should be optional - // Set the priority low so this is only called when the other thread is idle -// if (!SetThreadPriority(newThread,THREAD_PRIORITY_IDLE)) -// fprintf(stderr,"Couldn't set paging thread priority.\n"); + // Note: Should be optional + // Set the priority low so this is only called when the other thread is idle +// if (!SetThreadPriority(newThread,THREAD_PRIORITY_IDLE)) +// fprintf(stderr,"Couldn't set paging thread priority.\n"); - // Was successfull - if (newThread != NULL) - threadMode = mode; + // Was successfull + if (newThread != NULL) + threadMode = mode; #else - //locationChangeEvent is self-initialized. - pthread_mutex_init( &changeListMutex, 0L ); - pthread_mutex_init( &locationMutex, 0L ); - threadMode = mode; - if( pthread_create( &newThread, 0L, ThreadFunc, (void *)this ) != 0 ) - threadMode = ThreadNone; + //locationChangeEvent is self-initialized. + pthread_mutex_init( &changeListMutex, 0L ); + pthread_mutex_init( &locationMutex, 0L ); + threadMode = mode; + if( pthread_create( &newThread, 0L, ThreadFunc, (void *)this ) != 0 ) + threadMode = ThreadNone; #endif - return threadMode != ThreadNone; + return threadMode != ThreadNone; } /* End Thread - Note: Do this + Note: Do this */ bool OSGPageManager::EndThread() { @@ -237,179 +237,188 @@ bool OSGPageManager::EndThread() } /* Thread Loop - This method is the main loop for the pager when it's - working in its own thread. - --- Paging Thread --- + This method is the main loop for the pager when it's + working in its own thread. + --- Paging Thread --- */ bool OSGPageManager::ThreadLoop() { - // Note: Only works for free running thread - if (threadMode != ThreadFree) - throw 1; + // Note: Only works for free running thread + if (threadMode != ThreadFree) + throw 1; - bool done = false; - bool pagingActive = false; - while (!done) { - /* Here's how we do it: - Wait for position change - Update manager w/ position - Form delete list - Load tile (if needed) - Add tile to merge list - */ - // Position has already changed or we'll wait for it to do so - osgWaitEvent(locationChangeEvent); - double myLocX,myLocY; - { - osgLockMutex(locationMutex); - myLocX = locX; - myLocY = locY; - osgUnLockMutex(locationMutex); - } + bool done = false; + bool pagingActive = false; + while (!done) { + /* Here's how we do it: + Wait for position change + Update manager w/ position + Form delete list + Load tile (if needed) + Add tile to merge list + */ + // Position has already changed or we'll wait for it to do so + osgWaitEvent(locationChangeEvent); + double myLocX,myLocY; + { + osgLockMutex(locationMutex); + myLocX = locX; + myLocY = locY; + osgUnLockMutex(locationMutex); + } - // Pass the new location on to page manager - trpg2dPoint loc; - loc.x = myLocX; - loc.y = myLocY; - if (pageManage->SetLocation(loc) || pagingActive) { - // If there were changes, process them - // Form the delete list first - trpgManagedTile *tile=NULL; - std::vector unhook; - while ((tile = pageManage->GetNextUnload())) { - unhook.push_back((Group *)tile->GetLocalData()); - pageManage->AckUnload(); - } + // Pass the new location on to page manager + trpg2dPoint loc; + loc.x = myLocX; + loc.y = myLocY; + if (pageManage->SetLocation(loc) || pagingActive) { + // If there were changes, process them + // Form the delete list first + trpgManagedTile *tile=NULL; + std::vector unhook; + while ((tile = pageManage->GetNextUnload())) { + unhook.push_back((Group *)(tile->GetLocalData())); + pageManage->AckUnload(); + } - // Tell the main thread to unhook tiles - // and get the groups to delete as well. - std::vector nextDelete; - { - osgLockMutex(changeListMutex); - // Add to the unhook list - for (unsigned int ti=0;ti nextDelete; + { + osgLockMutex(changeListMutex); + // Add to the unhook list + for (unsigned int ti=0;tiunref(); +#ifdef USE_THREADLOOP_DELETE + // Unreference whatever we're supposed to delete + for (unsigned int gi=0;giunref(); +#endif - // Now do a single load - tile = pageManage->GetNextLoad(); - osg::Group *tileGroup=NULL; - pagingActive = false; - if (tile) { - osg::Group *parentNode = NULL; - tileGroup = archive->LoadTile(NULL,pageManage,tile,&parentNode); - pageManage->AckLoad(); - if (tileGroup) { - // Make an extra reference to it because we want it back for deletion - tileGroup->ref(); - } else { - int x,y,lod; - tile->GetTileLoc(x,y,lod); - fprintf(stderr,"Failed to load tile (%d,%d,%d)\n",x,y,lod); - } + // Now do a single load + tile = pageManage->GetNextLoad(); + osg::Group *tileGroup=NULL; + pagingActive = false; + if (tile) { + osg::Group *parentNode = NULL; + tileGroup = archive->LoadTile(NULL,pageManage,tile,&parentNode); + pageManage->AckLoad(); + if (tileGroup) { +#ifdef USE_THREADLOOP_DELETE + // Make an extra reference to it because we want it back for deletion + // RO, commenting out as we don't want to do delete here, we want it to happen in the merge thread. + tileGroup->ref(); +#endif + } else { + int x,y,lod; + tile->GetTileLoc(x,y,lod); + fprintf(stderr,"Failed to load tile (%d,%d,%d)\n",x,y,lod); + } - // Now add this tile to the merge list - if (tileGroup) { - osgLockMutex(changeListMutex); - toMerge.push_back(tileGroup); - toMergeParent.push_back(parentNode); - osgUnLockMutex(changeListMutex); - } - // We're not necessarily done paging, we're just handing control back - // It's likely we'll be back here - pagingActive = true; - } - } - } + // Now add this tile to the merge list + if (tileGroup) { + osgLockMutex(changeListMutex); + toMerge.push_back(tileGroup); + toMergeParent.push_back(parentNode); + osgUnLockMutex(changeListMutex); + } + // We're not necessarily done paging, we're just handing control back + // It's likely we'll be back here + pagingActive = true; + } + } + } - return true; + return true; } /* Update Position Thread - This method updates the location for the paging thread to use. - --- Main Thread --- + This method updates the location for the paging thread to use. + --- Main Thread --- */ void OSGPageManager::UpdatePositionThread(double inLocX,double inLocY) { - // Update the position - { - // Get the location mutex - osgLockMutex(locationMutex); - positionValid = true; - locX = inLocX-originX; - locY = inLocY-originY; - osgUnLockMutex(locationMutex); - } + // Update the position + { + // Get the location mutex + osgLockMutex(locationMutex); + positionValid = true; + locX = inLocX-originX; + locY = inLocY-originY; + osgUnLockMutex(locationMutex); + } - // Notify the paging thread there's been a position update - osgSetEvent(locationChangeEvent); + // Notify the paging thread there's been a position update + osgSetEvent(locationChangeEvent); } /* Merge Updates - Merge in the new tiles and unhook the ones we'll be deleting. - Actually, we'll hand these back to the paging thread for deletion. - --- Main Thread --- + Merge in the new tiles and unhook the ones we'll be deleting. + Actually, we'll hand these back to the paging thread for deletion. + --- Main Thread --- */ bool OSGPageManager::MergeUpdateThread(osg::Group *rootNode) { - std::vector mergeList; - std::vector mergeParentList; - std::vector unhookList; + std::vector mergeList; + std::vector mergeParentList; + std::vector unhookList; - // Make local copies of the merge and unhook lists - { - osgLockMutex(changeListMutex); - mergeList = toMerge; - mergeParentList = toMergeParent; - unhookList = toUnhook; + // Make local copies of the merge and unhook lists + { + osgLockMutex(changeListMutex); + mergeList = toMerge; + mergeParentList = toMergeParent; + unhookList = toUnhook; - toMerge.clear(); - toMergeParent.clear(); - toUnhook.clear(); - osgUnLockMutex(changeListMutex); - } + toMerge.clear(); + toMergeParent.clear(); + toUnhook.clear(); + osgUnLockMutex(changeListMutex); + } - // Do the unhooking first - for (unsigned int ui=0;uigetParents(); - for (unsigned int pi=0;piremoveChild(unhookMe); - } - } + // Look for its parent(s) + // Only expecting one, but it doesn't matter + const osg::Node::ParentList &parents = unhookMe->getParents(); + for (unsigned int pi=0;piremoveChild(unhookMe); + } + } + +#ifdef USE_THREADLOOP_DELETE + // Put the unhooked things on the list to delete + { + osgLockMutex(changeListMutex); + for (unsigned int di=0;diaddChild(mergeMe); + else + rootNode->addChild(mergeMe); + } + } - // Do the merging last - { - for (unsigned int mi=0;miaddChild(mergeMe); - else - rootNode->addChild(mergeMe); - } - } - - return true; + return true; } diff --git a/src/osgPlugins/txp/trPagePageManager.h b/src/osgPlugins/txp/trPagePageManager.h index 7325de1d2..29e87adbf 100644 --- a/src/osgPlugins/txp/trPagePageManager.h +++ b/src/osgPlugins/txp/trPagePageManager.h @@ -35,6 +35,10 @@ #include "WaitBlock.h" #include "TrPageArchive.h" +// Dec 2002, Robert Osfield -> comment out now, as we actually do want to delete in the main thread +// as the deletion of the display and texture object isn't thread safe. +// #define USE_THREADLOOP_DELETE + namespace txp { /* Thread Identifier @@ -136,8 +140,11 @@ namespace txp std::vector toMergeParent; // Unhook list is filled in by the paging thread std::vector toUnhook; + +#ifdef USE_THREADLOOP_DELETE // Main thread moves groups to the delete list as soon as they are unhooked std::vector toDelete; +#endif }; };