Use high precision vertex attributes.

Use high precision vertex attributes.

Specifying vertex attributes and the view matrix as mediump causes bad artifacts on Mali T6XX.

Covered by the bigmatrix GM in msaa4 mode.

BUG:skia=3545,3771

Committed: https://skia.googlesource.com/skia/+/f2539d50f911914af0f80f0092ff8c654869e650

[Stephen White, 2015-05-20 19:11:29]

senorblanco@chromium.org changed reviewers:
+ bsalomon@google.com

[Stephen White, 2015-05-20 19:11:53]

If this change causes perf regressions on non-Mali platforms, we should consider
limiting the change to Mali-only.

[Stephen White, 2015-05-20 19:41:15]

New patch up: limit highp to only inPosition in default GeoProcFactory.

[bsalomon, 2015-05-20 19:46:21]

bsalomon@google.com changed reviewers:
+ joshualitt@google.com

[bsalomon, 2015-05-20 19:46:21]

Passing the buck on this one to Joshua as he knows the builder and GP code
better than I do.

[bsalomon, 2015-05-20 19:46:22]

Passing the buck on this one to Joshua as he knows the builder and GP code
better than I do.

[joshualitt, 2015-05-20 19:54:40]

On 2015/05/20 19:46:22, bsalomon wrote:
> Passing the buck on this one to Joshua as he knows the builder and GP code
> better than I do.

What code path is triggering this? It'd be nice to narrow it down so we can make
it an option to GrDefaultGeoProcFactory::Create, and then only enable it when we
need it.

[Stephen White, 2015-05-20 20:08:53]

On 2015/05/20 19:54:40, joshualitt wrote:
> On 2015/05/20 19:46:22, bsalomon wrote:
> > Passing the buck on this one to Joshua as he knows the builder and GP code
> > better than I do.
> 
> What code path is triggering this? It'd be nice to narrow it down so we can
make
> it an option to GrDefaultGeoProcFactory::Create, and then only enable it when
we
> need it.

I noticed the artifacts in both the tessellating path renderer, as well as the
stencil-and-cover
one. Honestly, I suspect we'd have the problem in any shader that multiplies the
view matrix by
the position. This includes some shaders not modified here, including
AAConvexPathRenderer, 
Oval, Bezier, BitmapText, Dashing, DistanceFieldGeoProc (just guessing by the
fact that they
have an attribute called inPosition).

[Stephen White, 2015-05-20 20:33:50]

New patch up: also modify oval, beziereffect and GrDistanceFieldGeoProc to use
highp for inPosition.

[joshualitt, 2015-05-20 20:36:11]

nits

https://codereview.chromium.org/1145273002/diff/40001/src/gpu/GrDefaultGeoPro...
File src/gpu/GrDefaultGeoProcFactory.cpp (right):

https://codereview.chromium.org/1145273002/diff/40001/src/gpu/GrDefaultGeoPro...
src/gpu/GrDefaultGeoProcFactory.cpp:196: fInPosition =
&this->addVertexAttrib(Attribute("inPosition", kVec2f_GrVertexAttribType,
kHigh_GrSLPrecision));
newline

https://codereview.chromium.org/1145273002/diff/40001/src/gpu/GrOvalRenderer.cpp
File src/gpu/GrOvalRenderer.cpp (right):

https://codereview.chromium.org/1145273002/diff/40001/src/gpu/GrOvalRenderer....
src/gpu/GrOvalRenderer.cpp:186: fInPosition =
&this->addVertexAttrib(Attribute("inPosition", kVec2f_GrVertexAttribType,
kHigh_GrSLPrecision));
newline

https://codereview.chromium.org/1145273002/diff/40001/src/gpu/GrOvalRenderer....
src/gpu/GrOvalRenderer.cpp:559: fInPosition =
&this->addVertexAttrib(Attribute("inPosition", kVec2f_GrVertexAttribType,
kHigh_GrSLPrecision));
newline

https://codereview.chromium.org/1145273002/diff/40001/src/gpu/GrPrimitiveProc...
File src/gpu/GrPrimitiveProcessor.h (right):

https://codereview.chromium.org/1145273002/diff/40001/src/gpu/GrPrimitiveProc...
src/gpu/GrPrimitiveProcessor.h:129: Attribute(const char* name,
GrVertexAttribType type, GrSLPrecision precision = kDefault_GrSLPrecision)
newline

