Implement WEBGL_EXPERIMENTAL_compressed_textures WebGL extension
Created attachment 114619 [details] Patch
Please wait for approval from fishd@chromium.org before submitting because this patch contains changes to the Chromium public API.
Comment on attachment 114619 [details] Patch Attachment 114619 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/10429290
Comment on attachment 114619 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=114619&action=review Looks great overall. A few minor comments, plus the GraphicsContext3DOpenGL build fix we discussed offline. > Source/WebCore/html/canvas/WebGLExperimentalCompressedTextures.cpp:48 > + if (gc3DContext->getExtensions()->supports("GL_EXT_texture_compression_dxt1")) { Consider caching the result of getExtensions() in a local variable. > Source/WebCore/html/canvas/WebGLExperimentalCompressedTextures.cpp:104 > + return gc3DContext->getExtensions()->supports("GL_EXT_texture_compression_dxt1") Here too. > Source/WebCore/html/canvas/WebGLExperimentalCompressedTextures.cpp:116 > + if (m_supportsDxt1) Could just return m_supportsDxt1. Here and below. > Source/WebCore/platform/graphics/efl/GraphicsContext3DEfl.cpp:200 > + // m_internal->compressedTexImage2D(target, level, internalformat, width, height, border, imageSize, data); Add a FIXME here and below. > Source/WebKit/chromium/public/WebGraphicsContext3D.h:226 > + virtual void compressedTexSubImage2D(WGC3Denum target, WGC3Dint level, WGC3Dint xoffset, WGC3Dint yoffset, WGC3Dsizei width, WGC3Dsizei height, WGC3Denum format, WGC3Dsizei imageSize, const void* data) = 0; Since these are pure virtual, you'll need to roll forward chromium_rev in Source/WebKit/chromium/DEPS to the revision where you added these entry points, if it hasn't been already.
Created attachment 114775 [details] Patch
Created attachment 114806 [details] Patch
Comment on attachment 114806 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=114806&action=review Looks great. Did you want this submitted via the commit queue? Please mark the patch cq? if so. > Source/WebKit/chromium/DEPS:35 > + 'chromium_rev': '109568' I heard from someone else today that there was a problem rolling forward the Chromium revision, and that the most recent roll was reverted. Could you check on IRC with the Chromium WebKit gardener to see if that problem was resolved?
(In reply to comment #7) > (From update of attachment 114806 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=114806&action=review > > > Source/WebKit/chromium/DEPS:35 > > + 'chromium_rev': '109568' > > I heard from someone else today that there was a problem rolling forward the Chromium revision, and that the most recent roll was reverted. Could you check on IRC with the Chromium WebKit gardener to see if that problem was resolved? Ojan is about to roll to 109696 in https://bugs.webkit.org/attachment.cgi?id=114762&action=prettypatch .
(In reply to comment #8) > (In reply to comment #7) > > (From update of attachment 114806 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=114806&action=review > > > > > Source/WebKit/chromium/DEPS:35 > > > + 'chromium_rev': '109568' > > > > I heard from someone else today that there was a problem rolling forward the Chromium revision, and that the most recent roll was reverted. Could you check on IRC with the Chromium WebKit gardener to see if that problem was resolved? > > Ojan is about to roll to 109696 in https://bugs.webkit.org/attachment.cgi?id=114762&action=prettypatch . Great, thanks. In that case, Gregg, could you re-upload the patch with that section removed? Any committer should be able to cq+ that without needing a re-review.
Created attachment 114818 [details] Patch
Comment on attachment 114818 [details] Patch Looks great.
Comment on attachment 114818 [details] Patch Clearing flags on attachment: 114818 Committed r100176: <http://trac.webkit.org/changeset/100176>
All reviewed patches have been landed. Closing bug.
This broke the Lion build: http://build.webkit.org/builders/Lion%20Intel%20Debug%20%28Build%29/builds/3595/steps/compile-webkit/logs/stdio Undefined symbols for architecture x86_64: "__ZN7WebCore35WebGLExperimentalCompressedTextures6createEPNS_21WebGLRenderingContextE", referenced from: __ZN7WebCore21WebGLRenderingContext12getExtensionERKN3WTF6StringE in WebGLRenderingContext.o "__ZN7WebCore35WebGLExperimentalCompressedTextures27getCompressedTextureFormatsEv", referenced from: __ZN7WebCore21WebGLRenderingContext12getParameterEjRi in WebGLRenderingContext.o "__ZN7WebCore35WebGLExperimentalCompressedTextures9supportedEPNS_21WebGLRenderingContextE", referenced from: __ZN7WebCore21WebGLRenderingContext22getSupportedExtensionsEv in WebGLRenderingContext.o
(In reply to comment #14) > This broke the Lion build: > > http://build.webkit.org/builders/Lion%20Intel%20Debug%20%28Build%29/builds/3595/steps/compile-webkit/logs/stdio > > Undefined symbols for architecture x86_64: > "__ZN7WebCore35WebGLExperimentalCompressedTextures6createEPNS_21WebGLRenderingContextE", referenced from: > __ZN7WebCore21WebGLRenderingContext12getExtensionERKN3WTF6StringE in WebGLRenderingContext.o > "__ZN7WebCore35WebGLExperimentalCompressedTextures27getCompressedTextureFormatsEv", referenced from: > __ZN7WebCore21WebGLRenderingContext12getParameterEjRi in WebGLRenderingContext.o > "__ZN7WebCore35WebGLExperimentalCompressedTextures9supportedEPNS_21WebGLRenderingContextE", referenced from: > __ZN7WebCore21WebGLRenderingContext22getSupportedExtensionsEv in WebGLRenderingContext.o How did this patch make it past the commit queue?
(In reply to comment #15) > (In reply to comment #14) > > This broke the Lion build: > > > > http://build.webkit.org/builders/Lion%20Intel%20Debug%20%28Build%29/builds/3595/steps/compile-webkit/logs/stdio > > > > Undefined symbols for architecture x86_64: > > "__ZN7WebCore35WebGLExperimentalCompressedTextures6createEPNS_21WebGLRenderingContextE", referenced from: > > __ZN7WebCore21WebGLRenderingContext12getExtensionERKN3WTF6StringE in WebGLRenderingContext.o > > "__ZN7WebCore35WebGLExperimentalCompressedTextures27getCompressedTextureFormatsEv", referenced from: > > __ZN7WebCore21WebGLRenderingContext12getParameterEjRi in WebGLRenderingContext.o > > "__ZN7WebCore35WebGLExperimentalCompressedTextures9supportedEPNS_21WebGLRenderingContextE", referenced from: > > __ZN7WebCore21WebGLRenderingContext22getSupportedExtensionsEv in WebGLRenderingContext.o > > How did this patch make it past the commit queue? Oh it's the bots-can't-check-patches-by-non-committers rule. If we land the patch, it seems like the buildbots will be building the patch anyway, so why not test it before landing via the commit bot? At least it's had a review by a trusted person.
Rolled the patch in http://trac.webkit.org/changeset/100191 because of the previous missing symbols.
I'll figure out the missing symbol problem and resubmit the patch. It really shouldn't have made it through the commit queue.
Created attachment 115313 [details] Patch
Updated DerivedSources.make and WebCore.xcodeproj to fix Mac build.
Comment on attachment 115313 [details] Patch Attachment 115313 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/10484643
Created attachment 115476 [details] Patch
Merged with move of ArrayBufferView from WebCore to WTF.
Comment on attachment 115476 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=115476&action=review > Source/WebCore/html/canvas/WebGLRenderingContext.cpp:2117 > + if (equalIgnoringCase(name, "WEBGL_EXPERIMENTAL_compressed_textures")) { Based on the discussions on the WebGL mailing list, we should hold off adding an extension with this name. My feeling is that this should be called WEBKIT_WEBGL_compressed_textures, but I know we haven't made a decision. So until we do, we shouldn't go forward with this name.
Created attachment 117286 [details] Patch after extension rename
Per feedback above and after discussion on public_webgl mailing list, exposed the extension under the name WEBKIT_WEBGL_compressed_textures. Submitting to EWS bots before committing. Further feedback welcome, but review not necessary; it's Gregg's patch and I've already reviewed it.
Comment on attachment 117286 [details] Patch after extension rename Attachment 117286 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/10687354
Created attachment 117293 [details] Fixed more build issues
Committed r101578: <http://trac.webkit.org/changeset/101578>
Probably not the right place - but... Thank you, thank you, thank you! Ben