Skip to content

Support OffscreenCanvas #90

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

Closed
ixcviw7bw opened this issue Jul 16, 2018 · 14 comments · Fixed by #129
Closed

Support OffscreenCanvas #90

ixcviw7bw opened this issue Jul 16, 2018 · 14 comments · Fixed by #129
Assignees
Labels

Comments

@ixcviw7bw
Copy link

ixcviw7bw commented Jul 16, 2018

Spector does not detect canvas controlled by a WebWorker.
When explicitly capturing canvas, it produces error.

    const offscreenCanvas = (renderCanvas as any).transferControlToOffscreen();
    const spector = new SPECTOR.Spector();
    spector.displayUI();
    spector.captureCanvas(offscreenCanvas);

This produces error:

TypeError: e.getAttribute is not a function

Or:

No webgl context available on the chosen canvas.

If the DOM canvas is passed to captureCanvas().

I tried using it inside WebWorker to capture data in JSON, send it to DOM thread and display it there.
But it throws error when imported:

ReferenceError: window is not defined

@sebavan
Copy link
Member

sebavan commented Jul 16, 2018

Yup it is true i have never tried this use case. Iam currently moving so please bare with a bit of delay :-( I ll fix it in a week

@sebavan
Copy link
Member

sebavan commented Jul 23, 2018

I ll probably need a bit more time as my move is taking longer than expected :-(

@sebavan
Copy link
Member

sebavan commented Jul 30, 2018

I have started looking into the issue, and it requires bigger changes than expected. Idea is it should be able to work inside the worker cause once the control has been transfered we can not use the context anymore.

The main issue is that the method used to capture texture fails as it relies on a canvas 2d.

I can probaby quickly have a way to make it work without the textures.

@bilgorajskim, would that work for you ?

Capturing the textures will require more dev.

@sebavan sebavan self-assigned this Jul 30, 2018
@sebavan sebavan added the bug label Jul 30, 2018
@ixcviw7bw
Copy link
Author

ixcviw7bw commented Jul 30, 2018

2D context on OffscreenCanvas works fine on Chrome (I'm using it on Chrome 68 with Experimental Web Platform features enabled in flags), but it's not implemented yet on Firefox.

So I guess texture capturing should work on Chrome, it would only need to make OffscreenCanvas instead of creating DOM canvas (just gave a quick look on how it works, I might be wrong..).

I think it's not worth hacking around to make it work on Firefox, as they will add 2d support eventually.

I don't really need texture capturing anyway :)

Thank you for your interest, I really appreciate it.

@sebavan
Copy link
Member

sebavan commented Feb 15, 2019

Ok I am coming back to this issue and now that context 2d seems good, I ll do the integration just after the front end move to react. Sorry for the huge wait, the module effort we did on babylon took me a longer than expected.

@elalish
Copy link

elalish commented Dec 20, 2019

This is now a blocker for debugging on our project. I get the sense OffscreenCanvas is now on its way to increasing adoption. Are there still major hurdles to getting this working in Spector?

@sebavan
Copy link
Member

sebavan commented Dec 20, 2019

The main blocker for me is to find time :-)

I guess supporting Offscreen Canvas on the main window as I have seen Google doing more and more should be quite straightforward as we still have access to all the rest of the dom.

Also with the support for 2d context offscreen, All the capture mecanism we use should be available without two many changes off the main thread.

@elalish, what is the actual perf gain of using an offscreen canvas from the main thrad. I was wondering if the final blit would not overcome the potential gain ?

I hope to find time soon for this.

@elalish
Copy link

elalish commented Dec 20, 2019

I believe the idea is to use this in concert with ImageBitmapRenderingContext, which we haven't quite put in yet. Also, we plan to get ourselves off the main thread, so this is a stepping stone.

@cdata
Copy link
Contributor

cdata commented Dec 20, 2019

@sebavan the immediate gain for us is super marginal: we don't create a DOM element.

The secondary gain is a lot more significant (WIP): we can start using ImageBitmapRenderingContext and reduce the memory footprint of our render path.

The longer-term reality for us is that we have plans to move all rendering off the main thread, and we absolutely need to be able to debug OffscreenCanvas.

@sebavan
Copy link
Member

sebavan commented Dec 20, 2019

Yup makes much more sense to me :-)

If you need it pretty soon, feel free to make a PR, I am currently focusing my spare time (pretty tight) to not drop Firefox support due to review issues.

@cdata
Copy link
Contributor

cdata commented Dec 21, 2019

@sebavan I made an attempt in #129 - LMKWYT!

@sebavan
Copy link
Member

sebavan commented Dec 21, 2019

I have almost fixed the list for the extension stay tuned :-)
image

@sebavan
Copy link
Member

sebavan commented Dec 21, 2019

You ll just have to think to check the offscreen canvas box at the bottom of the window:
image

@sebavan
Copy link
Member

sebavan commented Dec 21, 2019

So it is under review for Chrome at the moment :-)

Hope it will be available in your extension in a couple of business days.

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

Successfully merging a pull request may close this issue.

4 participants