Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Prep Attribute class for partial update #3091

Merged
merged 2 commits into from May 15, 2019
Merged

Prep Attribute class for partial update #3091

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5d215ca
prep for partial update
May 13, 2019
77b481f
clean up
May 13, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
23 changes: 18 additions & 5 deletions modules/core/src/lib/attribute.js
View file
Open in desktop
Expand Up @@ -171,6 +171,11 @@ export default class Attribute extends BaseAttribute {

this.constant = false;
this.value = new ArrayType(this.size * allocCount);

if (this.buffer && this.buffer.byteLength < this.value.byteLength) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ibgreen @tsherif This is the part I'm hoping to get some ideas - in the incremental update use case (e.g. data loaded by chunks), every time we reallocate the buffer, we need to re-upload the entire value array.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to know ahead of time in that incremental case what the total size of the data will be?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally, no.

Alternatively, every time overflow happens, we could over-allocate the buffer instead of setting it to the exact new size.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That makes sense to me. Something like doubling the size of the buffer on each overflow.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How sensitive should we be regarding GPU memory usage?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It could be a problem on mobile, but I think we have some leeway because we don't use textures much. It would be helpful to profile some typical use cases and see what our current memory usage is like.

this.buffer.reallocate(this.value.byteLength);
}

state.needsUpdate = true;
state.allocedInstances = allocCount;
return true;
Expand All @@ -186,16 +191,24 @@ export default class Attribute extends BaseAttribute {

const state = this.userData;

const {update} = state;
const {update, noAlloc} = state;

let updated = true;
if (update) {
// Custom updater - typically for non-instanced layers
update.call(context, this, {data, props, numInstances, bufferLayout});
this.update({
value: this.value,
constant: this.constant
});
if (noAlloc || this.constant || !this.buffer) {
// Full update
this.update({
value: this.value,
constant: this.constant
});
} else {
// Only update the changed part of the attribute
this.buffer.subData({
data: this.value.subarray(0, numInstances * this.size)
});
}
this._checkAttributeArray();
} else {
updated = false;
Expand Down
40 changes: 22 additions & 18 deletions modules/core/src/lib/base-attribute.js
View file
Open in desktop
Expand Up @@ -76,24 +76,7 @@ export default class BaseAttribute {

// Create buffer if needed
if (!constant && this.gl) {
// Move accessor fields to accessor object
const props = {
...opts,
id: this.id,
target: this.target,
accessor: {
type: this.type
}
};
if (Number.isFinite(props.divisor)) {
props.accessor.divisor = props.divisor;
}
delete props.divisor;
if (Number.isFinite(props.size)) {
props.accessor.size = props.size;
}
delete props.size;
this.buffer = this.buffer || new Buffer(this.gl, props);
this.buffer = this.buffer || this._createBuffer(opts);
this.buffer.setData({data: value});
this.type = this.buffer.accessor.type;
}
Expand All @@ -120,6 +103,27 @@ export default class BaseAttribute {
return null;
}

_createBuffer(opts) {
// Move accessor fields to accessor object
const props = Object.assign({}, opts, {
id: this.id,
target: this.target,
accessor: {
type: this.type
}
});
if (Number.isFinite(props.divisor)) {
props.accessor.divisor = props.divisor;
}
delete props.divisor;
if (Number.isFinite(props.size)) {
props.accessor.size = props.size;
}
delete props.size;

return new Buffer(this.gl, props);
}

// Sets all accessor props except type
// TODO - store on `this.accessor`
_setAccessor(opts) {
Expand Down