Closed
Bug 816181
Opened 12 years ago
Closed 12 years ago
Split out WebGLShader into separate files
Categories
(Core :: Graphics: CanvasWebGL, defect)
Core
Graphics: CanvasWebGL
Tracking
()
RESOLVED
FIXED
mozilla20
People
(Reporter: bjacob, Assigned: sawrubh)
References
Details
(Whiteboard: [mentor=bjacob][lang=c++] webgl-next)
Attachments
(1 file)
12.48 KB,
patch
|
bjacob
:
review+
|
Details | Diff | Splinter Review |
This is a class living almost entirely inside WebGLContext.h. Split it into a new separate header, WebGLShader.h, and try to move most (or all) of its actual code to WebGLShader.cpp. Typically WebGLShader.h would only include WebGLObjectModel.h and not WebGLContext.h. The point is to make it so that we can #include WebGLShader.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 WebGLShader.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 WebGLShader.cpp, then it can make sense for it to stay in WebGLShader.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.
Assignee | ||
Comment 1•12 years ago
|
||
Assignee: nobody → saurabhanandiit
Status: NEW → ASSIGNED
Attachment #687782 -
Flags: review?(bjacob)
Reporter | ||
Comment 2•12 years ago
|
||
Comment on attachment 687782 [details] [diff] [review] Patch v1 Review of attachment 687782 [details] [diff] [review]: ----------------------------------------------------------------- Looks great!
Attachment #687782 -
Flags: review?(bjacob) → review+
Reporter | ||
Comment 3•12 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/ef5d80327785
Target Milestone: --- → mozilla20
Comment 4•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/ef5d80327785
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•