From c2fbeae724d13074ddf0ff254922f2728a67f4d3 Mon Sep 17 00:00:00 2001 From: Nicky Date: Mon, 10 Mar 2014 00:00:49 +0100 Subject: [PATCH 1/3] FIRE-12968/FIRE-12929; Fix crash for AMD driver and random rendering artifacts. --- indra/llrender/llshadermgr.cpp | 5 +++++ .../app_settings/shaders/class1/avatar/objectSkinV.glsl | 9 +++++++++ 2 files changed, 14 insertions(+) diff --git a/indra/llrender/llshadermgr.cpp b/indra/llrender/llshadermgr.cpp index a565570db8..a83a131ba3 100755 --- a/indra/llrender/llshadermgr.cpp +++ b/indra/llrender/llshadermgr.cpp @@ -684,6 +684,11 @@ GLhandleARB LLShaderMgr::loadShaderFile(const std::string& filename, S32 & shade } } + // add define for ATI/AMD so we can do some special ifdef magic in shaders. + if( gGLManager.mIsATI ) + text[ count++ ] = strdup( "#define ND_IS_AMD_CARD 1\n" ); + // + if (texture_index_channels > 0 && type == GL_FRAGMENT_SHADER_ARB) { //use specified number of texture channels for indexed texture rendering diff --git a/indra/newview/app_settings/shaders/class1/avatar/objectSkinV.glsl b/indra/newview/app_settings/shaders/class1/avatar/objectSkinV.glsl index 57129c3bd1..ca1a46b4e3 100755 --- a/indra/newview/app_settings/shaders/class1/avatar/objectSkinV.glsl +++ b/indra/newview/app_settings/shaders/class1/avatar/objectSkinV.glsl @@ -63,5 +63,14 @@ mat4 getObjectSkinnedTransform() ret[3] = vec4(trans, 1.0); return ret; + +#ifdef ND_IS_AMD_CARD + // If it's AMD make sure the GLSL compiler sees the arrays referenced once by static index. Otherwise it seems to optimise the storage awawy which leads to unfun crashes and artifacts. + mat3 dummy1 = matrixPalette[0]; + vec3 dummy2 = translationPalette[0]; + mat3 dummy3 = matrixPalette[51]; + vec3 dummy4 = translationPalette[51]; +#endif + } From 8c9146347659efe0f9eea7935b6f1f430ad16755 Mon Sep 17 00:00:00 2001 From: Nicky Date: Mon, 10 Mar 2014 00:45:41 +0100 Subject: [PATCH 2/3] FIRE-12968; Even though the root cause is likely the shader memory overrun, preseed the VBO pools to take some strain from the driver. Remove the LLStrider changes while at it, those are not used in the Havok version and have not triggered anything in the OS version since month. --- indra/llcommon/llstrider.h | 120 +-------- indra/llrender/llrender.cpp | 6 - indra/llrender/llvertexbuffer.cpp | 328 ++++++++++-------------- indra/llrender/llvertexbuffer.h | 22 +- indra/newview/llfloaterimagepreview.cpp | 6 - indra/newview/llvopartgroup.cpp | 3 +- 6 files changed, 153 insertions(+), 332 deletions(-) diff --git a/indra/llcommon/llstrider.h b/indra/llcommon/llstrider.h index 9a71523afb..653b133c61 100755 --- a/indra/llcommon/llstrider.h +++ b/indra/llcommon/llstrider.h @@ -37,17 +37,13 @@ template class LLStrider }; U32 mSkip; public: + + LLStrider() { mObjectp = NULL; mSkip = sizeof(Object); } ~LLStrider() { } + const LLStrider& operator = (Object *first) { mObjectp = first; return *this;} void setStride (S32 skipBytes) { mSkip = (skipBytes ? skipBytes : sizeof(Object));} - void skip(const U32 index) { mBytep += mSkip*index;} - U32 getSkip() const { return mSkip; } - -#ifndef OPENSIM // protect against buffer overflows, but only for non HAvok builds. Otherwise changing the object size plays really foul when used in the binary Havok blob - LLStrider() { mObjectp = NULL; mSkip = sizeof(Object); } - const LLStrider& operator = (Object *first) { mObjectp = first; return *this;} - LLStrider operator+(const S32& index) { LLStrider ret; @@ -56,111 +52,15 @@ public: return ret; } + void skip(const U32 index) { mBytep += mSkip*index;} + U32 getSkip() const { return mSkip; } Object* get() { return mObjectp; } - Object const* get() const { return mObjectp; } // - Object* operator->() { return mObjectp; } - Object& operator *() { return *mObjectp; } - Object* operator ++(int) { Object* old = mObjectp; mBytep += mSkip; return old; } - Object* operator +=(int i) { mBytep += mSkip*i; return mObjectp; } + Object* operator->() { return mObjectp; } + Object& operator *() { return *mObjectp; } + Object* operator ++(int) { Object* old = mObjectp; mBytep += mSkip; return old; } + Object* operator +=(int i) { mBytep += mSkip*i; return mObjectp; } - Object& operator[](U32 index) { return *(Object*)(mBytep + (mSkip * index)); } -#else - LLStrider() { mObjectp = NULL; mSkip = sizeof(Object); mBufferEnd = 0; } - const LLStrider& operator = (Object *first) { mObjectp = first; mBufferEnd = 0; return *this;} - - LLStrider operator+(const S32& index) - { - LLStrider ret; - ret.mBytep = mBytep + mSkip*index; - ret.mSkip = mSkip; - ret.mBufferEnd = mBufferEnd; - - return ret; - } - - Object* get() - { - if( !assertValid( mBytep ) ) - return &mDummy; - - return mObjectp; - } - - Object const* get() const - { - return mObjectp; - } - - Object* operator->() - { - if( !assertValid( mBytep ) ) - return &mDummy; - - return mObjectp; - } - - Object& operator *() - { - if( !assertValid( mBytep ) ) - return mDummy; - - return *mObjectp; - } - - Object* operator ++(int) - { - Object* old = mObjectp; - mBytep += mSkip; - - if( !assertValid( (U8*)old ) ) - return &mDummy; - - return old; - } - - Object* operator +=(int i) - { - mBytep += mSkip*i; - assertValid( mBytep ); - return mObjectp; - } - - Object& operator[](U32 index) - { - if( !assertValid( mBytep + mSkip*index ) ) - return mDummy; - - return *(Object*)(mBytep + (mSkip * index)); - } - - void setCount( U32 aCount ) - { - mBufferEnd = mBytep + mSkip*aCount; -#if LL_RELEASE_WITH_DEBUG_INFO || LL_DEBUG - mCount = aCount; -#endif - } - - bool assertValid( U8 const *aBuffer ) - { - if( !aBuffer || !mBufferEnd ) - return true; - if( aBuffer < mBufferEnd ) - return true; - - llerrs << "Vertex buffer access beyond end of VBO" << llendl; - return false; - } - -private: - U8 *mBufferEnd; - -#if LL_RELEASE_WITH_DEBUG_INFO || LL_DEBUG - U32 mCount; -#endif - - Object mDummy; -#endif + Object& operator[](U32 index) { return *(Object*)(mBytep + (mSkip * index)); } }; #endif // LL_LLSTRIDER_H diff --git a/indra/llrender/llrender.cpp b/indra/llrender/llrender.cpp index 45b651f1cd..aaac8fb65b 100755 --- a/indra/llrender/llrender.cpp +++ b/indra/llrender/llrender.cpp @@ -1927,12 +1927,6 @@ void LLRender::flush() mBuffer->getVertexStrider(mVerticesp, 0, count); mBuffer->getTexCoord0Strider(mTexcoordsp, 0, count); mBuffer->getColorStrider(mColorsp, 0, count); - - #ifdef OPENSIM // protect against buffer overflows - mVerticesp.setCount( mBuffer->getNumVerts() ); - mTexcoordsp.setCount( mBuffer->getNumVerts() ); - mColorsp.setCount( mBuffer->getNumVerts() ); - #endif // } mBuffer->flush(); diff --git a/indra/llrender/llvertexbuffer.cpp b/indra/llrender/llvertexbuffer.cpp index ebe664c529..e0efac3d9e 100755 --- a/indra/llrender/llvertexbuffer.cpp +++ b/indra/llrender/llvertexbuffer.cpp @@ -147,8 +147,8 @@ void LLVBOPool::deleteBuffer(U32 name) LLVBOPool::LLVBOPool(U32 vboUsage, U32 vboType) : mUsage(vboUsage), mType(vboType) { - // mMissCount.resize(LL_VBO_POOL_SEED_COUNT); - // std::fill(mMissCount.begin(), mMissCount.end(), 0); + mMissCount.resize(LL_VBO_POOL_SEED_COUNT); + std::fill(mMissCount.begin(), mMissCount.end(), 0); } volatile U8* LLVBOPool::allocate(U32& name, U32 size, bool for_seed) @@ -157,112 +157,85 @@ volatile U8* LLVBOPool::allocate(U32& name, U32 size, bool for_seed) volatile U8* ret = NULL; - // We're not using the free list + U32 i = vbo_block_index(size); - // U32 i = vbo_block_index(size); - // - // if (mFreeList.size() <= i) - // { - // mFreeList.resize(i+1); - // } - // - // if (mFreeList[i].empty() || for_seed) - // { - // //make a new buffer - // name = genBuffer(); - // - // glBindBufferARB(mType, name); - // - // if (!for_seed && i < LL_VBO_POOL_SEED_COUNT) - // { //record this miss - // mMissCount[i]++; - // } - // - // if (mType == GL_ARRAY_BUFFER_ARB) - // { - // LLVertexBuffer::sAllocatedBytes += size; - // } - // else - // { - // LLVertexBuffer::sAllocatedIndexBytes += size; - // } - // - // if (LLVertexBuffer::sDisableVBOMapping || mUsage != GL_DYNAMIC_DRAW_ARB) - // { - // glBufferDataARB(mType, size, 0, mUsage); - // ret = (U8*) ll_aligned_malloc_16(size); - // } - // else - // { //always use a true hint of static draw when allocating non-client-backed buffers - // glBufferDataARB(mType, size, 0, GL_STATIC_DRAW_ARB); - // } - // - // glBindBufferARB(mType, 0); - // - // if (for_seed) - // { //put into pool for future use - // llassert(mFreeList.size() > i); - // - // Record rec; - // rec.mGLName = name; - // rec.mClientData = ret; - // - // if (mType == GL_ARRAY_BUFFER_ARB) - // { - // sBytesPooled += size; - // } - // else - // { - // sIndexBytesPooled += size; - // } - // mFreeList[i].push_back(rec); - // } - // } - // else - // { - // name = mFreeList[i].front().mGLName; - // ret = mFreeList[i].front().mClientData; - // - // if (mType == GL_ARRAY_BUFFER_ARB) - // { - // sBytesPooled -= size; - // } - // else - // { - // sIndexBytesPooled -= size; - // } - // - // mFreeList[i].pop_front(); - // } - - - name = genBuffer(); - - glBindBufferARB(mType, name); - - if (mType == GL_ARRAY_BUFFER_ARB) - LLVertexBuffer::sAllocatedBytes += size; - else - LLVertexBuffer::sAllocatedIndexBytes += size; - - if (LLVertexBuffer::sDisableVBOMapping || mUsage != GL_DYNAMIC_DRAW_ARB) + if (mFreeList.size() <= i) { - glBufferDataARB(mType, size, 0, mUsage); - if (mUsage != GL_DYNAMIC_COPY_ARB) - { //data will be provided by application - ret = (U8*) ll_aligned_malloc(size, 64); + mFreeList.resize(i+1); + } + + if (mFreeList[i].empty() || for_seed) + { + //make a new buffer + name = genBuffer(); + + glBindBufferARB(mType, name); + + if (!for_seed && i < LL_VBO_POOL_SEED_COUNT) + { //record this miss + mMissCount[i]++; + } + + if (mType == GL_ARRAY_BUFFER_ARB) + { + LLVertexBuffer::sAllocatedBytes += size; + } + else + { + LLVertexBuffer::sAllocatedIndexBytes += size; + } + + if (LLVertexBuffer::sDisableVBOMapping || mUsage != GL_DYNAMIC_DRAW_ARB) + { + glBufferDataARB(mType, size, 0, mUsage); + if (mUsage != GL_DYNAMIC_COPY_ARB) + { //data will be provided by application + ret = (U8*) ll_aligned_malloc(size, 64); + } + } + else + { //always use a true hint of static draw when allocating non-client-backed buffers + glBufferDataARB(mType, size, 0, GL_STATIC_DRAW_ARB); + } + + glBindBufferARB(mType, 0); + + if (for_seed) + { //put into pool for future use + llassert(mFreeList.size() > i); + + Record rec; + rec.mGLName = name; + rec.mClientData = ret; + + if (mType == GL_ARRAY_BUFFER_ARB) + { + sBytesPooled += size; + } + else + { + sIndexBytesPooled += size; + } + mFreeList[i].push_back(rec); } } else { - //always use a true hint of static draw when allocating non-client-backed buffers - glBufferDataARB(mType, size, 0, GL_STATIC_DRAW_ARB); + name = mFreeList[i].front().mGLName; + ret = mFreeList[i].front().mClientData; + + if (mType == GL_ARRAY_BUFFER_ARB) + { + sBytesPooled -= size; + } + else + { + sIndexBytesPooled -= size; + } + + mFreeList[i].pop_front(); } - glBindBufferARB(mType, 0); - - // - return ret; } @@ -285,74 +258,68 @@ void LLVBOPool::release(U32 name, volatile U8* buffer, U32 size) void LLVBOPool::seedPool() { - // We're not using the freelist + U32 dummy_name = 0; - // U32 dummy_name = 0; - // - // if (mFreeList.size() < LL_VBO_POOL_SEED_COUNT) - // { - // mFreeList.resize(LL_VBO_POOL_SEED_COUNT); - // } - // - // for (U32 i = 0; i < LL_VBO_POOL_SEED_COUNT; i++) - // { - // if (mMissCount[i] > mFreeList[i].size()) - // { - // U32 size = i*LL_VBO_BLOCK_SIZE; - // - // S32 count = mMissCount[i] - mFreeList[i].size(); - // for (U32 j = 0; j < count; ++j) - // { - // allocate(dummy_name, size, true); - // } - // } - // } + if (mFreeList.size() < LL_VBO_POOL_SEED_COUNT) + { + mFreeList.resize(LL_VBO_POOL_SEED_COUNT); + } - // + for (U32 i = 0; i < LL_VBO_POOL_SEED_COUNT; i++) + { + if (mMissCount[i] > mFreeList[i].size()) + { + U32 size = i*LL_VBO_BLOCK_SIZE; + + S32 count = mMissCount[i] - mFreeList[i].size(); + for (U32 j = 0; j < count; ++j) + { + allocate(dummy_name, size, true); + } + } + } } - - void LLVBOPool::cleanup() { - // U32 size = LL_VBO_BLOCK_SIZE; - // - // for (U32 i = 0; i < mFreeList.size(); ++i) - // { - // record_list_t& l = mFreeList[i]; - // - // while (!l.empty()) - // { - // Record& r = l.front(); - // - // deleteBuffer(r.mGLName); - // - // if (r.mClientData) - // { - // ll_aligned_free((void*) r.mClientData); - // } - // - // l.pop_front(); - // - // if (mType == GL_ARRAY_BUFFER_ARB) - // { - // sBytesPooled -= size; - // LLVertexBuffer::sAllocatedBytes -= size; - // } - // else - // { - // sIndexBytesPooled -= size; - // LLVertexBuffer::sAllocatedIndexBytes -= size; - // } - // } - // - // size += LL_VBO_BLOCK_SIZE; - // } - // - // //reset miss counts - // std::fill(mMissCount.begin(), mMissCount.end(), 0); + U32 size = LL_VBO_BLOCK_SIZE; + + for (U32 i = 0; i < mFreeList.size(); ++i) + { + record_list_t& l = mFreeList[i]; + + while (!l.empty()) + { + Record& r = l.front(); + + deleteBuffer(r.mGLName); + + if (r.mClientData) + { + ll_aligned_free((void*) r.mClientData); + } + + l.pop_front(); + + if (mType == GL_ARRAY_BUFFER_ARB) + { + sBytesPooled -= size; + LLVertexBuffer::sAllocatedBytes -= size; + } + else + { + sIndexBytesPooled -= size; + LLVertexBuffer::sAllocatedIndexBytes -= size; + } + } + + size += LL_VBO_BLOCK_SIZE; + } + + //reset miss counts + std::fill(mMissCount.begin(), mMissCount.end(), 0); } @@ -1307,13 +1274,8 @@ void LLVertexBuffer::updateNumVerts(S32 nverts) if (nverts > 65536) { - // FIRE-5077; Just print an info if there are more than 0xFFFF, for now just so there is a message in the logs where in older version #vertices would have been capped. - - // llwarns << "Vertex buffer overflow!" << llendl; - // nverts = 65536; - llinfos << "More vertices than 65536 (#" << nverts << ")" < + llwarns << "Vertex buffer overflow!" << llendl; + nverts = 65536; } U32 needed_size = calcOffsets(mTypeMask, mOffsets, nverts); @@ -1348,21 +1310,11 @@ void LLVertexBuffer::allocateBuffer(S32 nverts, S32 nindices, bool create) { stop_glerror(); - // FIRE-5077; Just print an info if there are more than 0xFFFF, for now just so there is a message in the logs where in older version #vertices would have been capped. - - // if (nverts < 0 || nindices < 0 || - // nverts > 65536) - // { - // llerrs << "Bad vertex buffer allocation: " << nverts << " : " << nindices << llendl; - // } - - if( nverts < 0 || nindices < 0 ) + if (nverts < 0 || nindices < 0 || + nverts > 65536) + { llerrs << "Bad vertex buffer allocation: " << nverts << " : " << nindices << llendl; - - if( nverts > 0xFFFF ) - llinfos << "More vertices than 65535 (#" << nverts << ")" < + } updateNumVerts(nverts); updateNumIndices(nindices); @@ -2087,13 +2039,6 @@ template struct VertexBufferStrider strider = (T*)ptr; strider.setStride(0); - - #ifdef OPENSIM // protect against buffer overflows - if( count == -1 ) - count = vbo.getNumIndices()-index; - strider.setCount( count ); - #endif // - return true; } else if (vbo.hasDataType(type)) @@ -2110,13 +2055,6 @@ template struct VertexBufferStrider strider = (T*)ptr; strider.setStride(stride); - - #ifdef OPENSIM // protect against buffer overflows - if( count == -1 ) - count = vbo.getNumVerts()-index; - strider.setCount( count ); - #endif // - return true; } else diff --git a/indra/llrender/llvertexbuffer.h b/indra/llrender/llvertexbuffer.h index df62e02119..c065a86135 100755 --- a/indra/llrender/llvertexbuffer.h +++ b/indra/llrender/llvertexbuffer.h @@ -77,21 +77,17 @@ public: U32 genBuffer(); void deleteBuffer(U32 name); + class Record + { + public: + U32 mGLName; + volatile U8* mClientData; + }; - // We're not using any of this + typedef std::list record_list_t; + std::vector mFreeList; + std::vector mMissCount; - // class Record - // { - // public: - // U32 mGLName; - // volatile U8* mClientData; - // }; - // - // typedef std::list record_list_t; - // std::vector mFreeList; - // std::vector mMissCount; - - // }; diff --git a/indra/newview/llfloaterimagepreview.cpp b/indra/newview/llfloaterimagepreview.cpp index 2a2512254f..64b1c1fc2c 100755 --- a/indra/newview/llfloaterimagepreview.cpp +++ b/indra/newview/llfloaterimagepreview.cpp @@ -835,12 +835,6 @@ void LLImagePreviewSculpted::setPreviewTarget(LLImageRaw* imagep, F32 distance) LLStrider tc; tc = (LLVector2*) vf.mTexCoords; tc.setStride(8); - #ifdef OPENSIM // protect against buffer overflows - pos.setCount( vf.mNumVertices ); - norm.setCount( vf.mNumVertices ); - tc.setCount( vf.mNumVertices ); - #endif // - for (U32 i = 0; i < num_vertices; i++) { *(vertex_strider++) = *pos++; diff --git a/indra/newview/llvopartgroup.cpp b/indra/newview/llvopartgroup.cpp index 0f4a8ba484..f3b87a7d05 100755 --- a/indra/newview/llvopartgroup.cpp +++ b/indra/newview/llvopartgroup.cpp @@ -896,8 +896,7 @@ void LLParticlePartition::getGeometry(LLSpatialGroup* group) BOOL has_glow = FALSE; - // if (cur_glow.get() != start_glow) - if (const_cast< LLStrider const&>(cur_glow).get() != start_glow) + if (cur_glow.get() != start_glow) { has_glow = TRUE; } From e1af334f5f6d5a874266a2bdb8ce524461ca4321 Mon Sep 17 00:00:00 2001 From: Nicky Date: Mon, 10 Mar 2014 00:49:14 +0100 Subject: [PATCH 3/3] Do proper bounds checking for maximum joints. --- indra/newview/lldrawpoolavatar.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/indra/newview/lldrawpoolavatar.cpp b/indra/newview/lldrawpoolavatar.cpp index b94f3e7d97..79d9c2adc6 100755 --- a/indra/newview/lldrawpoolavatar.cpp +++ b/indra/newview/lldrawpoolavatar.cpp @@ -1720,7 +1720,11 @@ void LLDrawPoolAvatar::updateRiggedFaceVertexBuffer(LLVOAvatar* avatar, LLFace* { F32 w = weight[j][k]; - idx[k] = llclamp((S32) floorf(w), 0, 63); + // proper bounds checking, the maximum changed from 64 to 52(JOINT_COUNT). + // idx[k] = llclamp((S32) floorf(w), 0, 63); + idx[k] = llclamp((S32) floorf(w), 0, JOINT_COUNT-1); + // + wght[k] = w - floorf(w); scale += wght[k]; }