RendererGLContext supports reparenting a GL context.

RendererGLContext supports reparenting a GL context.

This will allow the parenting of offscreen canvas contexts to be deferred until the compositor's view context exists, which in some cases needs to be deferred until the window is asynchronously created by the browser's UI thread. An example is JavaScript opening a popup window and then immediately using canvas to attempt to render to it.

This patch alone does not fix the bug.

BUG=80703
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=89715

[Ken Russell (switch to Gerrit), 2011-06-17 23:12:43]

LGTM

[apatrick_chromium, 2011-06-17 23:17:06]

+darin for change in content/renderer

[darin (slow to review), 2011-06-18 03:16:09]

LGTM

[commit-bot: I haz the power, 2011-06-20 19:58:26]

Change committed as 89715

[piman, 2011-06-22 00:10:59]

http://codereview.chromium.org/7205012/diff/1031/content/renderer/gpu/rendere...
File content/renderer/gpu/renderer_gl_context.cc (right):

http://codereview.chromium.org/7205012/diff/1031/content/renderer/gpu/rendere...
content/renderer/gpu/renderer_gl_context.cc:225: new_parent_texture_id)) {
Drive-by: since this happens out-of-band (with a message, not from the command
buffers), can this race with, say, latches ?
Case scenario:
child->SetLatch(child_to_parent)
parent1->WaitLatch(child_to_parent);
child->SetParent(parent2)
parent1->Finish()

What happens if the SetParent is executed before the SetLatch is executed ? Or
the WaitLatch ?

[apatrick_chromium, 2011-06-22 00:21:02]

http://codereview.chromium.org/7205012/diff/1031/content/renderer/gpu/rendere...
File content/renderer/gpu/renderer_gl_context.cc (right):

http://codereview.chromium.org/7205012/diff/1031/content/renderer/gpu/rendere...
content/renderer/gpu/renderer_gl_context.cc:225: new_parent_texture_id)) {
On 2011/06/22 00:10:59, piman wrote:
> Drive-by: since this happens out-of-band (with a message, not from the command
> buffers), can this race with, say, latches ?
> Case scenario:
> child->SetLatch(child_to_parent)
> parent1->WaitLatch(child_to_parent);
> child->SetParent(parent2)
> parent1->Finish()
> 
> What happens if the SetParent is executed before the SetLatch is executed ? Or
> the WaitLatch ?

It should be alright at the moment because SetParent is always called
immediately after the context is created with no latch related stuff in between.
There have been some latch related issues since this landed when trying to
create a canvas and using it to render to a window that doesn't yet exist (like
JavaScript synchronously opening a popup and then sticking a canvas in its DOM).
That doesn't work anyway though.

Going forward, I'm going to see if I can replace latches with proper glFlush /
glBindTexture semantics. I.e. call glFlush and the side effects are visible on
any other context on the channel, whether or not it was the parent at the time.

[piman, 2011-06-22 00:36:07]

On Tue, Jun 21, 2011 at 5:21 PM, <apatrick@chromium.org> wrote:

>
> http://codereview.chromium.**org/7205012/diff/1031/content/**
>
renderer/gpu/renderer_gl_**context.cc<http://codereview.chromium.org/7205012/diff/1031/content/renderer/gpu/renderer_gl_context.cc>
> File content/renderer/gpu/renderer_**gl_context.cc (right):
>
> http://codereview.chromium.**org/7205012/diff/1031/content/**
>
renderer/gpu/renderer_gl_**context.cc#newcode225<http://codereview.chromium.org/7205012/diff/1031/content/renderer/gpu/renderer_gl_context.cc#newcode225>
> content/renderer/gpu/renderer_**gl_context.cc:225: new_parent_texture_id))
> {
> On 2011/06/22 00:10:59, piman wrote:
>
>> Drive-by: since this happens out-of-band (with a message, not from the
>>
> command
>
>> buffers), can this race with, say, latches ?
>> Case scenario:
>> child->SetLatch(child_to_**parent)
>> parent1->WaitLatch(child_to_**parent);
>> child->SetParent(parent2)
>> parent1->Finish()
>>
>
>  What happens if the SetParent is executed before the SetLatch is
>>
> executed ? Or
>
>> the WaitLatch ?
>>
>
> It should be alright at the moment because SetParent is always called
> immediately after the context is created with no latch related stuff in
> between. There have been some latch related issues since this landed
> when trying to create a canvas and using it to render to a window that
> doesn't yet exist (like JavaScript synchronously opening a popup and
> then sticking a canvas in its DOM). That doesn't work anyway though.
>
> Going forward, I'm going to see if I can replace latches with proper
> glFlush / glBindTexture semantics. I.e. call glFlush and the side
> effects are visible on any other context on the channel, whether or not
> it was the parent at the time.


