Fix issues with insetting and outsetting quads.

Need more degrees of freedom when moving 3D points to project to 2D
points that don't fall on the projected quad edges.

Need to check geometry subset in shader to avoid positive coverage in
outset quads with nearly parallel edges.

Bug: chromium:1177833
Change-Id: I0759382d9221ba44aacd537254e08d9f2716a6af
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/372196
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
This commit is contained in:
Brian Salomon 2021-02-22 09:32:31 -05:00 committed by Skia Commit-Bot
parent ffee4766fa
commit 4a281dc8ee
4 changed files with 134 additions and 4 deletions

98
gm/crbug_1177833.cpp Normal file
View File

@ -0,0 +1,98 @@
/*
* Copyright 2021 Google LLC
*
* Use of this source code is governed by a BSD-style license that can be
* found in the LICENSE file.
*/
#include "gm/gm.h"
#include "include/core/SkCanvas.h"
#include "include/core/SkMatrix.h"
#include "include/core/SkRect.h"
// Bad quads dumped from SkiaRenderer in crbug.com/1178833. These should all draw as really thin
// lines.
DEF_SIMPLE_GM(crbug_1177833, canvas, 400, 400) {
canvas->clear(SK_ColorBLACK);
canvas->translate(-700, -700);
// This quad had two issues. The inset collapsed the inner 2D projected quad to a point but
// didn't enable enough degrees of freedom to adjust the 4 3D points to project to that point.
// Also, the outset produced a 2D projected point far away from the original quad but the
// shader was not checking the geometric subset and so pixels far away from the projection of
// the quad would have positive coverage.
{
canvas->save();
canvas->concat(SkMatrix::MakeAll(SkBits2Float(0xbf79250e), SkBits2Float(0x3e9da860), SkBits2Float(0x44914c8a),
SkBits2Float(0xbf982962), SkBits2Float(0xbf280002), SkBits2Float(0x44c3116e),
SkBits2Float(0xba9bfe62), SkBits2Float(0x39d10455), SkBits2Float(0x3fc9b377)));
SkRect rect = {SkBits2Float(0x00000000),
SkBits2Float(0x00000000),
SkBits2Float(0x40a00000),
SkBits2Float(0x43560000)};
SkPoint clip[4] = {{SkBits2Float(0x409fff57), SkBits2Float(0x40c86a18)},
{SkBits2Float(0x409fff57), SkBits2Float(0x4314dc8c)},
{SkBits2Float(0x407f6b0d), SkBits2Float(0x43157fff)},
{SkBits2Float(0x4040859c), SkBits2Float(0x43140374)}};
SkCanvas::QuadAAFlags aaFlags = static_cast<SkCanvas::QuadAAFlags>(0x00000002);
SkColor4f color = {SkBits2Float(0x3f6eeef0),
SkBits2Float(0x3f6eeef0),
SkBits2Float(0x3f6eeef0),
SkBits2Float(0x3f800000)};
SkBlendMode mode = static_cast<SkBlendMode>(0x00000003);
canvas->experimental_DrawEdgeAAQuad(rect, clip, aaFlags, color, mode);
canvas->restore();
}
// This quad also exposed the inset collapse to a point without enough degrees of freedom issue.
canvas->save();
canvas->translate(-300, 0);
{
canvas->save();
canvas->concat(SkMatrix::MakeAll(SkBits2Float(0x3f54dd8a), SkBits2Float(0xbf9096a4), SkBits2Float(0x447eae34),
SkBits2Float(0x3f3f6905), SkBits2Float(0xbe5208ba), SkBits2Float(0x4418118b),
SkBits2Float(0x3aa134a1), SkBits2Float(0xb93ef249), SkBits2Float(0x3f580bd4)));
SkRect rect = {SkBits2Float(0x00000000),
SkBits2Float(0x00000000),
SkBits2Float(0x40a00000),
SkBits2Float(0x43560000)};
SkPoint clip[4] = {{SkBits2Float(0x40a0000e), SkBits2Float(0x40c86b5a)},
{SkBits2Float(0x40a0001e), SkBits2Float(0x4314dd5f)},
{SkBits2Float(0x407f76eb), SkBits2Float(0x431580c2)},
{SkBits2Float(0x404092e7), SkBits2Float(0x43140445)}};
SkCanvas::QuadAAFlags aaFlags = static_cast<SkCanvas::QuadAAFlags>(0x00000002);
SkColor4f color = {SkBits2Float(0x3f6eeef0),
SkBits2Float(0x3f6eeef0),
SkBits2Float(0x3f6eeef0),
SkBits2Float(0x3f800000)};
SkBlendMode mode = static_cast<SkBlendMode>(0x00000003);
canvas->experimental_DrawEdgeAAQuad(rect, clip, aaFlags, color, mode);
canvas->restore();
}
canvas->restore();
// This quad exposed a similar issue to the point issue above, but when collapsing to a
// triangle. When a 2D quad edge collapsed from insetting we'd replace it with a point off of
// its adjacent edges. We need to ensure the code that moves the 3D point that projects to
// the 2D point has 2 degrees of freedom so it can find the correct 3D point.
{
canvas->save();
canvas->concat(SkMatrix::MakeAll(SkBits2Float(0x3f54b255), SkBits2Float(0x3eb5a94d), SkBits2Float(0x443d7419),
SkBits2Float(0x3f885d66), SkBits2Float(0x3f5a6b9c), SkBits2Float(0x443c7334),
SkBits2Float(0x3aa95ea5), SkBits2Float(0xb8a1391e), SkBits2Float(0x3f84dde5)));
SkRect rect = {SkBits2Float(0x00000000),
SkBits2Float(0x00000000),
SkBits2Float(0x40a00000),
SkBits2Float(0x43100000)};
SkPoint clip[4] = {{SkBits2Float(0x405a654c), SkBits2Float(0x42e8c790)},
{SkBits2Float(0x3728c61b), SkBits2Float(0x42e7df31)},
{SkBits2Float(0xb678ecc5), SkBits2Float(0x412db4e0)},
{SkBits2Float(0x4024b2ad), SkBits2Float(0x413ab3ed)}};
SkCanvas::QuadAAFlags aaFlags = static_cast<SkCanvas::QuadAAFlags>(0x00000004);
SkColor4f color = {SkBits2Float(0x3f800000),
SkBits2Float(0x3f800000),
SkBits2Float(0x3f800000),
SkBits2Float(0x3f800000)};
SkBlendMode mode = static_cast<SkBlendMode>(0x00000003);
canvas->experimental_DrawEdgeAAQuad(rect, clip, aaFlags, color, mode);
canvas->restore();
}
}

