Revert "Reland "Add new method for storing DrawOpAtlas texture index.""

This reverts commit dea2f34f09.

Reason for revert: Seeing unexplained glitches on Pixel3 and Pixel4 in fontcache-mt.

Original change's description:
> Reland "Add new method for storing DrawOpAtlas texture index."
> 
> This is a reland of c8b2e61540
> 
> Original change's description:
> > Add new method for storing DrawOpAtlas texture index.
> > 
> > Storing the texture index in the lower bit of each texture coordinate
> > seems to have issues on certain iOS devices. Rather than do that, we
> > use the sign of the texture coordinate to act as our storage bit.
> > To manage encoding 0 we map [0, N] to [-1, -N-1] to represent a bit.
> > 
> > Change-Id: Ic588ee92cf858915a1833cf482d4b23bd11c1000
> > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/263561
> > Commit-Queue: Jim Van Verth <jvanverth@google.com>
> > Reviewed-by: Brian Osman <brianosman@google.com>
> 
> Change-Id: I901502c3d83ff9727c51ad4447b0cee733257649
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/264566
> Reviewed-by: Jim Van Verth <jvanverth@google.com>
> Commit-Queue: Jim Van Verth <jvanverth@google.com>

TBR=jvanverth@google.com,brianosman@google.com

Change-Id: I8e5cbd61ba768e5b4b6df66189239e077b7327c4
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/264653
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Commit-Queue: Jim Van Verth <jvanverth@google.com>
This commit is contained in:
Jim Van Verth 2020-01-15 21:56:46 +00:00 committed by Skia Commit-Bot
parent 9bd8351ef3
commit 7edb0eb8a4
9 changed files with 92 additions and 140 deletions

View File

