Fix IOSurface synchronization issues
It appears that not only render targets, but also samplers that use
IOSurfaces require SwiftShader to perform synchronized draws,
otherwise it seems like using a shared IOSurface's CPU memory is
unsafe and can cause crashes in Layout Tests.
Bug chromium:846693
Change-Id: I0ce24700d34c657ac2447ceb2f6f837bfa3a9a58
Reviewed-on: https://swiftshader-review.googlesource.com/19288
Tested-by: Alexis Hétu <sugoi@google.com>
Reviewed-by: Nicolas Capens <nicolascapens@google.com>
diff --git a/src/OpenGL/common/Image.cpp b/src/OpenGL/common/Image.cpp
index 447222c..988b532 100644
--- a/src/OpenGL/common/Image.cpp
+++ b/src/OpenGL/common/Image.cpp
@@ -1332,7 +1332,7 @@
#endif
}
- bool ClientBuffer::targetRequiresSync() const
+ bool ClientBuffer::requiresSync() const
{
#if defined(__APPLE__)
return true;
@@ -1431,9 +1431,9 @@
sw::Surface::unlockExternal();
}
- bool targetRequiresSync() const override
+ bool requiresSync() const override
{
- return clientBuffer.targetRequiresSync();
+ return clientBuffer.requiresSync();
}
void release() override
diff --git a/src/OpenGL/common/Image.hpp b/src/OpenGL/common/Image.hpp
index d7117c5..a744763 100644
--- a/src/OpenGL/common/Image.hpp
+++ b/src/OpenGL/common/Image.hpp
@@ -79,7 +79,7 @@
void release();
void* lock(int x, int y, int z);
void unlock();
- bool targetRequiresSync() const;
+ bool requiresSync() const;
private:
int width;
diff --git a/src/OpenGL/libGLESv2/Context.cpp b/src/OpenGL/libGLESv2/Context.cpp
index 7bd9fea..a479f18 100644
--- a/src/OpenGL/libGLESv2/Context.cpp
+++ b/src/OpenGL/libGLESv2/Context.cpp
@@ -3180,6 +3180,7 @@
device->setMipmapFilter(samplerType, samplerIndex, es2sw::ConvertMipMapFilter(minFilter));
device->setMaxAnisotropy(samplerType, samplerIndex, maxAnisotropy);
device->setHighPrecisionFiltering(samplerType, samplerIndex, mState.textureFilteringHint == GL_NICEST);
+ device->setSyncRequired(samplerType, samplerIndex, texture->requiresSync());
applyTexture(samplerType, samplerIndex, texture);
}
diff --git a/src/OpenGL/libGLESv2/Texture.cpp b/src/OpenGL/libGLESv2/Texture.cpp
index ca28ed9..a218cd3 100644
--- a/src/OpenGL/libGLESv2/Texture.cpp
+++ b/src/OpenGL/libGLESv2/Texture.cpp
@@ -528,6 +528,19 @@
return level - 1;
}
+bool Texture2D::requiresSync() const
+{
+ for(int level = 0; level < IMPLEMENTATION_MAX_TEXTURE_LEVELS; level++)
+ {
+ if(image[level] && image[level]->requiresSync())
+ {
+ return true;
+ }
+ }
+
+ return false;
+}
+
void Texture2D::setImage(GLint level, GLsizei width, GLsizei height, GLint internalformat, GLenum format, GLenum type, const gl::PixelStorageModes &unpackParameters, const void *pixels)
{
if(image[level])
@@ -1019,6 +1032,22 @@
return level - 1;
}
+bool TextureCubeMap::requiresSync() const
+{
+ for(int level = 0; level < IMPLEMENTATION_MAX_TEXTURE_LEVELS; level++)
+ {
+ for(int face = 0; face < 6; face++)
+ {
+ if(image[face][level] && image[face][level]->requiresSync())
+ {
+ return true;
+ }
+ }
+ }
+
+ return false;
+}
+
void TextureCubeMap::setCompressedImage(GLenum target, GLint level, GLenum format, GLsizei width, GLsizei height, GLsizei imageSize, const void *pixels)
{
int face = CubeFaceIndex(target);
@@ -1531,6 +1560,19 @@
return level - 1;
}
+bool Texture3D::requiresSync() const
+{
+ for(int level = 0; level < IMPLEMENTATION_MAX_TEXTURE_LEVELS; level++)
+ {
+ if(image[level] && image[level]->requiresSync())
+ {
+ return true;
+ }
+ }
+
+ return false;
+}
+
void Texture3D::setImage(GLint level, GLsizei width, GLsizei height, GLsizei depth, GLint internalformat, GLenum format, GLenum type, const gl::PixelStorageModes &unpackParameters, const void *pixels)
{
if(image[level])
diff --git a/src/OpenGL/libGLESv2/Texture.h b/src/OpenGL/libGLESv2/Texture.h
index 3b4bfc5..ddd915e 100644
--- a/src/OpenGL/libGLESv2/Texture.h
+++ b/src/OpenGL/libGLESv2/Texture.h
@@ -97,6 +97,7 @@
virtual GLsizei getDepth(GLenum target, GLint level) const;
virtual GLint getFormat(GLenum target, GLint level) const = 0;
virtual int getTopLevel() const = 0;
+ virtual bool requiresSync() const = 0;
virtual bool isSamplerComplete() const = 0;
virtual bool isCompressed(GLenum target, GLint level) const = 0;
@@ -159,6 +160,7 @@
GLsizei getHeight(GLenum target, GLint level) const override;
GLint getFormat(GLenum target, GLint level) const override;
int getTopLevel() const override;
+ bool requiresSync() const override;
void setImage(GLint level, GLsizei width, GLsizei height, GLint internalformat, GLenum format, GLenum type, const gl::PixelStorageModes &unpackParameters, const void *pixels);
void setCompressedImage(GLint level, GLenum format, GLsizei width, GLsizei height, GLsizei imageSize, const void *pixels);
@@ -226,6 +228,7 @@
GLsizei getHeight(GLenum target, GLint level) const override;
GLint getFormat(GLenum target, GLint level) const override;
int getTopLevel() const override;
+ bool requiresSync() const override;
void setImage(GLenum target, GLint level, GLsizei width, GLsizei height, GLint internalformat, GLenum format, GLenum type, const gl::PixelStorageModes &unpackParameters, const void *pixels);
void setCompressedImage(GLenum target, GLint level, GLenum format, GLsizei width, GLsizei height, GLsizei imageSize, const void *pixels);
@@ -287,6 +290,7 @@
GLsizei getDepth(GLenum target, GLint level) const override;
GLint getFormat(GLenum target, GLint level) const override;
int getTopLevel() const override;
+ bool requiresSync() const override;
void setImage(GLint level, GLsizei width, GLsizei height, GLsizei depth, GLint internalformat, GLenum format, GLenum type, const gl::PixelStorageModes &unpackParameters, const void *pixels);
void setCompressedImage(GLint level, GLenum format, GLsizei width, GLsizei height, GLsizei depth, GLsizei imageSize, const void *pixels);
diff --git a/src/Renderer/PixelProcessor.cpp b/src/Renderer/PixelProcessor.cpp
index 7ae45e9..3903701 100644
--- a/src/Renderer/PixelProcessor.cpp
+++ b/src/Renderer/PixelProcessor.cpp
@@ -537,6 +537,15 @@
else ASSERT(false);
}
+ void PixelProcessor::setSyncRequired(unsigned int sampler, bool isSincRequired)
+ {
+ if(sampler < TEXTURE_IMAGE_UNITS)
+ {
+ context->sampler[sampler].setSyncRequired(isSincRequired);
+ }
+ else ASSERT(false);
+ }
+
void PixelProcessor::setWriteSRGB(bool sRGB)
{
context->setWriteSRGB(sRGB);
diff --git a/src/Renderer/PixelProcessor.hpp b/src/Renderer/PixelProcessor.hpp
index 1954610..cea417d 100644
--- a/src/Renderer/PixelProcessor.hpp
+++ b/src/Renderer/PixelProcessor.hpp
@@ -242,6 +242,7 @@
void setMaxLevel(unsigned int sampler, int maxLevel);
void setMinLod(unsigned int sampler, float minLod);
void setMaxLod(unsigned int sampler, float maxLod);
+ void setSyncRequired(unsigned int sampler, bool isSincRequired);
void setWriteSRGB(bool sRGB);
void setDepthBufferEnable(bool depthBufferEnable);
diff --git a/src/Renderer/Renderer.cpp b/src/Renderer/Renderer.cpp
index 1334e70..b560f41 100644
--- a/src/Renderer/Renderer.cpp
+++ b/src/Renderer/Renderer.cpp
@@ -224,7 +224,7 @@
int ss = context->getSuperSampleCount();
int ms = context->getMultiSampleCount();
- bool targetRequiresSync = false;
+ bool requiresSync = false;
for(int q = 0; q < ss; q++)
{
@@ -368,6 +368,8 @@
draw->texture[sampler]->lock(PUBLIC, isReadWriteTexture(sampler) ? MANAGED : PRIVATE); // If the texure is both read and written, use the same read/write lock as render targets
data->mipmap[sampler] = context->sampler[sampler].getTextureData();
+
+ requiresSync |= context->sampler[sampler].requiresSync();
}
}
@@ -426,6 +428,8 @@
draw->texture[TEXTURE_IMAGE_UNITS + sampler]->lock(PUBLIC, PRIVATE);
data->mipmap[TEXTURE_IMAGE_UNITS + sampler] = context->sampler[TEXTURE_IMAGE_UNITS + sampler].getTextureData();
+
+ requiresSync |= context->sampler[TEXTURE_IMAGE_UNITS + sampler].requiresSync();
}
}
}
@@ -608,7 +612,7 @@
if(draw->renderTarget[index])
{
unsigned int layer = context->renderTargetLayer[index];
- targetRequiresSync |= context->renderTarget[index]->targetRequiresSync();
+ requiresSync |= context->renderTarget[index]->requiresSync();
data->colorBuffer[index] = (unsigned int*)context->renderTarget[index]->lockInternal(0, 0, layer, LOCK_READWRITE, MANAGED);
data->colorBuffer[index] += q * ms * context->renderTarget[index]->getSliceB(true);
data->colorPitchB[index] = context->renderTarget[index]->getInternalPitchB();
@@ -622,7 +626,7 @@
if(draw->depthBuffer)
{
unsigned int layer = context->depthBufferLayer;
- targetRequiresSync |= context->depthBuffer->targetRequiresSync();
+ requiresSync |= context->depthBuffer->requiresSync();
data->depthBuffer = (float*)context->depthBuffer->lockInternal(0, 0, layer, LOCK_READWRITE, MANAGED);
data->depthBuffer += q * ms * context->depthBuffer->getSliceB(true);
data->depthPitchB = context->depthBuffer->getInternalPitchB();
@@ -632,7 +636,7 @@
if(draw->stencilBuffer)
{
unsigned int layer = context->stencilBufferLayer;
- targetRequiresSync |= context->stencilBuffer->targetRequiresSync();
+ requiresSync |= context->stencilBuffer->requiresSync();
data->stencilBuffer = (unsigned char*)context->stencilBuffer->lockStencil(0, 0, layer, MANAGED);
data->stencilBuffer += q * ms * context->stencilBuffer->getSliceB(true);
data->stencilPitchB = context->stencilBuffer->getStencilPitchB();
@@ -681,7 +685,7 @@
}
// TODO(sugoi): This is a temporary brute-force workaround to ensure IOSurface synchronization.
- if(targetRequiresSync)
+ if(requiresSync)
{
synchronize();
}
@@ -2469,6 +2473,18 @@
}
}
+ void Renderer::setSyncRequired(SamplerType type, int sampler, bool syncRequired)
+ {
+ if(type == SAMPLER_PIXEL)
+ {
+ PixelProcessor::setSyncRequired(sampler, syncRequired);
+ }
+ else
+ {
+ VertexProcessor::setSyncRequired(sampler, syncRequired);
+ }
+ }
+
void Renderer::setPointSpriteEnable(bool pointSpriteEnable)
{
context->setPointSpriteEnable(pointSpriteEnable);
diff --git a/src/Renderer/Renderer.hpp b/src/Renderer/Renderer.hpp
index b37da5f..ce22866 100644
--- a/src/Renderer/Renderer.hpp
+++ b/src/Renderer/Renderer.hpp
@@ -304,6 +304,7 @@
void setMaxLevel(SamplerType type, int sampler, int maxLevel);
void setMinLod(SamplerType type, int sampler, float minLod);
void setMaxLod(SamplerType type, int sampler, float maxLod);
+ void setSyncRequired(SamplerType type, int sampler, bool syncRequired);
void setPointSpriteEnable(bool pointSpriteEnable);
void setPointScaleEnable(bool pointScaleEnable);
diff --git a/src/Renderer/Sampler.cpp b/src/Renderer/Sampler.cpp
index 748fb4e..efac4c6 100644
--- a/src/Renderer/Sampler.cpp
+++ b/src/Renderer/Sampler.cpp
@@ -401,6 +401,16 @@
return textureType == TEXTURE_3D || textureType == TEXTURE_2D_ARRAY;
}
+ void Sampler::setSyncRequired(bool isSyncRequired)
+ {
+ syncRequired = isSyncRequired;
+ }
+
+ bool Sampler::requiresSync() const
+ {
+ return syncRequired;
+ }
+
const Texture &Sampler::getTextureData()
{
return texture;
diff --git a/src/Renderer/Sampler.hpp b/src/Renderer/Sampler.hpp
index c73721d..af225c5 100644
--- a/src/Renderer/Sampler.hpp
+++ b/src/Renderer/Sampler.hpp
@@ -193,6 +193,7 @@
void setMaxLevel(int maxLevel);
void setMinLod(float minLod);
void setMaxLod(float maxLod);
+ void setSyncRequired(bool isSincRequired);
static void setFilterQuality(FilterType maximumFilterQuality);
static void setMipmapQuality(MipmapType maximumFilterQuality);
@@ -202,6 +203,7 @@
bool hasUnsignedTexture() const;
bool hasCubeTexture() const;
bool hasVolumeTexture() const;
+ bool requiresSync() const;
const Texture &getTextureData();
@@ -226,6 +228,7 @@
bool sRGB;
bool gather;
bool highPrecisionFiltering;
+ bool syncRequired;
int border;
SwizzleType swizzleR;
diff --git a/src/Renderer/Surface.hpp b/src/Renderer/Surface.hpp
index 2b1b906..a6ef15b 100644
--- a/src/Renderer/Surface.hpp
+++ b/src/Renderer/Surface.hpp
@@ -321,7 +321,7 @@
inline int getStencilSliceB() const;
void sync(); // Wait for lock(s) to be released.
- virtual bool targetRequiresSync() const { return false; }
+ virtual bool requiresSync() const { return false; }
inline bool isUnlocked() const; // Only reliable after sync().
inline int getSamples() const;
diff --git a/src/Renderer/VertexProcessor.cpp b/src/Renderer/VertexProcessor.cpp
index cc9bd25..976ea2b 100644
--- a/src/Renderer/VertexProcessor.cpp
+++ b/src/Renderer/VertexProcessor.cpp
@@ -692,6 +692,15 @@
else ASSERT(false);
}
+ void VertexProcessor::setSyncRequired(unsigned int sampler, bool isSincRequired)
+ {
+ if(sampler < TEXTURE_IMAGE_UNITS)
+ {
+ context->sampler[sampler].setSyncRequired(isSincRequired);
+ }
+ else ASSERT(false);
+ }
+
void VertexProcessor::setPointSize(float pointSize)
{
point.pointSize = replicate(pointSize);
diff --git a/src/Renderer/VertexProcessor.hpp b/src/Renderer/VertexProcessor.hpp
index cd3849a..277a155 100644
--- a/src/Renderer/VertexProcessor.hpp
+++ b/src/Renderer/VertexProcessor.hpp
@@ -268,6 +268,7 @@
void setMaxLevel(unsigned int sampler, int maxLevel);
void setMinLod(unsigned int sampler, float minLod);
void setMaxLod(unsigned int sampler, float maxLod);
+ void setSyncRequired(unsigned int sampler, bool isSincRequired);
void setPointSize(float pointSize);
void setPointSizeMin(float pointSizeMin);
