From b852a77a79af613601c576d6a65a78cb04eed5ef Mon Sep 17 00:00:00 2001 From: Aleric Inglewood Date: Tue, 21 Jun 2011 02:49:34 +0200 Subject: [PATCH] AIFilePicker related bug fixes. Bug fix in LLPreviewAnim::gotAssetForSave_continued: the if() that tests if the result from the filepicker can be used was accidently negated, mostly causing a crash when cancelling an animation preview download (open animation, File -> Save Texture As..), and canceling the save when a filename is picked. The lifetime of AIFileUpload is actually till the very end of the main(), causing it's member mPicker to be reused. This leads to problems. When someone tries to open a file picker for a file upload of the same time before the previous filepicker called the callback function (ie, when two filepickers are opened at the same time, or when the plugin crashes). With this fix it is possible to open any number of file pickers. Finally, for linux, LLFastTimers was using assembly with gives rather random results on multicore machines. Since AIStateMachine is using this for wait timing, it had a negative effect on how well the file picker worked (the last message wasn't flushed for several seconds). --- indra/llcommon/llfasttimer.cpp | 175 +----------------- indra/llcommon/llfasttimer.h | 1 - indra/newview/llpreviewanim.cpp | 2 +- indra/newview/llviewermenufile.cpp | 26 ++- indra/newview/statemachine/aistatemachine.cpp | 4 +- 5 files changed, 20 insertions(+), 188 deletions(-) diff --git a/indra/llcommon/llfasttimer.cpp b/indra/llcommon/llfasttimer.cpp index d3783c582..f1d7eb305 100644 --- a/indra/llcommon/llfasttimer.cpp +++ b/indra/llcommon/llfasttimer.cpp @@ -29,33 +29,22 @@ * COMPLETENESS OR PERFORMANCE. * $/LicenseInfo$ */ + #include "linden_common.h" #include "llfasttimer.h" - #include "llmemory.h" #include "llprocessor.h" - #if LL_WINDOWS #define WIN32_LEAN_AND_MEAN #include -#include "lltimer.h" -#elif LL_LINUX || LL_SOLARIS -#include -#include -#include "lltimer.h" -#elif LL_DARWIN -#include -#include "lltimer.h" // get_clock_count() -#else -#error "architecture not supported" #endif +#include "lltimer.h" ////////////////////////////////////////////////////////////////////////////// // statics - LLFastTimer::EFastTimerType LLFastTimer::sCurType = LLFastTimer::FTM_OTHER; int LLFastTimer::sCurDepth = 0; U64 LLFastTimer::sStart[LLFastTimer::FTM_MAX_DEPTH]; @@ -70,44 +59,12 @@ S32 LLFastTimer::sLastFrameIndex = -1; int LLFastTimer::sPauseHistory = 0; int LLFastTimer::sResetHistory = 0; -#define USE_RDTSC 0 +U64 LLFastTimer::sClockResolution = calc_clock_frequency(50U); // Resolution of get_clock_count() -#if LL_LINUX || LL_SOLARIS -U64 LLFastTimer::sClockResolution = 1000000000; // 1e9, Nanosecond resolution -#else -U64 LLFastTimer::sClockResolution = 1000000; // 1e6, Microsecond resolution -#endif - - -//static -#if (LL_DARWIN || LL_LINUX || LL_SOLARIS) && !(defined(__i386__) || defined(__amd64__)) U64 LLFastTimer::countsPerSecond() // counts per second for the *32-bit* timer { return sClockResolution >> 8; } -#else // windows or x86-mac or x86-linux or x86-solaris -U64 LLFastTimer::countsPerSecond() // counts per second for the *32-bit* timer -{ -#if USE_RDTSC || !LL_WINDOWS - //getCPUFrequency returns MHz and sCPUClockFrequency wants to be in Hz - static U64 sCPUClockFrequency = U64(LLProcessorInfo().getCPUFrequency()*1000000.0); - - // we drop the low-order byte in our timers, so report a lower frequency -#else - // If we're not using RDTSC, each fasttimer tick is just a performance counter tick. - // Not redefining the clock frequency itself (in llprocessor.cpp/calculate_cpu_frequency()) - // since that would change displayed MHz stats for CPUs - static bool firstcall = true; - static U64 sCPUClockFrequency; - if (firstcall) - { - QueryPerformanceFrequency((LARGE_INTEGER*)&sCPUClockFrequency); - firstcall = false; - } -#endif - return sCPUClockFrequency >> 8; -} -#endif void LLFastTimer::reset() { @@ -162,139 +119,17 @@ void LLFastTimer::reset() // Important note: These implementations must be FAST! // - -#if LL_WINDOWS -// -// Windows implementation of CPU clock -// - -// -// NOTE: put back in when we aren't using platform sdk anymore -// -// because MS has different signatures for these functions in winnt.h -// need to rename them to avoid conflicts -//#define _interlockedbittestandset _renamed_interlockedbittestandset -//#define _interlockedbittestandreset _renamed_interlockedbittestandreset -//#include -//#undef _interlockedbittestandset -//#undef _interlockedbittestandreset - -//inline U32 LLFastTimer::getCPUClockCount32() -//{ -// U64 time_stamp = __rdtsc(); -// return (U32)(time_stamp >> 8); -//} -// -//// return full timer value, *not* shifted by 8 bits -//inline U64 LLFastTimer::getCPUClockCount64() -//{ -// return __rdtsc(); -//} - // shift off lower 8 bits for lower resolution but longer term timing // on 1Ghz machine, a 32-bit word will hold ~1000 seconds of timing -#if USE_RDTSC -U32 LLFastTimer::getCPUClockCount32() -{ - U32 ret_val; - __asm - { - _emit 0x0f - _emit 0x31 - shr eax,8 - shl edx,24 - or eax, edx - mov dword ptr [ret_val], eax - } - return ret_val; -} -// return full timer value, *not* shifted by 8 bits -U64 LLFastTimer::getCPUClockCount64() -{ - U64 ret_val; - __asm - { - _emit 0x0f - _emit 0x31 - mov eax,eax - mov edx,edx - mov dword ptr [ret_val+4], edx - mov dword ptr [ret_val], eax - } - return ret_val; -} - -std::string LLFastTimer::sClockType = "rdtsc"; - -#else //LL_COMMON_API U64 get_clock_count(); // in lltimer.cpp -// These use QueryPerformanceCounter, which is arguably fine and also works on amd architectures. +// On windows these use QueryPerformanceCounter, which is arguably fine and also works on amd architectures. U32 LLFastTimer::getCPUClockCount32() { - return (U32)(get_clock_count()>>8); + return get_clock_count() >> 8; } U64 LLFastTimer::getCPUClockCount64() { return get_clock_count(); } - -std::string LLFastTimer::sClockType = "QueryPerformanceCounter"; -#endif - -#endif - - -#if (LL_LINUX || LL_SOLARIS) && !(defined(__i386__) || defined(__amd64__)) -// -// Linux and Solaris implementation of CPU clock - non-x86. -// This is accurate but SLOW! Only use out of desperation. -// -// Try to use the MONOTONIC clock if available, this is a constant time counter -// with nanosecond resolution (but not necessarily accuracy) and attempts are -// made to synchronize this value between cores at kernel start. It should not -// be affected by CPU frequency. If not available use the REALTIME clock, but -// this may be affected by NTP adjustments or other user activity affecting -// the system time. -U64 LLFastTimer::getCPUClockCount64() -{ - struct timespec tp; - -#ifdef CLOCK_MONOTONIC // MONOTONIC supported at build-time? - if (-1 == clock_gettime(CLOCK_MONOTONIC,&tp)) // if MONOTONIC isn't supported at runtime then ouch, try REALTIME -#endif - clock_gettime(CLOCK_REALTIME,&tp); - - return (tp.tv_sec*LLFastTimer::sClockResolution)+tp.tv_nsec; -} - -U32 LLFastTimer::getCPUClockCount32() -{ - return (U32)(LLFastTimer::getCPUClockCount64() >> 8); -} - -std::string LLFastTimer::sClockType = "clock_gettime"; - -#endif // (LL_LINUX || LL_SOLARIS) && !(defined(__i386__) || defined(__amd64__)) - - -#if (LL_LINUX || LL_SOLARIS || LL_DARWIN) && (defined(__i386__) || defined(__amd64__)) -// -// Mac+Linux+Solaris FAST x86 implementation of CPU clock -U32 LLFastTimer::getCPUClockCount32() -{ - U64 x; - __asm__ volatile (".byte 0x0f, 0x31": "=A"(x)); - return (U32)(x >> 8); -} - -U64 LLFastTimer::getCPUClockCount64() -{ - U64 x; - __asm__ volatile (".byte 0x0f, 0x31": "=A"(x)); - return x; -} - -std::string LLFastTimer::sClockType = "rdtsc"; -#endif \ No newline at end of file diff --git a/indra/llcommon/llfasttimer.h b/indra/llcommon/llfasttimer.h index 3a7f30511..5b34d361c 100644 --- a/indra/llcommon/llfasttimer.h +++ b/indra/llcommon/llfasttimer.h @@ -256,7 +256,6 @@ public: static void reset(); static U64 countsPerSecond(); - static std::string sClockType; public: static int sCurDepth; static U64 sStart[FTM_MAX_DEPTH]; diff --git a/indra/newview/llpreviewanim.cpp b/indra/newview/llpreviewanim.cpp index 845129a38..6963f6a84 100644 --- a/indra/newview/llpreviewanim.cpp +++ b/indra/newview/llpreviewanim.cpp @@ -400,7 +400,7 @@ void LLPreviewAnim::gotAssetForSave(LLVFS *vfs, // static void LLPreviewAnim::gotAssetForSave_continued(char* buffer, S32 size, AIFilePicker* filepicker) { - if (!filepicker->hasFilename()) + if (filepicker->hasFilename()) { std::string filename = filepicker->getFilename(); std::ofstream export_file(filename.c_str(), std::ofstream::binary); diff --git a/indra/newview/llviewermenufile.cpp b/indra/newview/llviewermenufile.cpp index d0b6d61c6..3eddf6df5 100644 --- a/indra/newview/llviewermenufile.cpp +++ b/indra/newview/llviewermenufile.cpp @@ -154,16 +154,13 @@ std::string build_extensions_string(ELoadFilter filter) } class AIFileUpload { - protected: - AIFilePicker* mPicker; - public: - AIFileUpload(void) : mPicker(NULL) { } - virtual ~AIFileUpload() { llassert(!mPicker); if (mPicker) { mPicker->abort(); mPicker = NULL; } } + AIFileUpload(void) { } + virtual ~AIFileUpload() { } public: bool is_valid(std::string const& filename, ELoadFilter type); - void filepicker_callback(ELoadFilter type); + void filepicker_callback(ELoadFilter type, AIFilePicker* picker); void start_filepicker(ELoadFilter type, char const* context); protected: @@ -179,21 +176,22 @@ void AIFileUpload::start_filepicker(ELoadFilter filter, char const* context) // display(); } - llassert(!mPicker); - mPicker = AIFilePicker::create(); - mPicker->open(filter, "", context); - mPicker->run(boost::bind(&AIFileUpload::filepicker_callback, this, filter)); + AIFilePicker* picker = AIFilePicker::create(); + picker->open(filter, "", context); + // Note that when the call back is called then we're still in the main loop of + // the viewer and therefore the AIFileUpload still exists, since that is only + // destructed at the end of main when exiting the viewer. + picker->run(boost::bind(&AIFileUpload::filepicker_callback, this, filter, picker)); } -void AIFileUpload::filepicker_callback(ELoadFilter type) +void AIFileUpload::filepicker_callback(ELoadFilter type, AIFilePicker* picker) { - if (mPicker->hasFilename()) + if (picker->hasFilename()) { - std::string filename = mPicker->getFilename(); + std::string filename = picker->getFilename(); if (is_valid(filename, type)) handle_event(filename); } - mPicker = NULL; } bool AIFileUpload::is_valid(std::string const& filename, ELoadFilter type) diff --git a/indra/newview/statemachine/aistatemachine.cpp b/indra/newview/statemachine/aistatemachine.cpp index 2eccfb53e..bdf43fcdc 100644 --- a/indra/newview/statemachine/aistatemachine.cpp +++ b/indra/newview/statemachine/aistatemachine.cpp @@ -79,7 +79,7 @@ AITHREADSAFESIMPLE(U64, AIStateMachine::sMaxCount, ); void AIStateMachine::updateSettings(void) { Dout(dc::statemachine, "Initializing AIStateMachine::sMaxCount"); - *AIAccess(sMaxCount) = LLFastTimer::countsPerSecond() * gSavedSettings.getU32("StateMachineMaxTime") / 1000; + *AIAccess(sMaxCount) = LLFastTimer::sClockResolution * gSavedSettings.getU32("StateMachineMaxTime") / 1000; } //---------------------------------------------------------------------------- @@ -319,7 +319,7 @@ void AIStateMachine::mainloop(void*) if (total_clocks >= max_count) { #ifndef LL_RELEASE_FOR_DOWNLOAD - llwarns << "AIStateMachine::mainloop did run for " << (total_clocks * 1000 / LLFastTimer::countsPerSecond()) << " ms." << llendl; + llwarns << "AIStateMachine::mainloop did run for " << (total_clocks * 1000 / LLFastTimer::sClockResolution) << " ms." << llendl; #endif std::sort(active_statemachines.begin(), active_statemachines.end(), QueueElementComp()); break;