Use GLES2Interface in GLRenderer

Use GLES2Interface in GLRenderer

This ports GLRenderer to (almost) exclusively use GLES2Interface as the
context type instead of WebGraphicsContext3D. The remaining users within
gl_renderer.cc are being addressed in parallel patches. Many callers
outside of gl_renderer.cc still use the WebGraphicsContext3D type, so
GLRenderer still exposes a pointer of that type. They'll be migrated piece
by piece.

Since this patch changes the line length of many lines due to changing the
context pointer type from "context_" to "gl_" and changing the length
of some of the context functions, I've clang-format'd the whole file.

BUG=181120
R=danakj@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=240041

[jamesr, 2013-12-05 23:58:35]

This patch is on top of https://codereview.chromium.org/93433004/.  It'll
probably be the most disruptive patch in terms of line count (at least until we
get to the unit test churn) since it involves changing and clang-formatting 500
lines in the renderer, but them's the breaks.

Dana and/or Enne - care to take a look?

[jamesr, 2013-12-05 23:59:40]

https://codereview.chromium.org/105553003/diff/1/cc/test/test_gles2_interface.cc
File cc/test/test_gles2_interface.cc (right):

https://codereview.chromium.org/105553003/diff/1/cc/test/test_gles2_interface...
cc/test/test_gles2_interface.cc:76: void TestGLES2Interface::BindTexture(GLenum
target, GLuint texture) {
fyi: i've expanded all of these forwarding calls out "by hand" instead of using
macros or a generator script in the hope that this will make it easier to move
the test functionality over to this interface from TestWebGraphicsContext3D.  If
we used macros or a generator, we'd have to then later expand them to change
them from being simple pass-throughs to having logic here.

[piman, 2013-12-06 00:49:43]

https://codereview.chromium.org/105553003/diff/1/cc/output/gl_renderer.cc
File cc/output/gl_renderer.cc (left):

https://codereview.chromium.org/105553003/diff/1/cc/output/gl_renderer.cc#old...
cc/output/gl_renderer.cc:2290: GLC(context_, context_->flush());
This is where the FreeEverything happens inside the wgc3dcbi, because we sent
new commands. Should we do it explicitly now? Or patch GLES2Implmentation to do
it somehow?

https://codereview.chromium.org/105553003/diff/1/cc/test/test_web_graphics_co...
File cc/test/test_web_graphics_context_3d.h (right):

https://codereview.chromium.org/105553003/diff/1/cc/test/test_web_graphics_co...
cc/test/test_web_graphics_context_3d.h:201: unsigned
AllocateTestTextureForExternalUse();
You again!

[jamesr, 2013-12-06 01:35:18]

On 2013/12/06 00:49:43, piman wrote:
> https://codereview.chromium.org/105553003/diff/1/cc/output/gl_renderer.cc
> File cc/output/gl_renderer.cc (left):
> 
>
https://codereview.chromium.org/105553003/diff/1/cc/output/gl_renderer.cc#old...
> cc/output/gl_renderer.cc:2290: GLC(context_, context_->flush());
> This is where the FreeEverything happens inside the wgc3dcbi, because we sent
> new commands. Should we do it explicitly now? Or patch GLES2Implmentation to
do
> it somehow?

I'll redo this in a prereq patch so this patch stays mechanical.

[jamesr, 2013-12-09 01:25:57]

OK, this patch applies ToT now and
https://src.chromium.org/viewvc/chrome?revision=239390&view=revision took care
of the visibility side effects.  PTAL.

[danakj, 2013-12-09 19:10:21]

I think there was some chat with piman about that reinterpretconstcastholycrap.

So LGTM but piman@ to double check that.

https://codereview.chromium.org/105553003/diff/40001/cc/test/test_gles2_inter...
File cc/test/test_gles2_interface.cc (right):

https://codereview.chromium.org/105553003/diff/40001/cc/test/test_gles2_inter...
cc/test/test_gles2_interface.cc:134:
reinterpret_cast<intptr_t>(const_cast<void*>(indices)));
omg

[jamesr, 2013-12-09 19:57:34]

On 2013/12/09 19:10:21, danakj wrote:
>
https://codereview.chromium.org/105553003/diff/40001/cc/test/test_gles2_inter...
> cc/test/test_gles2_interface.cc:134:
> reinterpret_cast<intptr_t>(const_cast<void*>(indices)));
> omg

It's definitely gross but it's due to a type error in OpenGL - this parameter's
type is 'const void*' since it used to refer to client buffer space, but now
it's actually just used to store a numerical offset.  This is just test code so
I'll go ahead and land and fix up later if needed.

[commit-bot: I haz the power, 2013-12-09 20:13:15]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jamesr@chromium.org/105553003/60001

[piman, 2013-12-10 00:23:20]

On Mon, Dec 9, 2013 at 11:57 AM, <jamesr@chromium.org> wrote:

> On 2013/12/09 19:10:21, danakj wrote:
>
> https://codereview.chromium.org/105553003/diff/40001/cc/
> test/test_gles2_interface.cc#newcode134
>
>> cc/test/test_gles2_interface.cc:134:
>> reinterpret_cast<intptr_t>(const_cast<void*>(indices)));
>> omg
>>
>
> It's definitely gross but it's due to a type error in OpenGL - this
> parameter's
> type is 'const void*' since it used to refer to client buffer space, but
> now
> it's actually just used to store a numerical offset.  This is just test
> code so
> I'll go ahead and land and fix up later if needed.
>

I think you shouldn't need the const_cast part.


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

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

[jamesr, 2013-12-10 00:26:53]

On 2013/12/10 00:23:20, piman wrote:
> I think you shouldn't need the const_cast part.

True, reinterpret_cast<> on its own appears to be enough.

[jamesr, 2013-12-11 06:52:59]

Committed patchset #6 manually as r240041 (presubmit successful).