Commit b91d2630 by Jamie Madill Committed by Commit Bot

Speculative fix for assertion failure with samplers.

This was crashing for example here: https://chromium-swarm.appspot.com/task?id=4b174a9710848410 https://chromium-swarm.appspot.com/task?id=4b214baec2ee7e10 With this stack: libglesv2!gl::TextureState::isBoundAsSamplerTexture libglesv2!gl::Texture::onUnbindAsSamplerTexture libglesv2!gl::State::unsetActiveTexture+0x9 libglesv2!gl::State::updateActiveTextureState+0x2d2 libglesv2!gl::State::updateActiveTexture+0x3aa libglesv2!gl::State::setSamplerTexture+0x4a1 libglesv2!gl::Context::bindTexture+0x1ab libglesv2!gl::BindTexture+0x99 chrome!GrGLFunction+0x1f chrome!GrGLGpu::bindTexture+0x1a0 chrome!GrGLProgram::bindTextures+0x1b9 chrome!GrGLOpsRenderPass::onBindTextures+0x50 chrome!GrOpsRenderPass::bindTextures+0x106 chrome!GrOpFlushState::bindTextures+0xf chrome!`anonymous namespace'::FillRectOp::onExecute+0xd3 It's unclear how we could end up with a Texture bound that doesn't go through the normal setter functions. I did see a potential hole where textures might not get an unbind call when a Context is torn down. This could lead to bugs in multi-context situations. This protects the set/unset functions in a helper class to ensure we always call onBind/onUnbind and forces the destructor to call unbind. Bug: angleproject:4490 Change-Id: Ied64e02bbe3a37efcab6cbdd4bf2d1b6dcb8b3ac Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2118254 Commit-Queue: Jamie Madill <jmadill@chromium.org> Reviewed-by: 's avatarCody Northrop <cnorthrop@google.com>
parent e39d055d
...@@ -253,6 +253,56 @@ const angle::PackedEnumMap<BufferBinding, State::BufferBindingSetter> State::kBu ...@@ -253,6 +253,56 @@ const angle::PackedEnumMap<BufferBinding, State::BufferBindingSetter> State::kBu
GetBufferBindingSetter<BufferBinding::Uniform>(), GetBufferBindingSetter<BufferBinding::Uniform>(),
}}; }};
ActiveTexturesCache::ActiveTexturesCache() : mTextures{} {}
ActiveTexturesCache::~ActiveTexturesCache()
{
ASSERT(empty());
}
void ActiveTexturesCache::clear()
{
for (size_t textureIndex = 0; textureIndex < mTextures.size(); ++textureIndex)
{
reset(textureIndex);
}
}
bool ActiveTexturesCache::empty() const
{
for (Texture *texture : mTextures)
{
if (texture)
{
return false;
}
}
return true;
}
ANGLE_INLINE void ActiveTexturesCache::reset(size_t textureIndex)
{
if (mTextures[textureIndex])
{
mTextures[textureIndex]->onUnbindAsSamplerTexture();
mTextures[textureIndex] = nullptr;
}
}
ANGLE_INLINE void ActiveTexturesCache::set(size_t textureIndex, Texture *texture)
{
// We don't call reset() here to avoid setting nullptr before rebind.
if (mTextures[textureIndex])
{
mTextures[textureIndex]->onUnbindAsSamplerTexture();
}
ASSERT(texture);
texture->onBindAsSamplerTexture();
mTextures[textureIndex] = texture;
}
State::State(ContextID contextIn, State::State(ContextID contextIn,
const State *shareContextState, const State *shareContextState,
TextureManager *shareTextures, TextureManager *shareTextures,
...@@ -313,7 +363,6 @@ State::State(ContextID contextIn, ...@@ -313,7 +363,6 @@ State::State(ContextID contextIn,
mProvokingVertex(gl::ProvokingVertexConvention::LastVertexConvention), mProvokingVertex(gl::ProvokingVertexConvention::LastVertexConvention),
mVertexArray(nullptr), mVertexArray(nullptr),
mActiveSampler(0), mActiveSampler(0),
mActiveTexturesCache{},
mTexturesIncompatibleWithSamplers(0), mTexturesIncompatibleWithSamplers(0),
mPrimitiveRestart(false), mPrimitiveRestart(false),
mDebug(debug), mDebug(debug),
...@@ -528,16 +577,9 @@ void State::reset(const Context *context) ...@@ -528,16 +577,9 @@ void State::reset(const Context *context)
UpdateIndexedBufferBinding(context, &buf, nullptr, BufferBinding::ShaderStorage, 0, 0); UpdateIndexedBufferBinding(context, &buf, nullptr, BufferBinding::ShaderStorage, 0, 0);
} }
setAllDirtyBits(); mActiveTexturesCache.clear();
}
ANGLE_INLINE void State::unsetActiveTexture(size_t textureIndex) setAllDirtyBits();
{
if (mActiveTexturesCache[textureIndex])
{
mActiveTexturesCache[textureIndex]->onUnbindAsSamplerTexture();
mActiveTexturesCache[textureIndex] = nullptr;
}
} }
ANGLE_INLINE void State::unsetActiveTextures(ActiveTextureMask textureMask) ANGLE_INLINE void State::unsetActiveTextures(ActiveTextureMask textureMask)
...@@ -545,22 +587,11 @@ ANGLE_INLINE void State::unsetActiveTextures(ActiveTextureMask textureMask) ...@@ -545,22 +587,11 @@ ANGLE_INLINE void State::unsetActiveTextures(ActiveTextureMask textureMask)
// Unset any relevant bound textures. // Unset any relevant bound textures.
for (size_t textureIndex : mProgram->getActiveSamplersMask()) for (size_t textureIndex : mProgram->getActiveSamplersMask())
{ {
unsetActiveTexture(textureIndex); mActiveTexturesCache.reset(textureIndex);
mCompleteTextureBindings[textureIndex].reset(); mCompleteTextureBindings[textureIndex].reset();
} }
} }
ANGLE_INLINE void State::setActiveTexture(size_t textureIndex, Texture *texture)
{
if (mActiveTexturesCache[textureIndex])
{
mActiveTexturesCache[textureIndex]->onUnbindAsSamplerTexture();
}
texture->onBindAsSamplerTexture();
mActiveTexturesCache[textureIndex] = texture;
}
ANGLE_INLINE void State::updateActiveTextureState(const Context *context, ANGLE_INLINE void State::updateActiveTextureState(const Context *context,
size_t textureIndex, size_t textureIndex,
const Sampler *sampler, const Sampler *sampler,
...@@ -568,11 +599,11 @@ ANGLE_INLINE void State::updateActiveTextureState(const Context *context, ...@@ -568,11 +599,11 @@ ANGLE_INLINE void State::updateActiveTextureState(const Context *context,
{ {
if (!texture->isSamplerComplete(context, sampler)) if (!texture->isSamplerComplete(context, sampler))
{ {
unsetActiveTexture(textureIndex); mActiveTexturesCache.reset(textureIndex);
} }
else else
{ {
setActiveTexture(textureIndex, texture); mActiveTexturesCache.set(textureIndex, texture);
if (texture->hasAnyDirtyBit()) if (texture->hasAnyDirtyBit())
{ {
...@@ -612,7 +643,7 @@ ANGLE_INLINE void State::updateActiveTexture(const Context *context, ...@@ -612,7 +643,7 @@ ANGLE_INLINE void State::updateActiveTexture(const Context *context,
if (!texture) if (!texture)
{ {
unsetActiveTexture(textureIndex); mActiveTexturesCache.reset(textureIndex);
mDirtyBits.set(DIRTY_BIT_TEXTURE_BINDINGS); mDirtyBits.set(DIRTY_BIT_TEXTURE_BINDINGS);
return; return;
} }
......
...@@ -63,6 +63,23 @@ using TextureBindingVector = std::vector<BindingPointer<Texture>>; ...@@ -63,6 +63,23 @@ using TextureBindingVector = std::vector<BindingPointer<Texture>>;
using TextureBindingMap = angle::PackedEnumMap<TextureType, TextureBindingVector>; using TextureBindingMap = angle::PackedEnumMap<TextureType, TextureBindingVector>;
using ActiveQueryMap = angle::PackedEnumMap<QueryType, BindingPointer<Query>>; using ActiveQueryMap = angle::PackedEnumMap<QueryType, BindingPointer<Query>>;
class ActiveTexturesCache final : angle::NonCopyable
{
public:
ActiveTexturesCache();
~ActiveTexturesCache();
Texture *operator[](size_t textureIndex) const { return mTextures[textureIndex]; }
void clear();
void set(size_t textureIndex, Texture *texture);
void reset(size_t textureIndex);
bool empty() const;
private:
ActiveTextureArray<Texture *> mTextures;
};
class State : angle::NonCopyable class State : angle::NonCopyable
{ {
public: public:
...@@ -661,7 +678,7 @@ class State : angle::NonCopyable ...@@ -661,7 +678,7 @@ class State : angle::NonCopyable
GLenum format); GLenum format);
const ImageUnit &getImageUnit(size_t unit) const { return mImageUnits[unit]; } const ImageUnit &getImageUnit(size_t unit) const { return mImageUnits[unit]; }
const ActiveTexturePointerArray &getActiveTexturesCache() const { return mActiveTexturesCache; } const ActiveTexturesCache &getActiveTexturesCache() const { return mActiveTexturesCache; }
ComponentTypeMask getCurrentValuesTypeMask() const { return mCurrentValuesTypeMask; } ComponentTypeMask getCurrentValuesTypeMask() const { return mCurrentValuesTypeMask; }
// "onActiveTextureChange" is called when a texture binding changes. // "onActiveTextureChange" is called when a texture binding changes.
...@@ -749,9 +766,7 @@ class State : angle::NonCopyable ...@@ -749,9 +766,7 @@ class State : angle::NonCopyable
private: private:
friend class Context; friend class Context;
void unsetActiveTexture(size_t textureIndex);
void unsetActiveTextures(ActiveTextureMask textureMask); void unsetActiveTextures(ActiveTextureMask textureMask);
void setActiveTexture(size_t textureIndex, Texture *texture);
void updateActiveTexture(const Context *context, size_t textureIndex, Texture *texture); void updateActiveTexture(const Context *context, size_t textureIndex, Texture *texture);
void updateActiveTextureState(const Context *context, void updateActiveTextureState(const Context *context,
size_t textureIndex, size_t textureIndex,
...@@ -900,7 +915,7 @@ class State : angle::NonCopyable ...@@ -900,7 +915,7 @@ class State : angle::NonCopyable
// A cache of complete textures. nullptr indicates unbound or incomplete. // A cache of complete textures. nullptr indicates unbound or incomplete.
// Don't use BindingPointer because this cache is only valid within a draw call. // Don't use BindingPointer because this cache is only valid within a draw call.
// Also stores a notification channel to the texture itself to handle texture change events. // Also stores a notification channel to the texture itself to handle texture change events.
ActiveTexturePointerArray mActiveTexturesCache; ActiveTexturesCache mActiveTexturesCache;
std::vector<angle::ObserverBinding> mCompleteTextureBindings; std::vector<angle::ObserverBinding> mCompleteTextureBindings;
ActiveTextureMask mTexturesIncompatibleWithSamplers; ActiveTextureMask mTexturesIncompatibleWithSamplers;
......
...@@ -428,6 +428,7 @@ class Texture final : public RefCountObject<TextureID>, ...@@ -428,6 +428,7 @@ class Texture final : public RefCountObject<TextureID>,
ANGLE_INLINE void onBindAsSamplerTexture() ANGLE_INLINE void onBindAsSamplerTexture()
{ {
ASSERT(mState.mSamplerBindingCount < std::numeric_limits<uint32_t>::max());
mState.mSamplerBindingCount++; mState.mSamplerBindingCount++;
if (mState.mSamplerBindingCount == 1) if (mState.mSamplerBindingCount == 1)
{ {
......
...@@ -487,8 +487,7 @@ using ActiveTextureMask = angle::BitSet<IMPLEMENTATION_MAX_ACTIVE_TEXTURES>; ...@@ -487,8 +487,7 @@ using ActiveTextureMask = angle::BitSet<IMPLEMENTATION_MAX_ACTIVE_TEXTURES>;
template <typename T> template <typename T>
using ActiveTextureArray = std::array<T, IMPLEMENTATION_MAX_ACTIVE_TEXTURES>; using ActiveTextureArray = std::array<T, IMPLEMENTATION_MAX_ACTIVE_TEXTURES>;
using ActiveTexturePointerArray = ActiveTextureArray<Texture *>; using ActiveTextureTypeArray = ActiveTextureArray<TextureType>;
using ActiveTextureTypeArray = ActiveTextureArray<TextureType>;
template <typename T> template <typename T>
using UniformBuffersArray = std::array<T, IMPLEMENTATION_MAX_UNIFORM_BUFFER_BINDINGS>; using UniformBuffersArray = std::array<T, IMPLEMENTATION_MAX_UNIFORM_BUFFER_BINDINGS>;
......
...@@ -2729,7 +2729,7 @@ angle::Result StateManager11::applyTexturesForSRVs(const gl::Context *context, ...@@ -2729,7 +2729,7 @@ angle::Result StateManager11::applyTexturesForSRVs(const gl::Context *context,
ASSERT(!mProgramD3D->isSamplerMappingDirty()); ASSERT(!mProgramD3D->isSamplerMappingDirty());
// TODO(jmadill): Use the Program's sampler bindings. // TODO(jmadill): Use the Program's sampler bindings.
const gl::ActiveTexturePointerArray &completeTextures = glState.getActiveTexturesCache(); const gl::ActiveTexturesCache &completeTextures = glState.getActiveTexturesCache();
const gl::RangeUI samplerRange = mProgramD3D->getUsedSamplerRange(shaderType); const gl::RangeUI samplerRange = mProgramD3D->getUsedSamplerRange(shaderType);
for (unsigned int samplerIndex = samplerRange.low(); samplerIndex < samplerRange.high(); for (unsigned int samplerIndex = samplerRange.low(); samplerIndex < samplerRange.high();
......
...@@ -3184,7 +3184,7 @@ angle::Result Renderer9::applyTextures(const gl::Context *context, gl::ShaderTyp ...@@ -3184,7 +3184,7 @@ angle::Result Renderer9::applyTextures(const gl::Context *context, gl::ShaderTyp
ASSERT(!programD3D->isSamplerMappingDirty()); ASSERT(!programD3D->isSamplerMappingDirty());
// TODO(jmadill): Use the Program's sampler bindings. // TODO(jmadill): Use the Program's sampler bindings.
const gl::ActiveTexturePointerArray &activeTextures = glState.getActiveTexturesCache(); const gl::ActiveTexturesCache &activeTextures = glState.getActiveTexturesCache();
const gl::RangeUI samplerRange = programD3D->getUsedSamplerRange(shaderType); const gl::RangeUI samplerRange = programD3D->getUsedSamplerRange(shaderType);
for (unsigned int samplerIndex = samplerRange.low(); samplerIndex < samplerRange.high(); for (unsigned int samplerIndex = samplerRange.low(); samplerIndex < samplerRange.high();
......
...@@ -817,7 +817,7 @@ void StateManagerGL::updateProgramTextureBindings(const gl::Context *context) ...@@ -817,7 +817,7 @@ void StateManagerGL::updateProgramTextureBindings(const gl::Context *context)
const gl::State &glState = context->getState(); const gl::State &glState = context->getState();
const gl::Program *program = glState.getProgram(); const gl::Program *program = glState.getProgram();
const gl::ActiveTexturePointerArray &textures = glState.getActiveTexturesCache(); const gl::ActiveTexturesCache &textures = glState.getActiveTexturesCache();
const gl::ActiveTextureMask &activeTextures = program->getActiveSamplersMask(); const gl::ActiveTextureMask &activeTextures = program->getActiveSamplersMask();
const gl::ActiveTextureTypeArray &textureTypes = program->getActiveSamplerTypes(); const gl::ActiveTextureTypeArray &textureTypes = program->getActiveSamplerTypes();
......
...@@ -1537,8 +1537,8 @@ angle::Result ContextMtl::handleDirtyActiveTextures(const gl::Context *context) ...@@ -1537,8 +1537,8 @@ angle::Result ContextMtl::handleDirtyActiveTextures(const gl::Context *context)
const gl::State &glState = mState; const gl::State &glState = mState;
const gl::Program *program = glState.getProgram(); const gl::Program *program = glState.getProgram();
const gl::ActiveTexturePointerArray &textures = glState.getActiveTexturesCache(); const gl::ActiveTexturesCache &textures = glState.getActiveTexturesCache();
const gl::ActiveTextureMask &activeTextures = program->getActiveSamplersMask(); const gl::ActiveTextureMask &activeTextures = program->getActiveSamplersMask();
for (size_t textureUnit : activeTextures) for (size_t textureUnit : activeTextures)
{ {
......
...@@ -869,7 +869,7 @@ angle::Result ProgramMtl::updateTextures(const gl::Context *glContext, ...@@ -869,7 +869,7 @@ angle::Result ProgramMtl::updateTextures(const gl::Context *glContext,
{ {
const auto &glState = glContext->getState(); const auto &glState = glContext->getState();
const gl::ActiveTexturePointerArray &completeTextures = glState.getActiveTexturesCache(); const gl::ActiveTexturesCache &completeTextures = glState.getActiveTexturesCache();
for (gl::ShaderType shaderType : gl::AllGLES2ShaderTypes()) for (gl::ShaderType shaderType : gl::AllGLES2ShaderTypes())
{ {
......
...@@ -3512,7 +3512,7 @@ angle::Result ContextVk::updateActiveTextures(const gl::Context *context) ...@@ -3512,7 +3512,7 @@ angle::Result ContextVk::updateActiveTextures(const gl::Context *context)
memset(mActiveTextures.data(), 0, sizeof(mActiveTextures[0]) * prevMaxIndex); memset(mActiveTextures.data(), 0, sizeof(mActiveTextures[0]) * prevMaxIndex);
mActiveTexturesDesc.reset(); mActiveTexturesDesc.reset();
const gl::ActiveTexturePointerArray &textures = glState.getActiveTexturesCache(); const gl::ActiveTexturesCache &textures = glState.getActiveTexturesCache();
const gl::ActiveTextureMask &activeTextures = program->getActiveSamplersMask(); const gl::ActiveTextureMask &activeTextures = program->getActiveSamplersMask();
const gl::ActiveTextureTypeArray &textureTypes = program->getActiveSamplerTypes(); const gl::ActiveTextureTypeArray &textureTypes = program->getActiveSamplerTypes();
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment