DjDeveloperr / skia_canvas

Fast HTML Canvas API implementation for Deno using Google Skia
https://deno.land/x/skia_canvas
Apache License 2.0
124 stars 7 forks source link

feat(Canvas): add support for displaying output in Jupyter Notebook #60

Closed DjDeveloperr closed 11 months ago

rgbkrk commented 11 months ago

Giving your branch a spin now to see if I can find the issue you mentioned on discord.

After getting everything to build I've gotten to the same issue you ran into. I even tried defining the Symbol.for(Jupyter.display) earlier in the code and referencing that directly.

image
rgbkrk commented 11 months ago

Well I got it to work and it must be a TypeScript or Deno quirk. If you bind it in the constructor then the Deno kernel picks it up:

  constructor(width: number, height: number, gpu = false) {
    this[$display] = this[$display].bind(this);
image

👀 Something weird with Deno/Rust picking up the available function on this class @bartlomieju. We gotta find a minimal version that has this odd behavior.

bartlomieju commented 11 months ago

Could you point me which one is not working here? Is it canvas.ts or svgcanvas.ts?

DjDeveloperr commented 11 months ago

Could you point me which one is not working here? Is it canvas.ts or svgcanvas.ts?

@bartlomieju it's canvas.ts

bartlomieju commented 11 months ago

Thanks, I'll try to take a look ASAP.

bartlomieju commented 11 months ago

I just tried this and I got:

Screenshot 2023-09-23 at 10 22 58

I think this is related to https://github.com/denoland/deno/issues/20528

bartlomieju commented 11 months ago

So the underlying issue is that this error is raised:

TypeError: Cannot read properties of undefined (reading 'encode')\n    at [Jupyter.display] (file:///Users/ib/dev/skia_canvas/src/canvas.ts:215:38)\n    at <anonymous>:4:33

However I don't yet understand why that's the case. The transpiled output looks correct and AFAIK this should be the class intance in this case. I will consult with the team and get back to you.

rgbkrk commented 11 months ago

The bug causing it to not render properly has been fixed: https://github.com/denoland/deno/pull/20789

image

I had to jump to https://github.com/denoland/deno/commit/cbddf5756e07fbbe10bd35d23bb9e9c0c685442a to make it work though because deno main can't load this module right now (#61).

DjDeveloperr commented 11 months ago

Awesome fix @rgbkrk - I'll cut a release once that fix makes it to stable!

rgbkrk commented 11 months ago

The fix for ffi is on main.

rgbkrk commented 11 months ago

1.37.2 is out with the fix, feel free to ship 🚢

DjDeveloperr commented 11 months ago

I've released 0.5.5:

image