Cleanup legacy NVPR-related definitions

Cleanup legacy NVPR-related definitions

Fixed-function NVPR codepaths were removed a while ago. Only NVPR API
version 1.3 (PathFragmentInputGen) was left working. Remove
backwards-compatibility code that was left behind.

Remove some NVPR API function typedefs that were left from initial
commits.

Remove PathCoords function pointer from GrGLInterface, it has
never been called and causes problems in the future, since it will
not be implemented in the Chromium pseudo extension.

Avoid failing interface creation even if nvprmsaaXX config is
requested but the driver is not recent enough. The SAN bots have
old driver, but try to run nvprmsaa16 configs. Instead, print
out a warning.

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

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

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

[Kimmo Kinnunen, 2015-06-18 11:48:33]

kkinnunen@nvidia.com changed reviewers:
+ joshualitt@google.com

[joshualitt, 2015-06-18 15:12:43]

On 2015/06/18 11:48:34, Kimmo Kinnunen wrote:

lgtm

[Chris Dalton, 2015-06-18 18:10:17]

cdalton@nvidia.com changed reviewers:
+ cdalton@nvidia.com

[Chris Dalton, 2015-06-18 18:10:17]

Good change. We can probably get rid of GrGLProgram::didSetData and
GrGLPathProcessor::didSetData while we're at it removing legacy code.

[Kimmo Kinnunen, 2015-06-24 11:27:12]

The CQ bit was checked by kkinnunen@nvidia.com

[commit-bot: I haz the power, 2015-06-24 11:27:19]

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

[commit-bot: I haz the power, 2015-06-24 11:27:21]

The author kkinnunen@nvidia.com has not signed Google Contributor License
Agreement. Please visit https://cla.developers.google.com to sign and manage
CLA.

[commit-bot: I haz the power, 2015-06-24 11:28:09]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-06-24 11:28:10]

