{"action":"created","issue":{"url":"https://api.github.com/repos/mapbox/mapbox-gl-js/issues/1526","labels_url":"https://api.github.com/repos/mapbox/mapbox-gl-js/issues/1526/labels{/name}","comments_url":"https://api.github.com/repos/mapbox/mapbox-gl-js/issues/1526/comments","events_url":"https://api.github.com/repos/mapbox/mapbox-gl-js/issues/1526/events","html_url":"https://github.com/mapbox/mapbox-gl-js/pull/1526","id":107825585,"number":1526,"title":"Added Buffer2 class","user":{"login":"lucaswoj","id":281306,"avatar_url":"https://avatars.githubusercontent.com/u/281306?v=3","gravatar_id":"","url":"https://api.github.com/users/lucaswoj","html_url":"https://github.com/lucaswoj","followers_url":"https://api.github.com/users/lucaswoj/followers","following_url":"https://api.github.com/users/lucaswoj/following{/other_user}","gists_url":"https://api.github.com/users/lucaswoj/gists{/gist_id}","starred_url":"https://api.github.com/users/lucaswoj/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/lucaswoj/subscriptions","organizations_url":"https://api.github.com/users/lucaswoj/orgs","repos_url":"https://api.github.com/users/lucaswoj/repos","events_url":"https://api.github.com/users/lucaswoj/events{/privacy}","received_events_url":"https://api.github.com/users/lucaswoj/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":19,"created_at":"2015-09-22T23:48:23Z","updated_at":"2015-09-30T15:57:58Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/mapbox/mapbox-gl-js/pulls/1526","html_url":"https://github.com/mapbox/mapbox-gl-js/pull/1526","diff_url":"https://github.com/mapbox/mapbox-gl-js/pull/1526.diff","patch_url":"https://github.com/mapbox/mapbox-gl-js/pull/1526.patch"},"body":"**recut from #1414**\r\n\r\nUp until now, GL JS has used hand-coded classes to specify buffer layouts and buffer manipulation operations. This worked well pre-data-driven-styling because there were < 10 possible buffer layouts. For example, [here is the `TriangleElementBuffer` class](https://github.com/mapbox/mapbox-gl-js/blob/master/js/data/buffer/triangle_element_buffer.js)\r\n\r\n```js\r\nmodule.exports = TriangleElementBuffer;\r\n\r\nfunction TriangleElementBuffer(buffer) {\r\n    Buffer.call(this, buffer);\r\n}\r\n\r\nTriangleElementBuffer.prototype = util.inherit(Buffer, {\r\n    itemSize: 6, // bytes per triangle (3 * unsigned short == 6 bytes)\r\n    arrayType: 'ELEMENT_ARRAY_BUFFER',\r\n\r\n    add: function(a, b, c) {\r\n        var pos2 = this.pos / 2;\r\n\r\n        this.resize();\r\n\r\n        this.ushorts[pos2 + 0] = a;\r\n        this.ushorts[pos2 + 1] = b;\r\n        this.ushorts[pos2 + 2] = c;\r\n\r\n        this.pos += this.itemSize;\r\n    }\r\n});\r\n```\r\n\r\nPost-data-driven styling, however, the number of possible buffer layouts is [practically infinite](https://en.wikipedia.org/wiki/Zero_one_infinity_rule) because any subset of paint properties might be included in the buffer. \r\n\r\nThe first step towards proper data driven styling was to create a more general and powerful framework for buffers (temporarily called `Buffer2`) which allows buffer layouts to be specified by configuration rather than code. Using `Buffer2`, We could write the same `TriangleElementBuffer` class as\r\n\r\n```js\r\nmodule.exports = function() {\r\n    Buffer2.apply(this, {\r\n        type: Buffer2.BufferType.ELEMENT,\r\n        attributes: {\r\n            verticies: {\r\n                components: 3,\r\n                type: Buffer2.AttributeType.UNSIGNED_SHORT\r\n            }\r\n        }\r\n    });\r\n}\r\n```\r\n\r\n... but in practice we *wouldn't* write it that way. GL JS would choose the attributes for that buffer at runtime for the best performance. \r\n\r\nThe `Buffer2` class will know how to construct, write to, and read from GL buffers but have no semantic understanding of their contents. \r\n\r\nInstances of `Buffer2` can be ([destructively](https://developers.google.com/web/updates/2011/12/Transferable-Objects-Lightning-Fast?hl=en)) serialized and transferred across threads. \r\n\r\ncc @tmcw @mourner @kkaefer "},"comment":{"url":"https://api.github.com/repos/mapbox/mapbox-gl-js/issues/comments/144458537","html_url":"https://github.com/mapbox/mapbox-gl-js/pull/1526#issuecomment-144458537","issue_url":"https://api.github.com/repos/mapbox/mapbox-gl-js/issues/1526","id":144458537,"user":{"login":"mourner","id":25395,"avatar_url":"https://avatars.githubusercontent.com/u/25395?v=3","gravatar_id":"","url":"https://api.github.com/users/mourner","html_url":"https://github.com/mourner","followers_url":"https://api.github.com/users/mourner/followers","following_url":"https://api.github.com/users/mourner/following{/other_user}","gists_url":"https://api.github.com/users/mourner/gists{/gist_id}","starred_url":"https://api.github.com/users/mourner/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mourner/subscriptions","organizations_url":"https://api.github.com/users/mourner/orgs","repos_url":"https://api.github.com/users/mourner/repos","events_url":"https://api.github.com/users/mourner/events{/privacy}","received_events_url":"https://api.github.com/users/mourner/received_events","type":"User","site_admin":false},"created_at":"2015-09-30T15:57:58Z","updated_at":"2015-09-30T15:57:58Z","body":"Also, `line_element_buffer` is identical to `triangle_element_buffer` so can be removed as well.\r\n\r\nGenerally I see that we're in a great position to remove `*_buffer.js` classes in this PR and turn them into sets of attribute configs like @lucaswoj envisioned (even before writing `Layer`). For that to happen, we need to eliminate custom `add` and `bind` methods.\r\n\r\nTo eliminate `add` (that's already identical to `push` in my branch for many buffer types), we simply move the packing logic to the corresponding bucket implementations, so this:\r\n\r\n```js\r\nadd: function(x, y, extrudeX, extrudeY) {\r\n    this.push(\r\n        (x * 2) + ((extrudeX + 1) / 2),\r\n        (y * 2) + ((extrudeY + 1) / 2));\r\n},\r\n...\r\ncircleVertex.add(x, y, -1, -1);\r\n```\r\n\r\nBecomes:\r\n\r\n```js\r\ncircleVertex.push(x * 2, y * 2);\r\n```\r\n\r\nWe also may want to keep the multiply-floor packing logic like this in the attribute configs:\r\n\r\n```js\r\nthis.push(\r\n    x, y,\r\n    Math.floor(ox * 64), // use 1/64 pixels for placement\r\n    Math.floor(oy * 64),\r\n    Math.floor(tx / 4), /* tex */\r\n    Math.floor(ty / 4), /* tex */\r\n```\r\n\r\nTo do this, we simply add a `multiplier` option to attributes so this becomes:\r\n\r\n```js\r\nthis.push(\r\n    x, y,\r\n    ox, oy,\r\n    tx, ty,\r\n...\r\nattributes: [{\r\n    name: 'pos',\r\n    components: 2,\r\n    type: Buffer.AttributeType.SHORT\r\n}, {\r\n    name: 'extrude',\r\n    components: 2,\r\n    type: Buffer.AttributeType.SHORT,\r\n    multiplier: 64, // use 1/64 pixels for placement\r\n}, {\r\n    name: 'data1',\r\n    components: 4,\r\n    type: Buffer.AttributeType.UNSIGNED_BYTE,\r\n    multiplier: 1 / 4\r\n}, {\r\n```\r\n\r\nTo eliminate custom `bind`, we can simply write a generic implementation that uses the attributes configuration of the buffer. Currently this kind of code is redundant:\r\n\r\n```js\r\ngl.vertexAttribPointer(shader.a_pos, 2, gl.SHORT, false, stride, offset + 0);\r\ngl.vertexAttribPointer(shader.a_offset, 2, gl.SHORT, false, stride, offset + 4);\r\ngl.vertexAttribPointer(shader.a_data1, 4, gl.UNSIGNED_BYTE, false, stride, offset + 8);\r\ngl.vertexAttribPointer(shader.a_data2, 2, gl.UNSIGNED_BYTE, false, stride, offset + 12);\r\n```\r\n\r\nTo make things more confusing, vertexAttribPointer configuration happens in two places. For some buffer types like line vertex, it's inside `draw_*.js`, and for others like icon vertex it's in `*_buffer.js`. So writing a generic `bind` in `buffer.js` will make the code much cleaner."}}