| Summary: | Support EXT_texture_compression_rgtc WebGL extension | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | James Darpinian <jdarpinian> | ||||||||||||
| Component: | WebGL | Assignee: | James Darpinian <jdarpinian> | ||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||
| Severity: | Normal | CC: | aakash_jain, annulen, ap, cdumez, changseok, commit-queue, dino, esprehn+autocc, ews-watchlist, graouts, gyuyoung.kim, kbr, kondapallykalyan, ryanhaddad, ryuan.choi, sergio, webkit-bug-importer | ||||||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||||||
| Version: | WebKit Nightly Build | ||||||||||||||
| Hardware: | Unspecified | ||||||||||||||
| OS: | Unspecified | ||||||||||||||
| See Also: |
https://bugs.webkit.org/show_bug.cgi?id=219219 https://bugs.webkit.org/show_bug.cgi?id=217813 |
||||||||||||||
| Bug Depends on: | 217424 | ||||||||||||||
| Bug Blocks: | 217761, 217813, 219219 | ||||||||||||||
| Attachments: |
|
||||||||||||||
|
Description
James Darpinian
2020-10-01 17:11:03 PDT
Created attachment 410284 [details]
Patch
Will add conformance test tomorrow. Created attachment 410383 [details]
Patch
Comment on attachment 410383 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=410383&action=review Looks great! One question. r+ > Source/WebCore/html/canvas/WebGLRenderingContextBase.cpp:6739 > + const int kBlockHeight = 4; Have you confirmed that these values (from the extension spec) are correct, and that the tests both successfully enable the extension, and pass? Comment on attachment 410383 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=410383&action=review >> Source/WebCore/html/canvas/WebGLRenderingContextBase.cpp:6739 >> + const int kBlockHeight = 4; > > Have you confirmed that these values (from the extension spec) are correct, and that the tests both successfully enable the extension, and pass? Yes. Although the existing conformance test doesn't appear to load any images or do any rendering. So I guess we don't truly know if any existing implementation actually works. I will try to find some sample images. Created attachment 410704 [details]
Patch
Comment on attachment 410704 [details]
Patch
Added very thorough rendering test based on the existing S3TC test.
Committed r268085: <https://trac.webkit.org/changeset/268085> All reviewed patches have been landed. Closing bug and clearing flags on attachment 410704 [details]. (In reply to EWS from comment #8) > Committed r268085: <https://trac.webkit.org/changeset/268085> The newly added test: webgl/conformance/extensions/s3tc-and-rgtc.html seems to be consistently failing on ios-wk2. EWS indicated this failure on previous version of this patch. Should have waited for ios-wk2 EWS to complete before landing. History: https://results.webkit.org/?suite=layout-tests&test=webgl%2Fconformance%2Fextensions%2Fs3tc-and-rgtc.html Re-opened since this is blocked by bug 217424 From this run: https://build.webkit.org/results/Apple-iOS-14-Simulator-Release-WK2-Tests/r268117%20(171)/results.html https://build.webkit.org/results/Apple-iOS-14-Simulator-Release-WK2-Tests/r268117%20(171)/webgl/conformance/extensions/s3tc-and-rgtc-pretty-diff.html it looks like the bug in the test is that it doesn't handle the case where the extension isn't available: 1CONSOLE MESSAGE: TypeError: undefined is not an object (evaluating 'ext_rgtc.COMPRESSED_RED_RGTC1_EXT') Created attachment 410784 [details]
Patch
Sorry for the breakage. The code to check whether the extension was supported was buggy. Will wait for EWS before relanding. Comment on attachment 410784 [details]
Patch
Looks good assuming the revised test has been tested locally when EXT_texture_compression_rgtc is not supported. r+
Created attachment 410868 [details]
Patch
Committed r268234: <https://trac.webkit.org/changeset/268234> All reviewed patches have been landed. Closing bug and clearing flags on attachment 410868 [details]. |