125293 2013-12-05 02:19:32 -0800 GL_HALF_FLOAT_ARB causes a build break when using GLES. 2013-12-16 22:03:40 -0800 1 1 1 Unclassified WebKit WebGL 528+ (Nightly build) Unspecified Linux RESOLVED DUPLICATE 125541 P2 Normal --- 1 changseok changseok bfulgham commit-queue dino elima kondapallykalyan mrobinson noam roger_fong oldest_to_newest 956815 0 changseok 2013-12-05 02:19:32 -0800 GL_HALF_FLOAT_ARB is not a valid parameter for GLES. Maybe r160119 is the related changeset. 956819 1 218494 changseok 2013-12-05 02:30:02 -0800 Created attachment 218494 Patch 956820 2 commit-queue 2013-12-05 02:32:34 -0800 Attachment 218494 did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCore/platform/graphics/opengl/GraphicsContext3DOpenGL.cpp', u'Source/WebCore/platform/graphics/opengl/GraphicsContext3DOpenGLCommon.cpp', u'Source/WebCore/platform/graphics/opengl/GraphicsContext3DOpenGLES.cpp', '--commit-queue']" exit_code: 1 ERROR: Source/WebCore/platform/graphics/opengl/GraphicsContext3DOpenGL.cpp:283: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WebCore/platform/graphics/opengl/GraphicsContext3DOpenGL.cpp:283: Comma should be at the beggining of the line in a member initialization list. [whitespace/init] [4] ERROR: Source/WebCore/platform/graphics/opengl/GraphicsContext3DOpenGLES.cpp:216: Wrong number of spaces before statement. (expected: 8) [whitespace/indent] [4] ERROR: Source/WebCore/platform/graphics/opengl/GraphicsContext3DOpenGLES.cpp:216: Comma should be at the beggining of the line in a member initialization list. [whitespace/init] [4] Total errors found: 4 in 3 files If any of these errors are false positives, please file a bug against check-webkit-style. 956822 3 changseok 2013-12-05 02:34:18 -0800 (In reply to comment #2) > Attachment 218494 [details] did not pass style-queue: > > Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCore/platform/graphics/opengl/GraphicsContext3DOpenGL.cpp', u'Source/WebCore/platform/graphics/opengl/GraphicsContext3DOpenGLCommon.cpp', u'Source/WebCore/platform/graphics/opengl/GraphicsContext3DOpenGLES.cpp', '--commit-queue']" exit_code: 1 > ERROR: Source/WebCore/platform/graphics/opengl/GraphicsContext3DOpenGL.cpp:283: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] > ERROR: Source/WebCore/platform/graphics/opengl/GraphicsContext3DOpenGL.cpp:283: Comma should be at the beggining of the line in a member initialization list. [whitespace/init] [4] > ERROR: Source/WebCore/platform/graphics/opengl/GraphicsContext3DOpenGLES.cpp:216: Wrong number of spaces before statement. (expected: 8) [whitespace/indent] [4] > ERROR: Source/WebCore/platform/graphics/opengl/GraphicsContext3DOpenGLES.cpp:216: Comma should be at the beggining of the line in a member initialization list. [whitespace/init] [4] > Total errors found: 4 in 3 files > > > If any of these errors are false positives, please file a bug against check-webkit-style. I think this is a false alarm. 957517 4 218494 mrobinson 2013-12-07 09:17:45 -0800 Comment on attachment 218494 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=218494&action=review > Source/WebCore/platform/graphics/opengl/GraphicsContext3DOpenGLES.cpp:216 > + // FIXME: we will need to deal with PixelStore params when dealing with image buffers that differ from the subimage size. > + ::glTexSubImage2D(target, level, xoff, yoff, width, height, format, type, pixels); Is HALF_FLOAT_OES supported properly without any translation? 958240 5 elima 2013-12-10 01:31:50 -0800 Looks like the style errors are indeed false positives. Can we move on and review the it? I'm having the same problem. 958620 6 mrobinson 2013-12-11 00:49:28 -0800 So it looks like HALF_FLOAT_OES is supported if http://www.khronos.org/registry/gles/extensions/OES/OES_texture_float.txt is supported, but Extensions3DOpenGLES::supportsExtension is not written correctly to detect OpenGLES extensions. It should be written to detect if m_availableExtensions contains the appropriate extensions. Proper support for OES_texture_float also requires properly reporting that the extension is available. We should do more than fix the build here and implement a proper fix. 958629 7 changseok 2013-12-11 01:20:32 -0800 (In reply to comment #6) > So it looks like HALF_FLOAT_OES is supported if http://www.khronos.org/registry/gles/extensions/OES/OES_texture_float.txt is supported, but Extensions3DOpenGLES::supportsExtension is not written correctly to detect OpenGLES extensions. It should be written to detect if m_availableExtensions contains the appropriate extensions. Proper support for OES_texture_float also requires properly reporting that the extension is available. We should do more than fix the build here and implement a proper fix. got it. I'll do! 959212 8 218494 bfulgham 2013-12-12 09:24:10 -0800 Comment on attachment 218494 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=218494&action=review I don't think this patch is needed now that Bug 125541 has landed. > Source/WebCore/platform/graphics/opengl/GraphicsContext3DOpenGLCommon.cpp:-1366 > - Why is this being removed? ANGLE-based code uses it, so this will break Windows at the very least. https://bugs.webkit.org/show_bug.cgi?id=125541 protects against converting HALF_FLOAT_OES to HALF_FLOAT_ARB on GLES-based platforms, so this should be safe. > Source/WebCore/platform/graphics/opengl/GraphicsContext3DOpenGLES.cpp:218 > + I don't think this should be moved here. 960396 9 changseok 2013-12-16 21:53:16 -0800 *** This bug has been marked as a duplicate of bug 125541 *** 960401 10 218494 changseok 2013-12-16 22:03:40 -0800 Comment on attachment 218494 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=218494&action=review >> Source/WebCore/platform/graphics/opengl/GraphicsContext3DOpenGLES.cpp:218 >> + > > I don't think this should be moved here. Well.. AFAIK, why we split GC3D into GC3DOpenGLCommon, GC3DOpenGL and GC3DOpenGLES was to prevent abusing build flags like USE(OPENGL_ES_2). That was why I moved texSubImage2D here. 218494 2013-12-05 02:30:02 -0800 2013-12-16 22:03:40 -0800 Patch bug-125293-20131205193001.patch text/plain 4643 changseok U3VidmVyc2lvbiBSZXZpc2lvbjogMTYwMTYxCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggMzQyYjU0YmZiMzY5ZWEy NTYzYzhmODQ5MTUxOTljNWY4ZDlhOWFmNy4uMjMwZDAwYTVlZmUwOTc1Njk1MjljYzZiZjRmM2Vi NzBjNzI4ODgwNSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDIxIEBACisyMDEzLTEyLTA1ICBDaGFu Z1Nlb2sgT2ggIDxjaGFuZ3Nlb2sub2hAY29sbGFib3JhLmNvbT4KKworICAgICAgICBHTF9IQUxG X0ZMT0FUX0FSQiBjYXVzZXMgYSBidWlsZCBicmVhayB3aGVuIHVzaW5nIEdMRVMuCisgICAgICAg IGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xMjUyOTMKKworICAgICAg ICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBHTF9IQUxGX0ZMT0FUX0FS QiBpcyBub3QgdmFsaWQgZm9yIEdMRVMuIFNvIEkgdGhpbmsgaXQncyBiZXR0ZXIgdG8gbW92ZQor ICAgICAgICB0aGUgdGV4U3ViSW1hZ2UyRCB0byBHQzNER0wuY3BwIGFuZCBHQzNER0xFUy5jcHAu CisKKyAgICAgICAgTm8gZnVuY3Rpb25hbGl0eSBjaGFuZ2VkLgorCisgICAgICAgICogcGxhdGZv cm0vZ3JhcGhpY3Mvb3BlbmdsL0dyYXBoaWNzQ29udGV4dDNET3BlbkdMLmNwcDoKKyAgICAgICAg KFdlYkNvcmU6OkdyYXBoaWNzQ29udGV4dDNEOjp0ZXhTdWJJbWFnZTJEKToKKyAgICAgICAgKiBw bGF0Zm9ybS9ncmFwaGljcy9vcGVuZ2wvR3JhcGhpY3NDb250ZXh0M0RPcGVuR0xDb21tb24uY3Bw OgorICAgICAgICAqIHBsYXRmb3JtL2dyYXBoaWNzL29wZW5nbC9HcmFwaGljc0NvbnRleHQzRE9w ZW5HTEVTLmNwcDoKKyAgICAgICAgKFdlYkNvcmU6OkdyYXBoaWNzQ29udGV4dDNEOjp0ZXhTdWJJ bWFnZTJEKToKKwogMjAxMy0xMi0wNCAgR3VycHJlZXQgS2F1ciAgPGsuZ3VycHJlZXRAc2Ftc3Vu Zy5jb20+CiAKICAgICAgICAgJSB1bml0IGhlaWdodHMgZG9uJ3Qgd29yayBpZiBwYXJlbnQgYmxv Y2sgaGVpZ2h0IGlzIHNldCBpbiB2aApkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUvcGxhdGZv cm0vZ3JhcGhpY3Mvb3BlbmdsL0dyYXBoaWNzQ29udGV4dDNET3BlbkdMLmNwcCBiL1NvdXJjZS9X ZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL29wZW5nbC9HcmFwaGljc0NvbnRleHQzRE9wZW5HTC5j cHAKaW5kZXggODY0NGM5YzQ2MmYwYzNjYjlkN2ZkMTY3ZWQwOTNjNGMwNWQwYmRlNy4uMmJlZWFm ZDY1NmMxNTRhOWM5ZmQ5NjMwOTVkN2Y4Mzk5ZmFjODVkNyAxMDA2NDQKLS0tIGEvU291cmNlL1dl YkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3Mvb3BlbmdsL0dyYXBoaWNzQ29udGV4dDNET3BlbkdMLmNw cAorKysgYi9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy9vcGVuZ2wvR3JhcGhpY3ND b250ZXh0M0RPcGVuR0wuY3BwCkBAIC0yNzIsNiArMjcyLDE3IEBAIGJvb2wgR3JhcGhpY3NDb250 ZXh0M0Q6OnRleEltYWdlMkQoR0MzRGVudW0gdGFyZ2V0LCBHQzNEaW50IGxldmVsLCBHQzNEZW51 bSBpbnRlCiAgICAgcmV0dXJuIHRydWU7CiB9CiAKK3ZvaWQgR3JhcGhpY3NDb250ZXh0M0Q6OnRl eFN1YkltYWdlMkQoR0MzRGVudW0gdGFyZ2V0LCBHQzNEaW50IGxldmVsLCBHQzNEaW50IHhvZmYs IEdDM0RpbnQgeW9mZiwgR0MzRHNpemVpIHdpZHRoLCBHQzNEc2l6ZWkgaGVpZ2h0LCBHQzNEZW51 bSBmb3JtYXQsIEdDM0RlbnVtIHR5cGUsIGNvbnN0IHZvaWQqIHBpeGVscykKK3sKKyAgICBtYWtl Q29udGV4dEN1cnJlbnQoKTsKKworICAgIGlmICh0eXBlID09IEhBTEZfRkxPQVRfT0VTKQorICAg ICAgICB0eXBlID0gR0xfSEFMRl9GTE9BVF9BUkI7CisKKyAgICAvLyBGSVhNRTogd2Ugd2lsbCBu ZWVkIHRvIGRlYWwgd2l0aCBQaXhlbFN0b3JlIHBhcmFtcyB3aGVuIGRlYWxpbmcgd2l0aCBpbWFn ZSBidWZmZXJzIHRoYXQgZGlmZmVyIGZyb20gdGhlIHN1YmltYWdlIHNpemUuCisgICAgOjpnbFRl eFN1YkltYWdlMkQodGFyZ2V0LCBsZXZlbCwgeG9mZiwgeW9mZiwgd2lkdGgsIGhlaWdodCwgZm9y bWF0LCB0eXBlLCBwaXhlbHMpOworfQorCiB2b2lkIEdyYXBoaWNzQ29udGV4dDNEOjpkZXB0aFJh bmdlKEdDM0RjbGFtcGYgek5lYXIsIEdDM0RjbGFtcGYgekZhcikKIHsKICAgICBtYWtlQ29udGV4 dEN1cnJlbnQoKTsKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNz L29wZW5nbC9HcmFwaGljc0NvbnRleHQzRE9wZW5HTENvbW1vbi5jcHAgYi9Tb3VyY2UvV2ViQ29y ZS9wbGF0Zm9ybS9ncmFwaGljcy9vcGVuZ2wvR3JhcGhpY3NDb250ZXh0M0RPcGVuR0xDb21tb24u Y3BwCmluZGV4IGYyZjcwYmM2M2MyMjYyNDU0OTI0YjUzMGIxYjA3ZTRlZGUxY2I3YzYuLjE5YTkz YTA0OGNlNmQ2ZmQ5ZTdlZmQ5NjZmZWI0ODA4ODAzYWM1ZmEgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9X ZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL29wZW5nbC9HcmFwaGljc0NvbnRleHQzRE9wZW5HTENv bW1vbi5jcHAKKysrIGIvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3Mvb3BlbmdsL0dy YXBoaWNzQ29udGV4dDNET3BlbkdMQ29tbW9uLmNwcApAQCAtMTM1MywxNyArMTM1Myw2IEBAIEdD M0RzaXplaXB0ciBHcmFwaGljc0NvbnRleHQzRDo6Z2V0VmVydGV4QXR0cmliT2Zmc2V0KEdDM0R1 aW50IGluZGV4LCBHQzNEZW51bSBwCiAgICAgcmV0dXJuIHN0YXRpY19jYXN0PEdDM0RzaXplaXB0 cj4ocmVpbnRlcnByZXRfY2FzdDxpbnRwdHJfdD4ocG9pbnRlcikpOwogfQogCi12b2lkIEdyYXBo aWNzQ29udGV4dDNEOjp0ZXhTdWJJbWFnZTJEKEdDM0RlbnVtIHRhcmdldCwgR0MzRGludCBsZXZl bCwgR0MzRGludCB4b2ZmLCBHQzNEaW50IHlvZmYsIEdDM0RzaXplaSB3aWR0aCwgR0MzRHNpemVp IGhlaWdodCwgR0MzRGVudW0gZm9ybWF0LCBHQzNEZW51bSB0eXBlLCBjb25zdCB2b2lkKiBwaXhl bHMpCi17Ci0gICAgbWFrZUNvbnRleHRDdXJyZW50KCk7Ci0KLSAgICBpZiAodHlwZSA9PSBIQUxG X0ZMT0FUX09FUykKLSAgICAgICAgdHlwZSA9IEdMX0hBTEZfRkxPQVRfQVJCOwotCi0gICAgLy8g RklYTUU6IHdlIHdpbGwgbmVlZCB0byBkZWFsIHdpdGggUGl4ZWxTdG9yZSBwYXJhbXMgd2hlbiBk ZWFsaW5nIHdpdGggaW1hZ2UgYnVmZmVycyB0aGF0IGRpZmZlciBmcm9tIHRoZSBzdWJpbWFnZSBz aXplLgotICAgIDo6Z2xUZXhTdWJJbWFnZTJEKHRhcmdldCwgbGV2ZWwsIHhvZmYsIHlvZmYsIHdp ZHRoLCBoZWlnaHQsIGZvcm1hdCwgdHlwZSwgcGl4ZWxzKTsKLX0KLQogdm9pZCBHcmFwaGljc0Nv bnRleHQzRDo6Y29tcHJlc3NlZFRleEltYWdlMkQoR0MzRGVudW0gdGFyZ2V0LCBHQzNEaW50IGxl dmVsLCBHQzNEZW51bSBpbnRlcm5hbGZvcm1hdCwgR0MzRHNpemVpIHdpZHRoLCBHQzNEc2l6ZWkg aGVpZ2h0LCBHQzNEaW50IGJvcmRlciwgR0MzRHNpemVpIGltYWdlU2l6ZSwgY29uc3Qgdm9pZCog ZGF0YSkKIHsKICAgICBtYWtlQ29udGV4dEN1cnJlbnQoKTsKZGlmZiAtLWdpdCBhL1NvdXJjZS9X ZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL29wZW5nbC9HcmFwaGljc0NvbnRleHQzRE9wZW5HTEVT LmNwcCBiL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL29wZW5nbC9HcmFwaGljc0Nv bnRleHQzRE9wZW5HTEVTLmNwcAppbmRleCA5ZWIzMDVlOTJjZTk3NzhkOWI1YTJjNDU3OTk5ZWQ3 NTAzM2U2MTIyLi5mYTIxNTNlNmEwNDg0NDZhY2UyMzQwMzE3NzE2OWJlN2U3ZDdjYWVlIDEwMDY0 NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy9vcGVuZ2wvR3JhcGhpY3ND b250ZXh0M0RPcGVuR0xFUy5jcHAKKysrIGIvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vZ3JhcGhp Y3Mvb3BlbmdsL0dyYXBoaWNzQ29udGV4dDNET3BlbkdMRVMuY3BwCkBAIC0yMDgsNiArMjA4LDE0 IEBAIGJvb2wgR3JhcGhpY3NDb250ZXh0M0Q6OnRleEltYWdlMkQoR0MzRGVudW0gdGFyZ2V0LCBH QzNEaW50IGxldmVsLCBHQzNEZW51bSBpbnRlCiAgICAgcmV0dXJuIHRydWU7CiB9CiAKK3ZvaWQg R3JhcGhpY3NDb250ZXh0M0Q6OnRleFN1YkltYWdlMkQoR0MzRGVudW0gdGFyZ2V0LCBHQzNEaW50 IGxldmVsLCBHQzNEaW50IHhvZmYsIEdDM0RpbnQgeW9mZiwgR0MzRHNpemVpIHdpZHRoLCBHQzNE c2l6ZWkgaGVpZ2h0LCBHQzNEZW51bSBmb3JtYXQsIEdDM0RlbnVtIHR5cGUsIGNvbnN0IHZvaWQq IHBpeGVscykKK3sKKyAgICBtYWtlQ29udGV4dEN1cnJlbnQoKTsKKworICAgIC8vIEZJWE1FOiB3 ZSB3aWxsIG5lZWQgdG8gZGVhbCB3aXRoIFBpeGVsU3RvcmUgcGFyYW1zIHdoZW4gZGVhbGluZyB3 aXRoIGltYWdlIGJ1ZmZlcnMgdGhhdCBkaWZmZXIgZnJvbSB0aGUgc3ViaW1hZ2Ugc2l6ZS4KKyAg ICA6OmdsVGV4U3ViSW1hZ2UyRCh0YXJnZXQsIGxldmVsLCB4b2ZmLCB5b2ZmLCB3aWR0aCwgaGVp Z2h0LCBmb3JtYXQsIHR5cGUsIHBpeGVscyk7Cit9CisKIHZvaWQgR3JhcGhpY3NDb250ZXh0M0Q6 OnZhbGlkYXRlQXR0cmlidXRlcygpCiB7CiAgICAgdmFsaWRhdGVEZXB0aFN0ZW5jaWwoIkdMX09F U19wYWNrZWRfZGVwdGhfc3RlbmNpbCIpOwo=