Try jobs failed on following builders:
  skia_presubmit-Trybot on client.skia.fyi (JOB_FAILED,
http://build.chromium.org/p/client.skia.fyi/builders/skia_presubmit-Trybot/bu...)

[Kimmo Kinnunen, 2015-06-24 12:00:02]

kkinnunen@nvidia.com changed reviewers:
+ bsalomon@google.com

[Kimmo Kinnunen, 2015-06-24 12:00:03]

Brian, this would need your review (public api)

[bsalomon, 2015-06-24 13:49:27]

On 2015/06/24 12:00:03, Kimmo Kinnunen wrote:
> Brian, this would need your review (public api)

lgtm

[Kimmo Kinnunen, 2015-06-25 05:59:45]

The CQ bit was checked by kkinnunen@nvidia.com

[Kimmo Kinnunen, 2015-06-25 05:59:47]

The patchset sent to the CQ was uploaded after l-g-t-m from
joshualitt@google.com
Link to the patchset: https://chromiumcodereview.appspot.com/1177243004/#ps40001
(title: "address review comments")

[commit-bot: I haz the power, 2015-06-25 09:00:52]

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

[commit-bot: I haz the power, 2015-06-25 09:00:54]

The author kkinnunen@nvidia.com has not signed Google Contributor License
Agreement. Please visit https://cla.developers.google.com to sign and manage
CLA.

[commit-bot: I haz the power, 2015-06-25 09:03:31]

The author kkinnunen@nvidia.com has not signed Google Contributor License
Agreement. Please visit https://cla.developers.google.com to sign and manage
CLA.

[commit-bot: I haz the power, 2015-06-25 09:05:44]

The author kkinnunen@nvidia.com has not signed Google Contributor License
Agreement. Please visit https://cla.developers.google.com to sign and manage
CLA.

[commit-bot: I haz the power, 2015-06-25 09:07:03]

Committed patchset #3 (id:40001) as
https://skia.googlesource.com/skia/+/fb8d6884e0e01d0c2f8596adf5af1efb0d08de7e

[fmalita_google_do_not_use, 2015-06-25 15:17:18]

On 2015/06/25 09:07:03, commit-bot: I haz the power wrote:
> Committed patchset #3 (id:40001) as
> https://skia.googlesource.com/skia/+/fb8d6884e0e01d0c2f8596adf5af1efb0d08de7e

The GPU SAN bots seem to be failing after this CL:
https://build.chromium.org/p/client.skia/builders/Test-Ubuntu-GCC-Golo-GPU-GT...

../../../src/gpu/gl/GrGLInterface.cpp:509 GrGLInterface::validate() failed.
Failed to validate gl interfaceCould not run gpu: Could not create a surface.

[f(malita), 2015-06-25 15:42:05]

A revert of this CL (patchset #3 id:40001) has been created in
https://codereview.chromium.org/1206333003/ by fmalita@chromium.org.

The reason for reverting is: Broke the GPU SAN bots:
https://build.chromium.org/p/client.skia/builders/Test-Ubuntu-GCC-Golo-GPU-GT....

[Kimmo Kinnunen, 2015-06-26 07:55:27]

Brian, Joshua: Please re-review, there was a problem causing revert.

IMHO this #if 0 business points out a small code duplication problem with GL
interface verification. 

Alternative approach would be that the context would just check the
GrGLInterface as it determines the caps. 

Is there a reason to fail the interface if particular extension tag does not
agree with the Skia's list of needed functions? The extension could just be not
used. Maybe the function ptr could be null'ed if we are afraid that we'd call
the function nevertheless.. I can work on related changes, if this ok.

[bsalomon, 2015-06-26 13:30:55]

On 2015/06/26 07:55:27, Kimmo Kinnunen wrote:
> Brian, Joshua: Please re-review, there was a problem causing revert.
> 
> IMHO this #if 0 business points out a small code duplication problem with GL
> interface verification. 
> 
> Alternative approach would be that the context would just check the
> GrGLInterface as it determines the caps. 
> 
> Is there a reason to fail the interface if particular extension tag does not
> agree with the Skia's list of needed functions? The extension could just be
not
> used. Maybe the function ptr could be null'ed if we are afraid that we'd call
> the function nevertheless.. I can work on related changes, if this ok.

Personally I find the caps logic complex enough without the addition of function
pointer checking there too. I like the idea of keeping it as a mapping from
versions/extensions to the set of (sometimes higher level) knobs Ganesh cares
about.

The reason for failing hard was really to give a strong signal to the client
that they hadn't setup their GL interface correctly and get them to fix it.
Basically it puts the burden on the client to give us an interface that
accurately reflects the GL_EXTENSION string and leaves us out of the business of
patching up missing functions. Most extensions we use have been stable for a
long time and so don't have the versioning complexity of NVPR.

But I agree that the current system is strained. 

I'm wondering whether the interface setup and validation code could be generated
from a table that describes the mappings between extension strings/GL version
numbers to function names. Perhaps the interface's function pointer array could
be generated from a table as well. Generated code could remove the extension
name if a function was missing and print some sort of error to alert the client
that they're missing something. Some extensions are required on older desktop GL
so for those we'd have to fail for them (e.g. vertex buffer objects). I'm not
sure what to do about functions that are missing but should be there by GL
version number. Perhaps we could lower the version or fail. To add complexity,
some functions are required only on certain context types (e.g. we have to use
VAOs on core profiles but don't on ES or compatibility). It seems like such a
table could also be used to generate the interface assembler code, too.

[Kimmo Kinnunen, 2015-06-29 05:24:02]

On 2015/06/26 13:30:55, bsalomon wrote:
> On 2015/06/26 07:55:27, Kimmo Kinnunen wrote:
> > Brian, Joshua: Please re-review, there was a problem causing revert.

> > Is there a reason to fail the interface if particular extension tag does not
> > agree with the Skia's list of needed functions? The extension could just be
> not
> > used. Maybe the function ptr could be null'ed if we are afraid that we'd
call
> > the function nevertheless.. I can work on related changes, if this ok.
> 
> Personally I find the caps logic complex enough without the addition of
function
> pointer checking there too. I like the idea of keeping it as a mapping from
> versions/extensions to the set of (sometimes higher level) knobs Ganesh cares
> about.

Ok, thanks for pointing that out. No problem keeping it as is.
 

> I'm wondering whether the interface setup and validation code could be
generated
> from a table that describes the mappings between extension strings/GL version
> numbers to function names. ....

Sounds like something I can not get reviewed, so I have to pass on trying that..


This patch #4 would still need a review, if anybody has time :)


> On 2015/06/26 07:55:27, Kimmo Kinnunen wrote:
> > Brian, Joshua: Please re-review, there was a problem causing revert.

[bsalomon, 2015-06-29 14:10:29]

On 2015/06/29 05:24:02, Kimmo Kinnunen wrote:
> On 2015/06/26 13:30:55, bsalomon wrote:
> > On 2015/06/26 07:55:27, Kimmo Kinnunen wrote:
> > > Brian, Joshua: Please re-review, there was a problem causing revert.
> 
> > > Is there a reason to fail the interface if particular extension tag does
not
> > > agree with the Skia's list of needed functions? The extension could just
be
> > not
> > > used. Maybe the function ptr could be null'ed if we are afraid that we'd
> call
> > > the function nevertheless.. I can work on related changes, if this ok.
> > 
> > Personally I find the caps logic complex enough without the addition of
> function
> > pointer checking there too. I like the idea of keeping it as a mapping from
> > versions/extensions to the set of (sometimes higher level) knobs Ganesh
cares
> > about.
> 
> Ok, thanks for pointing that out. No problem keeping it as is.
>  
> 
> > I'm wondering whether the interface setup and validation code could be
> generated
> > from a table that describes the mappings between extension strings/GL
version
> > numbers to function names. ....
> 
> Sounds like something I can not get reviewed, so I have to pass on trying
that..
> 
> 
> This patch #4 would still need a review, if anybody has time :)
> 
> 
> > On 2015/06/26 07:55:27, Kimmo Kinnunen wrote:
> > > Brian, Joshua: Please re-review, there was a problem causing revert.

lgtm

[Kimmo Kinnunen, 2015-06-30 10:52:40]

The CQ bit was checked by kkinnunen@nvidia.com

[Kimmo Kinnunen, 2015-06-30 10:52:41]

The patchset sent to the CQ was uploaded after l-g-t-m from
joshualitt@google.com
Link to the patchset: https://chromiumcodereview.appspot.com/1177243004/#ps60001
(title: "fix the error causing the revert")

[commit-bot: I haz the power, 2015-06-30 10:52:50]

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

[commit-bot: I haz the power, 2015-06-30 10:52:51]

The author kkinnunen@nvidia.com has not signed Google Contributor License
Agreement. Please visit https://cla.developers.google.com to sign and manage
CLA.

[commit-bot: I haz the power, 2015-06-30 10:59:53]

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

[jvanverth1, 2015-06-30 16:40:16]

A revert of this CL (patchset #4 id:60001) has been created in
https://codereview.chromium.org/1219663005/ by jvanverth@google.com.

The reason for reverting is: Breaks the Ubuntu *SAN bots..

[Kimmo Kinnunen, 2015-07-01 09:51:32]

The CQ bit was checked by kkinnunen@nvidia.com

[Kimmo Kinnunen, 2015-07-01 09:51:33]

The patchset sent to the CQ was uploaded after l-g-t-m from
joshualitt@google.com, bsalomon@google.com
Link to the patchset: https://codereview.chromium.org/1177243004/#ps80001
(title: "change failure to a warning")

[commit-bot: I haz the power, 2015-07-01 09:51:46]

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

[commit-bot: I haz the power, 2015-07-01 09:51:47]

The author kkinnunen@nvidia.com has not signed Google Contributor License
Agreement. Please visit https://cla.developers.google.com to sign and manage
CLA.

[Kimmo Kinnunen, 2015-07-01 09:53:50]

On 2015/06/30 16:40:16, jvanverth1 wrote:
> A revert of this CL (patchset #4 id:60001) has been created in
> https://codereview.chromium.org/1219663005/ by mailto:jvanverth@google.com.
> 
> The reason for reverting is: Breaks the Ubuntu *SAN bots..

Sorry about that, again.

Fixed the error by not requiring nvprmsaa config to use nvpr. I believe this now
matches the previous functionality.
Added an warning SkDebugf, hope it is ok. 

Taking the liberty to submit to CQ with the existing LGTMs..

[commit-bot: I haz the power, 2015-07-01 09:58:56]

Committed patchset #5 (id:80001) as
https://skia.googlesource.com/skia/+/cfe62e30848eadead4358b0385e57723779b762b

[Kimmo Kinnunen, 2015-07-01 10:48:59]

My liberty taking did indeed break the bots. This should fix it without revert:
https://codereview.chromium.org/1220513005/
(feel free to revert, too)

[borenet, 2015-07-01 11:22:17]

A revert of this CL (patchset #6 id:100001) has been created in
https://codereview.chromium.org/1220883005/ by borenet@google.com.

The reason for reverting is: Breaks GLInterfaceValidationTest on Android bots. 
Also getting this warning:

WARNING: nvprmsaa config requested, but driver path rendering support not
available. Maybe update the driver? Your driver version string: "OpenGL ES 2.0
14.01003".