Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Commit 405d4cf

Browse files
kakashidinhoCommit Bot
authored andcommitted
Metal: multiple bug fixes
- ContextMtl: triangle fan draws should call setupDraw() with original parameters. Not the modified parameters. - SurfaceMtl: should initialize metal layer's drawableSize after layer's creation. - TextureMtl & FrameBufferMtl: Fix texture copySubImage CPU path incorrectly copied unflipped area. - mtl_render_utils: Fix wrong variable name used for trifan compute pipeline cache table. - mtl_resources: Fix texture & buffer memory leaks due to missing ANGLE_MTL_AUTORELEASE. - mtl_utils: Fix viewport flipping error due to arithmetic between unsigned & signed values. These bugs were discovered during dEQP tests running. Bug: angleproject:2634 Change-Id: Ie01380910ab68a2b876718d9dac0b5b4c41b607c Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/1906608 Commit-Queue: Jamie Madill <jmadill@chromium.org> Reviewed-by: Jamie Madill <jmadill@chromium.org> Reviewed-by: Jonah Ryan-Davis <jonahr@google.com>
1 parent c2f1f61 commit 405d4cf

File tree

8 files changed

+71
-47
lines changed

8 files changed

+71
-47
lines changed

src/libANGLE/renderer/metal/ContextMtl.mm

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -150,8 +150,8 @@
150150
context, {0, static_cast<uint32_t>(count), mTriFanArraysIndexBuffer, 0}));
151151
}
152152

153-
ANGLE_TRY(setupDraw(context, gl::PrimitiveMode::Triangles, first, genIndicesCount, 1,
154-
gl::DrawElementsType::UnsignedInt, reinterpret_cast<const void *>(0)));
153+
ANGLE_TRY(setupDraw(context, gl::PrimitiveMode::TriangleFan, first, count, 1,
154+
gl::DrawElementsType::InvalidEnum, reinterpret_cast<const void *>(0)));
155155

156156
// Draw with the zero starting index buffer, shift the vertex index using baseVertex instanced
157157
// draw:
@@ -173,9 +173,8 @@
173173
context, {static_cast<uint32_t>(first), static_cast<uint32_t>(count), genIdxBuffer,
174174
genIdxBufferOffset}));
175175

176-
ANGLE_TRY(setupDraw(context, gl::PrimitiveMode::Triangles, 0, genIndicesCount, 1,
177-
gl::DrawElementsType::UnsignedInt,
178-
reinterpret_cast<const void *>(genIdxBufferOffset)));
176+
ANGLE_TRY(setupDraw(context, gl::PrimitiveMode::TriangleFan, first, count, 1,
177+
gl::DrawElementsType::InvalidEnum, reinterpret_cast<const void *>(0)));
179178

180179
mRenderEncoder.drawIndexed(MTLPrimitiveTypeTriangle, genIndicesCount, MTLIndexTypeUInt32,
181180
genIdxBuffer, genIdxBufferOffset);
@@ -258,9 +257,7 @@
258257

259258
ANGLE_TRY(mTriFanIndexBuffer.commit(this));
260259

261-
ANGLE_TRY(setupDraw(context, gl::PrimitiveMode::Triangles, 0, genIndicesCount, 1,
262-
gl::DrawElementsType::UnsignedInt,
263-
reinterpret_cast<const void *>(genIdxBufferOffset)));
260+
ANGLE_TRY(setupDraw(context, gl::PrimitiveMode::TriangleFan, 0, count, 1, type, indices));
264261

265262
mRenderEncoder.drawIndexed(MTLPrimitiveTypeTriangle, genIndicesCount, MTLIndexTypeUInt32,
266263
genIdxBuffer, genIdxBufferOffset);

src/libANGLE/renderer/metal/FrameBufferMtl.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,11 @@ class FramebufferMtl : public FramebufferImpl
9595
void onFinishedDrawingToFrameBuffer(const gl::Context *context,
9696
mtl::RenderCommandEncoder *encoder);
9797

