x3dom / x3dom

X3DOM. A framework for integrating and manipulating X3D scenes as HTML5/DOM elements.
http://x3dom.org
Other
813 stars 271 forks source link

Removing a <movietexture> dynamically causes errors. #1226

Open microaaron opened 1 year ago

microaaron commented 1 year ago

Removing a <movietexture> dynamically causes errors.

Uncaught TypeError: Failed to execute 'texImage2D' on 'WebGL2RenderingContext': Overload resolution failed. at updateMovie (x3dom.debug.js:21188:16) https://github.com/x3dom/x3dom/blob/23bb1572587a1a9117280d1da26ffc33bc2f1f2e/src/Texture.js#L449

andreasplesch commented 1 year ago

I think this happens because the <video> DOM element is removed without emitting an ended event. .pause() in shutdown should cause an ended event. ended may only be emitted after a video started to play. If there is no user interaction it does not start to play.

andreasplesch commented 1 year ago

It turns out that shutdown actually should not cause an ended playing event because it should not try to loop the video. It seems easiest to just remove the updating in shutdown. I will also add better IOS compatibility, and better auto-starting.

microaaron commented 1 year ago

Thanks.

microaaron commented 1 year ago

This error still exists. https://github.com/x3dom/x3dom/blob/23bb1572587a1a9117280d1da26ffc33bc2f1f2e/src/Texture.js#L449 tex._video=null

Since the MovieTexture is removed, why is updateMovie() still being executed? It seems that in opengl the texture is not destroyed.

andreasplesch commented 1 year ago

I could reproduce on Linux, Chrome 103.0.5060.53 . I had only tested on Windows, latest Chrome where it worked. What platform did you use ?

microaaron commented 1 year ago

I could reproduce on Linux, Chrome 103.0.5060.53 . I had only tested on Windows, latest Chrome where it worked. What platform did you use ?

Windows10, Chrome 102.0.5005.115, Microsoft Edge 103.0.1264.71, Firefox 103.0

microaaron commented 1 year ago

setInterval() is called twice. line 470 and line 478. https://github.com/x3dom/x3dom/blob/835ef694bd3c150d7d819029321525f928dc6da9/src/Texture.js#L465-L479

1228

microaaron commented 1 year ago

And...Wouldn't it be better for the second parameter of setInterval() to match the frame rate of the video?

andreasplesch commented 1 year ago

Thx, a copy and paste mistake.

---on the phone---

On Thu, Jul 28, 2022, 3:50 AM microaaron @.***> wrote:

setInterval() is called twice.

https://github.com/x3dom/x3dom/blob/835ef694bd3c150d7d819029321525f928dc6da9/src/Texture.js#L465-L479

1228 https://github.com/x3dom/x3dom/pull/1228

— Reply to this email directly, view it on GitHub https://github.com/x3dom/x3dom/issues/1226#issuecomment-1197790984, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABPCT227UH2N7EJQG74N3TTVWI3VZANCNFSM54O7FNMA . You are receiving this because you commented.Message ID: @.***>

microaaron commented 1 year ago

Firefox reported two warnings: WEBGL_debug_renderer_info is deprecated in Firefox and will be removed. Please use RENDERER. WebGL warning: readPixels: Out-of-bounds reads with readPixels are deprecated, and may be slow.

andreasplesch commented 1 year ago

And...Wouldn't it be better for the second parameter of setInterval() to match the frame rate of the video?

Ideally, yes. https://developer.mozilla.org/en-US/docs/Web/API/HTMLMediaElement/playbackRate is only relative to the native file rate. Maybe there is a way to determine the file rate.

andreasplesch commented 1 year ago

https://github.com/x3dom/x3dom/commit/c4a27a8750f89cead2741620c836b27e8f7e43f6