Skip to content

Commit

Permalink
combine duplicate parts of Buffer and StructArray
Browse files Browse the repository at this point in the history
  • Loading branch information
@ansis
ansis committed Mar 14, 2016
1 parent ca23cca commit d40a059
Show file tree
Hide file tree
Showing 8 changed files with 334 additions and 336 deletions.
81 changes: 44 additions & 37 deletions js/data/bucket.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ var featureFilter = require('feature-filter');
var Buffer = require('./buffer');
var StyleLayer = require('../style/style_layer');
var util = require('../util/util');
var StructArrayType = require('../util/struct_array');

module.exports = Bucket;

Expand Down Expand Up @@ -79,8 +80,10 @@ function Bucket(options) {

if (options.elementGroups) {
this.elementGroups = options.elementGroups;
this.buffers = util.mapObject(options.buffers, function(options) {
return new Buffer(options);
this.buffers = util.mapObject(options.structArrays, function(structArray, bufferName) {
var structArrayType = options.structArrayTypes[bufferName];
var type = (structArrayType.members[0].name === 'vertices' ? Buffer.BufferType.ELEMENT : Buffer.BufferType.VERTEX);
return new Buffer(structArray, structArrayType, type);
});
}
}
Expand All @@ -91,13 +94,13 @@ function Bucket(options) {
*/
Bucket.prototype.populateBuffers = function() {
this.createStyleLayer();
this.createBuffers();
this.createStructArrays();

This comment has been minimized.

Sign in to view

Copy link
@lucaswoj

lucaswoj Mar 15, 2016

Contributor

Can we continue calling them Buffers within Bucket? It is an implementation detail one abstraction level away that Buffers are implemented with StructArrays.

This comment has been minimized.

Sign in to view

Copy link
@ansis

ansis Mar 15, 2016

Author Contributor

Would the createVertexAddMethod and createElementAddMethod be moved into buffer.js?

I think I prefer them separate since Bucket doesn't need any part of Buffer when creating the arraybuffers. And none of the StructArray methods are needed for rendering. If the StructArrays were moved to the inside of Buffer there would be a worker/data-creation side of Buffer and a rendering side of Buffer that wouldn't really share much.

I might be missing how this fits into the long-term vision for Bucket

This comment has been minimized.

Sign in to view

Copy link
@lucaswoj

lucaswoj Mar 15, 2016

Contributor

Would the createVertexAddMethod and createElementAddMethod be moved into buffer.js?

No, Buffer would be a wrapper around StructArray (via composition)

I might be missing how this fits into the long-term vision for Bucket

Every method that remains on the Buffer class in this branch I have moved to Bucket in the data-driven styling branch, in the spirit of "inline then refactor." How would you feel about entirely dropping the Buffer class?

This comment has been minimized.

Sign in to view

Copy link
@ansis

ansis Mar 15, 2016

Author Contributor

Every method that remains on the Buffer class in this branch I have moved to Bucket in the data-driven styling branch, in the spirit of "inline then refactor." How would you feel about entirely dropping the Buffer class?

sounds good to me. If it's going to be inlined with data-driven styling, does it make sense to just leave it as is until then?

This comment has been minimized.

Sign in to view

Copy link
@lucaswoj

lucaswoj Mar 15, 2016

Contributor

does it make sense to just leave it as is until then?

Yes, I think so. Let's leave it be 👍

for (var i = 0; i < this.features.length; i++) {
this.addFeature(this.features[i]);
}

this.trimBuffers();
this.trimArrays();
};

/**
Expand All @@ -113,14 +116,14 @@ Bucket.prototype.makeRoomFor = function(shaderName, numVertices) {
var currentGroup = groups.length && groups[groups.length - 1];

if (!currentGroup || currentGroup.vertexLength + numVertices > 65535) {
var vertexBuffer = this.buffers[this.getBufferName(shaderName, 'vertex')];
var elementBuffer = this.buffers[this.getBufferName(shaderName, 'element')];
var secondElementBuffer = this.buffers[this.getBufferName(shaderName, 'secondElement')];
var vertexArray = this.structArrays[this.getBufferName(shaderName, 'vertex')];
var elementArray = this.structArrays[this.getBufferName(shaderName, 'element')];
var secondElementArray = this.structArrays[this.getBufferName(shaderName, 'secondElement')];

currentGroup = new ElementGroup(
vertexBuffer.length,
elementBuffer && elementBuffer.length,
secondElementBuffer && secondElementBuffer.length
vertexArray.length,
elementArray && elementArray.length,
secondElementArray && secondElementArray.length
);
groups.push(currentGroup);
}
Expand All @@ -133,9 +136,10 @@ Bucket.prototype.makeRoomFor = function(shaderName, numVertices) {
* as necessary.
* @private
*/
Bucket.prototype.createBuffers = function() {
Bucket.prototype.createStructArrays = function() {
var elementGroups = this.elementGroups = {};
var buffers = this.buffers = {};
var structArrays = this.structArrays = {};
var structArrayTypes = this.structArrayTypes = {};

for (var shaderName in this.shaderInterfaces) {
var shaderInterface = this.shaderInterfaces[shaderName];
Expand All @@ -144,10 +148,10 @@ Bucket.prototype.createBuffers = function() {
var vertexBufferName = this.getBufferName(shaderName, 'vertex');
var vertexAddMethodName = this.getAddMethodName(shaderName, 'vertex');

buffers[vertexBufferName] = new Buffer({
type: Buffer.BufferType.VERTEX,
attributes: shaderInterface.attributes
});
var VertexArrayType = new StructArrayType(shaderInterface.attributes, { alignment: Buffer.VERTEX_ATTRIBUTE_ALIGNMENT });

structArrays[vertexBufferName] = new VertexArrayType();
structArrayTypes[vertexBufferName] = VertexArrayType.serialize();

this[vertexAddMethodName] = this[vertexAddMethodName] || createVertexAddMethod(
shaderName,
Expand All @@ -156,16 +160,21 @@ Bucket.prototype.createBuffers = function() {
);
}


if (shaderInterface.elementBuffer) {
var elementBufferName = this.getBufferName(shaderName, 'element');
buffers[elementBufferName] = createElementBuffer(shaderInterface.elementBufferComponents);
this[this.getAddMethodName(shaderName, 'element')] = createElementAddMethod(this.buffers[elementBufferName]);
var ElementArrayType = createElementBufferType(shaderInterface.elementBufferComponents);
structArrays[elementBufferName] = new ElementArrayType();
structArrayTypes[elementBufferName] = ElementArrayType.serialize();
this[this.getAddMethodName(shaderName, 'element')] = createElementAddMethod(this.structArrays[elementBufferName]);
}

if (shaderInterface.secondElementBuffer) {
var secondElementBufferName = this.getBufferName(shaderName, 'secondElement');
buffers[secondElementBufferName] = createElementBuffer(shaderInterface.secondElementBufferComponents);
this[this.getAddMethodName(shaderName, 'secondElement')] = createElementAddMethod(this.buffers[secondElementBufferName]);
var SecondElementArrayType = createElementBufferType(shaderInterface.secondElementBufferComponents);
structArrays[secondElementBufferName] = new SecondElementArrayType();
structArrayTypes[secondElementBufferName] = SecondElementArrayType.serialize();
this[this.getAddMethodName(shaderName, 'secondElement')] = createElementAddMethod(this.structArrays[secondElementBufferName]);
}

elementGroups[shaderName] = [];
Expand All @@ -178,9 +187,9 @@ Bucket.prototype.destroy = function(gl) {
}
};

Bucket.prototype.trimBuffers = function() {
for (var bufferName in this.buffers) {
this.buffers[bufferName].trim();
Bucket.prototype.trimArrays = function() {
for (var bufferName in this.structArrays) {
this.structArrays[bufferName].trim();
}
};

Expand Down Expand Up @@ -212,9 +221,10 @@ Bucket.prototype.serialize = function() {
},
zoom: this.zoom,
elementGroups: this.elementGroups,
buffers: util.mapObject(this.buffers, function(buffer) {
return buffer.serialize();
})
structArrays: util.mapObject(this.structArrays, function(structArray) {
return structArray.serialize();
}),
structArrayTypes: this.structArrayTypes
};
};

Expand All @@ -239,7 +249,7 @@ function createVertexAddMethod(shaderName, shaderInterface, bufferName) {
pushArgs = pushArgs.concat(shaderInterface.attributes[i].value);
}

var body = 'return this.buffers.' + bufferName + '.push(' + pushArgs.join(', ') + ');';
var body = 'return this.structArrays.' + bufferName + '.emplaceBack(' + pushArgs.join(', ') + ');';

if (!createVertexAddMethodCache[body]) {
createVertexAddMethodCache[body] = new Function(shaderInterface.attributeArgs, body);
Expand All @@ -250,19 +260,16 @@ function createVertexAddMethod(shaderName, shaderInterface, bufferName) {

function createElementAddMethod(buffer) {
return function(one, two, three) {
return buffer.push(one, two, three);
return buffer.emplaceBack(one, two, three);
};
}

function createElementBuffer(components) {
return new Buffer({
type: Buffer.BufferType.ELEMENT,
attributes: [{
name: 'vertices',
components: components || 3,
type: Buffer.ELEMENT_ATTRIBUTE_TYPE
}]
});
function createElementBufferType(components) {
return new StructArrayType([{
type: Buffer.ELEMENT_ATTRIBUTE_TYPE,
name: 'vertices',
components: components || 3
}]);
}

function capitalize(string) {
Expand Down
4 changes: 2 additions & 2 deletions js/data/bucket/symbol_bucket.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ SymbolBucket.prototype.shaderInterfaces = {

SymbolBucket.prototype.populateBuffers = function(collisionTile, stacks, icons) {
this.createStyleLayer();
this.createBuffers();
this.createStructArrays();

// To reduce the number of labels that jump around when zooming we need
// to use a text-size value that is the same for all zoom levels.
Expand Down Expand Up @@ -231,7 +231,7 @@ SymbolBucket.prototype.populateBuffers = function(collisionTile, stacks, icons)

this.placeFeatures(collisionTile, this.collisionDebug);

this.trimBuffers();
this.trimArrays();
};

SymbolBucket.prototype.addFeature = function(lines, shapedText, shapedIcon, featureIndex) {
Expand Down
Loading

0 comments on commit d40a059

Please sign in to comment.