https://codereview.chromium.org/1145273002/diff/40001/src/gpu/effects/GrBezie...
File src/gpu/effects/GrBezierEffect.cpp (right):

https://codereview.chromium.org/1145273002/diff/40001/src/gpu/effects/GrBezie...
src/gpu/effects/GrBezierEffect.cpp:203: fInPosition =
&this->addVertexAttrib(Attribute("inPosition", kVec2f_GrVertexAttribType,
kHigh_GrSLPrecision));
newline

https://codereview.chromium.org/1145273002/diff/40001/src/gpu/effects/GrBezie...
src/gpu/effects/GrBezierEffect.cpp:404: fInPosition =
&this->addVertexAttrib(Attribute("inPosition", kVec2f_GrVertexAttribType,
kHigh_GrSLPrecision));
newline

https://codereview.chromium.org/1145273002/diff/40001/src/gpu/effects/GrBezie...
src/gpu/effects/GrBezierEffect.cpp:617: fInPosition =
&this->addVertexAttrib(Attribute("inPosition", kVec2f_GrVertexAttribType,
kHigh_GrSLPrecision));
newline

https://codereview.chromium.org/1145273002/diff/40001/src/gpu/effects/GrDista...
File src/gpu/effects/GrDistanceFieldGeoProc.cpp (right):

https://codereview.chromium.org/1145273002/diff/40001/src/gpu/effects/GrDista...
src/gpu/effects/GrDistanceFieldGeoProc.cpp:208: fInPosition =
&this->addVertexAttrib(Attribute("inPosition", kVec2f_GrVertexAttribType,
kHigh_GrSLPrecision));
newline

https://codereview.chromium.org/1145273002/diff/40001/src/gpu/effects/GrDista...
src/gpu/effects/GrDistanceFieldGeoProc.cpp:423: fInPosition =
&this->addVertexAttrib(Attribute("inPosition", kVec2f_GrVertexAttribType,
kHigh_GrSLPrecision));
newline

https://codereview.chromium.org/1145273002/diff/40001/src/gpu/effects/GrDista...
src/gpu/effects/GrDistanceFieldGeoProc.cpp:677: fInPosition =
&this->addVertexAttrib(Attribute("inPosition", kVec2f_GrVertexAttribType,
kHigh_GrSLPrecision));
newline

[Stephen White, 2015-05-20 20:47:43]

New patch up: PTAL. Thanks!

https://codereview.chromium.org/1145273002/diff/40001/src/gpu/GrDefaultGeoPro...
File src/gpu/GrDefaultGeoProcFactory.cpp (right):

https://codereview.chromium.org/1145273002/diff/40001/src/gpu/GrDefaultGeoPro...
src/gpu/GrDefaultGeoProcFactory.cpp:196: fInPosition =
&this->addVertexAttrib(Attribute("inPosition", kVec2f_GrVertexAttribType,
kHigh_GrSLPrecision));
On 2015/05/20 20:36:10, joshualitt wrote:
> newline

Done.

https://codereview.chromium.org/1145273002/diff/40001/src/gpu/GrOvalRenderer.cpp
File src/gpu/GrOvalRenderer.cpp (right):

https://codereview.chromium.org/1145273002/diff/40001/src/gpu/GrOvalRenderer....
src/gpu/GrOvalRenderer.cpp:186: fInPosition =
&this->addVertexAttrib(Attribute("inPosition", kVec2f_GrVertexAttribType,
kHigh_GrSLPrecision));
On 2015/05/20 20:36:10, joshualitt wrote:
> newline

Done.

https://codereview.chromium.org/1145273002/diff/40001/src/gpu/GrOvalRenderer....
src/gpu/GrOvalRenderer.cpp:559: fInPosition =
&this->addVertexAttrib(Attribute("inPosition", kVec2f_GrVertexAttribType,
kHigh_GrSLPrecision));
On 2015/05/20 20:36:10, joshualitt wrote:
> newline

Done.

https://codereview.chromium.org/1145273002/diff/40001/src/gpu/GrPrimitiveProc...
File src/gpu/GrPrimitiveProcessor.h (right):

https://codereview.chromium.org/1145273002/diff/40001/src/gpu/GrPrimitiveProc...
src/gpu/GrPrimitiveProcessor.h:129: Attribute(const char* name,
GrVertexAttribType type, GrSLPrecision precision = kDefault_GrSLPrecision)
On 2015/05/20 20:36:10, joshualitt wrote:
> newline

