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

Implementation of loadBytes #2674

Closed
shiffman opened this issue Feb 28, 2018 · 7 comments · Fixed by #2771
Closed

Implementation of loadBytes #2674

shiffman opened this issue Feb 28, 2018 · 7 comments · Fixed by #2771
Assignees

Comments

@shiffman
Copy link
Member

I am opening a new issue since #40 about file i/o is a long thread and this relates specifically to loadBytes(). I am working on an example that loads binary data and thought I would take a crack at adding loadBytes() to p5. I have something working and am happy to pull request an implementation, however, I have a couple questions I'd love thoughts or advice on:

The working new method is towards the bottom.

  1. As discussed in Example of async function for callback and preload() on the libraries wiki, it's required that an original object pointer is retained. I've done this by placing the Uint8Array into a values property of a blank object. Thoughts on how this could be improved? Can I set the values of a Uint8Array if I don't know the size in advance?

  2. I think I am running into registerPreloadMethod mechanism is broken #2568 as I can only get this to work in 0.5.11 (no later versions).

p5.prototype.registerPreloadMethod('loadBytes');

function loadBytes(file, callback) {
  let data = {};
  let oReq = new XMLHttpRequest();
  oReq.open("GET", file, true);
  oReq.responseType = "arraybuffer";
  oReq.onload = function(oEvent) {
    let arrayBuffer = oReq.response;
    if (arrayBuffer) {
      data.values = new Uint8Array(arrayBuffer);
      if (callback) {
        callback(data);
      }
    }
  }
  oReq.send(null);
  return data;
}
@Spongman
Copy link
Contributor

Spongman commented Mar 1, 2018

you'll only run into #2568 if you're using p5 in instance mode. probably the reason you're not seeing preload() complete is you're not calling _decrementPreload() in your loadBytes callback which is necessary to make the preload refcounting thing work.

i was working on something similar a while back using an augmented version of httpDo() that handles Blobs and ArrayBuffers:

    p5.prototype.loadBytes(file, callback, errorCallback) {
      let data = {};

      var self = this;
      this.httpDo(
        file,
        'GET',
        'arrayBuffer',
        function(arrayBuffer) {
          data.bytes = new Uint8Array(arrayBuffer);

          if (typeof callback !== 'undefined') {
            callback(data);
          }

          self._decrementPreload();
        },
        errorCallback
      );
      return data;
    };

note the call to self._decrementPreload(); at the end of the callback.

@Dhakshith
Copy link

@Spongman
It Is Working, But Error:
Uncaught TypeError: Failed to construct 'Request': Request with GET/HEAD method cannot have body.

@shiffman
Copy link
Member Author

shiffman commented Mar 8, 2018

For reference here is my latest "working" version. I have on my list to pull request this to p5. Any comments before I do so? Is it preferable to use httpDo() or any reason?

p5.prototype.registerPreloadMethod('loadBytes');

p5.prototype.loadBytes = function(file, callback) {
  var self = this;
  var data = {};
  var oReq = new XMLHttpRequest();
  oReq.open("GET", file, true);
  oReq.responseType = "arraybuffer";
  oReq.onload = function(oEvent) {
    var arrayBuffer = oReq.response;
    if (arrayBuffer) {
      data.bytes = new Uint8Array(arrayBuffer);
      if (callback) {
        callback(data);
      }
      self._decrementPreload();
    }
  }
  oReq.send(null);
  return data;
}

@shiffman shiffman self-assigned this Mar 8, 2018
@limzykenneth
Copy link
Member

@shiffman Using httpDo is just more consistent with the rest of the loading code base, also makes any future development based on promise easier. However, I don't think it's a must at this point, more of a nice to have.

@Spongman
Copy link
Contributor

Spongman commented Mar 9, 2018

i dug up the httpDo implementation i did, if you're interested. it's here: master...Spongman:loadBytes

@shiffman
Copy link
Member Author

shiffman commented Mar 9, 2018

FYI I am writing this while streaming. I would like to make a video tutorial where I walk though the steps of how to contribute this "enhancement" but I did not get it today. I think this would be useful (?) so I am holding off on submitting until another recording session.

@lmccart
Copy link
Member

lmccart commented Mar 12, 2018

@shiffman this would be great, thank you!

shiffman added a commit to CodingTrain/p5.js that referenced this issue Mar 12, 2018
This commit adds the loadBytes() function to p5.js as demonstrated
during a livestream on The Coding Train. There are some remaining issues
here. For example, there are no tests. I am also not using httpDo()
which would be more of the convention here for p5 as discussed
in processing#2674. I am also handling errors like 404 or CORS in perhaps
an odd way and would be happy to any feedback.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants