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.

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

Remove Buffer#getData() calls #8363

Closed
7 tasks
Tracked by #7457
donmccurdy opened this issue Dec 19, 2023 · 2 comments
Closed
7 tasks
Tracked by #7457

Remove Buffer#getData() calls #8363

donmccurdy opened this issue Dec 19, 2023 · 2 comments
Assignees
Labels
Milestone

Comments

@donmccurdy
Copy link
Collaborator

Description

Buffer#getData() will not be available in luma.gl v9, so we'll need to update parts of the deckgl codebase that depend on it. Primarily those seem to be layers using Transform. Where possible, readAsync() would be preferred. If we cannot make all uses asynchronous then we may need a synchronous helper function, either here or in luma.gl.

Flavors

  • Script tag
  • React
  • Python/Jupyter notebook
  • MapboxOverlay
  • GoogleMapsOverlay
  • CartoLayer
  • ArcGIS

Expected Behavior

n/a

Steps to Reproduce

For a general idea of the areas affected:

https://github.com/search?q=repo%3Avisgl%2Fdeck.gl+%22getData%28%22&type=code

Environment

n/a

Logs

n/a

@donmccurdy
Copy link
Collaborator Author

donmccurdy commented Feb 14, 2024

It looks like the last remaining uses of Buffer#getData are:

  1. A disabled unit test:

    // TODO v9 re-enable
    test.skip('Attribute#allocate - partial', t => {
    if (device.info.type !== 'webgl2') {
    // buffer.getData() is WebGL2 only
    t.comment('This test requires WebGL2');
    t.end();
    return;
    }
    let positions = new Attribute(device, {
    id: 'positions',
    update: attr => {
    attr.value[0] = 180;
    attr.value[1] = 90;
    },
    size: 2
    });
    positions.allocate(1);
    let value = positions.value;
    value[0] = 180;
    value[1] = 90;
    // make sure buffer is created
    positions.updateBuffer({});
    t.deepEqual(positions.buffer.getData().slice(0, 2), [180, 90], 'value uploaded to buffer');

  2. GPU/CPU Grid Aggregator implementations and tests

  3. The 'wind' showcase, which is written against deck.gl v5. As discussed in feat(examples): Port bezier example shaders to GLSL 300 ES #8433, we may remove that showcase.

@donmccurdy
Copy link
Collaborator Author

The work originally intended when filing this issue was already addressed in PRs like:

Changes to GPU Grid Aggregator are tracked in many other issues, what to do with showcases is being discussed, and the currently-disabled unit test is one of many that need updates, so I don't think we need this issue to track any of that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant