Description
Dean Jackson
2017-09-18 19:39:05 PDT
Created attachment 321207 [details]
Patch
Comment on attachment 321207 [details] Patch Attachment 321207 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/4595252 New failing tests: fast/canvas/webgl/tex-image-and-sub-image-2d-with-video.html Created attachment 321219 [details]
Archive of layout-test-results from ews100 for mac-elcapitan
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews100 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Comment on attachment 321207 [details] Patch Attachment 321207 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/4595250 New failing tests: fast/canvas/webgl/tex-image-and-sub-image-2d-with-video.html Created attachment 321220 [details]
Archive of layout-test-results from ews104 for mac-elcapitan-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104 Port: mac-elcapitan-wk2 Platform: Mac OS X 10.11.6
Comment on attachment 321207 [details] Patch Attachment 321207 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/4595312 New failing tests: fast/canvas/webgl/tex-image-and-sub-image-2d-with-video.html Created attachment 321225 [details]
Archive of layout-test-results from ews112 for mac-elcapitan
The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews112 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Comment on attachment 321207 [details] Patch Attachment 321207 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/4595324 New failing tests: webgl/1.0.2/conformance/textures/tex-image-and-sub-image-2d-with-video.html fast/canvas/webgl/tex-image-and-sub-image-2d-with-video.html Created attachment 321228 [details]
Archive of layout-test-results from ews125 for ios-simulator-wk2
The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews125 Port: ios-simulator-wk2 Platform: Mac OS X 10.12.5
Created attachment 327233 [details]
Patch
Comment on attachment 327233 [details] Patch Attachment 327233 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/5280900 New failing tests: fast/canvas/webgl/texImage2D-video-flipY-true.html fast/canvas/webgl/texImage2D-mse-flipY-true.html Created attachment 327247 [details]
Archive of layout-test-results from ews103 for mac-elcapitan
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Comment on attachment 327233 [details] Patch Attachment 327233 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/5281052 New failing tests: fast/canvas/webgl/texImage2D-video-flipY-true.html fast/canvas/webgl/texImage2D-mse-flipY-true.html fast/canvas/webgl/texImage2D-mse-flipY-false.html Created attachment 327251 [details]
Archive of layout-test-results from ews107 for mac-elcapitan-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107 Port: mac-elcapitan-wk2 Platform: Mac OS X 10.11.6
Comment on attachment 327233 [details] Patch Attachment 327233 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/5281647 New failing tests: fast/canvas/webgl/texImage2D-mse-flipY-true.html fast/canvas/webgl/texImage2D-mse-flipY-false.html fast/canvas/webgl/texImage2D-video-flipY-true.html fast/canvas/webgl/texImage2D-video-flipY-false.html platform/mac/media/media-source/videoplaybackquality-decompressionsession.html fast/canvas/webgl/tex-image-and-sub-image-2d-with-video.html Created attachment 327262 [details]
Archive of layout-test-results from ews115 for mac-elcapitan
The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews115 Port: mac-elcapitan Platform: Mac OS X 10.11.6
(In reply to Build Bot from comment #14) > Comment on attachment 327233 [details] > Patch > > Attachment 327233 [details] did not pass mac-wk2-ews (mac-wk2): > Output: http://webkit-queues.webkit.org/results/5281052 > > New failing tests: > fast/canvas/webgl/texImage2D-video-flipY-true.html > fast/canvas/webgl/texImage2D-mse-flipY-true.html > fast/canvas/webgl/texImage2D-mse-flipY-false.html Looks like these tests are failing because the blue is not quite blue enough and the red is not quite red enough (i.e., the red channel in the blue test is 20/255 when it's testing for <20/255). Created attachment 328217 [details]
Patch
Comment on attachment 328217 [details] Patch Attachment 328217 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/5464414 New failing tests: fast/canvas/webgl/texImage2D-mse-flipY-false.html Created attachment 328219 [details]
Archive of layout-test-results from ews103 for mac-elcapitan
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Comment on attachment 328217 [details] Patch Attachment 328217 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/5464518 New failing tests: fast/canvas/webgl/texImage2D-mse-flipY-false.html Created attachment 328222 [details]
Archive of layout-test-results from ews105 for mac-elcapitan-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105 Port: mac-elcapitan-wk2 Platform: Mac OS X 10.11.6
Comment on attachment 328217 [details] Patch Attachment 328217 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/5464507 New failing tests: fast/canvas/webgl/texImage2D-mse-flipY-false.html Created attachment 328223 [details]
Archive of layout-test-results from ews116 for mac-elcapitan
The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews116 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Comment on attachment 328217 [details] Patch Attachment 328217 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/5464516 New failing tests: fast/canvas/webgl/tex-image-and-sub-image-2d-with-video.html Created attachment 328224 [details]
Archive of layout-test-results from ews126 for ios-simulator-wk2
The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews126 Port: ios-simulator-wk2 Platform: Mac OS X 10.12.6
Created attachment 328231 [details]
Patch
Comment on attachment 328231 [details] Patch Attachment 328231 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/5467367 New failing tests: fast/canvas/webgl/tex-image-and-sub-image-2d-with-video.html Created attachment 328236 [details]
Archive of layout-test-results from ews125 for ios-simulator-wk2
The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews125 Port: ios-simulator-wk2 Platform: Mac OS X 10.12.6
And now it looks like iOS is getting the color channels reversed Created attachment 328452 [details]
Patch
Comment on attachment 328452 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=328452&action=review > Source/WebCore/ChangeLog:39 > + uploading BGRA pixel formatted data to a texture; work aronud this bug for now. Typo: around > Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:2333 > - if (!pixelBuffer) > + if (!updateLastPixelBuffer() && (m_lastImage || !m_lastPixelBuffer)) I don't quite understand the logic change here. The !pixelBuffer and !updateLastPixelBuffer() are effectively the same thing... but why did you add && (m_lastImage || !m_lastPixelBuffer)? > Source/WebCore/platform/graphics/cv/VideoTextureCopierCV.cpp:329 > + fragmentShaderSource.appendLiteral("uniform int u_swapColors;\n"); naming: flipColorChannels? swapColorChannels? > Source/WebCore/platform/graphics/cv/VideoTextureCopierCV.cpp:452 > - " yuv.gb = TEXTUREFUNC(u_uvTexture, v_uvTextureCoordinate).rg - vec2(0.5, 0.5);\n" > - " gl_FragColor = vec4(yuv * u_colorMatrix, 1);\n" > + " yuv.gb = TEXTUREFUNC(u_uvTexture, v_uvTextureCoordinate).rg;\n" Why aren't you subtracting 0.5, 0.5 any more? > Source/WebCore/platform/graphics/cv/VideoTextureCopierCV.cpp:-723 > -VideoTextureCopierCV::GC3DStateSaver::GC3DStateSaver(GraphicsContext3D& context) > - : m_context(context) Booohooooo.... this had so much promise :) Comment on attachment 328452 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=328452&action=review >> Source/WebCore/ChangeLog:39 >> + uploading BGRA pixel formatted data to a texture; work aronud this bug for now. > > Typo: around Will fix. >> Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:2333 >> + if (!updateLastPixelBuffer() && (m_lastImage || !m_lastPixelBuffer)) > > I don't quite understand the logic change here. The !pixelBuffer and !updateLastPixelBuffer() are effectively the same thing... but why did you add && (m_lastImage || !m_lastPixelBuffer)? The logic is: "If updateLastPixelBuffer() didn't end up with anything new (and returned false), and we either already converted the last valid m_lastPixelBuffer to an m_lastImage, or there's no m_lastPixelBuffer in the first place, then bail out early." If we didn't get a new m_lastPixelBuffer, and we also don't have a m_lastImage, then we should _not_ do an early return and do the YUV->RGB conversion in the conformer below. The key (I think) to understanding why we do this check are the lines above in updateLastPixelBuffer(): > Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:2310 > + m_lastPixelBuffer = adoptCF([m_videoOutput.get() copyPixelBufferForItemTime:currentTime itemTimeForDisplay:nil]); > + m_lastImage = nullptr; Whenever we update m_lastPixelBuffer with a new value, we clear out m_lastImage. So a call to paint into a WebGL canvas context will result in a non-null m_lastPixelBuffer, and a subsequent call to paint into a 2d canvas context will conform that existing m_lastPixelBuffer into a m_lastImage. >> Source/WebCore/platform/graphics/cv/VideoTextureCopierCV.cpp:452 >> + " yuv.gb = TEXTUREFUNC(u_uvTexture, v_uvTextureCoordinate).rg;\n" > > Why aren't you subtracting 0.5, 0.5 any more? The subtraction is now part of the u_colorMatrix in the "w" column. >> Source/WebCore/platform/graphics/cv/VideoTextureCopierCV.cpp:-723 >> - : m_context(context) > > Booohooooo.... this had so much promise :) It did! Created attachment 328709 [details]
Patch for landing
The commit-queue encountered the following flaky tests while processing attachment 328709 [details]: http/tests/xmlhttprequest/redirect-cross-origin-tripmine.html bug 179840 (authors: ap@webkit.org and rniwa@webkit.org) The commit-queue is continuing to process your patch. Comment on attachment 328709 [details] Patch for landing Clearing flags on attachment: 328709 Committed r225638: <https://trac.webkit.org/changeset/225638> All reviewed patches have been landed. Closing bug. |