RESOLVED FIXED 170325
captureStream is getting black frames with webgl canvas
https://bugs.webkit.org/show_bug.cgi?id=170325
Summary captureStream is getting black frames with webgl canvas
youenn fablet
Reported 2017-03-30 21:45:40 PDT
captureStream is getting black frames with webgl canvas
Attachments
Patch (28.82 KB, patch)
2017-03-30 22:02 PDT, youenn fablet
no flags
Fixing release builds (28.83 KB, patch)
2017-03-31 09:58 PDT, youenn fablet
no flags
Updated according review (29.25 KB, patch)
2017-03-31 16:08 PDT, youenn fablet
no flags
Patch for landing (30.14 KB, patch)
2017-04-03 09:18 PDT, youenn fablet
no flags
youenn fablet
Comment 1 2017-03-30 22:02:07 PDT
youenn fablet
Comment 2 2017-03-30 22:03:06 PDT
(In reply to youenn fablet from comment #0) > captureStream is getting black frames with webgl canvas Regression is due to the timer introduced to not produce too many frames.
youenn fablet
Comment 3 2017-03-31 09:58:06 PDT
Created attachment 305979 [details] Fixing release builds
Dean Jackson
Comment 4 2017-03-31 15:08:14 PDT
Comment on attachment 305979 [details] Fixing release builds View in context: https://bugs.webkit.org/attachment.cgi?id=305979&action=review > Source/WebCore/Modules/mediastream/CanvasCaptureMediaStreamTrack.cpp:136 > + if (canvas.renderingContext() && canvas.renderingContext()->isWebGL()) > + static_cast<WebGLRenderingContextBase*>(canvas.renderingContext())->preserveDrawingBuffer(); Doing this does have a negative performance effect on the WebGL context. Maybe until we have the recording synced with the endPaint, we should print a warning to the console saying this might be slow (if the context wasn't already created with preserveDrawingBuffer = true) > Source/WebCore/html/canvas/WebGLRenderingContextBase.h:210 > + void preserveDrawingBuffer() { m_attributes.preserveDrawingBuffer = true; } I think this should be setPreserveDrawingBuffer(bool), with a preserveDrawingBuffer() getter. > Source/WebCore/testing/Internals.h:590 > + std::optional<TrackFramePromise> m_nextFramePromise; Maybe m_nextTrackFramePromise?
youenn fablet
Comment 5 2017-03-31 16:08:56 PDT
Created attachment 306023 [details] Updated according review
youenn fablet
Comment 6 2017-04-03 08:57:25 PDT
(In reply to Dean Jackson from comment #4) > Comment on attachment 305979 [details] > Fixing release builds > > View in context: > https://bugs.webkit.org/attachment.cgi?id=305979&action=review > > > Source/WebCore/Modules/mediastream/CanvasCaptureMediaStreamTrack.cpp:136 > > + if (canvas.renderingContext() && canvas.renderingContext()->isWebGL()) > > + static_cast<WebGLRenderingContextBase*>(canvas.renderingContext())->preserveDrawingBuffer(); > > Doing this does have a negative performance effect on the WebGL context. > Maybe until we have the recording synced with the endPaint, we should print > a warning to the console saying this might be slow (if the context wasn't > already created with preserveDrawingBuffer = true) Added > > > Source/WebCore/html/canvas/WebGLRenderingContextBase.h:210 > > + void preserveDrawingBuffer() { m_attributes.preserveDrawingBuffer = true; } > > I think this should be setPreserveDrawingBuffer(bool), with a > preserveDrawingBuffer() getter. OK > > Source/WebCore/testing/Internals.h:590 > > + std::optional<TrackFramePromise> m_nextFramePromise; > > Maybe m_nextTrackFramePromise? OK
youenn fablet
Comment 7 2017-04-03 09:18:41 PDT
Created attachment 306076 [details] Patch for landing
youenn fablet
Comment 8 2017-04-03 09:19:45 PDT
Thanks for the review. > Doing this does have a negative performance effect on the WebGL context. > Maybe until we have the recording synced with the endPaint, we should print > a warning to the console saying this might be slow (if the context wasn't > already created with preserveDrawingBuffer = true) I added the log message and added a test to cover "preserveDrawingBuffer = true" case.
WebKit Commit Bot
Comment 9 2017-04-03 09:59:44 PDT
Comment on attachment 306076 [details] Patch for landing Clearing flags on attachment: 306076 Committed r214806: <http://trac.webkit.org/changeset/214806>
WebKit Commit Bot
Comment 10 2017-04-03 09:59:46 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.