Done.

https://codereview.chromium.org/1145273002/diff/40001/src/gpu/effects/GrBezie...
File src/gpu/effects/GrBezierEffect.cpp (right):

https://codereview.chromium.org/1145273002/diff/40001/src/gpu/effects/GrBezie...
src/gpu/effects/GrBezierEffect.cpp:203: fInPosition =
&this->addVertexAttrib(Attribute("inPosition", kVec2f_GrVertexAttribType,
kHigh_GrSLPrecision));
On 2015/05/20 20:36:11, joshualitt wrote:
> newline

Done.

https://codereview.chromium.org/1145273002/diff/40001/src/gpu/effects/GrBezie...
src/gpu/effects/GrBezierEffect.cpp:404: fInPosition =
&this->addVertexAttrib(Attribute("inPosition", kVec2f_GrVertexAttribType,
kHigh_GrSLPrecision));
On 2015/05/20 20:36:10, joshualitt wrote:
> newline

Done.

https://codereview.chromium.org/1145273002/diff/40001/src/gpu/effects/GrBezie...
src/gpu/effects/GrBezierEffect.cpp:617: fInPosition =
&this->addVertexAttrib(Attribute("inPosition", kVec2f_GrVertexAttribType,
kHigh_GrSLPrecision));
On 2015/05/20 20:36:10, joshualitt wrote:
> newline

Done.

https://codereview.chromium.org/1145273002/diff/40001/src/gpu/effects/GrDista...
File src/gpu/effects/GrDistanceFieldGeoProc.cpp (right):

https://codereview.chromium.org/1145273002/diff/40001/src/gpu/effects/GrDista...
src/gpu/effects/GrDistanceFieldGeoProc.cpp:208: fInPosition =
&this->addVertexAttrib(Attribute("inPosition", kVec2f_GrVertexAttribType,
kHigh_GrSLPrecision));
On 2015/05/20 20:36:11, joshualitt wrote:
> newline

Done.

https://codereview.chromium.org/1145273002/diff/40001/src/gpu/effects/GrDista...
src/gpu/effects/GrDistanceFieldGeoProc.cpp:423: fInPosition =
&this->addVertexAttrib(Attribute("inPosition", kVec2f_GrVertexAttribType,
kHigh_GrSLPrecision));
On 2015/05/20 20:36:11, joshualitt wrote:
> newline

Done.

https://codereview.chromium.org/1145273002/diff/40001/src/gpu/effects/GrDista...
src/gpu/effects/GrDistanceFieldGeoProc.cpp:677: fInPosition =
&this->addVertexAttrib(Attribute("inPosition", kVec2f_GrVertexAttribType,
kHigh_GrSLPrecision));
On 2015/05/20 20:36:11, joshualitt wrote:
> newline

Done.

[joshualitt, 2015-05-20 20:49:23]

