Revert of Use unorm shorts for texture coordinates when rendering text. (patchset #3 id:40001 of https://codereview.chromium.org/1713693002/ )
Reason for revert:
Causing issues with text on Mali 400s. Examples: largeglyphblur, imageblurtiled. It appears that there are precision problems.
Original issue's description:
> Use unorm shorts for texture coordinates when rendering text.
>
> There are a couple of reasons for this:
> - Vulkan does not guarantee conversions from integral vertex attributes
> to floating point shader variables
> - This may be faster and more precise on some platforms, as it avoids
> the aforementioned conversion and changes a multiply by a very small
> value to a multiply by a medium-sized value.
> GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1713693002
> TBR=bsalomon@google.com
>
> Committed: https://skia.googlesource.com/skia/+/e507ff0460f4f878214b9454fb5b9ab8d64d8063
TBR=joshualitt@google.com
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
Review URL: https://codereview.chromium.org/1709133003
diff --git a/src/gpu/GrBatchAtlas.h b/src/gpu/GrBatchAtlas.h
index 43dd692..61fc7e1 100644
--- a/src/gpu/GrBatchAtlas.h
+++ b/src/gpu/GrBatchAtlas.h
@@ -22,8 +22,6 @@
int numPlotsY() const { return fHeight / fPlotWidth; }
int fWidth;
int fHeight;
- int fLog2Width;
- int fLog2Height;
int fPlotWidth;
int fPlotHeight;
};
diff --git a/src/gpu/GrTest.cpp b/src/gpu/GrTest.cpp
index b4fb27b..04122e1 100644
--- a/src/gpu/GrTest.cpp
+++ b/src/gpu/GrTest.cpp
@@ -29,22 +29,16 @@
GrBatchAtlasConfig configs[3];
configs[kA8_GrMaskFormat].fWidth = dim;
configs[kA8_GrMaskFormat].fHeight = dim;
- configs[kA8_GrMaskFormat].fLog2Width = SkNextLog2(dim);
- configs[kA8_GrMaskFormat].fLog2Height = SkNextLog2(dim);
configs[kA8_GrMaskFormat].fPlotWidth = dim;
configs[kA8_GrMaskFormat].fPlotHeight = dim;
configs[kA565_GrMaskFormat].fWidth = dim;
configs[kA565_GrMaskFormat].fHeight = dim;
- configs[kA565_GrMaskFormat].fLog2Width = SkNextLog2(dim);
- configs[kA565_GrMaskFormat].fLog2Height = SkNextLog2(dim);
configs[kA565_GrMaskFormat].fPlotWidth = dim;
configs[kA565_GrMaskFormat].fPlotHeight = dim;
configs[kARGB_GrMaskFormat].fWidth = dim;
configs[kARGB_GrMaskFormat].fHeight = dim;
- configs[kARGB_GrMaskFormat].fLog2Width = SkNextLog2(dim);
- configs[kARGB_GrMaskFormat].fLog2Height = SkNextLog2(dim);
configs[kARGB_GrMaskFormat].fPlotWidth = dim;
configs[kARGB_GrMaskFormat].fPlotHeight = dim;
diff --git a/src/gpu/effects/GrBitmapTextGeoProc.cpp b/src/gpu/effects/GrBitmapTextGeoProc.cpp
index b4f6b7e..263676d 100644
--- a/src/gpu/effects/GrBitmapTextGeoProc.cpp
+++ b/src/gpu/effects/GrBitmapTextGeoProc.cpp
@@ -31,12 +31,16 @@
// compute numbers to be hardcoded to convert texture coordinates from int to float
SkASSERT(cte.numTextures() == 1);
- SkDEBUGCODE(GrTexture* atlas = cte.textureAccess(0).getTexture());
+ GrTexture* atlas = cte.textureAccess(0).getTexture();
SkASSERT(atlas && SkIsPow2(atlas->width()) && SkIsPow2(atlas->height()));
+ SkScalar recipWidth = 1.0f / atlas->width();
+ SkScalar recipHeight = 1.0f / atlas->height();
GrGLSLVertToFrag v(kVec2f_GrSLType);
varyingHandler->addVarying("TextureCoords", &v);
- vertBuilder->codeAppendf("%s = %s;", v.vsOut(),
+ vertBuilder->codeAppendf("%s = vec2(%.*f, %.*f) * %s;", v.vsOut(),
+ GR_SIGNIFICANT_POW2_DECIMAL_DIG, recipWidth,
+ GR_SIGNIFICANT_POW2_DECIMAL_DIG, recipHeight,
cte.inTextureCoords()->fName);
GrGLSLPPFragmentBuilder* fragBuilder = args.fFragBuilder;
@@ -146,7 +150,7 @@
fInColor = &this->addVertexAttrib(Attribute("inColor", kVec4ub_GrVertexAttribType));
}
fInTextureCoords = &this->addVertexAttrib(Attribute("inTextureCoords",
- kVec2us_GrVertexAttribType));
+ kVec2s_GrVertexAttribType));
this->addTextureAccess(&fTextureAccess);
}
diff --git a/src/gpu/effects/GrDistanceFieldGeoProc.cpp b/src/gpu/effects/GrDistanceFieldGeoProc.cpp
index dc01b62..50e78cc 100644
--- a/src/gpu/effects/GrDistanceFieldGeoProc.cpp
+++ b/src/gpu/effects/GrDistanceFieldGeoProc.cpp
@@ -78,20 +78,23 @@
// add varyings
GrGLSLVertToFrag recipScale(kFloat_GrSLType);
- GrGLSLVertToFrag uv(kVec2f_GrSLType);
+ GrGLSLVertToFrag st(kVec2f_GrSLType);
bool isSimilarity = SkToBool(dfTexEffect.getFlags() & kSimilarity_DistanceFieldEffectFlag);
- varyingHandler->addVarying("TextureCoords", &uv, kHigh_GrSLPrecision);
- vertBuilder->codeAppendf("%s = %s;", uv.vsOut(), dfTexEffect.inTextureCoords()->fName);
+ varyingHandler->addVarying("IntTextureCoords", &st, kHigh_GrSLPrecision);
+ vertBuilder->codeAppendf("%s = %s;", st.vsOut(), dfTexEffect.inTextureCoords()->fName);
// compute numbers to be hardcoded to convert texture coordinates from int to float
SkASSERT(dfTexEffect.numTextures() == 1);
GrTexture* atlas = dfTexEffect.textureAccess(0).getTexture();
SkASSERT(atlas && SkIsPow2(atlas->width()) && SkIsPow2(atlas->height()));
+ SkScalar recipWidth = 1.0f / atlas->width();
+ SkScalar recipHeight = 1.0f / atlas->height();
- GrGLSLVertToFrag st(kVec2f_GrSLType);
- varyingHandler->addVarying("IntTextureCoords", &st, kHigh_GrSLPrecision);
- vertBuilder->codeAppendf("%s = vec2(%d, %d) * %s;", st.vsOut(),
- atlas->width(), atlas->height(),
+ GrGLSLVertToFrag uv(kVec2f_GrSLType);
+ varyingHandler->addVarying("TextureCoords", &uv, kHigh_GrSLPrecision);
+ vertBuilder->codeAppendf("%s = vec2(%.*f, %.*f) * %s;", uv.vsOut(),
+ GR_SIGNIFICANT_POW2_DECIMAL_DIG, recipWidth,
+ GR_SIGNIFICANT_POW2_DECIMAL_DIG, recipHeight,
dfTexEffect.inTextureCoords()->fName);
// Use highp to work around aliasing issues
@@ -221,7 +224,7 @@
kHigh_GrSLPrecision));
fInColor = &this->addVertexAttrib(Attribute("inColor", kVec4ub_GrVertexAttribType));
fInTextureCoords = &this->addVertexAttrib(Attribute("inTextureCoords",
- kVec2us_GrVertexAttribType));
+ kVec2s_GrVertexAttribType));
this->addTextureAccess(&fTextureAccess);
}
@@ -519,19 +522,22 @@
// set up varyings
bool isUniformScale = SkToBool(dfTexEffect.getFlags() & kUniformScale_DistanceFieldEffectMask);
GrGLSLVertToFrag recipScale(kFloat_GrSLType);
- GrGLSLVertToFrag uv(kVec2f_GrSLType);
- varyingHandler->addVarying("TextureCoords", &uv, kHigh_GrSLPrecision);
- vertBuilder->codeAppendf("%s = %s;", uv.vsOut(), dfTexEffect.inTextureCoords()->fName);
+ GrGLSLVertToFrag st(kVec2f_GrSLType);
+ varyingHandler->addVarying("IntTextureCoords", &st, kHigh_GrSLPrecision);
+ vertBuilder->codeAppendf("%s = %s;", st.vsOut(), dfTexEffect.inTextureCoords()->fName);
// compute numbers to be hardcoded to convert texture coordinates from int to float
SkASSERT(dfTexEffect.numTextures() == 1);
GrTexture* atlas = dfTexEffect.textureAccess(0).getTexture();
SkASSERT(atlas && SkIsPow2(atlas->width()) && SkIsPow2(atlas->height()));
+ SkScalar recipWidth = 1.0f / atlas->width();
+ SkScalar recipHeight = 1.0f / atlas->height();
- GrGLSLVertToFrag st(kVec2f_GrSLType);
- varyingHandler->addVarying("IntTextureCoords", &st, kHigh_GrSLPrecision);
- vertBuilder->codeAppendf("%s = vec2(%d, %d) * %s;", st.vsOut(),
- atlas->width(), atlas->height(),
+ GrGLSLVertToFrag uv(kVec2f_GrSLType);
+ varyingHandler->addVarying("TextureCoords", &uv, kHigh_GrSLPrecision);
+ vertBuilder->codeAppendf("%s = vec2(%.*f, %.*f) * %s;", uv.vsOut(),
+ GR_SIGNIFICANT_POW2_DECIMAL_DIG, recipWidth,
+ GR_SIGNIFICANT_POW2_DECIMAL_DIG, recipHeight,
dfTexEffect.inTextureCoords()->fName);
// add frag shader code
@@ -702,7 +708,7 @@
kHigh_GrSLPrecision));
fInColor = &this->addVertexAttrib(Attribute("inColor", kVec4ub_GrVertexAttribType));
fInTextureCoords = &this->addVertexAttrib(Attribute("inTextureCoords",
- kVec2us_GrVertexAttribType));
+ kVec2s_GrVertexAttribType));
this->addTextureAccess(&fTextureAccess);
}
diff --git a/src/gpu/gl/GrGLVertexArray.cpp b/src/gpu/gl/GrGLVertexArray.cpp
index 8f3262d..2048e2c 100644
--- a/src/gpu/gl/GrGLVertexArray.cpp
+++ b/src/gpu/gl/GrGLVertexArray.cpp
@@ -21,7 +21,7 @@
{4, GR_GL_FLOAT, false}, // kVec4f_GrVertexAttribType
{1, GR_GL_UNSIGNED_BYTE, true}, // kUByte_GrVertexAttribType
{4, GR_GL_UNSIGNED_BYTE, true}, // kVec4ub_GrVertexAttribType
- {2, GR_GL_UNSIGNED_SHORT, true}, // kVec2s_GrVertexAttribType
+ {2, GR_GL_SHORT, false}, // kVec2s_GrVertexAttribType
{1, GR_GL_INT, false}, // kInt_GrVertexAttribType
{1, GR_GL_UNSIGNED_INT, false}, // kUint_GrVertexAttribType
};
@@ -32,7 +32,7 @@
GR_STATIC_ASSERT(3 == kVec4f_GrVertexAttribType);
GR_STATIC_ASSERT(4 == kUByte_GrVertexAttribType);
GR_STATIC_ASSERT(5 == kVec4ub_GrVertexAttribType);
-GR_STATIC_ASSERT(6 == kVec2us_GrVertexAttribType);
+GR_STATIC_ASSERT(6 == kVec2s_GrVertexAttribType);
GR_STATIC_ASSERT(7 == kInt_GrVertexAttribType);
GR_STATIC_ASSERT(8 == kUint_GrVertexAttribType);
diff --git a/src/gpu/text/GrAtlasTextBlob_regenInBatch.cpp b/src/gpu/text/GrAtlasTextBlob_regenInBatch.cpp
index 18eb23c..14bf4a5 100644
--- a/src/gpu/text/GrAtlasTextBlob_regenInBatch.cpp
+++ b/src/gpu/text/GrAtlasTextBlob_regenInBatch.cpp
@@ -21,7 +21,6 @@
template <bool regenPos, bool regenCol, bool regenTexCoords>
inline void regen_vertices(intptr_t vertex, const GrGlyph* glyph, size_t vertexStride,
bool useDistanceFields, SkScalar transX, SkScalar transY,
- int32_t log2Width, int32_t log2Height,
GrColor color) {
int u0, v0, u1, v1;
if (regenTexCoords) {
@@ -40,20 +39,6 @@
u1 = u0 + width;
v1 = v0 + height;
}
-
- // normalize
- u0 *= 65535;
- u0 >>= log2Width;
- u1 *= 65535;
- u1 >>= log2Width;
- v0 *= 65535;
- v0 >>= log2Height;
- v1 *= 65535;
- v1 >>= log2Height;
- SkASSERT(u0 >= 0 && u0 <= 65535);
- SkASSERT(u1 >= 0 && u1 <= 65535);
- SkASSERT(v0 >= 0 && v0 <= 65535);
- SkASSERT(v1 >= 0 && v1 <= 65535);
}
// This is a bit wonky, but sometimes we have LCD text, in which case we won't have color
@@ -74,9 +59,8 @@
}
if (regenTexCoords) {
- uint16_t* textureCoords = reinterpret_cast<uint16_t*>(vertex + texCoordOffset);
- textureCoords[0] = (uint16_t) u0;
- textureCoords[1] = (uint16_t) v0;
+ SkIPoint16* textureCoords = reinterpret_cast<SkIPoint16*>(vertex + texCoordOffset);
+ textureCoords->set(u0, v0);
}
vertex += vertexStride;
@@ -93,9 +77,8 @@
}
if (regenTexCoords) {
- uint16_t* textureCoords = reinterpret_cast<uint16_t*>(vertex + texCoordOffset);
- textureCoords[0] = (uint16_t)u0;
- textureCoords[1] = (uint16_t)v1;
+ SkIPoint16* textureCoords = reinterpret_cast<SkIPoint16*>(vertex + texCoordOffset);
+ textureCoords->set(u0, v1);
}
vertex += vertexStride;
@@ -112,9 +95,8 @@
}
if (regenTexCoords) {
- uint16_t* textureCoords = reinterpret_cast<uint16_t*>(vertex + texCoordOffset);
- textureCoords[0] = (uint16_t)u1;
- textureCoords[1] = (uint16_t)v1;
+ SkIPoint16* textureCoords = reinterpret_cast<SkIPoint16*>(vertex + texCoordOffset);
+ textureCoords->set(u1, v1);
}
vertex += vertexStride;
@@ -131,9 +113,8 @@
}
if (regenTexCoords) {
- uint16_t* textureCoords = reinterpret_cast<uint16_t*>(vertex + texCoordOffset);
- textureCoords[0] = (uint16_t)u1;
- textureCoords[1] = (uint16_t)v0;
+ SkIPoint16* textureCoords = reinterpret_cast<SkIPoint16*>(vertex + texCoordOffset);
+ textureCoords->set(u1, v0);
}
}
@@ -180,7 +161,6 @@
bool brokenRun = false;
for (int glyphIdx = 0; glyphIdx < glyphCount; glyphIdx++) {
GrGlyph* glyph = nullptr;
- int log2Width = 0, log2Height = 0;
if (regenTexCoords) {
size_t glyphOffset = glyphIdx + info->glyphStartIndex();
@@ -207,8 +187,6 @@
}
fontCache->addGlyphToBulkAndSetUseToken(info->bulkUseToken(), glyph,
target->currentToken());
- log2Width = fontCache->log2Width(info->maskFormat());
- log2Height = fontCache->log2Height(info->maskFormat());
}
intptr_t vertex = reinterpret_cast<intptr_t>(fVertices);
@@ -216,7 +194,7 @@
vertex += vertexStride * glyphIdx * GrAtlasTextBatch::kVerticesPerGlyph;
regen_vertices<regenPos, regenCol, regenTexCoords>(vertex, glyph, vertexStride,
info->drawAsDistanceFields(), transX,
- transY, log2Width, log2Height, color);
+ transY, color);
helper->incGlyphCount();
}
diff --git a/src/gpu/text/GrBatchFontCache.cpp b/src/gpu/text/GrBatchFontCache.cpp
index d99df1d..ad76e4d 100644
--- a/src/gpu/text/GrBatchFontCache.cpp
+++ b/src/gpu/text/GrBatchFontCache.cpp
@@ -48,22 +48,16 @@
// setup default atlas configs
fAtlasConfigs[kA8_GrMaskFormat].fWidth = 2048;
fAtlasConfigs[kA8_GrMaskFormat].fHeight = 2048;
- fAtlasConfigs[kA8_GrMaskFormat].fLog2Width = 11;
- fAtlasConfigs[kA8_GrMaskFormat].fLog2Height = 11;
fAtlasConfigs[kA8_GrMaskFormat].fPlotWidth = 512;
fAtlasConfigs[kA8_GrMaskFormat].fPlotHeight = 256;
fAtlasConfigs[kA565_GrMaskFormat].fWidth = 1024;
fAtlasConfigs[kA565_GrMaskFormat].fHeight = 2048;
- fAtlasConfigs[kA565_GrMaskFormat].fLog2Width = 10;
- fAtlasConfigs[kA565_GrMaskFormat].fLog2Height = 11;
fAtlasConfigs[kA565_GrMaskFormat].fPlotWidth = 256;
fAtlasConfigs[kA565_GrMaskFormat].fPlotHeight = 256;
fAtlasConfigs[kARGB_GrMaskFormat].fWidth = 1024;
fAtlasConfigs[kARGB_GrMaskFormat].fHeight = 2048;
- fAtlasConfigs[kARGB_GrMaskFormat].fLog2Width = 10;
- fAtlasConfigs[kARGB_GrMaskFormat].fLog2Height = 11;
fAtlasConfigs[kARGB_GrMaskFormat].fPlotWidth = 256;
fAtlasConfigs[kARGB_GrMaskFormat].fPlotHeight = 256;
}
diff --git a/src/gpu/text/GrBatchFontCache.h b/src/gpu/text/GrBatchFontCache.h
index 7ff1b9e..46ab1c8 100644
--- a/src/gpu/text/GrBatchFontCache.h
+++ b/src/gpu/text/GrBatchFontCache.h
@@ -171,9 +171,6 @@
return this->getAtlas(format)->atlasGeneration();
}
- int log2Width(GrMaskFormat format) { return fAtlasConfigs[format].fLog2Width; }
- int log2Height(GrMaskFormat format) { return fAtlasConfigs[format].fLog2Height; }
-
///////////////////////////////////////////////////////////////////////////
// Functions intended debug only
void dump() const;