gl_tests: Use zero size gl surface for offscreen rendering

gl_tests: Use zero size gl surface for offscreen rendering

In GLX, most of the drivers can cope with both PBuffer and Surfaceless modes
for offscreen rendering. In EGL though, Intel driver only works with
Surfaceless.

This CL changes all hardware platforms to use Surfaceless by forcing a zero
size GL surface initialization. Ozone-GBM wasn't previously working because of
that but now works like a charm ("[  PASSED  ] 94 tests").

This CL also fixes gpu_unittests under Ozone-GBM ("[  PASSED  ] 2544 tests")
and cleans a bit the message loop creation, bringing it to run earlier in the
RunTestSuite.

BUG=471261
TEST=gl_tests, gpu_unittests on both GBM and X11 (Linux)
NOTRY=true

Committed: https://crrev.com/825c082c407341474d2083c180d90c3480b4ebbd
Cr-Commit-Position: refs/heads/master@{#324925}

[vignatti (out of this project), 2015-03-23 23:14:20]

tiago.vignatti@intel.com changed reviewers:
+ bajones@chromium.org

[vignatti (out of this project), 2015-03-23 23:15:42]

bajones@, I want your consent first for the bigger changes and then we can
review the Ozone related stuff next. Thanks.

[bajones, 2015-03-23 23:50:19]

On 2015/03/23 23:15:42, vignatti wrote:
> bajones@, I want your consent first for the bigger changes and then we can
> review the Ozone related stuff next. Thanks.

I'm not familiar with Ozone, but the change to a zero size surface LGTM if it
passes on all platforms.

[vignatti (out of this project), 2015-03-23 23:56:38]

tiago.vignatti@intel.com changed reviewers:
+ dnicoara@chromium.org, piman@chromium.org

[vignatti (out of this project), 2015-03-23 23:58:01]

On 2015/03/23 23:50:19, bajones wrote:
> On 2015/03/23 23:15:42, vignatti wrote:
> > bajones@, I want your consent first for the bigger changes and then we can
> > review the Ozone related stuff next. Thanks.
> 
> I'm not familiar with Ozone, but the change to a zero size surface LGTM if it
> passes on all platforms.

okay, thank you.

Now adding piman@ for reviewing gpu/DEPS and dnicoara@ for double-checking Ozone
stuff.

[piman, 2015-03-24 00:36:30]

https://codereview.chromium.org/1025523005/diff/20001/gpu/command_buffer/clie...
File gpu/command_buffer/client/cmd_buffer_helper_test.cc (right):

https://codereview.chromium.org/1025523005/diff/20001/gpu/command_buffer/clie...
gpu/command_buffer/client/cmd_buffer_helper_test.cc:264: #endif
Could this (and the autorelease_pool_) be moved to unittest_main.cc so that it's
all in one place?

https://codereview.chromium.org/1025523005/diff/20001/gpu/command_buffer/test...
File gpu/command_buffer/tests/gl_tests_main.cc (right):

https://codereview.chromium.org/1025523005/diff/20001/gpu/command_buffer/test...
gpu/command_buffer/tests/gl_tests_main.cc:31: base::MessageLoopForIO
message_loop;
Can this move to main, so that the message loop creation is localized?

#if defined(USE_OZONE)
base::MessageLoopForUI main_loop;
ui::OzonePlatform::InitializeForUI();
ui::OzonePlatform::InitializeForGPU();
#else
base::MessageLoopForIO message_loop;
#endif


That way too, we can remove RunHelper, and Bind(&base::TestSuite::Run,
base::Unretained(&test_suite)) directly.

[vignatti (out of this project), 2015-03-24 14:01:48]

tiago.vignatti@intel.com changed reviewers:
+ spang@chromium.org

[vignatti (out of this project), 2015-03-24 14:01:49]

PTAL now.

Adding spang@ also for dependencies added to DEPS: '+ui/ozone/public'

https://codereview.chromium.org/1025523005/diff/20001/gpu/command_buffer/clie...
File gpu/command_buffer/client/cmd_buffer_helper_test.cc (right):

https://codereview.chromium.org/1025523005/diff/20001/gpu/command_buffer/clie...
gpu/command_buffer/client/cmd_buffer_helper_test.cc:264: #endif
On 2015/03/24 00:36:30, piman (Very slow to review) wrote:
> Could this (and the autorelease_pool_) be moved to unittest_main.cc so that
it's
> all in one place?

Done.

https://codereview.chromium.org/1025523005/diff/20001/gpu/command_buffer/test...
File gpu/command_buffer/tests/gl_tests_main.cc (right):

https://codereview.chromium.org/1025523005/diff/20001/gpu/command_buffer/test...
gpu/command_buffer/tests/gl_tests_main.cc:31: base::MessageLoopForIO
message_loop;
On 2015/03/24 00:36:30, piman (Very slow to review) wrote:
> Can this move to main, so that the message loop creation is localized?
> 
> #if defined(USE_OZONE)
> base::MessageLoopForUI main_loop;
> ui::OzonePlatform::InitializeForUI();
> ui::OzonePlatform::InitializeForGPU();
> #else
> base::MessageLoopForIO message_loop;
> #endif
> 
> 
> That way too, we can remove RunHelper, and Bind(&base::TestSuite::Run,
> base::Unretained(&test_suite)) directly.

Done. Much better in fact!

[spang, 2015-03-24 15:56:25]

lgtm

[piman, 2015-03-25 01:35:58]

lgtm

[vignatti (out of this project), 2015-03-25 13:18:19]

The CQ bit was checked by tiago.vignatti@intel.com

[vignatti (out of this project), 2015-03-25 13:18:19]

The patchset sent to the CQ was uploaded after l-g-t-m from bajones@chromium.org
Link to the patchset: https://codereview.chromium.org/1025523005/#ps40001
(title: "localized message loop creation")

[commit-bot: I haz the power, 2015-03-25 13:18:31]

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

[commit-bot: I haz the power, 2015-03-25 13:58:51]

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

[commit-bot: I haz the power, 2015-03-25 13:58:52]

Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[vignatti (out of this project), 2015-03-25 15:38:33]

On 2015/03/25 13:58:52, I haz the power (commit-bot) wrote:
> Try jobs failed on following builders:
>   linux_chromium_chromeos_ozone_rel_ng on tryserver.chromium.linux
(JOB_FAILED,
>
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

Guys, the results output is not being shown for some reason. No JSON summary
output, neither gtest XML. So it's difficult to track down the exact issue
causing failure to the bots. Any idea?

[dnicoara, 2015-03-25 15:46:43]

On 2015/03/25 15:38:33, vignatti wrote:
> On 2015/03/25 13:58:52, I haz the power (commit-bot) wrote:
> > Try jobs failed on following builders:
> >   linux_chromium_chromeos_ozone_rel_ng on tryserver.chromium.linux
> (JOB_FAILED,
> >
>
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
> 
> Guys, the results output is not being shown for some reason. No JSON summary
> output, neither gtest XML. So it's difficult to track down the exact issue
> causing failure to the bots. Any idea?

The error is:
[0325/062436:FATAL:message_loop.cc(387)] Check failed: !current(). should only
have one message loop per thread

Run a debug build. My guess you missed removing a MessageLoop somewhere.

[vignatti (out of this project), 2015-03-26 20:39:12]

On 2015/03/25 15:46:43, dnicoara wrote:
> 
> The error is:
> [0325/062436:FATAL:message_loop.cc(387)] Check failed: !current(). should only
> have one message loop per thread
> 
> Run a debug build. My guess you missed removing a MessageLoop somewhere.

Trybots were mad at me because my changes only worked for gl_tests and
gpu_unittests in single process mode. So in multi-processed,
base::LaunchUnitTestsInternal needs to start its own message loop but the
changes I made previously (Patch Set 3) were impossible cause they introduced
another message loop in the same process, which is invalid.

So in Patch Set 4 now, I've moved the message loop creation for the runner
helper instead, which runs in a different process than the launcher. PTAL.

[piman, 2015-03-26 20:50:13]

https://codereview.chromium.org/1025523005/diff/60001/gpu/command_buffer/test...
File gpu/command_buffer/tests/gl_tests_main.cc (right):

https://codereview.chromium.org/1025523005/diff/60001/gpu/command_buffer/test...
gpu/command_buffer/tests/gl_tests_main.cc:55: #endif
This concerns me slightly because ApplyGpuDriverBugWorkarounds below needs GL
(i.e. that gfx::GLSurface::InitializeOneOff ran).

Could everything move to the RunHelper?

[vignatti (out of this project), 2015-03-26 21:18:28]

https://codereview.chromium.org/1025523005/diff/60001/gpu/command_buffer/test...
File gpu/command_buffer/tests/gl_tests_main.cc (right):

https://codereview.chromium.org/1025523005/diff/60001/gpu/command_buffer/test...
gpu/command_buffer/tests/gl_tests_main.cc:55: #endif
On 2015/03/26 20:50:13, piman (Very slow to review) wrote:
> This concerns me slightly because ApplyGpuDriverBugWorkarounds below needs GL
> (i.e. that gfx::GLSurface::InitializeOneOff ran).
> 
> Could everything move to the RunHelper?

I tried to be conservative and not tackle other OSs and architectures. The only
problem I found by moving this snip to the RunHelper is that InitializeOneOff
will be called several times, but things seems to work fine -- I've in fact ran
that in Linux/X11.

Do you prefer changing like that then?

[piman, 2015-03-26 21:32:50]

https://codereview.chromium.org/1025523005/diff/60001/gpu/command_buffer/test...
File gpu/command_buffer/tests/gl_tests_main.cc (right):

https://codereview.chromium.org/1025523005/diff/60001/gpu/command_buffer/test...
gpu/command_buffer/tests/gl_tests_main.cc:55: #endif
On 2015/03/26 21:18:28, vignatti wrote:
> On 2015/03/26 20:50:13, piman (Very slow to review) wrote:
> > This concerns me slightly because ApplyGpuDriverBugWorkarounds below needs
GL
> > (i.e. that gfx::GLSurface::InitializeOneOff ran).
> > 
> > Could everything move to the RunHelper?
> 
> I tried to be conservative and not tackle other OSs and architectures. The
only
> problem I found by moving this snip to the RunHelper is that InitializeOneOff
> will be called several times, but things seems to work fine -- I've in fact
ran
> that in Linux/X11.
> 
> Do you prefer changing like that then?

Why would InitializeOneOff run twice?

[vignatti (out of this project), 2015-03-27 16:00:16]

https://codereview.chromium.org/1025523005/diff/60001/gpu/command_buffer/test...
File gpu/command_buffer/tests/gl_tests_main.cc (right):

https://codereview.chromium.org/1025523005/diff/60001/gpu/command_buffer/test...
gpu/command_buffer/tests/gl_tests_main.cc:55: #endif
On 2015/03/26 21:32:49, piman (Very slow to review) wrote:
> On 2015/03/26 21:18:28, vignatti wrote:
> > On 2015/03/26 20:50:13, piman (Very slow to review) wrote:
> > > This concerns me slightly because ApplyGpuDriverBugWorkarounds below needs
> GL
> > > (i.e. that gfx::GLSurface::InitializeOneOff ran).
> > > 
> > > Could everything move to the RunHelper?
> > 
> > I tried to be conservative and not tackle other OSs and architectures. The
> only
> > problem I found by moving this snip to the RunHelper is that
InitializeOneOff
> > will be called several times, but things seems to work fine -- I've in fact
> ran
> > that in Linux/X11.
> > 
> > Do you prefer changing like that then?
> 
> Why would InitializeOneOff run twice?

I'm not sure how the batch logic works, but the RunHelper callback will be
called every time a new batch is sent run. For example, the the default number
of tests per batch is 10 (kDefaultTestBatchLimit) and gl_tests have 94 tests, so
InitializeOneOff will be called 10 times in this case.

[vignatti (out of this project), 2015-03-27 23:51:11]

In Patch Set #6 now, I've moved the GL initialization to the runner callback
(piman's suggestion) and removed a few Ozone unnecessary symbols
(OzonePlatform::InitializeForGPU will be called implicitly via
gfx::InitializeStaticGLBindings). I've tried my changes in both X11 and GBM, and
couldn't spot any penalty/problems by running gfx::GLSurface::InitializeOneOff
inside the RunHelper(). 

piman@ PTAL.

[piman, 2015-03-31 03:58:45]

lgtm

[vignatti (out of this project), 2015-03-31 18:19:31]

The CQ bit was checked by tiago.vignatti@intel.com

[vignatti (out of this project), 2015-03-31 18:19:32]

The patchset sent to the CQ was uploaded after l-g-t-m from
bajones@chromium.org, spang@chromium.org
Link to the patchset: https://codereview.chromium.org/1025523005/#ps100001
(title: "remove unneeded ozone symbols and add gl init inside callback")

[commit-bot: I haz the power, 2015-03-31 18:20:17]

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

[commit-bot: I haz the power, 2015-03-31 19:31:29]

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

[commit-bot: I haz the power, 2015-03-31 19:31:31]

Try jobs failed on following builders:
  linux_chromium_clobber_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[vignatti (out of this project), 2015-04-02 19:47:59]

The CQ bit was checked by tiago.vignatti@intel.com

[commit-bot: I haz the power, 2015-04-02 19:51:12]

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

[commit-bot: I haz the power, 2015-04-02 20:57:56]

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

[commit-bot: I haz the power, 2015-04-02 20:57:57]

Try jobs failed on following builders:
  linux_chromium_clobber_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[vignatti (out of this project), 2015-04-08 22:25:17]

On 2015/04/02 20:57:57, I haz the power (commit-bot) wrote:
> Try jobs failed on following builders:
>   linux_chromium_clobber_rel_ng on tryserver.chromium.linux (JOB_FAILED,
>
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

hey guys, I need a guidance here please.

I'm unable to commit this because linux_chromium_clobber_rel_ng complains about
gpu_unittests.isolate not containing the xdisplaycheck binary. I'm not sure why
this is being triggered to be honest and I'm thinking even if we should push
manually this CL. Maybe I'm just missing something else.. any ideas?

[piman, 2015-04-08 22:30:17]

On Wed, Apr 8, 2015 at 3:25 PM, <tiago.vignatti@intel.com> wrote:

> On 2015/04/02 20:57:57, I haz the power (commit-bot) wrote:
>
>> Try jobs failed on following builders:
>>    linux_chromium_clobber_rel_ng on tryserver.chromium.linux (JOB_FAILED,
>>
>
> http://build.chromium.org/p/tryserver.chromium.linux/
> builders/linux_chromium_clobber_rel_ng/builds/12645)
>
> hey guys, I need a guidance here please.
>
> I'm unable to commit this because linux_chromium_clobber_rel_ng complains
> about
> gpu_unittests.isolate not containing the xdisplaycheck binary. I'm not
> sure why
> this is being triggered to be honest and I'm thinking even if we should
> push
> manually this CL. Maybe I'm just missing something else.. any ideas?
>

It looks like the bot is having problem, because it fails even without the
patch. Maybe a bot issue?
+raldi trooper du jour



> https://codereview.chromium.org/1025523005/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[vignatti (out of this project), 2015-04-08 22:34:48]

tiago.vignatti@intel.com changed reviewers:
+ raldi@google.com

[vignatti (out of this project), 2015-04-08 22:35:35]

On 2015/04/08 22:30:17, piman (Very slow to review) wrote:
> On Wed, Apr 8, 2015 at 3:25 PM, <mailto:tiago.vignatti@intel.com> wrote:
> 
> > On 2015/04/02 20:57:57, I haz the power (commit-bot) wrote:
> >
> >> Try jobs failed on following builders:
> >>    linux_chromium_clobber_rel_ng on tryserver.chromium.linux (JOB_FAILED,
> >>
> >
> > http://build.chromium.org/p/tryserver.chromium.linux/
> > builders/linux_chromium_clobber_rel_ng/builds/12645)
> >
> > hey guys, I need a guidance here please.
> >
> > I'm unable to commit this because linux_chromium_clobber_rel_ng complains
> > about
> > gpu_unittests.isolate not containing the xdisplaycheck binary. I'm not
> > sure why
> > this is being triggered to be honest and I'm thinking even if we should
> > push
> > manually this CL. Maybe I'm just missing something else.. any ideas?
> >
> 
> It looks like the bot is having problem, because it fails even without the
> patch. Maybe a bot issue?
> +raldi trooper du jour

adding raldi@.

[raldi, 2015-04-08 22:58:09]

See https://code.google.com/p/chromium/issues/detail?id=475216

[vignatti (out of this project), 2015-04-08 23:11:52]

On 2015/04/08 22:58:09, raldi wrote:
> See https://code.google.com/p/chromium/issues/detail?id=475216

this is forbidden for me ("403. That’s an error."), raldi. Any other way for me
to check that?

[chromium-reviews, 2015-04-09 01:04:55]

Sorry; disregard the URL. There was an issue with
linux_chromium_clobber_rel_ng earlier today. Are you still seeing the
problem?

On Wed, Apr 8, 2015 at 4:11 PM,  <tiago.vignatti@intel.com> wrote:
> On 2015/04/08 22:58:09, raldi wrote:
>>
>> See https://code.google.com/p/chromium/issues/detail?id=475216
>
>
> this is forbidden for me ("403. That’s an error."), raldi. Any other way for
> me
> to check that?
>
> https://codereview.chromium.org/1025523005/

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[spang, 2015-04-09 01:05:14]

The CQ bit was checked by spang@chromium.org

[commit-bot: I haz the power, 2015-04-09 01:08:25]

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

[commit-bot: I haz the power, 2015-04-09 02:20:02]

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

[commit-bot: I haz the power, 2015-04-09 02:20:04]

Try jobs failed on following builders:
  linux_chromium_clobber_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[vignatti (out of this project), 2015-04-09 14:37:07]

On 2015/04/09 01:04:55, chromium-reviews wrote:
> Sorry; disregard the URL. There was an issue with
> linux_chromium_clobber_rel_ng earlier today. Are you still seeing the
> problem?

yes :| The problem still persists in the last build (#14406). Any idea?

[chromium-reviews, 2015-04-09 20:20:22]

+Current trooper

On Thu, Apr 9, 2015 at 7:37 AM, <tiago.vignatti@intel.com> wrote:

> On 2015/04/09 01:04:55, chromium-reviews wrote:
>
>> Sorry; disregard the URL. There was an issue with
>> linux_chromium_clobber_rel_ng earlier today. Are you still seeing the
>> problem?
>>
>
> yes :| The problem still persists in the last build (#14406). Any idea?
>
> https://codereview.chromium.org/1025523005/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[vignatti (out of this project), 2015-04-13 21:29:13]

On 2015/04/09 20:20:22, chromium-reviews wrote:
> +Current trooper


any trooper, please?

[spang, 2015-04-13 21:29:48]

The CQ bit was checked by spang@chromium.org

[commit-bot: I haz the power, 2015-04-13 21:31:59]

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

[commit-bot: I haz the power, 2015-04-13 22:41:54]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2015-04-13 22:43:00]

Patchset 6 (id:??) landed as
https://crrev.com/825c082c407341474d2083c180d90c3480b4ebbd
Cr-Commit-Position: refs/heads/master@{#324925}