| Summary: | webgl/1.0.x/conformance/textures/misc/texture-corner-case-videos.html fails on Cocoa | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Kimmo Kinnunen <kkinnunen> | ||||||||||
| Component: | WebGL | Assignee: | Kimmo Kinnunen <kkinnunen> | ||||||||||
| Status: | RESOLVED FIXED | ||||||||||||
| Severity: | Normal | CC: | cdumez, changseok, dino, esprehn+autocc, ews-watchlist, gyuyoung.kim, kbr, kkinnunen, kondapallykalyan, tony, webkit-bug-importer | ||||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||||
| Version: | WebKit Local Build | ||||||||||||
| Hardware: | Unspecified | ||||||||||||
| OS: | Unspecified | ||||||||||||
| See Also: |
https://bugs.webkit.org/show_bug.cgi?id=228997 https://bugs.webkit.org/show_bug.cgi?id=229052 https://bugs.webkit.org/show_bug.cgi?id=230617 |
||||||||||||
| Bug Depends on: | |||||||||||||
| Bug Blocks: | 222812 | ||||||||||||
| Attachments: |
|
||||||||||||
|
Description
Kimmo Kinnunen
2021-08-05 05:19:27 PDT
Created attachment 434981 [details]
Patch
Comment on attachment 434981 [details] Patch I guess this fix was the one we've just discussed on Slack? Let's finish the discussion there - not sure whether we should be using the natural size and scaling, or using the video's visible rectangle. The test doesn't seem to care about the size of the uploaded texture, only the contents of its various corners. I don't think Chrome scales up during texture upload per the latest changes in https://crbug.com/1175907 . Created attachment 435249 [details]
Patch
(In reply to Kenneth Russell from comment #3) > Comment on attachment 434981 [details] > Patch > > I guess this fix was the one we've just discussed on Slack? > > Let's finish the discussion there - not sure whether we should be using the > natural size and scaling, or using the video's visible rectangle. Thanks for working on this. How about now? Links for posteriority: https://github.com/KhronosGroup/WebGL/pull/2464 https://www.khronos.org/registry/webgl/specs/latest/1.0/#6.9 https://html.spec.whatwg.org/multipage/media.html#htmlvideoelement https://github.com/KhronosGroup/WebGL/issues/3309 Comment on attachment 435249 [details]
Patch
Looking good - let's see what's needed to get all the tests passing.
Created attachment 435338 [details]
Patch
Created attachment 435339 [details]
Patch
Comment on attachment 435339 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=435339&action=review Looks good and pleasingly small! r+ > Source/WebCore/html/canvas/WebGLRenderingContextBase.cpp:-5780 > - // FIXME: Turn this into a GPU-GPU texture copy instead of CPU readback. Is this FIXME still relevant (to ImageBuffer::copyImage), or was the paintCurrentFrameInContext call the CPU readback? I don't remember. (In reply to Kenneth Russell from comment #10) > Comment on attachment 435339 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=435339&action=review > > Looks good and pleasingly small! r+ > > > Source/WebCore/html/canvas/WebGLRenderingContextBase.cpp:-5780 > > - // FIXME: Turn this into a GPU-GPU texture copy instead of CPU readback. > > Is this FIXME still relevant (to ImageBuffer::copyImage), or was the > paintCurrentFrameInContext call the CPU readback? I don't remember. In practice, I think both. This function is called for the purpose of being able to use CPU routines to process the data. I thought it's a bit redundant comment. E.g. to process more stuff with GPU, new cases should be added to the existing GPU functionality that tries to run before this code runs.. ImageBuffer::copyImage() returns Image which is the processed with CPU routines. Committed r280963 (240469@main): <https://commits.webkit.org/240469@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 435339 [details]. |