Ok, thanks for the info :)

Antoine


>
>
>
http://codereview.chromium.**org/7205012/<http://codereview.chromium.org/7205...
>

[piman, 2011-06-22 00:53:27]

http://codereview.chromium.org/7205012/diff/1031/content/renderer/pepper_plat...
File content/renderer/pepper_platform_context_3d_impl.cc (right):

http://codereview.chromium.org/7205012/diff/1031/content/renderer/pepper_plat...
content/renderer/pepper_platform_context_3d_impl.cc:85: if
(!command_buffer_->SetParent(parent_command_buffer, parent_texture_id_))
Actually this broke pepper. At this point the GpuCommandBufferStub is created
but not yet Initialized (the CommandBufferProxy hasn't been initialized). It
causes a crash in the gpu process in GpuCommandBufferStub::OnSetParent
(scheduler_ is NULL).

[apatrick_chromium, 2011-06-22 17:53:14]

http://codereview.chromium.org/7205012/diff/1031/content/renderer/pepper_plat...
File content/renderer/pepper_platform_context_3d_impl.cc (right):

http://codereview.chromium.org/7205012/diff/1031/content/renderer/pepper_plat...
content/renderer/pepper_platform_context_3d_impl.cc:85: if
(!command_buffer_->SetParent(parent_command_buffer, parent_texture_id_))
On 2011/06/22 00:53:27, piman wrote:
> Actually this broke pepper. At this point the GpuCommandBufferStub is created
> but not yet Initialized (the CommandBufferProxy hasn't been initialized). It
> causes a crash in the gpu process in GpuCommandBufferStub::OnSetParent
> (scheduler_ is NULL).

Okay I'll look into it.

[piman, 2011-06-22 17:55:44]

On Wed, Jun 22, 2011 at 10:53 AM, <apatrick@chromium.org> wrote:

>
> http://codereview.chromium.**org/7205012/diff/1031/content/**
>
renderer/pepper_platform_**context_3d_impl.cc<http://codereview.chromium.org/7205012/diff/1031/content/renderer/pepper_platform_context_3d_impl.cc>
> File content/renderer/pepper_**platform_context_3d_impl.cc (right):
>
> http://codereview.chromium.**org/7205012/diff/1031/content/**
>
renderer/pepper_platform_**context_3d_impl.cc#newcode85<http://codereview.chromium.org/7205012/diff/1031/content/renderer/pepper_platform_context_3d_impl.cc#newcode85>
> content/renderer/pepper_**platform_context_3d_impl.cc:**85: if
> (!command_buffer_->SetParent(**parent_command_buffer, parent_texture_id_))
> On 2011/06/22 00:53:27, piman wrote:
>
>> Actually this broke pepper. At this point the GpuCommandBufferStub is
>>
> created
>
>> but not yet Initialized (the CommandBufferProxy hasn't been
>>
> initialized). It
>
>> causes a crash in the gpu process in GpuCommandBufferStub::**OnSetParent
>> (scheduler_ is NULL).
>>
>
> Okay I'll look into it.


http://codereview.chromium.org/7190026/ fixes it.

Antoine


>
>
>
http://codereview.chromium.**org/7205012/<http://codereview.chromium.org/7205...
>