98+
// The actual area will be adjusted based on framebuffer flipping property.
99+
gl::Rectangle getReadPixelArea(const gl::Rectangle &glArea);
100+
101+
// NOTE: this method doesn't do the flipping of area. Caller must do it if needed before
102+
// callling this. See getReadPixelsArea().
98103
angle::Result readPixelsImpl(const gl::Context *context,
99104
const gl::Rectangle &area,
100105
const PackPixelsParams &packPixelsParams,

src/libANGLE/renderer/metal/FrameBufferMtl.mm

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -188,11 +188,7 @@
188188
// nothing to read
189189
return angle::Result::Continue;
190190
}
191-
gl::Rectangle flippedArea = clippedArea;
192-
if (mFlipY)
193-
{
194-
flippedArea.y = fbRect.height - flippedArea.y - flippedArea.height;
195-
}
191+
gl::Rectangle flippedArea = getReadPixelArea(clippedArea);
196192

197193
ContextMtl *contextMtl = mtl::GetImpl(context);
198194
const gl::State &glState = context->getState();
@@ -673,6 +669,21 @@ PackPixelsParams params(flippedArea, angleFormat, outputPitch, packState.reverse
673669
return angle::Result::Continue;
674670
}
675671

672+
gl::Rectangle FramebufferMtl::getReadPixelArea(const gl::Rectangle &glArea)
673+
{
674+
RenderTargetMtl *colorReadRT = getColorReadRenderTarget();
675+
ASSERT(colorReadRT);
676+
gl::Rectangle flippedArea = glArea;
677+
if (mFlipY)
678+
{
679+
flippedArea.y =
680+
colorReadRT->getTexture()->height(static_cast<uint32_t>(colorReadRT->getLevelIndex())) -
681+
flippedArea.y - flippedArea.height;
682+
}
683+
684+
return flippedArea;
685+
}
686+
676687
angle::Result FramebufferMtl::readPixelsImpl(const gl::Context *context,
677688
const gl::Rectangle &area,
678689
const PackPixelsParams &packPixelsParams,

src/libANGLE/renderer/metal/SurfaceMtl.mm

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -263,6 +263,9 @@ void StopFrameCapture()
263263

264264
[mLayer addSublayer:mMetalLayer.get()];
265265
}
266+
267+
// ensure drawableSize is set to correct value:
268+
checkIfLayerResized();
266269
}
267270

268271
return egl::NoError();

