Make NVPR a GL context option instead of a GL context

Make NVPR a GL context option instead of a GL context

Make NVPR a GL context option instead of a GL context.
This may enable NVPR to be run with command buffer
interface.

No functionality change in DM or nanobench. NVPR can
only be run with normal GL APIs.

BUG=skia:2992
Committed: https://skia.googlesource.com/skia/+/eeebdb538d476c1bfc8b63a946094ca1b505ecd1

Committed: https://skia.googlesource.com/skia/+/64492c43c3faee7ab0f69b1c84e0267616f85e52

Committed: https://skia.googlesource.com/skia/+/5219fd9ff7aa6ebf64c905ad40f5e0027a39962b

[Kimmo Kinnunen, 2015-11-16 10:53:04]

kkinnunen@nvidia.com changed reviewers:
+ bsalomon@google.com, jvanverth@google.com, mtklein@google.com

[bsalomon, 2015-11-16 14:13:50]

lgtm

[Kimmo Kinnunen, 2015-12-01 12:54:50]

The CQ bit was checked by kkinnunen@nvidia.com

[Kimmo Kinnunen, 2015-12-01 12:54:50]

The patchset sent to the CQ was uploaded after l-g-t-m from bsalomon@google.com
Link to the patchset: https://chromiumcodereview.appspot.com/1448883002/#ps20001
(title: "rebase")

[commit-bot: I haz the power, 2015-12-01 12:55:01]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1448883002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1448883002/20001

[commit-bot: I haz the power, 2015-12-01 13:10:51]

Description was changed from

==========
Make NVPR a GL context option instead of a GL context

Make NVPR a GL context option instead of a GL context.
This may enable NVPR to be run with command buffer
interface.

No functionality change in DM or nanobench. NVPR can
only be run with normal GL APIs.

BUG=skia:2992
==========

to

==========
Make NVPR a GL context option instead of a GL context

Make NVPR a GL context option instead of a GL context.
This may enable NVPR to be run with command buffer
interface.

No functionality change in DM or nanobench. NVPR can
only be run with normal GL APIs.

BUG=skia:2992

Committed:
https://skia.googlesource.com/skia/+/eeebdb538d476c1bfc8b63a946094ca1b505ecd1
==========

[commit-bot: I haz the power, 2015-12-01 13:10:52]

Committed patchset #2 (id:20001) as
https://skia.googlesource.com/skia/+/eeebdb538d476c1bfc8b63a946094ca1b505ecd1

[bsalomon, 2015-12-01 15:58:25]