View File

@ -117,6 +117,7 @@ gm_sources = [
"$_gm/crbug_1162942.cpp",
"$_gm/crbug_1167277.cpp",
"$_gm/crbug_1174186.cpp",
"$_gm/crbug_1177833.cpp",
"$_gm/crbug_224618.cpp",
"$_gm/crbug_691386.cpp",
"$_gm/crbug_788500.cpp",

View File

@ -27,11 +27,13 @@ static constexpr float kInvDistTolerance = 1.f / kDistTolerance;
// These rotate the points/edge values either clockwise or counterclockwise assuming tri strip
// order.
static AI V4f next_cw(const V4f& v) {
template<typename T>
static AI skvx::Vec<4, T> next_cw(const skvx::Vec<4, T>& v) {
return skvx::shuffle<2, 0, 3, 1>(v);
}
static AI V4f next_ccw(const V4f& v) {
template<typename T>
static AI skvx::Vec<4, T> next_ccw(const skvx::Vec<4, T>& v) {
return skvx::shuffle<1, 3, 0, 2>(v);
}
@ -776,6 +778,7 @@ int TessellationHelper::EdgeEquations::computeDegenerateQuad(const V4f& signedEd
0.25f * ((*y2d)[0] + (*y2d)[1] + (*y2d)[2] + (*y2d)[3])};
*x2d = center.fX;
*y2d = center.fY;
*aaMask = any(*aaMask);
return 1;
} else if (all(d1Or2)) {
// Degenerates to a line. Compare p[2] and p[3] to edge 0. If they are on the wrong side,
@ -784,10 +787,15 @@ int TessellationHelper::EdgeEquations::computeDegenerateQuad(const V4f& signedEd
// Edges 0 and 3 have crossed over, so make the line from average of (p0,p2) and (p1,p3)
*x2d = 0.5f * (skvx::shuffle<0, 1, 0, 1>(px) + skvx::shuffle<2, 3, 2, 3>(px));
*y2d = 0.5f * (skvx::shuffle<0, 1, 0, 1>(py) + skvx::shuffle<2, 3, 2, 3>(py));
// If edges 0 and 3 crossed then one must have AA but we moved both 2D points on the
// edge so we need moveTo() to be able to move both 3D points along the shared edge. So
// ensure both have AA.
*aaMask = *aaMask | M4f({1, 0, 0, 1});
} else {
// Edges 1 and 2 have crossed over, so make the line from average of (p0,p1) and (p2,p3)
*x2d = 0.5f * (skvx::shuffle<0, 0, 2, 2>(px) + skvx::shuffle<1, 1, 3, 3>(px));
*y2d = 0.5f * (skvx::shuffle<0, 0, 2, 2>(py) + skvx::shuffle<1, 1, 3, 3>(py));
*aaMask = *aaMask | M4f({0, 1, 1, 0});
}
return 2;
} else {
@ -834,8 +842,8 @@ int TessellationHelper::EdgeEquations::computeDegenerateQuad(const V4f& signedEd
// points we're computing here. If we have an AA edge and a non-AA edge we
// can only move along 1 edge, but now the point we're moving toward isn't
// on that edge. Thus, we provide an additional degree of freedom by turning
// AA on for both edges if either edge is AA.
*aaMask = *aaMask | (d1Or2 & skvx::shuffle<2, 0, 3, 1>(*aaMask));
// AA on for both edges if either edge is AA at each point.
*aaMask = *aaMask | (d1Or2 & next_cw(*aaMask)) | (next_ccw(d1Or2) & next_ccw(*aaMask));
*x2d = px;
*y2d = py;
return 3;

View File

@ -327,8 +327,18 @@ void Tessellator::append(GrQuad* deviceQuad, GrQuad* localQuad,
// a geometry subset if corners are not right angles
SkRect geomSubset;
if (fVertexSpec.requiresGeometrySubset()) {
#ifdef SK_USE_LEGACY_AA_QUAD_SUBSET
geomSubset = deviceQuad->bounds();
geomSubset.outset(0.5f, 0.5f); // account for AA expansion
#else
// Our shader code will compute zero coverage a full pixel distance outside of this
// rectangle. To strictly clip against the bounds we should inset this by 0.5. However,
// since this is a sort of back-up clipping mechanism for outset geometry far away from
// the original quad we leave an extra 0.5 pad (effectively outset the bounds by 0.5).
// This avoids applying unwanted antialiasing to non-aa edges that are along or close
// to the device space bounds.
geomSubset = deviceQuad->bounds();
#endif
}
if (aaFlags == GrQuadAAFlags::kNone) {
@ -706,6 +716,7 @@ public:
args.fFragBuilder->codeAppend("float4 geoSubset;");
args.fVaryingHandler->addPassThroughAttribute(gp.fGeomSubset, "geoSubset",
Interpolation::kCanBeFlat);
#ifdef SK_USE_LEGACY_AA_QUAD_SUBSET
args.fFragBuilder->codeAppend(
"if (coverage < 0.5) {"
" float4 dists4 = clamp(float4(1, 1, -1, -1) * "
@ -713,6 +724,18 @@ public:
" float2 dists2 = dists4.xy * dists4.zw;"
" coverage = min(coverage, dists2.x * dists2.y);"
"}");
#else
args.fFragBuilder->codeAppend(
// This is lifted from GrAARectEffect. It'd be nice if we could
// invoke a FP from a GP rather than duplicate this code.
"half xSub = min(half(sk_FragCoord.x - geoSubset.x ), 0)"
" + min(half(geoSubset.z - sk_FragCoord.x), 0);"
"half ySub = min(half(sk_FragCoord.y - geoSubset.y ), 0)"
" + min(half(geoSubset.w - sk_FragCoord.y), 0);"
"half subsetCoverage = (1 + max(xSub, -1)) *"
" (1 + max(ySub, -1));"
"coverage = min(coverage, subsetCoverage);");
#endif
}
args.fFragBuilder->codeAppendf("%s = half4(half(coverage));",