816168 - Split out WebGLRenderbuffer into separate files

Status: RESOLVED FIXED

(Core :: Graphics: CanvasWebGL, defect)

Description

[Benoit Jacob [:bjacob] (mostly away)]

This is a class living almost entirely inside WebGLContext.h.

Split it into a new separate header, WebGLRenderbuffer.h, and try to move most (or all) of its actual code in WebGLRenderbuffer.cpp.

Typically WebGLRenderbuffer.h would only include WebGLObjectModel.h and not WebGLContext.h. The point is to make it so that we can #include WebGLRenderbuffer.h without dragging in too many dependencies, and to bring WebGLContext.h down to a reasonable size.

See bug 801499 for the work on splitting WebGLBuffer out (which had to do lots of extra work as WebGLObjectModel.h didn't exist yet).

When moving its code into WebGLRenderbuffer.cpp, use discretion to decide where to stop. If a method is very small, does not require dragging in any dependency, is performance-critical so that the cost of the function call matters, and is used outside of WebGLRenderbuffer.cpp, then it can make sense for it to stay in WebGLRenderbuffer.h so that the compiler can still inline it without relying on link-time optimization.

If you get something that builds and passes 1.0.1 conformance tests,
  https://www.khronos.org/registry/webgl/conformance-suites/1.0.1/webgl-conformance-tests.html
you'll know that you're on the right track.

Comment 1

[Saurabh Anand [:sawrubh]]

Attachment: Patch v1

Comment 2

[Benoit Jacob [:bjacob] (mostly away)]

Comment on attachment 687783 [details] [diff] [review]
Patch v1

Review of attachment 687783 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good!

::: content/canvas/src/WebGLObjectModel.cpp
@@ +32,5 @@
> +    } else {
> +        mWidth = 0;
> +        mHeight = 0;
> +    }
> +}

I think I'd rather keep these two methods in the header. They are very small, don't require any dependency, and there could conceivably be cases where the performance impact might matter.

Comment 3

[Saurabh Anand [:sawrubh]]

Attachment: Patch v2

Fixed nit.

Comment 4

[Benoit Jacob [:bjacob] (mostly away)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b8b88df64dba

Comment 5

[Benoit Jacob [:bjacob] (mostly away)]

Comment on attachment 687851 [details] [diff] [review]
Patch v2

By the way, don't clear the review+ flag (or don't obsolete the patch with a review+ flag). Thanks.

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/b8b88df64dba