@ -77,6 +77,10 @@ void SkInternalAtlasTextContext::recordDraw(const void* srcVertexData, int glyph
memcpy(vertexData, srcVertexData, vertexDataSize);
for (int i = 0; i < 4 * glyphCnt; ++i) {
auto* vertex = reinterpret_cast<SkAtlasTextRenderer::SDFVertex*>(vertexData) + i;
// GrTextContext encodes a texture index into the lower bit of each texture coord.
// This isn't expected by SkAtlasTextRenderer subclasses.
vertex->fTextureCoordX /= 2;
vertex->fTextureCoordY /= 2;
matrix.mapHomogeneousPoints(&vertex->fPosition, &vertex->fPosition, 1);
}
fDraws.append(&fArena,

View File

@ -21,10 +21,6 @@
#include "src/gpu/GrSurfaceProxyPriv.h"
#include "src/gpu/GrTracing.h"
#ifdef DUMP_ATLAS_DATA
static bool gDumpAtlasData = false;
#endif
// When proxy allocation is deferred until flush time the proxies acting as atlases require
// special handling. This is because the usage that can be determined from the ops themselves
// isn't sufficient. Independent of the ops there will be ASAP and inline uploads to the
@ -61,32 +57,9 @@ std::unique_ptr<GrDrawOpAtlas> GrDrawOpAtlas::Make(GrProxyProvider* proxyProvide
return atlas;
}
// The two bits that make up the texture index are packed into the u and v coordinate
// respectively. To represent a '1', we negate the coordinate and subtract 1 (to handle 0).
std::pair<int16_t, int16_t> GrDrawOpAtlas::PackIndexInTexCoords(int16_t u, int16_t v,
int texIndex) {
SkASSERT(texIndex >= 0 && texIndex < 4);
if (texIndex & 0x2) {
u = -u-1;
}
if (texIndex & 0x1) {
v = -v-1;
}
return std::make_pair(u, v);
}
std::tuple<int16_t, int16_t, int> GrDrawOpAtlas::UnpackIndexFromTexCoords(int16_t u, int16_t v) {
int texIndex = 0;
if (u < 0) {
u = -u-1;
texIndex |= 0x2;
}
if (v < 0) {
v = -v-1;
texIndex |= 0x1;
}
return std::make_tuple(u, v, texIndex);
}
#ifdef DUMP_ATLAS_DATA
static bool gDumpAtlasData = false;
#endif
////////////////////////////////////////////////////////////////////////////////
GrDrawOpAtlas::Plot::Plot(int pageIndex, int plotIndex, uint64_t genID, int offX, int offY,

View File

@ -99,28 +99,6 @@ public:
AllowMultitexturing allowMultitexturing,
GrDrawOpAtlas::EvictionFunc func, void* data);
/**
* Packs a texture atlas index into the signed int16 texture coordinates.
* @param u U texture coordinate
* @param v V texture coordinate
* @param texIndex index of the texture these coordinates apply to. Must be in the range [0, 3].
* @return The new u and v coordinates with the packed value
*/
static std::pair<int16_t, int16_t> PackIndexInTexCoords(int16_t u, int16_t v, int texIndex);
/**
* Unpacks a texture atlas index from signed int16 texture coordinates.
* @param u Packed U texture coordinate
* @param v Packed V texture coordinate
* @return The unpacked u and v coordinates with the texture index.
*/
static std::tuple<int16_t, int16_t, int> UnpackIndexFromTexCoords(int16_t u, int16_t v);
// Maximum texture size that can be used for atlases.
// On lower-end GPUs texture coordinates end up being half floats, which means we only
// have enough precision to represent 2048 texels.
static constexpr int kMaxTextureSize = 2048;
/**
* Adds a width x height subimage to the atlas. Upon success it returns 'kSucceeded' and returns
* the ID and the subimage's coordinates in the backing texture. 'kTryAgain' is returned if

View File

@ -15,30 +15,27 @@
#include "src/gpu/glsl/GrGLSLVertexGeoBuilder.h"
static void append_index_uv_varyings(GrGLSLPrimitiveProcessor::EmitArgs& args,
int numTextureSamplers,
const char* inTexCoordsName,
const char* atlasDimensionsInvName,
GrGLSLVarying* uv,
GrGLSLVarying* texIdx,
GrGLSLVarying* st) {
using Interpolation = GrGLSLVaryingHandler::Interpolation;
// This extracts the texture index and texel coordinates from the same variable
// Packing structure: to store an index bit, texel coordinate [0,N] is mapped to [-1,-N-1]
args.fVertBuilder->codeAppendf("float2 unormTexCoords = float2(%s.x, %s.y);",
inTexCoordsName, inTexCoordsName);
if (numTextureSamplers < 2) {
args.fVertBuilder->codeAppend("float texIdx = 0;");
// Packing structure: texel coordinates are multiplied by 2 (or shifted left 1)
// texture index is stored as lower bits of both x and y
if (args.fShaderCaps->integerSupport()) {
args.fVertBuilder->codeAppendf("int2 signedCoords = int2(%s.x, %s.y);",
inTexCoordsName, inTexCoordsName);
args.fVertBuilder->codeAppend("int texIdx = 2*(signedCoords.x & 0x1) + (signedCoords.y & 0x1);");
args.fVertBuilder->codeAppend("float2 unormTexCoords = float2(signedCoords.x/2, signedCoords.y/2);");
} else {
args.fVertBuilder->codeAppend("float indexX = 0; float indexY = 0;");
// Could possibly do this with mix() but not clear it's worth it.
args.fVertBuilder->codeAppend("if (unormTexCoords.x < 0) {");
args.fVertBuilder->codeAppend(" unormTexCoords.x = -unormTexCoords.x-1;");
args.fVertBuilder->codeAppend(" indexX = 2;");
args.fVertBuilder->codeAppend("}");
args.fVertBuilder->codeAppend("if (unormTexCoords.y < 0) {");
args.fVertBuilder->codeAppend(" unormTexCoords.y = -unormTexCoords.y-1;");
args.fVertBuilder->codeAppend(" indexY = 1;");
args.fVertBuilder->codeAppend("}");
args.fVertBuilder->codeAppend("float texIdx = indexX + indexY;");
args.fVertBuilder->codeAppendf("float2 indexTexCoords = float2(%s.x, %s.y);",
inTexCoordsName, inTexCoordsName);
args.fVertBuilder->codeAppend("float2 unormTexCoords = floor(0.5*indexTexCoords);");
args.fVertBuilder->codeAppend("float2 diff = indexTexCoords - 2.0*unormTexCoords;");
args.fVertBuilder->codeAppend("float texIdx = 2.0*diff.x + diff.y;");
}
// Multiply by 1/atlasDimensions to get normalized texture coordinates
@ -46,7 +43,9 @@ static void append_index_uv_varyings(GrGLSLPrimitiveProcessor::EmitArgs& args,
args.fVertBuilder->codeAppendf("%s = unormTexCoords * %s;", uv->vsOut(),
atlasDimensionsInvName);
args.fVaryingHandler->addVarying("TexIndex", texIdx);
args.fVaryingHandler->addVarying("TexIndex", texIdx, args.fShaderCaps->integerSupport()
? Interpolation::kMustBeFlat
: Interpolation::kCanBeFlat);
args.fVertBuilder->codeAppendf("%s = texIdx;", texIdx->vsOut());
if (st) {

View File

@ -37,9 +37,10 @@ public:
kVertex_GrShaderFlag, kFloat2_GrSLType, "AtlasSizeInv", &atlasDimensionsInvName);
GrGLSLVarying uv(kFloat2_GrSLType);
GrGLSLVarying texIdx(kFloat_GrSLType);
append_index_uv_varyings(args, btgp.numTextureSamplers(), btgp.inTextureCoords().name(),
atlasDimensionsInvName, &uv, &texIdx, nullptr);
GrSLType texIdxType = args.fShaderCaps->integerSupport() ? kInt_GrSLType : kFloat_GrSLType;
GrGLSLVarying texIdx(texIdxType);
append_index_uv_varyings(args, btgp.inTextureCoords().name(), atlasDimensionsInvName, &uv,
&texIdx, nullptr);
GrGLSLFPFragmentBuilder* fragBuilder = args.fFragBuilder;
// Setup pass through color
@ -145,8 +146,8 @@ GrBitmapTextGeoProc::GrBitmapTextGeoProc(const GrShaderCaps& caps,
fInColor = MakeColorAttribute("inColor", wideColor);
}
fInTextureCoords = {"inTextureCoords", kShort2_GrVertexAttribType,
caps.integerSupport() ? kShort2_GrSLType : kFloat2_GrSLType};
fInTextureCoords = {"inTextureCoords", kUShort2_GrVertexAttribType,
caps.integerSupport() ? kUShort2_GrSLType : kFloat2_GrSLType};
this->setVertexAttributes(&fInPosition, 3);
if (numActiveViews) {

View File

@ -67,10 +67,10 @@ public:
// add varyings
GrGLSLVarying uv(kFloat2_GrSLType);
GrGLSLVarying texIdx(kFloat_GrSLType);
GrSLType texIdxType = args.fShaderCaps->integerSupport() ? kInt_GrSLType : kFloat_GrSLType;
GrGLSLVarying texIdx(texIdxType);
GrGLSLVarying st(kFloat2_GrSLType);
append_index_uv_varyings(args, dfTexEffect.numTextureSamplers(),
dfTexEffect.inTextureCoords().name(), atlasDimensionsInvName,
append_index_uv_varyings(args, dfTexEffect.inTextureCoords().name(), atlasDimensionsInvName,
&uv, &texIdx, &st);
bool isUniformScale = (dfTexEffect.getFlags() & kUniformScale_DistanceFieldEffectMask) ==
@ -233,8 +233,8 @@ GrDistanceFieldA8TextGeoProc::GrDistanceFieldA8TextGeoProc(const GrShaderCaps& c
fInPosition = {"inPosition", kFloat2_GrVertexAttribType, kFloat2_GrSLType};
}
fInColor = {"inColor", kUByte4_norm_GrVertexAttribType, kHalf4_GrSLType };
fInTextureCoords = {"inTextureCoords", kShort2_GrVertexAttribType,
caps.integerSupport() ? kShort2_GrSLType : kFloat2_GrSLType};
fInTextureCoords = {"inTextureCoords", kUShort2_GrVertexAttribType,
caps.integerSupport() ? kUShort2_GrSLType : kFloat2_GrSLType};
this->setVertexAttributes(&fInPosition, 3);
if (numViews) {
@ -344,11 +344,11 @@ public:
&atlasDimensionsInvName);
GrGLSLVarying uv(kFloat2_GrSLType);
GrGLSLVarying texIdx(kFloat_GrSLType);
GrSLType texIdxType = args.fShaderCaps->integerSupport() ? kInt_GrSLType : kFloat_GrSLType;
GrGLSLVarying texIdx(texIdxType);
GrGLSLVarying st(kFloat2_GrSLType);
append_index_uv_varyings(args, dfPathEffect.numTextureSamplers(),
dfPathEffect.inTextureCoords().name(), atlasDimensionsInvName,
&uv, &texIdx, &st);
append_index_uv_varyings(args, dfPathEffect.inTextureCoords().name(),
atlasDimensionsInvName, &uv, &texIdx, &st);
// setup pass through color
varyingHandler->addPassThroughAttribute(dfPathEffect.inColor(), args.fOutputColor);
@ -525,8 +525,8 @@ GrDistanceFieldPathGeoProc::GrDistanceFieldPathGeoProc(const GrShaderCaps& caps,
fInPosition = {"inPosition", kFloat2_GrVertexAttribType, kFloat2_GrSLType};
fInColor = MakeColorAttribute("inColor", wideColor);
fInTextureCoords = {"inTextureCoords", kShort2_GrVertexAttribType,
caps.integerSupport() ? kShort2_GrSLType : kFloat2_GrSLType};
fInTextureCoords = {"inTextureCoords", kUShort2_GrVertexAttribType,
caps.integerSupport() ? kUShort2_GrSLType : kFloat2_GrSLType};
this->setVertexAttributes(&fInPosition, 3);
if (numViews) {
@ -649,10 +649,10 @@ public:
// set up varyings
GrGLSLVarying uv(kFloat2_GrSLType);
GrGLSLVarying texIdx(kFloat_GrSLType);
GrSLType texIdxType = args.fShaderCaps->integerSupport() ? kInt_GrSLType : kFloat_GrSLType;
GrGLSLVarying texIdx(texIdxType);
GrGLSLVarying st(kFloat2_GrSLType);
append_index_uv_varyings(args, dfTexEffect.numTextureSamplers(),
dfTexEffect.inTextureCoords().name(), atlasDimensionsInvName,
append_index_uv_varyings(args, dfTexEffect.inTextureCoords().name(), atlasDimensionsInvName,
&uv, &texIdx, &st);
GrGLSLVarying delta(kFloat_GrSLType);
@ -848,8 +848,8 @@ GrDistanceFieldLCDTextGeoProc::GrDistanceFieldLCDTextGeoProc(const GrShaderCaps&
fInPosition = {"inPosition", kFloat2_GrVertexAttribType, kFloat2_GrSLType};
}
fInColor = {"inColor", kUByte4_norm_GrVertexAttribType, kHalf4_GrSLType};
fInTextureCoords = {"inTextureCoords", kShort2_GrVertexAttribType,
caps.integerSupport() ? kShort2_GrSLType : kFloat2_GrSLType};
fInTextureCoords = {"inTextureCoords", kUShort2_GrVertexAttribType,
caps.integerSupport() ? kUShort2_GrSLType : kFloat2_GrSLType};
this->setVertexAttributes(&fInPosition, 3);
if (numViews) {

View File

@ -192,22 +192,17 @@ static void clip_quads(const SkIRect& clipRect, char* currVertex, const char* bl
// In the LCD case the color will be garbage, but we'll overwrite it with the texcoords
// and it avoids a lot of conditionals.
auto color = *reinterpret_cast<const SkColor*>(blobVertices + sizeof(SkPoint));
size_t coordOffset = vertexStride - 2*sizeof(int16_t);
auto* blobCoordsLT = reinterpret_cast<const int16_t*>(blobVertices + coordOffset);
auto* blobCoordsRB = reinterpret_cast<const int16_t*>(blobVertices + 3 * vertexStride +
size_t coordOffset = vertexStride - 2*sizeof(uint16_t);
auto* blobCoordsLT = reinterpret_cast<const uint16_t*>(blobVertices + coordOffset);
auto* blobCoordsRB = reinterpret_cast<const uint16_t*>(blobVertices + 3 * vertexStride +
coordOffset);
// Pull out the texel coordinates and texture index bits
int16_t coordsRectL = blobCoordsLT[0];
int16_t coordsRectT = blobCoordsLT[1];
int16_t coordsRectR = blobCoordsRB[0];
int16_t coordsRectB = blobCoordsRB[1];
int index0, index1;
std::tie(coordsRectL, coordsRectT, index0) =
GrDrawOpAtlas::UnpackIndexFromTexCoords(coordsRectL, coordsRectT);
std::tie(coordsRectR, coordsRectB, index1) =
GrDrawOpAtlas::UnpackIndexFromTexCoords(coordsRectR, coordsRectB);
SkASSERT(index0 == index1);
uint16_t coordsRectL = blobCoordsLT[0] >> 1;
uint16_t coordsRectT = blobCoordsLT[1] >> 1;
uint16_t coordsRectR = blobCoordsRB[0] >> 1;
uint16_t coordsRectB = blobCoordsRB[1] >> 1;
uint16_t pageIndexX = blobCoordsLT[0] & 0x1;
uint16_t pageIndexY = blobCoordsLT[1] & 0x1;
int positionRectWidth = positionRect.width();
int positionRectHeight = positionRect.height();
@ -233,10 +228,10 @@ static void clip_quads(const SkIRect& clipRect, char* currVertex, const char* bl
positionRect.fBottom -= delta;
// Repack texel coordinates and index
std::tie(coordsRectL, coordsRectT) =
GrDrawOpAtlas::PackIndexInTexCoords(coordsRectL, coordsRectT, index0);
std::tie(coordsRectR, coordsRectB) =
GrDrawOpAtlas::PackIndexInTexCoords(coordsRectR, coordsRectB, index1);
coordsRectL = coordsRectL << 1 | pageIndexX;
coordsRectT = coordsRectT << 1 | pageIndexY;
coordsRectR = coordsRectR << 1 | pageIndexX;
coordsRectB = coordsRectB << 1 | pageIndexY;
// Set new positions and coords
SkPoint* currPosition = reinterpret_cast<SkPoint*>(currVertex);

View File

@ -625,20 +625,17 @@ private:
shapeData->fBounds.fRight /= scale;
shapeData->fBounds.fBottom /= scale;
// Pack the page index into the u and v texture coords
// We pack the 2bit page index in the low bit of the u and v texture coords
uint16_t pageIndex = GrDrawOpAtlas::GetPageIndexFromID(id);
SkASSERT(pageIndex < 4);
int16_t left, top, right, bottom;
std::tie(left, top, right, bottom) =
std::make_tuple(atlasLocation.fX + SK_DistanceFieldPad,
atlasLocation.fY + SK_DistanceFieldPad,
atlasLocation.fX + SK_DistanceFieldPad + devPathBounds.width(),
atlasLocation.fY + SK_DistanceFieldPad + devPathBounds.height());
std::tie(left, top) =
GrDrawOpAtlas::PackIndexInTexCoords(left, top, pageIndex);
std::tie(right, bottom) =
GrDrawOpAtlas::PackIndexInTexCoords(right, bottom, pageIndex);
shapeData->fTextureCoords.set(left, top, right, bottom);
uint16_t uBit = (pageIndex >> 1) & 0x1;
uint16_t vBit = pageIndex & 0x1;
shapeData->fTextureCoords.set((atlasLocation.fX+SK_DistanceFieldPad) << 1 | uBit,
(atlasLocation.fY+SK_DistanceFieldPad) << 1 | vBit,
(atlasLocation.fX+SK_DistanceFieldPad+
devPathBounds.width()) << 1 | uBit,
(atlasLocation.fY+SK_DistanceFieldPad+
devPathBounds.height()) << 1 | vBit);
fShapeCache->add(shapeData);
fShapeList->addToTail(shapeData);
@ -726,18 +723,14 @@ private:
shapeData->fBounds = SkRect::Make(devPathBounds);
shapeData->fBounds.offset(-translateX, -translateY);
// Pack the page index into the u and v texture coords
// We pack the 2bit page index in the low bit of the u and v texture coords
uint16_t pageIndex = GrDrawOpAtlas::GetPageIndexFromID(id);
SkASSERT(pageIndex < 4);
int16_t left, top, right, bottom;
std::tie(left, top, right, bottom) = std::make_tuple(atlasLocation.fX, atlasLocation.fY,
atlasLocation.fX+width,
atlasLocation.fY+height);
std::tie(left, top) =
GrDrawOpAtlas::PackIndexInTexCoords(left, top, pageIndex);
std::tie(right, bottom) =
GrDrawOpAtlas::PackIndexInTexCoords(right, bottom, pageIndex);
shapeData->fTextureCoords.set(left, top, right, bottom);
uint16_t uBit = (pageIndex >> 1) & 0x1;
uint16_t vBit = pageIndex & 0x1;
shapeData->fTextureCoords.set(atlasLocation.fX << 1 | uBit, atlasLocation.fY << 1 | vBit,
(atlasLocation.fX+width) << 1 | uBit,
(atlasLocation.fY+height) << 1 | vBit);
fShapeCache->add(shapeData);
fShapeList->addToTail(shapeData);

View File

@ -231,14 +231,14 @@ void GrTextBlob::SubRun::updateTexCoords(int begin, int end) {
const size_t vertexStride = this->vertexStride();
const size_t texCoordOffset = this->texCoordOffset();
char* vertex = this->quadStart(begin);
int16_t* textureCoords = reinterpret_cast<int16_t*>(vertex + texCoordOffset);
uint16_t* textureCoords = reinterpret_cast<uint16_t*>(vertex + texCoordOffset);
for (int i = begin; i < end; i++) {
GrGlyph* glyph = this->fGlyphs[i];
SkASSERT(glyph != nullptr);
int width = glyph->fBounds.width();
int height = glyph->fBounds.height();
int16_t u0, v0, u1, v1;
uint16_t u0, v0, u1, v1;
if (this->drawAsDistanceFields()) {
u0 = glyph->fAtlasLocation.fX + SK_DistanceFieldInset;
v0 = glyph->fAtlasLocation.fY + SK_DistanceFieldInset;
@ -251,23 +251,32 @@ void GrTextBlob::SubRun::updateTexCoords(int begin, int end) {
v1 = v0 + height;
}
// We pack the 2bit page index as the sign bit of the u and v texture coords
// We pack the 2bit page index in the low bit of the u and v texture coords
uint32_t pageIndex = glyph->pageIndex();
std::tie(u0, v0) = GrDrawOpAtlas::PackIndexInTexCoords(u0, v0, pageIndex);
std::tie(u1, v1) = GrDrawOpAtlas::PackIndexInTexCoords(u1, v1, pageIndex);
SkASSERT(pageIndex < 4);
uint16_t uBit = (pageIndex >> 1u) & 0x1u;
uint16_t vBit = pageIndex & 0x1u;
u0 <<= 1u;
u0 |= uBit;
v0 <<= 1u;
v0 |= vBit;
u1 <<= 1u;
u1 |= uBit;
v1 <<= 1u;
v1 |= vBit;
textureCoords[0] = u0;
textureCoords[1] = v0;
textureCoords = SkTAddOffset<int16_t>(textureCoords, vertexStride);
textureCoords = SkTAddOffset<uint16_t>(textureCoords, vertexStride);
textureCoords[0] = u0;
textureCoords[1] = v1;
textureCoords = SkTAddOffset<int16_t>(textureCoords, vertexStride);
textureCoords = SkTAddOffset<uint16_t>(textureCoords, vertexStride);
textureCoords[0] = u1;
textureCoords[1] = v0;
textureCoords = SkTAddOffset<int16_t>(textureCoords, vertexStride);
textureCoords = SkTAddOffset<uint16_t>(textureCoords, vertexStride);
textureCoords[0] = u1;
textureCoords[1] = v1;
textureCoords = SkTAddOffset<int16_t>(textureCoords, vertexStride);
textureCoords = SkTAddOffset<uint16_t>(textureCoords, vertexStride);
}
}