Skip to content

A
angle
Commit e3db3d20 by Alexis Hétu Committed by Commit Bot
Options

Revert "Make copyTexImage2D robust when source area is out of bounds"

This reverts commit e1e6f294. Reason for revert: This fix is incorrect, it ends up being a noop. Reverting this first before landing a different fix. Original change's description: > Make copyTexImage2D robust when source area is out of bounds > > Whenever the source area of the texture being copied is out of > bounds and robust resource init is enabled, it is necessary to > clear the invalid portion of the source area to 0 in the copy > operation. > > Bug: angleproject:3930 > Change-Id: If1605b917097006c37e7d59c66babbc32d0ba6d8 > Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2378004 > Reviewed-by: Geoff Lang <geofflang@chromium.org> > Reviewed-by: back sept 10 - Jamie Madill <jmadill@chromium.org> > Commit-Queue: Alexis Hétu <sugoi@chromium.org> TBR=sugoi@chromium.org,geofflang@chromium.org,jmadill@chromium.org,sugoi@google.com # Not skipping CQ checks because original CL landed > 1 day ago. Bug: angleproject:3930 Change-Id: I9e37a3f739eb7ce5d6d6ab5ca59d11ce9aae8483 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2388718Reviewed-by: 's avatarAlexis Hétu <sugoi@chromium.org> Commit-Queue: Alexis Hétu <sugoi@chromium.org>
parent 121d9856
Showing with 14 additions and 26 deletions
src/libANGLE/Texture.cpp
...@@ -1101,7 +1101,7 @@ angle::Result Texture::setSubImage(Context *context, ...@@ -1101,7 +1101,7 @@ angle::Result Texture::setSubImage(Context *context,
ASSERT(TextureTargetToType(target) == mState.mType); ASSERT(TextureTargetToType(target) == mState.mType);
ImageIndex index = ImageIndex::MakeFromTarget(target, level, area.depth); ImageIndex index = ImageIndex::MakeFromTarget(target, level, area.depth);
ANGLE_TRY(ensureSubImageInitialized(context, index, area, false)); ANGLE_TRY(ensureSubImageInitialized(context, index, area));
ANGLE_TRY(mTexture->setSubImage(context, index, area, format, type, unpackState, unpackBuffer, ANGLE_TRY(mTexture->setSubImage(context, index, area, format, type, unpackState, unpackBuffer,
pixels)); pixels));
...@@ -1154,7 +1154,7 @@ angle::Result Texture::setCompressedSubImage(const Context *context, ...@@ -1154,7 +1154,7 @@ angle::Result Texture::setCompressedSubImage(const Context *context,
ASSERT(TextureTargetToType(target) == mState.mType); ASSERT(TextureTargetToType(target) == mState.mType);
ImageIndex index = ImageIndex::MakeFromTarget(target, level, area.depth); ImageIndex index = ImageIndex::MakeFromTarget(target, level, area.depth);
ANGLE_TRY(ensureSubImageInitialized(context, index, area, false)); ANGLE_TRY(ensureSubImageInitialized(context, index, area));
ANGLE_TRY(mTexture->setCompressedSubImage(context, index, area, format, unpackState, imageSize, ANGLE_TRY(mTexture->setCompressedSubImage(context, index, area, format, unpackState, imageSize,
pixels)); pixels));
...@@ -1188,13 +1188,9 @@ angle::Result Texture::copyImage(Context *context, ...@@ -1188,13 +1188,9 @@ angle::Result Texture::copyImage(Context *context,
// the copy lies entirely off the source framebuffer, initialize as though a zero-size box is // the copy lies entirely off the source framebuffer, initialize as though a zero-size box is
// going to be set during the copy operation. // going to be set during the copy operation.
Box destBox; Box destBox;
bool sourceAreaIsOutOfBounds = false;
if (context->isRobustResourceInitEnabled()) if (context->isRobustResourceInitEnabled())
{ {
Extents fbSize = source->getReadColorAttachment()->getSize(); Extents fbSize = source->getReadColorAttachment()->getSize();
sourceAreaIsOutOfBounds = (sourceArea.x < 0) || (sourceArea.y < 0) ||
((sourceArea.x + sourceArea.width) > fbSize.width) ||
((sourceArea.y + sourceArea.height) > fbSize.height);
Rectangle clippedArea; Rectangle clippedArea;
if (ClipRectangle(sourceArea, Rectangle(0, 0, fbSize.width, fbSize.height), &clippedArea)) if (ClipRectangle(sourceArea, Rectangle(0, 0, fbSize.width, fbSize.height), &clippedArea))
{ {
...@@ -1209,14 +1205,14 @@ angle::Result Texture::copyImage(Context *context, ...@@ -1209,14 +1205,14 @@ angle::Result Texture::copyImage(Context *context,
// an initializeContents call, and then a copySubImage call. This ensures the destination // an initializeContents call, and then a copySubImage call. This ensures the destination
// texture exists before we try to clear it. // texture exists before we try to clear it.
Extents size(sourceArea.width, sourceArea.height, 1); Extents size(sourceArea.width, sourceArea.height, 1);
if (doesSubImageNeedInit(context, index, destBox, sourceAreaIsOutOfBounds)) if (doesSubImageNeedInit(context, index, destBox))
{ {
ANGLE_TRY(mTexture->setImage(context, index, internalFormat, size, ANGLE_TRY(mTexture->setImage(context, index, internalFormat, size,
internalFormatInfo.format, internalFormatInfo.type, internalFormatInfo.format, internalFormatInfo.type,
PixelUnpackState(), nullptr, nullptr)); PixelUnpackState(), nullptr, nullptr));
mState.setImageDesc(target, level, mState.setImageDesc(target, level,
ImageDesc(size, Format(internalFormatInfo), InitState::MayNeedInit)); ImageDesc(size, Format(internalFormatInfo), InitState::MayNeedInit));
ANGLE_TRY(ensureSubImageInitialized(context, index, destBox, sourceAreaIsOutOfBounds)); ANGLE_TRY(ensureSubImageInitialized(context, index, destBox));
ANGLE_TRY(mTexture->copySubImage(context, index, Offset(), sourceArea, source)); ANGLE_TRY(mTexture->copySubImage(context, index, Offset(), sourceArea, source));
} }
else else
...@@ -1251,13 +1247,9 @@ angle::Result Texture::copySubImage(Context *context, ...@@ -1251,13 +1247,9 @@ angle::Result Texture::copySubImage(Context *context,
// ensureSubImageInitialized ensures initialization of the entire destination texture, and not // ensureSubImageInitialized ensures initialization of the entire destination texture, and not
// just a sub-region. // just a sub-region.
Box destBox; Box destBox;
bool sourceAreaIsOutOfBounds = false;
if (context->isRobustResourceInitEnabled()) if (context->isRobustResourceInitEnabled())
{ {
Extents fbSize = source->getReadColorAttachment()->getSize(); Extents fbSize = source->getReadColorAttachment()->getSize();
sourceAreaIsOutOfBounds = (sourceArea.x < 0) || (sourceArea.y < 0) ||
((sourceArea.x + sourceArea.width) > fbSize.width) ||
((sourceArea.y + sourceArea.height) > fbSize.height);
Rectangle clippedArea; Rectangle clippedArea;
if (ClipRectangle(sourceArea, Rectangle(0, 0, fbSize.width, fbSize.height), &clippedArea)) if (ClipRectangle(sourceArea, Rectangle(0, 0, fbSize.width, fbSize.height), &clippedArea))
{ {
...@@ -1268,7 +1260,7 @@ angle::Result Texture::copySubImage(Context *context, ...@@ -1268,7 +1260,7 @@ angle::Result Texture::copySubImage(Context *context,
} }
} }
ANGLE_TRY(ensureSubImageInitialized(context, index, destBox, sourceAreaIsOutOfBounds)); ANGLE_TRY(ensureSubImageInitialized(context, index, destBox));
ANGLE_TRY(mTexture->copySubImage(context, index, destOffset, sourceArea, source)); ANGLE_TRY(mTexture->copySubImage(context, index, destOffset, sourceArea, source));
ANGLE_TRY(handleMipmapGenerationHint(context, index.getLevelIndex())); ANGLE_TRY(handleMipmapGenerationHint(context, index.getLevelIndex()));
...@@ -1336,7 +1328,7 @@ angle::Result Texture::copySubTexture(const Context *context, ...@@ -1336,7 +1328,7 @@ angle::Result Texture::copySubTexture(const Context *context,
Box destBox(destOffset.x, destOffset.y, destOffset.z, sourceBox.width, sourceBox.height, Box destBox(destOffset.x, destOffset.y, destOffset.z, sourceBox.width, sourceBox.height,
sourceBox.depth); sourceBox.depth);
ImageIndex index = ImageIndex::MakeFromTarget(target, level, sourceBox.depth); ImageIndex index = ImageIndex::MakeFromTarget(target, level, sourceBox.depth);
ANGLE_TRY(ensureSubImageInitialized(context, index, destBox, false)); ANGLE_TRY(ensureSubImageInitialized(context, index, destBox));
ANGLE_TRY(mTexture->copySubTexture(context, index, destOffset, sourceLevel, sourceBox, ANGLE_TRY(mTexture->copySubTexture(context, index, destOffset, sourceLevel, sourceBox,
unpackFlipY, unpackPremultiplyAlpha, unpackUnmultiplyAlpha, unpackFlipY, unpackPremultiplyAlpha, unpackUnmultiplyAlpha,
...@@ -1926,8 +1918,7 @@ void Texture::setInitState(const ImageIndex &imageIndex, InitState initState) ...@@ -1926,8 +1918,7 @@ void Texture::setInitState(const ImageIndex &imageIndex, InitState initState)
bool Texture::doesSubImageNeedInit(const Context *context, bool Texture::doesSubImageNeedInit(const Context *context,
const ImageIndex &imageIndex, const ImageIndex &imageIndex,
const Box &area, const Box &area) const
bool sourceAreaIsOutOfBounds) const
{ {
if (!context->isRobustResourceInitEnabled() || mState.mInitState == InitState::Initialized) if (!context->isRobustResourceInitEnabled() || mState.mInitState == InitState::Initialized)
{ {
...@@ -1942,15 +1933,14 @@ bool Texture::doesSubImageNeedInit(const Context *context, ...@@ -1942,15 +1933,14 @@ bool Texture::doesSubImageNeedInit(const Context *context,
} }
ASSERT(mState.mInitState == InitState::MayNeedInit); ASSERT(mState.mInitState == InitState::MayNeedInit);
return sourceAreaIsOutOfBounds || !area.coversSameExtent(desc.size); return !area.coversSameExtent(desc.size);
} }
angle::Result Texture::ensureSubImageInitialized(const Context *context, angle::Result Texture::ensureSubImageInitialized(const Context *context,
const ImageIndex &imageIndex, const ImageIndex &imageIndex,
const Box &area, const Box &area)
bool sourceAreaIsOutOfBounds)
{ {
if (doesSubImageNeedInit(context, imageIndex, area, sourceAreaIsOutOfBounds)) if (doesSubImageNeedInit(context, imageIndex, area))
{ {
// NOTE: do not optimize this to only initialize the passed area of the texture, or the // NOTE: do not optimize this to only initialize the passed area of the texture, or the
// initialization logic in copySubImage will be incorrect. // initialization logic in copySubImage will be incorrect.
......
src/libANGLE/Texture.h
...@@ -579,12 +579,10 @@ class Texture final : public RefCountObject<TextureID>, ...@@ -579,12 +579,10 @@ class Texture final : public RefCountObject<TextureID>,
bool doesSubImageNeedInit(const Context *context, bool doesSubImageNeedInit(const Context *context,
const ImageIndex &imageIndex, const ImageIndex &imageIndex,
const Box &area, const Box &area) const;
bool sourceAreaIsOutOfBounds) const;
angle::Result ensureSubImageInitialized(const Context *context, angle::Result ensureSubImageInitialized(const Context *context,
const ImageIndex &imageIndex, const ImageIndex &imageIndex,
const Box &area, const Box &area);
bool sourceAreaIsOutOfBounds);
angle::Result handleMipmapGenerationHint(Context *context, int level); angle::Result handleMipmapGenerationHint(Context *context, int level);
......
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