src/libANGLE/renderer/metal/TextureMtl.mm

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -803,6 +803,7 @@ MTLColorWriteMask GetColorWriteMask(const mtl::Format &mtlFormat, bool *emulated
803803
blitParams.dstColorMask = mTexture->getColorWritableMask();
804804

805805
blitParams.src = colorReadRT->getTexture();
806+
blitParams.srcLevel = static_cast<uint32_t>(colorReadRT->getLevelIndex());
806807
blitParams.srcRect = clippedSourceArea;
807808
blitParams.srcYFlipped = framebufferMtl->flipY();
808809
blitParams.dstLuminance = internalFormat.isLUMA();
@@ -827,7 +828,7 @@ MTLColorWriteMask GetColorWriteMask(const mtl::Format &mtlFormat, bool *emulated
827828
angle::MemoryBuffer conversionRow;
828829
ANGLE_CHECK_GL_ALLOC(contextMtl, conversionRow.resize(dstRowPitch));
829830

830-
MTLRegion mtlDstRowArea = MTLRegionMake2D(clippedSourceArea.x, 0, clippedSourceArea.width, 1);
831+
MTLRegion mtlDstRowArea = MTLRegionMake2D(modifiedDestOffset.x, 0, clippedSourceArea.width, 1);
831832
gl::Rectangle srcRowArea = gl::Rectangle(clippedSourceArea.x, 0, clippedSourceArea.width, 1);
832833

833834
for (int r = 0; r < clippedSourceArea.height; ++r)
@@ -838,7 +839,8 @@ MTLColorWriteMask GetColorWriteMask(const mtl::Format &mtlFormat, bool *emulated
838839
PackPixelsParams packParams(srcRowArea, dstFormat, dstRowPitch, false, nullptr, 0);
839840

840841
// Read pixels from framebuffer to memory:
841-
ANGLE_TRY(framebufferMtl->readPixelsImpl(context, srcRowArea, packParams,
842+
gl::Rectangle flippedSrcRowArea = framebufferMtl->getReadPixelArea(srcRowArea);
843+
ANGLE_TRY(framebufferMtl->readPixelsImpl(context, flippedSrcRowArea, packParams,
842844
framebufferMtl->getColorReadRenderTarget(),
843845
conversionRow.data()));
844846

src/libANGLE/renderer/metal/mtl_render_utils.mm

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -605,7 +605,7 @@ void GetFirstLastIndicesFromClientElements(GLsizei count,
605605

606606
IndexConvesionPipelineCacheKey key = {srcType, aligned};
607607

608-
auto &cache = mIndexConversionPipelineCaches[key];
608+
auto &cache = mTriFanFromElemArrayGeneratorPipelineCaches[key];
609609

610610
if (!cache)
611611
{

src/libANGLE/renderer/metal/mtl_resources.mm

Lines changed: 36 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -161,48 +161,54 @@ void SetTextureSwizzle(ContextMtl *context,
161161
bool renderTargetOnly,
162162
bool supportTextureView)
163163
{
164-
id<MTLDevice> metalDevice = context->getMetalDevice();
165-
166-
if (mips > 1 && mips < desc.mipmapLevelCount)
164+
ANGLE_MTL_OBJC_SCOPE
167165
{
168-
desc.mipmapLevelCount = mips;
169-
}
166+
id<MTLDevice> metalDevice = context->getMetalDevice();
170167

171-
// Every texture will support being rendered for now
172-
desc.usage = 0;
168+
if (mips > 1 && mips < desc.mipmapLevelCount)
169+
{
170+
desc.mipmapLevelCount = mips;
171+
}
173172

174-
if (Format::FormatRenderable(desc.pixelFormat))
175-
{
176-
desc.usage |= MTLTextureUsageRenderTarget;
177-
}
173+
// Every texture will support being rendered for now
174+
desc.usage = 0;
178175

179-
if (!Format::FormatCPUReadable(desc.pixelFormat))
180-
{
181-
desc.resourceOptions = MTLResourceStorageModePrivate;
182-
}
176+
if (Format::FormatRenderable(desc.pixelFormat))
177+
{
178+
desc.usage |= MTLTextureUsageRenderTarget;
179+
}
183180

184-
if (!renderTargetOnly)
185-
{
186-
desc.usage = desc.usage | MTLTextureUsageShaderRead;
187-
}
181+
if (!Format::FormatCPUReadable(desc.pixelFormat))
182+
{
183+
desc.resourceOptions = MTLResourceStorageModePrivate;
184+
}
188185

189-
if (supportTextureView)
190-
{
191-
desc.usage = desc.usage | MTLTextureUsagePixelFormatView;
192-
}
186+
if (!renderTargetOnly)
187+
{
188+
desc.usage = desc.usage | MTLTextureUsageShaderRead;
189+
}
193190

194-
set([metalDevice newTextureWithDescriptor:desc]);
191+
if (supportTextureView)
192+
{
193+
desc.usage = desc.usage | MTLTextureUsagePixelFormatView;
194+
}
195+
196+
set([[metalDevice newTextureWithDescriptor:desc] ANGLE_MTL_AUTORELEASE]);
197+
}
195198
}
196199

197200
Texture::Texture(Texture *original, MTLTextureType type, NSRange mipmapLevelRange, uint32_t slice)
198201
: Resource(original)
199202
{
200-
auto view = [original->get() newTextureViewWithPixelFormat:original->pixelFormat()
201-
textureType:type
202-
levels:mipmapLevelRange
203-
slices:NSMakeRange(slice, 1)];
203+
ANGLE_MTL_OBJC_SCOPE
204+
{
205+
auto view = [original->get() newTextureViewWithPixelFormat:original->pixelFormat()
206+
textureType:type
207+
levels:mipmapLevelRange
208+
slices:NSMakeRange(slice, 1)];
204209

205-
set(view);
210+
set([view ANGLE_MTL_AUTORELEASE]);
211+
}
206212
}
207213

208214
void Texture::syncContent(ContextMtl *context)
@@ -380,7 +386,7 @@ void SetTextureSwizzle(ContextMtl *context,
380386
newBuffer = [metalDevice newBufferWithLength:size options:options];
381387
}
382388

383-
set(newBuffer);
389+
set([newBuffer ANGLE_MTL_AUTORELEASE]);
384390

385391
return angle::Result::Continue;
386392
}

src/libANGLE/renderer/metal/mtl_utils.mm

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ MTLViewport GetViewportFlipY(const gl::Rectangle &rect,
9999
MTLViewport re;
100100

101101
re.originX = rect.x;
102-
re.originY = screenHeight - rect.y1();
102+
re.originY = static_cast<double>(screenHeight) - rect.y1();
103103
re.width = rect.width;
104104
re.height = rect.height;
105105
re.znear = znear;

0 commit comments

Comments
 (0)