Fix flipY logic in CopyTextureForBrowser()
CopyTextureForBrowser() handles flipY by flipping source texture then
applying copy op. But this is not correct. Instead
it should find the copy rect and apply flipY op in copy rect only,
which is the same as WebGPU GPUImageCopyExternalImage.flipY definition
(https://www.w3.org/TR/webgpu/#dom-gpuimagecopyexternalimage-flipy).
This CL fixed the issue and updated related end2end tests.
Bug: dawn:1635
Change-Id: I3ea7c9de44fb45224bc438486e0e92385446bfb1
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/116924
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Shaobo Yan <shaobo.yan@intel.com>
diff --git a/src/dawn/native/CopyTextureForBrowserHelper.cpp b/src/dawn/native/CopyTextureForBrowserHelper.cpp
index 011ca85..ea1039f 100644
--- a/src/dawn/native/CopyTextureForBrowserHelper.cpp
+++ b/src/dawn/native/CopyTextureForBrowserHelper.cpp
@@ -96,26 +96,7 @@
var output : VertexOutputs;
output.position = vec4<f32>((texcoord[VertexIndex] * 2.0 - vec2<f32>(1.0, 1.0)), 0.0, 1.0);
-
- // Y component of scale is calculated by the copySizeHeight / textureHeight. Only
- // flipY case can get negative number.
- var flipY = uniforms.scale.y < 0.0;
-
- // Texture coordinate takes top-left as origin point. We need to map the
- // texture to triangle carefully.
- if (flipY) {
- // We need to get the mirror positions(mirrored based on y = 0.5) on flip cases.
- // Adopt transform to src texture and then mapping it to triangle coord which
- // do a +1 shift on Y dimension will help us got that mirror position perfectly.
- output.texcoords = (texcoord[VertexIndex] * uniforms.scale + uniforms.offset) *
- vec2<f32>(1.0, -1.0) + vec2<f32>(0.0, 1.0);
- } else {
- // For the normal case, we need to get the exact position.
- // So mapping texture to triangle firstly then adopt the transform.
- output.texcoords = (texcoord[VertexIndex] *
- vec2<f32>(1.0, -1.0) + vec2<f32>(0.0, 1.0)) *
- uniforms.scale + uniforms.offset;
- }
+ output.texcoords = texcoord[VertexIndex] * uniforms.scale + uniforms.offset;
return output;
}
@@ -435,10 +416,11 @@
sourceInfo->origin.y / static_cast<float>(sourceInfo->size.height) // offset
};
- // Handle flipY. FlipY here means we flip the source texture firstly and then
- // do copy. This helps on the case which source texture is flipped and the copy
- // need to unpack the flip.
- if (options->flipY) {
+ // The NDC to framebuffer space transform maps inverts the Y coordinate such that NDC [-1, 1]
+ // (resp [-1, -1]) maps to framebuffer space [0, 0] (resp [0, height-1]). So we need to undo
+ // this flip when converting positions to texcoords.
+ // https://www.w3.org/TR/webgpu/#coordinate-systems
+ if (!options->flipY) {
uniformData.scaleY *= -1.0;
uniformData.offsetY += copySize->height / static_cast<float>(sourceInfo->size.height);
}
diff --git a/src/dawn/tests/end2end/CopyExternalTextureForBrowserTests.cpp b/src/dawn/tests/end2end/CopyExternalTextureForBrowserTests.cpp
index 07ccb93..3bcdc33 100644
--- a/src/dawn/tests/end2end/CopyExternalTextureForBrowserTests.cpp
+++ b/src/dawn/tests/end2end/CopyExternalTextureForBrowserTests.cpp
@@ -124,17 +124,19 @@
}
std::vector<utils::RGBA8> GetDefaultExpectedData(bool flipY,
- wgpu::Origin3D origin,
+ wgpu::Origin3D srcOrigin,
wgpu::Extent3D rect) {
std::vector<utils::RGBA8> expected;
- for (uint32_t row = origin.y; row < origin.y + rect.height; ++row) {
- for (uint32_t col = origin.x; col < origin.x + rect.width; ++col) {
+ for (uint32_t rowInRect = 0; rowInRect < rect.height; ++rowInRect) {
+ for (uint32_t colInRect = 0; colInRect < rect.width; ++colInRect) {
+ uint32_t row = rowInRect + srcOrigin.y;
+ uint32_t col = colInRect + srcOrigin.x;
+
if (flipY) {
- uint32_t flippedRow = kHeight - row - 1;
- expected.push_back(kDefaultSourceRGBA[flippedRow][col]);
- } else {
- expected.push_back(kDefaultSourceRGBA[row][col]);
+ row = (rect.height - rowInRect - 1) + srcOrigin.y;
}
+
+ expected.push_back(kDefaultSourceRGBA[row][col]);
}
}
@@ -182,7 +184,7 @@
};
} // anonymous namespace
-TEST_P(CopyExternalTextureForBrowserTests_Basic, FullCopy) {
+TEST_P(CopyExternalTextureForBrowserTests_Basic, Copy) {
DAWN_SUPPRESS_TEST_IF(IsOpenGLES());
DAWN_SUPPRESS_TEST_IF(IsOpenGL() && IsLinux());
diff --git a/src/dawn/tests/end2end/CopyTextureForBrowserTests.cpp b/src/dawn/tests/end2end/CopyTextureForBrowserTests.cpp
index 21b1852..2fc7893 100644
--- a/src/dawn/tests/end2end/CopyTextureForBrowserTests.cpp
+++ b/src/dawn/tests/end2end/CopyTextureForBrowserTests.cpp
@@ -280,16 +280,15 @@
all(textureLoad(dst, vec2<i32>(dstTexCoord), 0) == nonCoveredColor);
} else {
// Calculate source texture coord.
- var srcTexCoord = dstTexCoord - uniforms.dstCopyOrigin +
- uniforms.srcCopyOrigin;
- // Note that |flipY| equals flip src texture firstly and then do copy from src
- // subrect to dst subrect. This helps on blink part to handle some input texture
- // which is flipped and need to unpack flip during the copy.
- // We need to calculate the expect y coord based on this rule.
+ var srcTexCoordInRect = dstTexCoord - uniforms.dstCopyOrigin;
+
+ // Note that |flipY| equals flip src texture in copy sub rect.
if (uniforms.dstTextureFlipY == 1u) {
- srcTexCoord.y = u32(srcSize.y) - srcTexCoord.y - 1u;
+ srcTexCoordInRect.y = uniforms.copySize.y - srcTexCoordInRect.y - 1;
}
+ var srcTexCoord = srcTexCoordInRect + uniforms.srcCopyOrigin;
+
var srcColor = textureLoad(src, vec2<i32>(srcTexCoord), 0);
var dstColor = textureLoad(dst, vec2<i32>(dstTexCoord), 0);
