Buffer2 follow-up refactoring (remove *_buffer classes) #1578
Conversation
also removes redundant *_buffer tests because they don’t actually test anything useful after buffer2 refactor.
mourner
force-pushed
the
refactor-buffers
branch
from
9cc8220 to
10fd95c
Compare
mourner
force-pushed
the
refactor-buffers
branch
from
10fd95c to
cfe5950
Compare
refactors the code so vertexAttribPointer calls are derived from buffer configuration rather than hardcoded into either individual buffer classes or draw code.
mourner
force-pushed
the
refactor-buffers
branch
from
7adae7a to
bc05f22
Compare
|
This is ready to be merged. I made sure every commit is self-contained and passes tests. |
| @@ -65,29 +64,19 @@ function Buffer(options) { | |||
| } | |||
|
|
|||
| // Start Buffer1 compatability code | |||
There was a problem hiding this comment.
Can we remove this line now?
|
|
||
| Buffer.prototype = { |
There was a problem hiding this comment.
We should use the same Buffer.prototype.bind = syntax here and add a JSdoc comment
|
|
||
| gl.vertexAttribPointer( | ||
| shader['a_' + attrib.name], attrib.components, gl[attrib.type.name], | ||
| false, this.itemSize, offset + attrib.offset); |
There was a problem hiding this comment.
We should use getOffset instead of manually calculating offset + attrib.offset here. That, or we should drop getOffset entirely. Thoughts?
There was a problem hiding this comment.
I'd say drop entirely. Simpler, less code, easier to understand. We can always reintroduce knew flexible methods later when we need them externally.
| * @param shader The active WebGL shader | ||
| * @param {number} offset The offset of the attrubute data in the currently bound GL buffer. | ||
| */ | ||
| Buffer.prototype.setAttribPointers = function(gl, shader, offset) { |
There was a problem hiding this comment.
👍 I was going to put this in the bind method but this is better
A part of the steps outlined here. Final goal of this PR is removing
*_bufferfiles.Cleaning up code feels so good after the Buffer2 refactor — everything finally fits into place. Great work @lucaswoj! cc @jfirebaugh