[webkit-dev] Protecting against stale pointers in DrawingBuffer and GraphicsContext3D

[James Robinson]

On Mon, Oct 11, 2010 at 4:03 PM, Chris Marrin <cmarrin at apple.com> wrote:

>
> On Oct 11, 2010, at 3:35 PM, James Robinson wrote:
>
> > On Mon, Oct 11, 2010 at 3:15 PM, Chris Marrin <cmarrin at apple.com> wrote:
> >
> > For accelerated 2D rendering we created a class called DrawingBuffer.
> This encapsulates the accelerated drawing surface (usually in the GPU) and
> the compositing layer used to display that surface on the page. The drawing
> surface (which is called a Framebuffer Object or FBO) is allocated by
> GraphicsContext3D, so DrawingBuffer needs a reference to that. Currently
> this is a weak reference. DrawingBuffer has a ::create() method and you pass
> the GraphicsContext3D to that method.
> >
> > If you destroy the GraphicsContext3D, DrawingBuffer has a stale pointer.
> If you were to try to destroy the DrawingBuffer it would attempt to use that
> pointer (to destroy its FBO) and crash or worse. Currently we have an
> implicit policy that you should never destroy a GraphicsContext3D before its
> DrawingBuffers are all destroyed. That works fine in the current use case,
> CanvasRenderingContext2D. And we can follow that same policy when in the
> future when we use DrawingBuffer in WebGLRenderingContext.
> >
> > My concern is that this sort of implicit policy can lead to errors in the
> future when other potential clients of these classes use them. So I posted
> https://bugs.webkit.org/show_bug.cgi?id=47501. In that patch I move the
> creation of DrawingBuffer to the GraphicsContext3D and keep back pointers to
> all the DrawingBuffers allocated so they can be cleaned up when
> GraphicsContext3D is destroyed. In talking to James R. he's concerned this
> adds unnecessary complexity and would rather stick with the implicit policy.
> >
> > So is this something I should be concerned about, or is an implicit
> policy sufficient in this case?
> >
> > Before somebody suggests it, I think Chris and I are in agreement that
> neither GraphicsContext3D nor DrawingBuffer should be RefCounted.  They both
> have clear single-ownership semantics.
>
> True, although Simon and I just chatted and he pointed out that Refcounting
> both classes would solve this problem. The fact that GraphicsContext3D
> wouldn't need a back pointer to DrawingBuffer means we wouldn't have any
> circular references. I don't think the overhead of refcounting is an issue
> here, so maybe that would be a simpler solution?
>

I'd really prefer not to make them RefCounted.  The problem is that
GraphicsContext3D and DrawingBuffer are very heavyweight objects, which
means we need to manage their lifetimes tightly and avoid leaving them lying
around longer than necessary.  The exact resource use of these objects
depends on the system but a GraphicsContext3D will typically be backed by an
OpenGL context, which implies some set of driver resources, and a
DrawingBuffer is normally backed by a few megabytes of VRAM.  In the current
code, there is always a single object responsible for managing the lifetime
of a given GraphicsContext3D or a DrawingBuffer which makes it very easy to
ensure that they live for as long as necessary but no longer.  With a
RefCounted object it can be difficult to ensure that all references to a
given object go away when necessary.

In this particular case DrawingBuffer exists at a slightly higher
abstraction layer than GraphicsContext3D.  DrawingBuffer depends on GC3D,
but there is no dependency the other way (and IMHO there should not be).
 This means that the lifetime of a DrawingBuffer depends on the underlying
GraphicsContext3D, but not the other way 'round.  All callers that use or
will want to use a DrawingBuffer already have to be aware of the lifetime of
the GraphicsContext3D associated with it since the only way to use a
DrawingBuffer is by using the GraphicsContext3D API.

For those following along at home, the current user of DrawingBuffer is
CanvasRenderingContext2D which has an OwnPtr<DrawingBuffer> and uses a
GraphicsContext3D that is guaranteed to outlive the
CanvasRenderingContext2D.  The next proposed user of DrawingBuffer is
WebGLRenderingContext which manages its own GraphicsContext3D via a member
OwnPtr<>.

- James


> -----
> ~Chris
> cmarrin at apple.com
>
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-dev/attachments/20101011/18ac2cd0/attachment.html>