[webkit-reviews] review granted: [Bug 240790] ANGLE: ERR: TextureMtl.mm:108 (GetLayerMipIndex): \t! Unreachable reached: GetLayerMipIndex(.../TextureMtl.mm:108) : [Attachment 460053] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Kyle Piddington
<kpiddington at apple.com>'s request for review:
Bug 240790: ANGLE: ERR: TextureMtl.mm:108 (GetLayerMipIndex): \t! Unreachable
reached: GetLayerMipIndex(.../TextureMtl.mm:108)
https://bugs.webkit.org/show_bug.cgi?id=240790

Attachment 460053: Patch

https://bugs.webkit.org/attachment.cgi?id=460053&action=review




--- Comment #11 from Darin Adler <darin at apple.com> ---
Comment on attachment 460053
  --> https://bugs.webkit.org/attachment.cgi?id=460053
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=460053&action=review

> Source/ThirdParty/ANGLE/src/libANGLE/renderer/metal/mtl_resources.mm:276
> +    ASSERT(refOut);

Not sure we need this assertion, but if we do, we probably need it in *both*
overloads of MakeTexture.

> Source/ThirdParty/ANGLE/src/libANGLE/renderer/metal/mtl_resources.mm:278
> +    ANGLE_MTL_CHECK(context, newTexture && newTexture->valid(),
GL_OUT_OF_MEMORY);

Pretty sure that "new" never returns nullptr in C++, so the "newTexture &&"
part of this is not needed.

> LayoutTests/fast/canvas/webgl/large-texture-creation.html:34
> +  gl.texImage2D(gl.TEXTURE_2D,

We changed two code paths, but with only one test case we must be covering only
one of the two.

> LayoutTests/fast/canvas/webgl/large-texture-creation.html:44
> +  glErrorShouldBe(gl, [gl.NO_ERROR, gl.OUT_OF_MEMORY], "unexpected result
when creating MAX_TEXTURE_SIZE texture")

Shouldn’t it always be OUT_OF_MEMORY? Why is NO_ERROR a possibility?