Skip to content

A
angle
Commit 5fd08af4 by James Darpinian Committed by Commit Bot
Options

Sampler state overrides texture state if set

The new validation added in http://crbug.com/809237 failed to consider that sampler object state overrides texture object state if a sampler object is bound. State caching makes this complicated to fix. Fixes WebGL conformance test incompatible-texture-type-for-sampler.html https://github.com/KhronosGroup/WebGL/pull/2823 Bug: 940080, 809237 Change-Id: I26b0fb35c5630c36248edae80f0298a0cb7e14b8 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/1522364 Commit-Queue: James Darpinian <jdarpinian@chromium.org> Reviewed-by: 's avatarGeoff Lang <geofflang@chromium.org>
parent f7f15ac2
Showing with 72 additions and 49 deletions
src/libANGLE/Context.cpp
...@@ -1100,6 +1100,7 @@ void Context::bindSampler(GLuint textureUnit, GLuint samplerHandle) ...@@ -1100,6 +1100,7 @@ void Context::bindSampler(GLuint textureUnit, GLuint samplerHandle)
mState.mSamplerManager->checkSamplerAllocation(mImplementation.get(), samplerHandle); mState.mSamplerManager->checkSamplerAllocation(mImplementation.get(), samplerHandle);
mState.setSamplerBinding(this, textureUnit, sampler); mState.setSamplerBinding(this, textureUnit, sampler);
mSamplerObserverBindings[textureUnit].bind(sampler); mSamplerObserverBindings[textureUnit].bind(sampler);
mStateCache.onActiveTextureChange(this);
} }
void Context::bindImageTexture(GLuint unit, void Context::bindImageTexture(GLuint unit,
...@@ -8015,6 +8016,7 @@ void Context::onSubjectStateChange(const Context *context, ...@@ -8015,6 +8016,7 @@ void Context::onSubjectStateChange(const Context *context,
{ {
ASSERT(index < kSamplerMaxSubjectIndex); ASSERT(index < kSamplerMaxSubjectIndex);
mState.setSamplerDirty(index - kSampler0SubjectIndex); mState.setSamplerDirty(index - kSampler0SubjectIndex);
mState.onActiveTextureStateChange(this, index - kSampler0SubjectIndex);
} }
break; break;
} }
......
src/libANGLE/Sampler.cpp
...@@ -36,9 +36,10 @@ const std::string &Sampler::getLabel() const ...@@ -36,9 +36,10 @@ const std::string &Sampler::getLabel() const
return mLabel; return mLabel;
} }
void Sampler::setMinFilter(GLenum minFilter) void Sampler::setMinFilter(const Context *context, GLenum minFilter)
{ {
mState.setMinFilter(minFilter); mState.setMinFilter(minFilter);
onStateChange(context, angle::SubjectMessage::DEPENDENT_DIRTY_BITS);
} }
GLenum Sampler::getMinFilter() const GLenum Sampler::getMinFilter() const
...@@ -46,9 +47,10 @@ GLenum Sampler::getMinFilter() const ...@@ -46,9 +47,10 @@ GLenum Sampler::getMinFilter() const
return mState.getMinFilter(); return mState.getMinFilter();
} }
void Sampler::setMagFilter(GLenum magFilter) void Sampler::setMagFilter(const Context *context, GLenum magFilter)
{ {
mState.setMagFilter(magFilter); mState.setMagFilter(magFilter);
onStateChange(context, angle::SubjectMessage::DEPENDENT_DIRTY_BITS);
} }
GLenum Sampler::getMagFilter() const GLenum Sampler::getMagFilter() const
...@@ -56,9 +58,10 @@ GLenum Sampler::getMagFilter() const ...@@ -56,9 +58,10 @@ GLenum Sampler::getMagFilter() const
return mState.getMagFilter(); return mState.getMagFilter();
} }
void Sampler::setWrapS(GLenum wrapS) void Sampler::setWrapS(const Context *context, GLenum wrapS)
{ {
mState.setWrapS(wrapS); mState.setWrapS(wrapS);
onStateChange(context, angle::SubjectMessage::DEPENDENT_DIRTY_BITS);
} }
GLenum Sampler::getWrapS() const GLenum Sampler::getWrapS() const
...@@ -66,9 +69,10 @@ GLenum Sampler::getWrapS() const ...@@ -66,9 +69,10 @@ GLenum Sampler::getWrapS() const
return mState.getWrapS(); return mState.getWrapS();
} }
void Sampler::setWrapT(GLenum wrapT) void Sampler::setWrapT(const Context *context, GLenum wrapT)
{ {
mState.setWrapT(wrapT); mState.setWrapT(wrapT);
onStateChange(context, angle::SubjectMessage::DEPENDENT_DIRTY_BITS);
} }
GLenum Sampler::getWrapT() const GLenum Sampler::getWrapT() const
...@@ -76,9 +80,10 @@ GLenum Sampler::getWrapT() const ...@@ -76,9 +80,10 @@ GLenum Sampler::getWrapT() const
return mState.getWrapT(); return mState.getWrapT();
} }
void Sampler::setWrapR(GLenum wrapR) void Sampler::setWrapR(const Context *context, GLenum wrapR)
{ {
mState.setWrapR(wrapR); mState.setWrapR(wrapR);
onStateChange(context, angle::SubjectMessage::DEPENDENT_DIRTY_BITS);
} }
GLenum Sampler::getWrapR() const GLenum Sampler::getWrapR() const
...@@ -86,9 +91,10 @@ GLenum Sampler::getWrapR() const ...@@ -86,9 +91,10 @@ GLenum Sampler::getWrapR() const
return mState.getWrapR(); return mState.getWrapR();
} }
void Sampler::setMaxAnisotropy(float maxAnisotropy) void Sampler::setMaxAnisotropy(const Context *context, float maxAnisotropy)
{ {
mState.setMaxAnisotropy(maxAnisotropy); mState.setMaxAnisotropy(maxAnisotropy);
onStateChange(context, angle::SubjectMessage::DEPENDENT_DIRTY_BITS);
} }
float Sampler::getMaxAnisotropy() const float Sampler::getMaxAnisotropy() const
...@@ -96,9 +102,10 @@ float Sampler::getMaxAnisotropy() const ...@@ -96,9 +102,10 @@ float Sampler::getMaxAnisotropy() const
return mState.getMaxAnisotropy(); return mState.getMaxAnisotropy();
} }
void Sampler::setMinLod(GLfloat minLod) void Sampler::setMinLod(const Context *context, GLfloat minLod)
{ {
mState.setMinLod(minLod); mState.setMinLod(minLod);
onStateChange(context, angle::SubjectMessage::DEPENDENT_DIRTY_BITS);
} }
GLfloat Sampler::getMinLod() const GLfloat Sampler::getMinLod() const
...@@ -106,9 +113,10 @@ GLfloat Sampler::getMinLod() const ...@@ -106,9 +113,10 @@ GLfloat Sampler::getMinLod() const
return mState.getMinLod(); return mState.getMinLod();
} }
void Sampler::setMaxLod(GLfloat maxLod) void Sampler::setMaxLod(const Context *context, GLfloat maxLod)
{ {
mState.setMaxLod(maxLod); mState.setMaxLod(maxLod);
onStateChange(context, angle::SubjectMessage::DEPENDENT_DIRTY_BITS);
} }
GLfloat Sampler::getMaxLod() const GLfloat Sampler::getMaxLod() const
...@@ -116,9 +124,10 @@ GLfloat Sampler::getMaxLod() const ...@@ -116,9 +124,10 @@ GLfloat Sampler::getMaxLod() const
return mState.getMaxLod(); return mState.getMaxLod();
} }
void Sampler::setCompareMode(GLenum compareMode) void Sampler::setCompareMode(const Context *context, GLenum compareMode)
{ {
mState.setCompareMode(compareMode); mState.setCompareMode(compareMode);
onStateChange(context, angle::SubjectMessage::DEPENDENT_DIRTY_BITS);
} }
GLenum Sampler::getCompareMode() const GLenum Sampler::getCompareMode() const
...@@ -126,9 +135,10 @@ GLenum Sampler::getCompareMode() const ...@@ -126,9 +135,10 @@ GLenum Sampler::getCompareMode() const
return mState.getCompareMode(); return mState.getCompareMode();
} }
void Sampler::setCompareFunc(GLenum compareFunc) void Sampler::setCompareFunc(const Context *context, GLenum compareFunc)
{ {
mState.setCompareFunc(compareFunc); mState.setCompareFunc(compareFunc);
onStateChange(context, angle::SubjectMessage::DEPENDENT_DIRTY_BITS);
} }
GLenum Sampler::getCompareFunc() const GLenum Sampler::getCompareFunc() const
...@@ -136,9 +146,10 @@ GLenum Sampler::getCompareFunc() const ...@@ -136,9 +146,10 @@ GLenum Sampler::getCompareFunc() const
return mState.getCompareFunc(); return mState.getCompareFunc();
} }
void Sampler::setSRGBDecode(GLenum sRGBDecode) void Sampler::setSRGBDecode(const Context *context, GLenum sRGBDecode)
{ {
mState.setSRGBDecode(sRGBDecode); mState.setSRGBDecode(sRGBDecode);
onStateChange(context, angle::SubjectMessage::DEPENDENT_DIRTY_BITS);
} }
GLenum Sampler::getSRGBDecode() const GLenum Sampler::getSRGBDecode() const
...@@ -146,9 +157,10 @@ GLenum Sampler::getSRGBDecode() const ...@@ -146,9 +157,10 @@ GLenum Sampler::getSRGBDecode() const
return mState.getSRGBDecode(); return mState.getSRGBDecode();
} }
void Sampler::setBorderColor(const ColorGeneric &color) void Sampler::setBorderColor(const Context *context, const ColorGeneric &color)
{ {
mState.setBorderColor(color); mState.setBorderColor(color);
onStateChange(context, angle::SubjectMessage::DEPENDENT_DIRTY_BITS);
} }
const ColorGeneric &Sampler::getBorderColor() const const ColorGeneric &Sampler::getBorderColor() const
......
src/libANGLE/Sampler.h
...@@ -35,40 +35,40 @@ class Sampler final : public RefCountObject, public LabeledObject, public angle: ...@@ -35,40 +35,40 @@ class Sampler final : public RefCountObject, public LabeledObject, public angle:
void setLabel(const Context *context, const std::string &label) override; void setLabel(const Context *context, const std::string &label) override;
const std::string &getLabel() const override; const std::string &getLabel() const override;
void setMinFilter(GLenum minFilter); void setMinFilter(const Context *context, GLenum minFilter);
GLenum getMinFilter() const; GLenum getMinFilter() const;
void setMagFilter(GLenum magFilter); void setMagFilter(const Context *context, GLenum magFilter);
GLenum getMagFilter() const; GLenum getMagFilter() const;
void setWrapS(GLenum wrapS); void setWrapS(const Context *context, GLenum wrapS);
GLenum getWrapS() const; GLenum getWrapS() const;
void setWrapT(GLenum wrapT); void setWrapT(const Context *context, GLenum wrapT);
GLenum getWrapT() const; GLenum getWrapT() const;
void setWrapR(GLenum wrapR); void setWrapR(const Context *context, GLenum wrapR);
GLenum getWrapR() const; GLenum getWrapR() const;
void setMaxAnisotropy(float maxAnisotropy); void setMaxAnisotropy(const Context *context, float maxAnisotropy);
float getMaxAnisotropy() const; float getMaxAnisotropy() const;
void setMinLod(GLfloat minLod); void setMinLod(const Context *context, GLfloat minLod);
GLfloat getMinLod() const; GLfloat getMinLod() const;
void setMaxLod(GLfloat maxLod); void setMaxLod(const Context *context, GLfloat maxLod);
GLfloat getMaxLod() const; GLfloat getMaxLod() const;
void setCompareMode(GLenum compareMode); void setCompareMode(const Context *context, GLenum compareMode);
GLenum getCompareMode() const; GLenum getCompareMode() const;
void setCompareFunc(GLenum compareFunc); void setCompareFunc(const Context *context, GLenum compareFunc);
GLenum getCompareFunc() const; GLenum getCompareFunc() const;
void setSRGBDecode(GLenum sRGBDecode); void setSRGBDecode(const Context *context, GLenum sRGBDecode);
GLenum getSRGBDecode() const; GLenum getSRGBDecode() const;
void setBorderColor(const ColorGeneric &color); void setBorderColor(const Context *context, const ColorGeneric &color);
const ColorGeneric &getBorderColor() const; const ColorGeneric &getBorderColor() const;
const SamplerState &getSamplerState() const; const SamplerState &getSamplerState() const;
......
src/libANGLE/State.cpp
...@@ -540,7 +540,8 @@ ANGLE_INLINE void State::updateActiveTextureState(const Context *context, ...@@ -540,7 +540,8 @@ ANGLE_INLINE void State::updateActiveTextureState(const Context *context,
mTexturesIncompatibleWithSamplers[textureIndex] = mTexturesIncompatibleWithSamplers[textureIndex] =
!texture->getTextureState().compatibleWithSamplerFormat( !texture->getTextureState().compatibleWithSamplerFormat(
mProgram->getState().getSamplerFormatForTextureUnitIndex(textureIndex)); mProgram->getState().getSamplerFormatForTextureUnitIndex(textureIndex),
sampler ? sampler->getSamplerState() : texture->getSamplerState());
mDirtyBits.set(DIRTY_BIT_TEXTURE_BINDINGS); mDirtyBits.set(DIRTY_BIT_TEXTURE_BINDINGS);
} }
...@@ -1184,6 +1185,7 @@ void State::setSamplerBinding(const Context *context, GLuint textureUnit, Sample ...@@ -1184,6 +1185,7 @@ void State::setSamplerBinding(const Context *context, GLuint textureUnit, Sample
// This is overly conservative as it assumes the sampler has never been bound. // This is overly conservative as it assumes the sampler has never been bound.
setSamplerDirty(textureUnit); setSamplerDirty(textureUnit);
onActiveTextureChange(context, textureUnit); onActiveTextureChange(context, textureUnit);
onActiveTextureStateChange(context, textureUnit);
} }
void State::detachSampler(const Context *context, GLuint sampler) void State::detachSampler(const Context *context, GLuint sampler)
...@@ -1192,12 +1194,11 @@ void State::detachSampler(const Context *context, GLuint sampler) ...@@ -1192,12 +1194,11 @@ void State::detachSampler(const Context *context, GLuint sampler)
// If a sampler object that is currently bound to one or more texture units is // If a sampler object that is currently bound to one or more texture units is
// deleted, it is as though BindSampler is called once for each texture unit to // deleted, it is as though BindSampler is called once for each texture unit to
// which the sampler is bound, with unit set to the texture unit and sampler set to zero. // which the sampler is bound, with unit set to the texture unit and sampler set to zero.
for (BindingPointer<Sampler> &samplerBinding : mSamplers) for (size_t i = 0; i < mSamplers.size(); i++)
{ {
if (samplerBinding.id() == sampler) if (mSamplers[i].id() == sampler)
{ {
samplerBinding.set(context, nullptr); setSamplerBinding(context, i, nullptr);
mDirtyBits.set(DIRTY_BIT_SAMPLER_BINDINGS);
} }
} }
} }
......
src/libANGLE/Texture.cpp
...@@ -108,6 +108,7 @@ TextureState::TextureState(TextureType type) ...@@ -108,6 +108,7 @@ TextureState::TextureState(TextureType type)
mGenerateMipmapHint(GL_FALSE), mGenerateMipmapHint(GL_FALSE),
mInitState(InitState::MayNeedInit), mInitState(InitState::MayNeedInit),
mCachedSamplerFormat(SamplerFormat::InvalidEnum), mCachedSamplerFormat(SamplerFormat::InvalidEnum),
mCachedSamplerCompareMode(GL_NONE),
mCachedSamplerFormatValid(false) mCachedSamplerFormatValid(false)
{} {}
...@@ -241,12 +242,12 @@ GLenum TextureState::getGenerateMipmapHint() const ...@@ -241,12 +242,12 @@ GLenum TextureState::getGenerateMipmapHint() const
return mGenerateMipmapHint; return mGenerateMipmapHint;
} }
SamplerFormat TextureState::computeRequiredSamplerFormat() const SamplerFormat TextureState::computeRequiredSamplerFormat(const SamplerState &samplerState) const
{ {
const ImageDesc &baseImageDesc = getImageDesc(getBaseImageTarget(), getEffectiveBaseLevel()); const ImageDesc &baseImageDesc = getImageDesc(getBaseImageTarget(), getEffectiveBaseLevel());
if ((baseImageDesc.format.info->format == GL_DEPTH_COMPONENT || if ((baseImageDesc.format.info->format == GL_DEPTH_COMPONENT ||
baseImageDesc.format.info->format == GL_DEPTH_STENCIL) && baseImageDesc.format.info->format == GL_DEPTH_STENCIL) &&
mSamplerState.getCompareMode() != GL_NONE) samplerState.getCompareMode() != GL_NONE)
{ {
return SamplerFormat::Shadow; return SamplerFormat::Shadow;
} }
......
src/libANGLE/Texture.h
...@@ -111,11 +111,14 @@ class TextureState final : private angle::NonCopyable ...@@ -111,11 +111,14 @@ class TextureState final : private angle::NonCopyable
bool isCubeComplete() const; bool isCubeComplete() const;
ANGLE_INLINE bool compatibleWithSamplerFormat(SamplerFormat format) const ANGLE_INLINE bool compatibleWithSamplerFormat(SamplerFormat format,
const SamplerState &samplerState) const
{ {
if (!mCachedSamplerFormatValid) if (!mCachedSamplerFormatValid ||
mCachedSamplerCompareMode != samplerState.getCompareMode())
{ {
mCachedSamplerFormat = computeRequiredSamplerFormat(); mCachedSamplerFormat = computeRequiredSamplerFormat(samplerState);
mCachedSamplerCompareMode = samplerState.getCompareMode();
mCachedSamplerFormatValid = true; mCachedSamplerFormatValid = true;
} }
// Incomplete textures are compatible with any sampler format. // Incomplete textures are compatible with any sampler format.
...@@ -152,7 +155,7 @@ class TextureState final : private angle::NonCopyable ...@@ -152,7 +155,7 @@ class TextureState final : private angle::NonCopyable
bool computeSamplerCompleteness(const SamplerState &samplerState, const State &data) const; bool computeSamplerCompleteness(const SamplerState &samplerState, const State &data) const;
bool computeMipmapCompleteness() const; bool computeMipmapCompleteness() const;
bool computeLevelCompleteness(TextureTarget target, size_t level) const; bool computeLevelCompleteness(TextureTarget target, size_t level) const;
SamplerFormat computeRequiredSamplerFormat() const; SamplerFormat computeRequiredSamplerFormat(const SamplerState &samplerState) const;
TextureTarget getBaseImageTarget() const; TextureTarget getBaseImageTarget() const;
...@@ -200,6 +203,7 @@ class TextureState final : private angle::NonCopyable ...@@ -200,6 +203,7 @@ class TextureState final : private angle::NonCopyable
InitState mInitState; InitState mInitState;
mutable SamplerFormat mCachedSamplerFormat; mutable SamplerFormat mCachedSamplerFormat;
mutable GLenum mCachedSamplerCompareMode;
mutable bool mCachedSamplerFormatValid; mutable bool mCachedSamplerFormatValid;
}; };
......
src/libANGLE/queryutils.cpp
...@@ -454,40 +454,40 @@ void SetSamplerParameterBase(Context *context, ...@@ -454,40 +454,40 @@ void SetSamplerParameterBase(Context *context,
switch (pname) switch (pname)
{ {
case GL_TEXTURE_WRAP_S: case GL_TEXTURE_WRAP_S:
sampler->setWrapS(ConvertToGLenum(pname, params[0])); sampler->setWrapS(context, ConvertToGLenum(pname, params[0]));
break; break;
case GL_TEXTURE_WRAP_T: case GL_TEXTURE_WRAP_T:
sampler->setWrapT(ConvertToGLenum(pname, params[0])); sampler->setWrapT(context, ConvertToGLenum(pname, params[0]));
break; break;
case GL_TEXTURE_WRAP_R: case GL_TEXTURE_WRAP_R:
sampler->setWrapR(ConvertToGLenum(pname, params[0])); sampler->setWrapR(context, ConvertToGLenum(pname, params[0]));
break; break;
case GL_TEXTURE_MIN_FILTER: case GL_TEXTURE_MIN_FILTER:
sampler->setMinFilter(ConvertToGLenum(pname, params[0])); sampler->setMinFilter(context, ConvertToGLenum(pname, params[0]));
break; break;
case GL_TEXTURE_MAG_FILTER: case GL_TEXTURE_MAG_FILTER:
sampler->setMagFilter(ConvertToGLenum(pname, params[0])); sampler->setMagFilter(context, ConvertToGLenum(pname, params[0]));
break; break;
case GL_TEXTURE_MAX_ANISOTROPY_EXT: case GL_TEXTURE_MAX_ANISOTROPY_EXT:
sampler->setMaxAnisotropy(CastQueryValueTo<GLfloat>(pname, params[0])); sampler->setMaxAnisotropy(context, CastQueryValueTo<GLfloat>(pname, params[0]));
break; break;
case GL_TEXTURE_COMPARE_MODE: case GL_TEXTURE_COMPARE_MODE:
sampler->setCompareMode(ConvertToGLenum(pname, params[0])); sampler->setCompareMode(context, ConvertToGLenum(pname, params[0]));
break; break;
case GL_TEXTURE_COMPARE_FUNC: case GL_TEXTURE_COMPARE_FUNC:
sampler->setCompareFunc(ConvertToGLenum(pname, params[0])); sampler->setCompareFunc(context, ConvertToGLenum(pname, params[0]));
break; break;
case GL_TEXTURE_MIN_LOD: case GL_TEXTURE_MIN_LOD:
sampler->setMinLod(CastQueryValueTo<GLfloat>(pname, params[0])); sampler->setMinLod(context, CastQueryValueTo<GLfloat>(pname, params[0]));
break; break;
case GL_TEXTURE_MAX_LOD: case GL_TEXTURE_MAX_LOD:
sampler->setMaxLod(CastQueryValueTo<GLfloat>(pname, params[0])); sampler->setMaxLod(context, CastQueryValueTo<GLfloat>(pname, params[0]));
break; break;
case GL_TEXTURE_SRGB_DECODE_EXT: case GL_TEXTURE_SRGB_DECODE_EXT:
sampler->setSRGBDecode(ConvertToGLenum(pname, params[0])); sampler->setSRGBDecode(context, ConvertToGLenum(pname, params[0]));
break; break;
case GL_TEXTURE_BORDER_COLOR: case GL_TEXTURE_BORDER_COLOR:
sampler->setBorderColor(ConvertToColor<isPureInteger>(params)); sampler->setBorderColor(context, ConvertToColor<isPureInteger>(params));
break; break;
default: default:
UNREACHABLE(); UNREACHABLE();
......
src/libANGLE/validationES.cpp
...@@ -2733,6 +2733,11 @@ const char *ValidateDrawStates(Context *context) ...@@ -2733,6 +2733,11 @@ const char *ValidateDrawStates(Context *context)
// Do some additonal WebGL-specific validation // Do some additonal WebGL-specific validation
if (extensions.webglCompatibility) if (extensions.webglCompatibility)
{ {
if (!state.validateSamplerFormats())
{
return kSamplerFormatMismatch;
}
const TransformFeedback *transformFeedbackObject = state.getCurrentTransformFeedback(); const TransformFeedback *transformFeedbackObject = state.getCurrentTransformFeedback();
if (transformFeedbackObject != nullptr && transformFeedbackObject->isActive() && if (transformFeedbackObject != nullptr && transformFeedbackObject->isActive() &&
transformFeedbackObject->buffersBoundForOtherUse()) transformFeedbackObject->buffersBoundForOtherUse())
......
src/tests/gl_tests/StateChangeTest.cpp
...@@ -3087,8 +3087,6 @@ void main() ...@@ -3087,8 +3087,6 @@ void main()
// Test sampler format validation caching works. // Test sampler format validation caching works.
TEST_P(WebGL2ValidationStateChangeTest, SamplerFormatCache) TEST_P(WebGL2ValidationStateChangeTest, SamplerFormatCache)
{ {
// TODO(jdarpinian): Re-enable this test when fixing http://crbug.com/940080
ANGLE_SKIP_TEST_IF(true);
constexpr char kFS[] = R"(#version 300 es constexpr char kFS[] = R"(#version 300 es
precision mediump float; precision mediump float;
uniform sampler2D sampler; uniform sampler2D sampler;
......
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