-
-
Notifications
You must be signed in to change notification settings - Fork 3.4k
Commit
- Loading branch information
There are no files selected for viewing
5 comments
on commit 08774d9
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @lmccart just tested commit #08774d9 in webgl and it's unfortunately a breaking change. Any chance we can roll back to commit #95e85fa in the meantime?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ahh I'm sorry, I thought I tested that! I just pushed a patch that should fix the issue and also addresses #998. do you want to give it a try?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, I'll take a stab. One thing I should note about multiple webgl instances on same page, this is known to be a tricky limitation in webgl in general. I don't think it's impossible but from what I recall THREE.js had some clever workarounds for multiple gl context on the same page. I'll first fix the breaking change and then tackle the gl instancing issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I fixed both already, if you want to try the latest patch I pushed. we can disable multiple webgl instances though and print an error instead if you think it's a bad idea.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems fine, but do you think you should keep
'defaultCanvas'
to avoid breaking existing sketches embedded in pages that rely on selecting#defaultCanvas
explicitly?