From 29870bb4f7ecd9b7a23acf73f2019a3f45c6af0f Mon Sep 17 00:00:00 2001 From: Kai Ninomiya Date: Mon, 21 Mar 2022 17:08:25 -0700 Subject: [PATCH] copyToTexture tests: remove paintOpaqueRects workaround and make color_space_conversion tests work without it --- .../web_platform/copyToTexture/canvas.spec.ts | 85 +++---------------- 1 file changed, 12 insertions(+), 73 deletions(-) diff --git a/src/webgpu/web_platform/copyToTexture/canvas.spec.ts b/src/webgpu/web_platform/copyToTexture/canvas.spec.ts index 916e4fef8d0a..f22b15ac2111 100644 --- a/src/webgpu/web_platform/copyToTexture/canvas.spec.ts +++ b/src/webgpu/web_platform/copyToTexture/canvas.spec.ts @@ -12,12 +12,10 @@ class F extends CopyToTextureUtils { init2DCanvasContentWithColorSpace({ width, height, - paintOpaqueRects, colorSpace, }: { width: number; height: number; - paintOpaqueRects: boolean; colorSpace: 'srgb' | 'display-p3'; }): { canvas: HTMLCanvasElement | OffscreenCanvas; @@ -45,7 +43,7 @@ class F extends CopyToTextureUtils { const rectWidth = Math.floor(width / 2); const rectHeight = Math.floor(height / 2); - const alphaValue = paintOpaqueRects ? 255 : 153; + const alphaValue = 153; let pixelStartPos = 0; // Red; @@ -110,12 +108,10 @@ class F extends CopyToTextureUtils { canvasType, width, height, - paintOpaqueRects, }: { canvasType: canvasTypes; width: number; height: number; - paintOpaqueRects: boolean; }): { canvas: HTMLCanvasElement | OffscreenCanvas; canvasContext: CanvasRenderingContext2D | OffscreenCanvasRenderingContext2D; @@ -132,12 +128,8 @@ class F extends CopyToTextureUtils { this.skip(canvasType + ' canvas 2d context not available'); } - // The rgb10a2unorm dst texture will have tiny errors when we compare actual and expectation. - // This is due to the convert from 8-bit to 10-bit combined with alpha value ops. So for - // rgb10a2unorm dst textures, we'll set alphaValue to 1.0 to test. - const alphaValue = paintOpaqueRects ? 1.0 : 0.6; const ctx = canvasContext as CanvasRenderingContext2D | OffscreenCanvasRenderingContext2D; - this.paint2DCanvas(ctx, width, height, alphaValue); + this.paint2DCanvas(ctx, width, height, 0.6); return { canvas, canvasContext }; } @@ -172,14 +164,12 @@ class F extends CopyToTextureUtils { width, height, premultiplied, - paintOpaqueRects, }: { canvasType: canvasTypes; contextName: 'webgl' | 'webgl2'; width: number; height: number; premultiplied: boolean; - paintOpaqueRects: boolean; }): { canvas: HTMLCanvasElement | OffscreenCanvas; canvasContext: WebGLRenderingContext | WebGL2RenderingContext; @@ -200,7 +190,7 @@ class F extends CopyToTextureUtils { const rectWidth = Math.floor(width / 2); const rectHeight = Math.floor(height / 2); - const alphaValue = paintOpaqueRects ? 1.0 : 0.6; + const alphaValue = 0.6; const colorValue = premultiplied ? alphaValue : 1.0; // For webgl/webgl2 context canvas, if the context created with premultipliedAlpha attributes, @@ -227,16 +217,11 @@ class F extends CopyToTextureUtils { return { canvas, canvasContext: gl }; } - getInitGPUCanvasData( - width: number, - height: number, - premultiplied: boolean, - paintOpaqueRects: boolean - ): Uint8ClampedArray { + getInitGPUCanvasData(width: number, height: number, premultiplied: boolean): Uint8ClampedArray { const rectWidth = Math.floor(width / 2); const rectHeight = Math.floor(height / 2); - const alphaValue = paintOpaqueRects ? 255 : 153; + const alphaValue = 153; const colorValue = premultiplied ? alphaValue : 255; // BGRA8Unorm texture @@ -287,14 +272,12 @@ class F extends CopyToTextureUtils { width, height, premultiplied, - paintOpaqueRects, }: { device: GPUDevice; canvasType: canvasTypes; width: number; height: number; premultiplied: boolean; - paintOpaqueRects: boolean; }): { canvas: HTMLCanvasElement | OffscreenCanvas; } { @@ -316,7 +299,7 @@ class F extends CopyToTextureUtils { }); // BGRA8Unorm texture - const initialData = this.getInitGPUCanvasData(width, height, premultiplied, paintOpaqueRects); + const initialData = this.getInitGPUCanvasData(width, height, premultiplied); const canvasTexture = gpuContext.getCurrentTexture(); device.queue.writeTexture( { texture: canvasTexture }, @@ -360,17 +343,11 @@ class F extends CopyToTextureUtils { calculateSourceContentOnCPU( width: number, height: number, - premultipliedAlpha: boolean, - paintOpaqueRects: boolean + premultipliedAlpha: boolean ): Uint8ClampedArray { const bytesPerPixel = 4; - const rgbaPixels = this.getInitGPUCanvasData( - width, - height, - premultipliedAlpha, - paintOpaqueRects - ); + const rgbaPixels = this.getInitGPUCanvasData(width, height, premultipliedAlpha); // The source canvas has bgra8unorm back resource. We // swizzle the channels to align with 2d/webgl canvas and @@ -421,7 +398,6 @@ g.test('copy_contents_from_2d_context_canvas') - Valid dest alphaMode - Valid 'flipY' config in 'GPUImageCopyExternalImage' (named 'srcDoFlipYDuringCopy' in cases) - TODO(#913): color space tests need to be added - - TODO: Add error tolerance for rgb10a2unorm dst texture format And the expected results are all passed. ` @@ -446,20 +422,10 @@ g.test('copy_contents_from_2d_context_canvas') srcDoFlipYDuringCopy, } = t.params; - // When dst texture format is rgb10a2unorm, the generated expected value of the result - // may have tiny errors compared to the actual result when the channel value is - // not 1.0 or 0.0. - // For example, we init the pixel with r channel to 0.6. And the denormalized value for - // 10-bit channel is 613.8, which needs to call "round" or other function to get an integer. - // It is possible that gpu adopt different "round" as our cpu implementation(we use Math.round()) - // and it will generate tiny errors. - // So the cases with rgb10a2unorm dst texture format are handled specially by painting opaque rects - // to ensure they will have stable result after alphaOps(should keep the same value). const { canvas, canvasContext } = t.init2DCanvasContent({ canvasType, width, height, - paintOpaqueRects: dstColorFormat === 'rgb10a2unorm', }); const dst = t.device.createTexture({ @@ -535,7 +501,6 @@ g.test('copy_contents_from_gl_context_canvas') - Valid dest alphaMode - Valid 'flipY' config in 'GPUImageCopyExternalImage'(named 'srcDoFlipYDuringCopy' in cases) - TODO: color space tests need to be added - - TODO: Add error tolerance for rgb10a2unorm dst texture format And the expected results are all passed. ` @@ -564,22 +529,12 @@ g.test('copy_contents_from_gl_context_canvas') srcDoFlipYDuringCopy, } = t.params; - // When dst texture format is rgb10a2unorm, the generated expected value of the result - // may have tiny errors compared to the actual result when the channel value is - // not 1.0 or 0.0. - // For example, we init the pixel with r channel to 0.6. And the denormalized value for - // 10-bit channel is 613.8, which needs to call "round" or other function to get an integer. - // It is possible that gpu adopt different "round" as our cpu implementation(we use Math.round()) - // and it will generate tiny errors. - // So the cases with rgb10a2unorm dst texture format are handled specially by by painting opaque rects - // to ensure they will have stable result after alphaOps(should keep the same value). const { canvas, canvasContext } = t.initGLCanvasContent({ canvasType, contextName, width, height, premultiplied: srcPremultiplied, - paintOpaqueRects: dstColorFormat === 'rgb10a2unorm', }); const dst = t.device.createTexture({ @@ -650,7 +605,6 @@ g.test('copy_contents_from_gpu_context_canvas') - Valid dest alphaMode - Valid 'flipY' config in 'GPUImageCopyExternalImage'(named 'srcDoFlipYDuringCopy' in cases) - TODO: color space tests need to be added - - TODO: Add error tolerance for rgb10a2unorm dst texture format And the expected results are all passed. ` @@ -688,22 +642,12 @@ g.test('copy_contents_from_gpu_context_canvas') device = t.device; } - // When dst texture format is rgb10a2unorm, the generated expected value of the result - // may have tiny errors compared to the actual result when the channel value is - // not 1.0 or 0.0. - // For example, we init the pixel with r channel to 0.6. And the denormalized value for - // 10-bit channel is 613.8, which needs to call "round" or other function to get an integer. - // It is possible that gpu adopt different "round" as our cpu implementation(we use Math.round()) - // and it will generate tiny errors. - // So the cases with rgb10a2unorm dst texture format are handled specially by by painting opaque rects - // to ensure they will have stable result after alphaOps(should keep the same value). const { canvas } = t.initGPUCanvasContent({ device, canvasType, width, height, premultiplied: srcPremultiplied, - paintOpaqueRects: dstColorFormat === 'rgb10a2unorm', }); const dst = t.device.createTexture({ @@ -720,12 +664,7 @@ g.test('copy_contents_from_gpu_context_canvas') // Construct expected value for different dst color format const info = kTextureFormatInfo[dstColorFormat]; const expFormat = info.baseFormat ?? dstColorFormat; - const sourcePixels = t.calculateSourceContentOnCPU( - width, - height, - srcPremultiplied, - dstColorFormat === 'rgb10a2unorm' - ); + const sourcePixels = t.calculateSourceContentOnCPU(width, height, srcPremultiplied); const expTexelView = t.getExpectedPixels( sourcePixels, width, @@ -811,7 +750,6 @@ g.test('color_space_conversion') const { canvas, canvasContext } = t.init2DCanvasContentWithColorSpace({ width, height, - paintOpaqueRects: true, colorSpace: srcColorSpace, }); @@ -828,7 +766,8 @@ g.test('color_space_conversion') sourcePixels, width, height, - dstColorFormat, + // copyExternalImageToTexture does not perform gamma-encoding into `-srgb` formats. + kTextureFormatInfo[dstColorFormat].baseFormat ?? dstColorFormat, srcDoFlipYDuringCopy, { srcPremultiplied: false, @@ -845,7 +784,7 @@ g.test('color_space_conversion') if (srcColorSpace !== dstColorSpace) { // Color space conversion seems prone to errors up to about 0.0003 on f32, 0.0007 on f16. texelCompareOptions.maxFractionalDiff = 0.001; - } else if (dstPremultiplied) { + } else { texelCompareOptions.maxDiffULPsForFloatFormat = 1; }