A revert of this CL (patchset #2 id:20001) has been created in
https://codereview.chromium.org/1486153002/ by bsalomon@google.com.

The reason for reverting is: BUG=skia:4609

skbug.com/4609

Seems like GrContextFactory needs to fail when the NVPR option is requested but
the driver version isn't sufficiently high.

.

[scroggo, 2015-12-01 16:10:00]

scroggo@google.com changed reviewers:
+ scroggo@google.com

[scroggo, 2015-12-01 16:10:01]

When relanding, please include the fix in crrec.com/1487033002

[Kimmo Kinnunen, 2015-12-02 13:51:23]

Description was changed from

==========
Make NVPR a GL context option instead of a GL context

Make NVPR a GL context option instead of a GL context.
This may enable NVPR to be run with command buffer
interface.

No functionality change in DM or nanobench. NVPR can
only be run with normal GL APIs.

BUG=skia:2992

Committed:
https://skia.googlesource.com/skia/+/eeebdb538d476c1bfc8b63a946094ca1b505ecd1
==========

to

==========
Make NVPR a GL context option instead of a GL context

Make NVPR a GL context option instead of a GL context.
This may enable NVPR to be run with command buffer
interface.

No functionality change in DM or nanobench. NVPR can
only be run with normal GL APIs.

BUG=skia:2992

Committed:
https://skia.googlesource.com/skia/+/eeebdb538d476c1bfc8b63a946094ca1b505ecd1
==========

[Kimmo Kinnunen, 2015-12-02 13:51:34]

The CQ bit was checked by kkinnunen@nvidia.com to run a CQ dry run

[commit-bot: I haz the power, 2015-12-02 13:51:38]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1448883002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1448883002/60001

[Kimmo Kinnunen, 2015-12-02 13:59:28]

The CQ bit was checked by kkinnunen@nvidia.com to run a CQ dry run

[commit-bot: I haz the power, 2015-12-02 13:59:38]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1448883002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1448883002/80001

[Kimmo Kinnunen, 2015-12-02 13:59:55]

Sorry for the trouble. This should fix the regressions.

[Kimmo Kinnunen, 2015-12-02 14:03:57]

https://codereview.chromium.org/1448883002/diff/80001/src/gpu/GrContextFactor...
File src/gpu/GrContextFactory.cpp (left):

https://codereview.chromium.org/1448883002/diff/80001/src/gpu/GrContextFactor...
src/gpu/GrContextFactory.cpp:102: GrGpu* gpu = grCtx->getGpu();
IIRC this hunk was interesting when NVPR was run by default. DM would quit if
NVPR was not supported and this would return nullptr.

Now it can return nullptr because nvpr is used only when --config nvprmsaa* is
passed, and in this case DM quitting should be desirable?

The only problem might be if current NVPR results are being generated with bots
that have old drivers. However, the current code should segfault in this case,
as far as I can see. As we don't have segfaults, this should be ok.

[commit-bot: I haz the power, 2015-12-02 14:14:36]

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

[commit-bot: I haz the power, 2015-12-02 14:14:37]

Dry run: This issue passed the CQ dry run.

[bsalomon, 2015-12-02 14:30:28]

https://codereview.chromium.org/1448883002/diff/80001/src/gpu/GrContextFactor...
File src/gpu/GrContextFactory.cpp (left):

https://codereview.chromium.org/1448883002/diff/80001/src/gpu/GrContextFactor...
src/gpu/GrContextFactory.cpp:102: GrGpu* gpu = grCtx->getGpu();
On 2015/12/02 14:03:57, Kimmo Kinnunen wrote:
> IIRC this hunk was interesting when NVPR was run by default. DM would quit if
> NVPR was not supported and this would return nullptr.
> 
> Now it can return nullptr because nvpr is used only when --config nvprmsaa* is
> passed, and in this case DM quitting should be desirable?

Does DM really fail or does it just print that it won't run the config? I think
we'd prefer the latter.

> 
> The only problem might be if current NVPR results are being generated with
bots
> that have old drivers. However, the current code should segfault in this case,
> as far as I can see. As we don't have segfaults, this should be ok.

[Kimmo Kinnunen, 2015-12-07 09:29:39]

On 2015/12/02 14:30:28, bsalomon wrote:
>
https://codereview.chromium.org/1448883002/diff/80001/src/gpu/GrContextFactor...
> File src/gpu/GrContextFactory.cpp (left):
> 
>
https://codereview.chromium.org/1448883002/diff/80001/src/gpu/GrContextFactor...
> src/gpu/GrContextFactory.cpp:102: GrGpu* gpu = grCtx->getGpu();
> On 2015/12/02 14:03:57, Kimmo Kinnunen wrote:
> > IIRC this hunk was interesting when NVPR was run by default. DM would quit
if
> > NVPR was not supported and this would return nullptr.
> > 
> > Now it can return nullptr because nvpr is used only when --config nvprmsaa*
is
> > passed, and in this case DM quitting should be desirable?
> 
> Does DM really fail or does it just print that it won't run the config? I
think
> we'd prefer the latter.
> 
> > 
> > The only problem might be if current NVPR results are being generated with
> bots
> > that have old drivers. However, the current code should segfault in this
case,
> > as far as I can see. As we don't have segfaults, this should be ok.


Is this ok to merge again, now that I fixed dm behavior to skip configs that can
not create contexts?

[bsalomon, 2015-12-07 14:24:26]

On 2015/12/07 09:29:39, Kimmo Kinnunen wrote:
> On 2015/12/02 14:30:28, bsalomon wrote:
> >
>
https://codereview.chromium.org/1448883002/diff/80001/src/gpu/GrContextFactor...
> > File src/gpu/GrContextFactory.cpp (left):
> > 
> >
>
https://codereview.chromium.org/1448883002/diff/80001/src/gpu/GrContextFactor...
> > src/gpu/GrContextFactory.cpp:102: GrGpu* gpu = grCtx->getGpu();
> > On 2015/12/02 14:03:57, Kimmo Kinnunen wrote:
> > > IIRC this hunk was interesting when NVPR was run by default. DM would quit
> if
> > > NVPR was not supported and this would return nullptr.
> > > 
> > > Now it can return nullptr because nvpr is used only when --config
nvprmsaa*
> is
> > > passed, and in this case DM quitting should be desirable?
> > 
> > Does DM really fail or does it just print that it won't run the config? I
> think
> > we'd prefer the latter.
> > 
> > > 
> > > The only problem might be if current NVPR results are being generated with
> > bots
> > > that have old drivers. However, the current code should segfault in this
> case,
> > > as far as I can see. As we don't have segfaults, this should be ok.
> 
> 
> Is this ok to merge again, now that I fixed dm behavior to skip configs that
can
> not create contexts?

yup, go for it.

[Kimmo Kinnunen, 2015-12-08 08:28:55]

The CQ bit was checked by kkinnunen@nvidia.com

[Kimmo Kinnunen, 2015-12-08 08:28:55]

The patchset sent to the CQ was uploaded after l-g-t-m from bsalomon@google.com
Link to the patchset: https://codereview.chromium.org/1448883002/#ps160001
(title: "rebase")

[commit-bot: I haz the power, 2015-12-08 08:29:02]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1448883002/160001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1448883002/160001

[commit-bot: I haz the power, 2015-12-08 09:24:44]

Description was changed from

==========
Make NVPR a GL context option instead of a GL context

Make NVPR a GL context option instead of a GL context.
This may enable NVPR to be run with command buffer
interface.

No functionality change in DM or nanobench. NVPR can
only be run with normal GL APIs.

BUG=skia:2992

Committed:
https://skia.googlesource.com/skia/+/eeebdb538d476c1bfc8b63a946094ca1b505ecd1
==========

to

==========
Make NVPR a GL context option instead of a GL context

Make NVPR a GL context option instead of a GL context.
This may enable NVPR to be run with command buffer
interface.

No functionality change in DM or nanobench. NVPR can
only be run with normal GL APIs.

BUG=skia:2992

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

Committed:
https://skia.googlesource.com/skia/+/64492c43c3faee7ab0f69b1c84e0267616f85e52
==========

[commit-bot: I haz the power, 2015-12-08 09:24:46]

Committed patchset #9 (id:160001) as
https://skia.googlesource.com/skia/+/64492c43c3faee7ab0f69b1c84e0267616f85e52

[borenet, 2015-12-09 15:03:29]

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

The reason for reverting is: "Could not create surface" on Linux GTX660 bots.

[bsalomon, 2015-12-09 15:03:47]

A revert of this CL (patchset #9 id:160001) has been created in
https://codereview.chromium.org/1511613004/ by bsalomon@google.com.

The reason for reverting is: Breaking bots:
http://build.chromium.org/p/client.skia/builders/Test-Ubuntu-GCC-ShuttleA-GPU....

[Kimmo Kinnunen, 2015-12-09 15:27:13]

Description was changed from

==========
Make NVPR a GL context option instead of a GL context

Make NVPR a GL context option instead of a GL context.
This may enable NVPR to be run with command buffer
interface.

No functionality change in DM or nanobench. NVPR can
only be run with normal GL APIs.

BUG=skia:2992

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

Committed:
https://skia.googlesource.com/skia/+/64492c43c3faee7ab0f69b1c84e0267616f85e52
==========

to

==========
Make NVPR a GL context option instead of a GL context

Make NVPR a GL context option instead of a GL context.
This may enable NVPR to be run with command buffer
interface.

No functionality change in DM or nanobench. NVPR can
only be run with normal GL APIs.

BUG=skia:2992

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

Committed:
https://skia.googlesource.com/skia/+/64492c43c3faee7ab0f69b1c84e0267616f85e52
==========

[Kimmo Kinnunen, 2015-12-09 15:33:44]

On 2015/12/09 15:03:47, bsalomon wrote:
> A revert of this CL (patchset #9 id:160001) has been created in
> https://codereview.chromium.org/1511613004/ by mailto:bsalomon@google.com.
> 
> The reason for reverting is: Breaking bots:
>
http://build.chromium.org/p/client.skia/builders/Test-Ubuntu-GCC-ShuttleA-GPU....

Ah, sorry about it again. Apparently I can not manage this many concurrent
patches..

It was misssing the contextOption in one check..

[Kimmo Kinnunen, 2015-12-09 15:57:23]

On 2015/12/09 15:33:44, Kimmo Kinnunen wrote:
> On 2015/12/09 15:03:47, bsalomon wrote:
> > A revert of this CL (patchset #9 id:160001) has been created in
> > https://codereview.chromium.org/1511613004/ by mailto:bsalomon@google.com.
> > 
> > The reason for reverting is: Breaking bots:
> >
>
http://build.chromium.org/p/client.skia/builders/Test-Ubuntu-GCC-ShuttleA-GPU....
> 
> Ah, sorry about it again. Apparently I can not manage this many concurrent
> patches..
> 
> It was misssing the contextOption in one check..

I tried to run this now on the testbots, see above. Should they catch the
problem? If so, would this be ok to merge (yet again..)

[bsalomon, 2015-12-09 17:12:19]

On 2015/12/09 15:57:23, Kimmo Kinnunen wrote:
> On 2015/12/09 15:33:44, Kimmo Kinnunen wrote:
> > On 2015/12/09 15:03:47, bsalomon wrote:
> > > A revert of this CL (patchset #9 id:160001) has been created in
> > > https://codereview.chromium.org/1511613004/ by mailto:bsalomon@google.com.
> > > 
> > > The reason for reverting is: Breaking bots:
> > >
> >
>
http://build.chromium.org/p/client.skia/builders/Test-Ubuntu-GCC-ShuttleA-GPU....
> > 
> > Ah, sorry about it again. Apparently I can not manage this many concurrent
> > patches..
> > 
> > It was misssing the contextOption in one check..
> 
> I tried to run this now on the testbots, see above. Should they catch the
> problem? If so, would this be ok to merge (yet again..)

yes, those would catch the bug. lgtm

[Kimmo Kinnunen, 2015-12-10 11:45:22]

The CQ bit was checked by kkinnunen@nvidia.com

[Kimmo Kinnunen, 2015-12-10 11:45:23]

The patchset sent to the CQ was uploaded after l-g-t-m from bsalomon@google.com
Link to the patchset: https://codereview.chromium.org/1448883002/#ps200001
(title: "rebase")

[commit-bot: I haz the power, 2015-12-10 11:45:24]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1448883002/200001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1448883002/200001

[commit-bot: I haz the power, 2015-12-10 14:28:16]

Description was changed from

==========
Make NVPR a GL context option instead of a GL context

Make NVPR a GL context option instead of a GL context.
This may enable NVPR to be run with command buffer
interface.

No functionality change in DM or nanobench. NVPR can
only be run with normal GL APIs.

BUG=skia:2992

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

Committed:
https://skia.googlesource.com/skia/+/64492c43c3faee7ab0f69b1c84e0267616f85e52
==========

to

==========
Make NVPR a GL context option instead of a GL context

Make NVPR a GL context option instead of a GL context.
This may enable NVPR to be run with command buffer
interface.

No functionality change in DM or nanobench. NVPR can
only be run with normal GL APIs.

BUG=skia:2992

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

Committed:
https://skia.googlesource.com/skia/+/64492c43c3faee7ab0f69b1c84e0267616f85e52

Committed:
https://skia.googlesource.com/skia/+/5219fd9ff7aa6ebf64c905ad40f5e0027a39962b
==========

[commit-bot: I haz the power, 2015-12-10 14:28:17]

Committed patchset #11 (id:200001) as
https://skia.googlesource.com/skia/+/5219fd9ff7aa6ebf64c905ad40f5e0027a39962b