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

Suggest optimizion for retrive image buffer. #230

Closed
MaxGraey opened this issue Jan 13, 2019 · 11 comments · Fixed by #264
Closed

Suggest optimizion for retrive image buffer. #230

MaxGraey opened this issue Jan 13, 2019 · 11 comments · Fixed by #264
Assignees

Comments

@MaxGraey
Copy link
Contributor

MaxGraey commented Jan 13, 2019

Current method: https://github.com/torch2424/wasmBoy/blob/bcc26f197b9516f136d385b7ffad1d7734b7d4da/lib/graphics/worker/imageData.js#L4

could be optimize to:

import { GAMEBOY_CAMERA_WIDTH, GAMEBOY_CAMERA_HEIGHT } from '../constants';
// move out creation of buffers, which is not cheap
const imageDataArray = new Uint8ClampedArray(GAMEBOY_CAMERA_HEIGHT * GAMEBOY_CAMERA_WIDTH * 4);

// use non-closure version for method
export function getImageDataFromGraphicsFrameBuffer(wasmByteMemory) {
   for (let y = 0; y < GAMEBOY_CAMERA_HEIGHT; ++y) {
    let stride1 = y * (GAMEBOY_CAMERA_WIDTH * 3);
    let stride2 = y * (GAMEBOY_CAMERA_WIDTH * 4);
    for (let x = 0; x < GAMEBOY_CAMERA_WIDTH; ++x) {
      const pixelStart = stride1 + x * 3;
      const imageDataIndex = stride2 + (x << 2);
      imageDataArray[imageDataIndex + 0] = wasmByteMemory[pixelStart + 0];
      imageDataArray[imageDataIndex + 1] = wasmByteMemory[pixelStart + 1];
      imageDataArray[imageDataIndex + 2] = wasmByteMemory[pixelStart + 2];
      // Alpha, no transparency
      imageDataArray[imageDataIndex + 3] = 255;
    }
  }
  return imageDataArray;
}
@torch2424 torch2424 self-assigned this Jan 13, 2019
@torch2424
Copy link
Owner

Ohhh! Thank you for this! 😄

@torch2424
Copy link
Owner

If I could ask, could you explain how this works?

@MaxGraey
Copy link
Contributor Author

MaxGraey commented Jan 13, 2019

Sure. At first I just cache some vars which relate to y index and could move out from internal loop: stride1 and stride2. Secondary, I remove unnecessary rgbColor array and loop and direct write from wasmByteMemory to imageDataArray instead doing this through rgbColor. Also I move out Uint8ClampedArray creation which don't require create every time when getImageDataFromGraphicsFrameBuffer call.

@torch2424
Copy link
Owner

Rad perfect, makes sense. Was asking, so I can optimize some other things. Thanks! 😄

@MaxGraey
Copy link
Contributor Author

Need to look. I just opened the random file and decide make this suggestion)

@MaxGraey
Copy link
Contributor Author

btw did you see this?
#221

@torch2424
Copy link
Owner

@MaxGraey Yes! I have seen that, just haven't gotten around to it 😂

@MaxGraey
Copy link
Contributor Author

MaxGraey commented Jan 13, 2019

Also are you try just do:

this.canvasImageData.data.set(this.imageDataArray);

instead:

for (let i = 0; i < this.imageDataArray.length; i++) {
   this.canvasImageData.data[i] = this.imageDataArray[i];
}

in this line?

@torch2424
Copy link
Owner

Yeah, I could definitely do that instead, been slowly replacing my for loops with set 😄

@MaxGraey
Copy link
Contributor Author

Also you could add @inline decorators for all functions in this and this files

@torch2424
Copy link
Owner

@MaxGraey Thanks! Yeah, I really need to learn/use the inline decorator. My plan was to use closure compiler over my JS output, but preserve function names in the output. And simply inline all of the functions that closure is inlining 😄 But I'll definitely go through and do those as well 😄

Thank you SOOOO much this is all really appreciated!

torch2424 added a commit that referenced this issue Feb 21, 2019
torch2424 added a commit that referenced this issue Feb 22, 2019
* Implemented all changes from #230 and #221

* Fixed off by one mentioned in #216

* Wrapped modulo in i32Portable as mentioned in #216

* issue #207, Allowed making closure builds for debugging, and then tried
to match its inlining

* Removed the legacy api

* Updated the package-lock for the branch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants