Summary: | [WebGL] Support for texImage2D of type HALF_FLOAT_OES with ArrayBufferView. | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Nayan Kumar K <nayankk> | ||||||||||||||
Component: | WebGL | Assignee: | Nayan Kumar K <nayankk> | ||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||
Severity: | Normal | CC: | abarth, bajones, buildbot, cmarrin, dglazkov, dino, esprehn+autocc, gman, gyuyoung.kim, haraken, japhet, kbr, noam, ojan.autocc, rakuco, rniwa, webkit.review.bot, zmo | ||||||||||||||
Priority: | P2 | ||||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||
OS: | Unspecified | ||||||||||||||||
Bug Depends on: | |||||||||||||||||
Bug Blocks: | 109853, 111267 | ||||||||||||||||
Attachments: |
|
Description
Nayan Kumar K
2013-02-25 16:12:12 PST
Created attachment 190335 [details]
oes_texture_half_float
(In reply to comment #1) > Created an attachment (id=190335) [details] > oes_texture_half_float Uploaded first patch towards implementing support for OES_texture_half_float in WebKit. With this patch, texImage2D and texSubImage2D will return failure when called with HALF_FLOAT_OES with non-null ArrayBufferView. Subsequent patches will add support for null ArrayBufferView. I am unable to get the test run properly with DumpRenderTree. For some reason DumpRenderTree doesn't advertise GL_OES_texture_half_float as one of the supported extension, whereas chromium/content_shell advertises this string on the same platform! Comment on attachment 190335 [details] oes_texture_half_float View in context: https://bugs.webkit.org/attachment.cgi?id=190335&action=review Thanks for continuing to push this patch forward. Before it is committed I think you should get it running in the Mac port of DRT. You probably need to check for the GL_ARB_half_float_pixel OpenGL extension, and if it's present, then have GraphicsContext3D claim that it supports GL_OES_texture_half_float. Also, the patch needs to be rebaselined and pass the EWS bots. > Source/WebCore/html/canvas/OESTextureHalfFloat.cpp:2 > + * Copyright (C) 2013 Google Inc. All rights reserved. You might want to change the attribution to Motorola. > Source/WebCore/html/canvas/WebGLRenderingContext.cpp:5140 > + break; In addition to these checks, please add validation to the texImage2D and texSubImage2D entry points making sure that you can't upload ImageData, HTMLImageElement, HTMLCanvasElement, or HTMLVideoElement as HALF_FLOAT textures yet. Also, add tests verifying this. > LayoutTests/fast/canvas/webgl/oes-texture-half-float-expected.txt:1 > +CONSOLE MESSAGE: WebGL: INVALID_ENUM: texImage2D: invalid texture type Is this console message coming from the Chromium port of DRT? If so, you should not check in this result into fast/canvas/webgl/ , but into platform/chromium/ instead. > LayoutTests/fast/canvas/webgl/oes-texture-half-float.html:16 > +<!-- Shaders for testing half-floating-point textures --> If you aren't going to add tests verifying rendering to HALF_FLOAT_OES textures yet, then remove these unused shaders and associated code for now. > LayoutTests/fast/canvas/webgl/oes-texture-half-float.html:131 > + gl.texImage2D(gl.TEXTURE_2D, 0, format, width, height, 0, format, gl.HALF_FLOAT_OES, data); You should also verify that allocation with null data succeeds. > LayoutTests/fast/canvas/webgl/oes-texture-half-float.html:138 > + } Please either add a test verifying that you can render to a HALF_FLOAT_OES texture, similar to the oes-texture-float test, or add a FIXME indicating that such a test is needed. > LayoutTests/fast/canvas/webgl/oes-texture-half-float.html:139 > +} Please add missing negative tests ensuring that you can't upload ImageData, HTMLImageElement, HTMLCanvasElement, or HTMLVideoElement into HALF_FLOAT_OES textures yet. Created attachment 190856 [details]
Patch
Attachment 190856 [details] did not pass style-queue:
Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast/canvas/webgl/constants.html', u'LayoutTests/fast/canvas/webgl/oes-texture-half-float-expected.txt', u'LayoutTests/fast/canvas/webgl/oes-texture-half-float-not-supported-expected.txt', u'LayoutTests/fast/canvas/webgl/oes-texture-half-float-not-supported.html', u'LayoutTests/fast/canvas/webgl/oes-texture-half-float.html', u'Source/WebCore/CMakeLists.txt', u'Source/WebCore/ChangeLog', u'Source/WebCore/DerivedSources.make', u'Source/WebCore/DerivedSources.pri', u'Source/WebCore/GNUmakefile.list.am', u'Source/WebCore/Target.pri', u'Source/WebCore/WebCore.gypi', u'Source/WebCore/WebCore.xcodeproj/project.pbxproj', u'Source/WebCore/bindings/js/JSWebGLRenderingContextCustom.cpp', u'Source/WebCore/bindings/v8/custom/V8WebGLRenderingContextCustom.cpp', u'Source/WebCore/html/canvas/OESTextureHalfFloat.cpp', u'Source/WebCore/html/canvas/OESTextureHalfFloat.h', u'Source/WebCore/html/canvas/OESTextureHalfFloat.idl', u'Source/WebCore/html/canvas/WebGLExtension.h', u'Source/WebCore/html/canvas/WebGLRenderingContext.cpp', u'Source/WebCore/html/canvas/WebGLRenderingContext.h', u'Source/WebCore/html/canvas/WebGLRenderingContext.idl', u'Source/WebCore/platform/graphics/Extensions3D.h', u'Source/WebCore/platform/graphics/GraphicsContext3D.cpp', u'Source/WebCore/platform/graphics/GraphicsContext3D.h', u'Source/WebCore/platform/graphics/GraphicsTypes3D.h', u'Source/WebCore/platform/graphics/opengl/GraphicsContext3DOpenGL.cpp']" exit_code: 1
Source/WebCore/platform/graphics/GraphicsContext3D.h:254: enum members should use InterCaps with an initial capital letter. [readability/enum_casing] [4]
Total errors found: 1 in 28 files
If any of these errors are false positives, please file a bug against check-webkit-style.
(In reply to comment #3) > (From update of attachment 190335 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=190335&action=review > > Thanks for continuing to push this patch forward. Before it is committed I think you should get it running in the Mac port of DRT. You probably need to check for the GL_ARB_half_float_pixel OpenGL extension, and if it's present, then have GraphicsContext3D claim that it supports GL_OES_texture_half_float. Also, the patch needs to be rebaselined and pass the EWS bots. Seem like availability of GL_ARB_texture_float (http://www.opengl.org/registry/specs/ARB/texture_float.txt) is enough to implement support for half float texture in dekstop gl. Specification defines the format to be used for half floating point texture. Also, Source/WebCore/platform/graphics/opengl/Extensions3DOpenGL.cpp already has the check to determine whether GL_ARB_texture_float is available before reporting GL_OES_texture_half_float as one of the supported extensions. I have incorporated rest of the comment. Comment on attachment 190856 [details] Patch Attachment 190856 [details] did not pass mac-ews (mac): Output: http://webkit-commit-queue.appspot.com/results/16789372 New failing tests: fast/canvas/webgl/oes-texture-half-float-not-supported.html Comment on attachment 190856 [details] Patch Attachment 190856 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-commit-queue.appspot.com/results/16863138 New failing tests: fast/canvas/webgl/oes-texture-half-float-not-supported.html
> New failing tests:
> fast/canvas/webgl/oes-texture-half-float-not-supported.html
Test passes in my local mac machine, but fails in mac-ews. Is there a way to get the test result from mac-ews?
(In reply to comment #9) > > New failing tests: > > fast/canvas/webgl/oes-texture-half-float-not-supported.html > > Test passes in my local mac machine, but fails in mac-ews. Is there a way to get the test result from mac-ews? I think abarth knows how to get results off the EWS bots. I suggest trying to find him on IRC tomorrow. Comment on attachment 190856 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=190856&action=review Looking good. Please make one more code change and incorporate the layout test in the new location for the WebGL tests. > Source/WebCore/html/canvas/WebGLRenderingContext.cpp:3769 > + // Uploading ImageData to half floating point texture is not supported yet Here and in the other places in this patch, add "FIXME:" at the start of the comment, and also file a bug about fixing this and link to that bug. Also, move this check down after the call to if (!validateSettableTexFormat(...)) in all these places. That way INVALID_ENUM will still be generated (by validateTexFuncFormatAndType) if the extension hasn't been enabled yet. > LayoutTests/fast/canvas/webgl/oes-texture-half-float.html:1 > +<!DOCTYPE html> The new location for these tests is LayoutTests/webgl/ and you can use the script LayoutTests/webgl/generate-webgl-tests.py script to pull in the script verbatim from the one you just contributed to Khronos. Unfortunately, the webgl/ folder isn't yet enabled on any port, so it might be worthwhile to add the test in this location temporarily. However, please also add it in the new location using that script. Also, in order to get this patch landed, you may want to mark the test as having text differences temporarily for the failing platforms (e.g. in LayoutTests/platform/mac/TestExpectations). Don't skip the test, just mark it as failing. Then you can get the results off the bots and see why it's failing. Created attachment 191045 [details]
Patch
> The new location for these tests is LayoutTests/webgl/ and you can use the script LayoutTests/webgl/generate-webgl-tests.py script to pull in the script verbatim from the one you just contributed to Khronos. Unfortunately, the webgl/ folder isn't yet enabled on any port, so it might be worthwhile to add the test in this location temporarily. However, please also add it in the new location using that script.
Currently generation-webgl-tests.py script generates test only for 1.0.2 version of webgl test suite. Test case submitted to Khronos was for 1.0.3. May be we should pull in all 1.0.3 version of test cases at once?
(In reply to comment #13) > Currently generation-webgl-tests.py script generates test only for 1.0.2 version of webgl test suite. Test case submitted to Khronos was for 1.0.3. May be we should pull in all 1.0.3 version of test cases at once? You can use a command line flag to specify where it pulls from. In my case I point it at my local repo's skd/tests. Run "generation-webgl-tests.py -w <path-to-source>" Comment on attachment 191045 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=191045&action=review Thanks for the updates. I suppose it's OK to add the webgl/ copies of the layout tests later. Please make the few corrections above before landing, and make sure it passes the EWS. r=me > Source/WebCore/html/canvas/WebGLRenderingContext.cpp:3816 > + // Uploading ImageElement to half floating point texture is not supported yet FIXME: > Source/WebCore/html/canvas/WebGLRenderingContext.cpp:3817 > + // https://bugs.webkit.org/show_bug.cgi?id=111194 Is it really worth having all of these bugs for all of these different types? I suspect that once you have the core data packing support in place that supporting each of them will be a few lines of code each. > Source/WebCore/html/canvas/WebGLRenderingContext.cpp:3844 > + // Uploading HTMLCanvasElement to half floating point texture is not supported yet FIXME: > Source/WebCore/html/canvas/WebGLRenderingContext.cpp:3906 > + // Uploading HTMLVideoElement to half floating point texture is not supported yet FIXME: > Source/WebCore/html/canvas/WebGLRenderingContext.cpp:4067 > + // Uploading ImageData to half floating point texture is not supported yet FIXME: > Source/WebCore/html/canvas/WebGLRenderingContext.cpp:4105 > + // Uploading HTMLImageElement to half floating point texture is not supported yet FIXME: > Source/WebCore/html/canvas/WebGLRenderingContext.cpp:4133 > + // Uploading HTMLCanvasElement to half floating point texture is not supported yet FIXME: > Source/WebCore/html/canvas/WebGLRenderingContext.cpp:4157 > + // Uploading HTMLVideoElement to half floating point texture is not supported yet FIXME: > Source/WebCore/html/canvas/WebGLRenderingContext.idl:1 > + /* Undo this change please. Comment on attachment 191045 [details] Patch Attachment 191045 [details] did not pass mac-ews (mac): Output: http://webkit-commit-queue.appspot.com/results/16803325 New failing tests: fast/css/hover-update.html Created attachment 191068 [details]
Implement HALF_FLOAT_OES
Attachment 191068 [details] did not pass style-queue:
Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast/canvas/webgl/constants.html', u'LayoutTests/fast/canvas/webgl/oes-texture-half-float-expected.txt', u'LayoutTests/fast/canvas/webgl/oes-texture-half-float-not-supported-expected.txt', u'LayoutTests/fast/canvas/webgl/oes-texture-half-float-not-supported.html', u'LayoutTests/fast/canvas/webgl/oes-texture-half-float.html', u'LayoutTests/platform/chromium/TestExpectations', u'Source/WebCore/CMakeLists.txt', u'Source/WebCore/ChangeLog', u'Source/WebCore/DerivedSources.make', u'Source/WebCore/DerivedSources.pri', u'Source/WebCore/GNUmakefile.list.am', u'Source/WebCore/Target.pri', u'Source/WebCore/WebCore.gypi', u'Source/WebCore/WebCore.xcodeproj/project.pbxproj', u'Source/WebCore/bindings/js/JSWebGLRenderingContextCustom.cpp', u'Source/WebCore/bindings/v8/custom/V8WebGLRenderingContextCustom.cpp', u'Source/WebCore/html/canvas/OESTextureHalfFloat.cpp', u'Source/WebCore/html/canvas/OESTextureHalfFloat.h', u'Source/WebCore/html/canvas/OESTextureHalfFloat.idl', u'Source/WebCore/html/canvas/WebGLExtension.h', u'Source/WebCore/html/canvas/WebGLRenderingContext.cpp', u'Source/WebCore/html/canvas/WebGLRenderingContext.h', u'Source/WebCore/html/canvas/WebGLRenderingContext.idl', u'Source/WebCore/platform/graphics/Extensions3D.h', u'Source/WebCore/platform/graphics/GraphicsContext3D.cpp', u'Source/WebCore/platform/graphics/GraphicsContext3D.h', u'Source/WebCore/platform/graphics/GraphicsTypes3D.h', u'Source/WebCore/platform/graphics/opengl/GraphicsContext3DOpenGL.cpp']" exit_code: 1
Source/WebCore/platform/graphics/GraphicsContext3D.h:254: enum members should use InterCaps with an initial capital letter. [readability/enum_casing] [4]
Total errors found: 1 in 29 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 191068 [details] Implement HALF_FLOAT_OES View in context: https://bugs.webkit.org/attachment.cgi?id=191068&action=review Looks good again. One thing I missed earlier. > Source/WebCore/platform/graphics/opengl/GraphicsContext3DOpenGL.cpp:270 > + type = FLOAT; I just realized -- this should almost certainly be "type = GL_HALF_FLOAT_ARB".
>
> You can use a command line flag to specify where it pulls from. In my case I point it at my local repo's skd/tests. Run "generation-webgl-tests.py -w <path-to-source>"
generate-webgl-tests.py filters out the tests marked 1.0.3 or above. Below is a snippet of generate-webgl-tests.py
GLOBAL_OPTIONS = {
# version use. Tests at or below this will be included.
"version": "1.0.2",
If I change this version to 1.0.3, it generates newly included test. Since other tests of version 1.0.2 is not yet checked-in to LayoutTests, I feel we should checkin all 1.0.3 tests at once.
Created attachment 191073 [details]
Patch
Comment on attachment 191073 [details]
Patch
FYI, for this bug at this point, you could just change the "Reviewed by NOBODY (OOPS!)." to "Reviewed by Kenneth Russell." by hand, and not set the r? bit.
Attachment 191073 [details] did not pass style-queue:
Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast/canvas/webgl/constants.html', u'LayoutTests/fast/canvas/webgl/oes-texture-half-float-expected.txt', u'LayoutTests/fast/canvas/webgl/oes-texture-half-float-not-supported-expected.txt', u'LayoutTests/fast/canvas/webgl/oes-texture-half-float-not-supported.html', u'LayoutTests/fast/canvas/webgl/oes-texture-half-float.html', u'LayoutTests/platform/chromium/TestExpectations', u'Source/WebCore/CMakeLists.txt', u'Source/WebCore/ChangeLog', u'Source/WebCore/DerivedSources.make', u'Source/WebCore/DerivedSources.pri', u'Source/WebCore/GNUmakefile.list.am', u'Source/WebCore/Target.pri', u'Source/WebCore/WebCore.gypi', u'Source/WebCore/WebCore.xcodeproj/project.pbxproj', u'Source/WebCore/bindings/js/JSWebGLRenderingContextCustom.cpp', u'Source/WebCore/bindings/v8/custom/V8WebGLRenderingContextCustom.cpp', u'Source/WebCore/html/canvas/OESTextureHalfFloat.cpp', u'Source/WebCore/html/canvas/OESTextureHalfFloat.h', u'Source/WebCore/html/canvas/OESTextureHalfFloat.idl', u'Source/WebCore/html/canvas/WebGLExtension.h', u'Source/WebCore/html/canvas/WebGLRenderingContext.cpp', u'Source/WebCore/html/canvas/WebGLRenderingContext.h', u'Source/WebCore/html/canvas/WebGLRenderingContext.idl', u'Source/WebCore/platform/graphics/Extensions3D.h', u'Source/WebCore/platform/graphics/GraphicsContext3D.cpp', u'Source/WebCore/platform/graphics/GraphicsContext3D.h', u'Source/WebCore/platform/graphics/GraphicsTypes3D.h', u'Source/WebCore/platform/graphics/opengl/GraphicsContext3DOpenGL.cpp']" exit_code: 1
Source/WebCore/platform/graphics/GraphicsContext3D.h:254: enum members should use InterCaps with an initial capital letter. [readability/enum_casing] [4]
Total errors found: 1 in 29 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 191073 [details] Patch Attachment 191073 [details] did not pass chromium-ews (chromium-xvfb): Output: http://webkit-commit-queue.appspot.com/results/16884013 New failing tests: platform/chromium/virtual/gpu/fast/canvas/webgl/oes-texture-half-float.html platform/chromium/virtual/gpu/fast/canvas/webgl/oes-texture-half-float-not-supported.html Created attachment 191097 [details]
Patch
Attachment 191097 [details] did not pass style-queue:
Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast/canvas/webgl/constants.html', u'LayoutTests/fast/canvas/webgl/oes-texture-half-float-expected.txt', u'LayoutTests/fast/canvas/webgl/oes-texture-half-float-not-supported-expected.txt', u'LayoutTests/fast/canvas/webgl/oes-texture-half-float-not-supported.html', u'LayoutTests/fast/canvas/webgl/oes-texture-half-float.html', u'LayoutTests/platform/chromium/TestExpectations', u'Source/WebCore/CMakeLists.txt', u'Source/WebCore/ChangeLog', u'Source/WebCore/DerivedSources.make', u'Source/WebCore/DerivedSources.pri', u'Source/WebCore/GNUmakefile.list.am', u'Source/WebCore/Target.pri', u'Source/WebCore/WebCore.gypi', u'Source/WebCore/WebCore.xcodeproj/project.pbxproj', u'Source/WebCore/bindings/js/JSWebGLRenderingContextCustom.cpp', u'Source/WebCore/bindings/v8/custom/V8WebGLRenderingContextCustom.cpp', u'Source/WebCore/html/canvas/OESTextureHalfFloat.cpp', u'Source/WebCore/html/canvas/OESTextureHalfFloat.h', u'Source/WebCore/html/canvas/OESTextureHalfFloat.idl', u'Source/WebCore/html/canvas/WebGLExtension.h', u'Source/WebCore/html/canvas/WebGLRenderingContext.cpp', u'Source/WebCore/html/canvas/WebGLRenderingContext.h', u'Source/WebCore/html/canvas/WebGLRenderingContext.idl', u'Source/WebCore/platform/graphics/Extensions3D.h', u'Source/WebCore/platform/graphics/GraphicsContext3D.cpp', u'Source/WebCore/platform/graphics/GraphicsContext3D.h', u'Source/WebCore/platform/graphics/GraphicsTypes3D.h', u'Source/WebCore/platform/graphics/opengl/GraphicsContext3DOpenGL.cpp']" exit_code: 1
Source/WebCore/platform/graphics/GraphicsContext3D.h:254: enum members should use InterCaps with an initial capital letter. [readability/enum_casing] [4]
Total errors found: 1 in 29 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 191097 [details] Patch Clearing flags on attachment: 191097 Committed r144535: <http://trac.webkit.org/changeset/144535> All reviewed patches have been landed. Closing bug. Why was this feature never announced on webkit-dev? FWIW, IDL with an empty definition makes very little senes. Note that this patch acausezd a build fialure on Windows: https://bugs.webkit.org/show_bug.cgi?id=111267 (In reply to comment #29) > Why was this feature never announced on webkit-dev? FWIW, IDL with an empty definition makes very little senes. Sorry about that. WebGL extensions are so narrowly focused that I never considered that their addition needed to be announced on webkit-dev. Do they need to be in the future? WebGL extensions are specified as instances of interfaces, and are defined so that the same object is guaranteed to be returned across multiple calls to WebGLRenderingContext.getExtension. For this reason they have been defined with IDL files, even if those IDL files are empty, so that we can return objects of distinct type for different extensions. Do you have a suggestion on how to implement these semantics more simply? (In reply to comment #30) > Note that this patch acausezd a build fialure on Windows: https://bugs.webkit.org/show_bug.cgi?id=111267 Sorry for breaking windows build. I missed this failure as it was not caught by EWS. |