On 2015/05/20 20:47:43, Stephen White wrote:
> New patch up: PTAL. Thanks!
> 
>
https://codereview.chromium.org/1145273002/diff/40001/src/gpu/GrDefaultGeoPro...
> File src/gpu/GrDefaultGeoProcFactory.cpp (right):
> 
>
https://codereview.chromium.org/1145273002/diff/40001/src/gpu/GrDefaultGeoPro...
> src/gpu/GrDefaultGeoProcFactory.cpp:196: fInPosition =
> &this->addVertexAttrib(Attribute("inPosition", kVec2f_GrVertexAttribType,
> kHigh_GrSLPrecision));
> On 2015/05/20 20:36:10, joshualitt wrote:
> > newline
> 
> Done.
> 
>
https://codereview.chromium.org/1145273002/diff/40001/src/gpu/GrOvalRenderer.cpp
> File src/gpu/GrOvalRenderer.cpp (right):
> 
>
https://codereview.chromium.org/1145273002/diff/40001/src/gpu/GrOvalRenderer....
> src/gpu/GrOvalRenderer.cpp:186: fInPosition =
> &this->addVertexAttrib(Attribute("inPosition", kVec2f_GrVertexAttribType,
> kHigh_GrSLPrecision));
> On 2015/05/20 20:36:10, joshualitt wrote:
> > newline
> 
> Done.
> 
>
https://codereview.chromium.org/1145273002/diff/40001/src/gpu/GrOvalRenderer....
> src/gpu/GrOvalRenderer.cpp:559: fInPosition =
> &this->addVertexAttrib(Attribute("inPosition", kVec2f_GrVertexAttribType,
> kHigh_GrSLPrecision));
> On 2015/05/20 20:36:10, joshualitt wrote:
> > newline
> 
> Done.
> 
>
https://codereview.chromium.org/1145273002/diff/40001/src/gpu/GrPrimitiveProc...
> File src/gpu/GrPrimitiveProcessor.h (right):
> 
>
https://codereview.chromium.org/1145273002/diff/40001/src/gpu/GrPrimitiveProc...
> src/gpu/GrPrimitiveProcessor.h:129: Attribute(const char* name,
> GrVertexAttribType type, GrSLPrecision precision = kDefault_GrSLPrecision)
> On 2015/05/20 20:36:10, joshualitt wrote:
> > newline
> 
> Done.
> 
>
https://codereview.chromium.org/1145273002/diff/40001/src/gpu/effects/GrBezie...
> File src/gpu/effects/GrBezierEffect.cpp (right):
> 
>
https://codereview.chromium.org/1145273002/diff/40001/src/gpu/effects/GrBezie...
> src/gpu/effects/GrBezierEffect.cpp:203: fInPosition =
> &this->addVertexAttrib(Attribute("inPosition", kVec2f_GrVertexAttribType,
> kHigh_GrSLPrecision));
> On 2015/05/20 20:36:11, joshualitt wrote:
> > newline
> 
> Done.
> 
>
https://codereview.chromium.org/1145273002/diff/40001/src/gpu/effects/GrBezie...
> src/gpu/effects/GrBezierEffect.cpp:404: fInPosition =
> &this->addVertexAttrib(Attribute("inPosition", kVec2f_GrVertexAttribType,
> kHigh_GrSLPrecision));
> On 2015/05/20 20:36:10, joshualitt wrote:
> > newline
> 
> Done.
> 
>
https://codereview.chromium.org/1145273002/diff/40001/src/gpu/effects/GrBezie...
> src/gpu/effects/GrBezierEffect.cpp:617: fInPosition =
> &this->addVertexAttrib(Attribute("inPosition", kVec2f_GrVertexAttribType,
> kHigh_GrSLPrecision));
> On 2015/05/20 20:36:10, joshualitt wrote:
> > newline
> 
> Done.
> 
>
https://codereview.chromium.org/1145273002/diff/40001/src/gpu/effects/GrDista...
> File src/gpu/effects/GrDistanceFieldGeoProc.cpp (right):
> 
>
https://codereview.chromium.org/1145273002/diff/40001/src/gpu/effects/GrDista...
> src/gpu/effects/GrDistanceFieldGeoProc.cpp:208: fInPosition =
> &this->addVertexAttrib(Attribute("inPosition", kVec2f_GrVertexAttribType,
> kHigh_GrSLPrecision));
> On 2015/05/20 20:36:11, joshualitt wrote:
> > newline
> 
> Done.
> 
>
https://codereview.chromium.org/1145273002/diff/40001/src/gpu/effects/GrDista...
> src/gpu/effects/GrDistanceFieldGeoProc.cpp:423: fInPosition =
> &this->addVertexAttrib(Attribute("inPosition", kVec2f_GrVertexAttribType,
> kHigh_GrSLPrecision));
> On 2015/05/20 20:36:11, joshualitt wrote:
> > newline
> 
> Done.
> 
>
https://codereview.chromium.org/1145273002/diff/40001/src/gpu/effects/GrDista...
> src/gpu/effects/GrDistanceFieldGeoProc.cpp:677: fInPosition =
> &this->addVertexAttrib(Attribute("inPosition", kVec2f_GrVertexAttribType,
> kHigh_GrSLPrecision));
> On 2015/05/20 20:36:11, joshualitt wrote:
> > newline
> 
> Done.

lgtm

[Stephen White, 2015-05-20 20:57:07]

The CQ bit was checked by senorblanco@chromium.org

[commit-bot: I haz the power, 2015-05-20 20:57:31]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1145273002/60001

[commit-bot: I haz the power, 2015-05-20 21:03:47]

Committed patchset #4 (id:60001) as
https://skia.googlesource.com/skia/+/f2539d50f911914af0f80f0092ff8c654869e650