{"action":"created","issue":{"url":"https://api.github.com/repos/tensorflow/tfjs-core/issues/1221","repository_url":"https://api.github.com/repos/tensorflow/tfjs-core","labels_url":"https://api.github.com/repos/tensorflow/tfjs-core/issues/1221/labels{/name}","comments_url":"https://api.github.com/repos/tensorflow/tfjs-core/issues/1221/comments","events_url":"https://api.github.com/repos/tensorflow/tfjs-core/issues/1221/events","html_url":"https://github.com/tensorflow/tfjs-core/pull/1221","id":349844588,"node_id":"MDExOlB1bGxSZXF1ZXN0MjA3ODQwODQw","number":1221,"title":"[WIP] Add support for using webgl in a worker via offscreen canvas","user":{"login":"bkazi","id":14239531,"node_id":"MDQ6VXNlcjE0MjM5NTMx","avatar_url":"https://avatars3.githubusercontent.com/u/14239531?v=4","gravatar_id":"","url":"https://api.github.com/users/bkazi","html_url":"https://github.com/bkazi","followers_url":"https://api.github.com/users/bkazi/followers","following_url":"https://api.github.com/users/bkazi/following{/other_user}","gists_url":"https://api.github.com/users/bkazi/gists{/gist_id}","starred_url":"https://api.github.com/users/bkazi/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/bkazi/subscriptions","organizations_url":"https://api.github.com/users/bkazi/orgs","repos_url":"https://api.github.com/users/bkazi/repos","events_url":"https://api.github.com/users/bkazi/events{/privacy}","received_events_url":"https://api.github.com/users/bkazi/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":3,"created_at":"2018-08-12T21:14:46Z","updated_at":"2018-08-14T21:05:15Z","closed_at":null,"author_association":"NONE","pull_request":{"url":"https://api.github.com/repos/tensorflow/tfjs-core/pulls/1221","html_url":"https://github.com/tensorflow/tfjs-core/pull/1221","diff_url":"https://github.com/tensorflow/tfjs-core/pull/1221.diff","patch_url":"https://github.com/tensorflow/tfjs-core/pull/1221.patch"},"body":"#### Description\r\n<!--\r\nPlease describe the pull request here.\r\nAlso, if this is an issue/bug fix, please add the issue link for reference here.\r\n-->\r\ntensorflow/tfjs#102\r\n\r\nOffscreen Canvas is currently only shipping in Chrome so this could break for users who try to use WebGL backend in a worker in unsupported browsers.  \r\nThe same issue applies to tests hence the support check in the test\r\n\r\nAdd feature to check if in worker\r\nAdd types for offscreen canvas\r\nAdd test for canvas when running in worker\r\n\r\n---\r\n<!-- Please do not delete this section -->\r\n##### For repository owners only:\r\n\r\nPlease remember to apply all applicable tags to your pull request.\r\nTags: FEATURE, BREAKING, BUG, PERF, DEV, DOC, SECURITY\r\n\r\nFor more info see: https://github.com/tensorflow/tfjs/blob/master/DEVELOPMENT.md\n\n<!-- Reviewable:start -->\n---\nThis change is [<img src=\"https://reviewable.io/review_button.svg\" height=\"34\" align=\"absmiddle\" alt=\"Reviewable\"/>](https://reviewable.io/reviews/tensorflow/tfjs-core/1221)\n<!-- Reviewable:end -->\n"},"comment":{"url":"https://api.github.com/repos/tensorflow/tfjs-core/issues/comments/413015810","html_url":"https://github.com/tensorflow/tfjs-core/pull/1221#issuecomment-413015810","issue_url":"https://api.github.com/repos/tensorflow/tfjs-core/issues/1221","id":413015810,"node_id":"MDEyOklzc3VlQ29tbWVudDQxMzAxNTgxMA==","user":{"login":"bkazi","id":14239531,"node_id":"MDQ6VXNlcjE0MjM5NTMx","avatar_url":"https://avatars3.githubusercontent.com/u/14239531?v=4","gravatar_id":"","url":"https://api.github.com/users/bkazi","html_url":"https://github.com/bkazi","followers_url":"https://api.github.com/users/bkazi/followers","following_url":"https://api.github.com/users/bkazi/following{/other_user}","gists_url":"https://api.github.com/users/bkazi/gists{/gist_id}","starred_url":"https://api.github.com/users/bkazi/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/bkazi/subscriptions","organizations_url":"https://api.github.com/users/bkazi/orgs","repos_url":"https://api.github.com/users/bkazi/repos","events_url":"https://api.github.com/users/bkazi/events{/privacy}","received_events_url":"https://api.github.com/users/bkazi/received_events","type":"User","site_admin":false},"created_at":"2018-08-14T21:05:15Z","updated_at":"2018-08-14T21:05:15Z","author_association":"NONE","body":"I have a few issues that I would like some discussion around\r\n- There is a lot of duplication of the `IS_WORKER` feature check in the previous commit since it is needed for multiple checks that rely on the canvas to figure out which kind of canvas to use. My idea to improve this was to abstract away the creation of canvases, and default to using `OffscreenCanvas` instead of the full canvas element, falling back to the canvas element on unsupported browsers. Does that seem too future facing for now?\r\n- `OffscreenCanvas` doesn't have TypeScript types since it hasn't shipped in many browsers so this means creating the types for the time being. As a quick fix I have created it such that the type is the same as that of `HTMLCanvasElement` since the API is the same. Do we continue with that or define a separate type for `OffscreenCanvas` and extend the class property to return type `HTMLCanvasElement | OffscreenCanvas`? I think the second one is the better option in the long run but just checking\r\n- the `NUM_BYTES_BEFORE_PAGING` is currently based on a screen size heuristic which isn't available in workers, is there a sensible default for this?"}}
{"actor":{"display_login":"bkazi"}}