Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2117183006:
gpu: Clarify sized texture format is available only if ES3 context or immutable texture is supported (Closed)
Descriptiongpu: Clarify the sized texture format is available only if ES3 context or immutable texture is supported.
This clarification makes it possible that gles2 decoder tracks true internal format of immutable texture. So this CL removes internal_format hack in GLES2DecoderImpl::TexStorageImpl()
In addition, it makes CMAA skip redundant copy path because CMAA can know true internal format.
BUG=535198
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Committed: https://crrev.com/730f8089f791fa31a3ee2a7f5059a1052b129d21
Cr-Commit-Position: refs/heads/master@{#425684}
Patch Set 1 #Patch Set 2 : rely on right extension #
Total comments: 1
Patch Set 3 : rely on right extension #Patch Set 4 : rebase to ToT #
Total comments: 5
Patch Set 5 : sized format is available if es3 context or immutable texture #
Total comments: 1
Patch Set 6 : add new test; InternalFormatBleedingToTexImage #
Total comments: 10
Patch Set 7 : fix oes-texture-float.html #
Total comments: 2
Messages
Total messages: 59 (39 generated)
Description was changed from ========== gpu: remove internal_format of texture hack Now GLES2 decoder tracks true internal_format of texture. In addition, it makes CMAA skip redundant copy path. BUG=535198 ========== to ========== gpu: remove internal_format of texture hack Now GLES2 decoder tracks true internal_format of texture. In addition, it makes CMAA skip redundant copy path. BUG=535198 CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel ==========
The CQ bit was checked by dongseong.hwang@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)
The CQ bit was checked by dongseong.hwang@intel.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by dongseong.hwang@intel.com
The CQ bit was checked by dongseong.hwang@intel.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
Description was changed from ========== gpu: remove internal_format of texture hack Now GLES2 decoder tracks true internal_format of texture. In addition, it makes CMAA skip redundant copy path. BUG=535198 CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel ========== to ========== gpu: remove internal_format of texture hack Now GLES2 decoder tracks true internal_format of texture. In addition, it makes CMAA skip redundant copy path. BUG=535198 ==========
The CQ bit was checked by dongseong.hwang@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
dongseong.hwang@intel.com changed reviewers: + zmo@chromium.org
zmo@, could you review? https://codereview.chromium.org/2117183006/diff/20001/gpu/command_buffer/serv... File gpu/command_buffer/service/gles2_cmd_decoder.cc (left): https://codereview.chromium.org/2117183006/diff/20001/gpu/command_buffer/serv... gpu/command_buffer/service/gles2_cmd_decoder.cc:15100: feature_info_->IsES3Enabled() ? internal_format : format; Remove this hack. Rationale 1. After ES3 support, gles2 decoder can handle both unsized format as well as sized format. 2. many gl2 extensions support sized format 3. texImage2D already handles sized format in es2 4. CMAA needs to know exact internal format https://cs.chromium.org/chromium/src/gpu/command_buffer/service/gles2_cmd_app...
https://codereview.chromium.org/2117183006/diff/60001/gpu/command_buffer/serv... File gpu/command_buffer/service/feature_info.cc (right): https://codereview.chromium.org/2117183006/diff/60001/gpu/command_buffer/serv... gpu/command_buffer/service/feature_info.cc:1121: if (!validators_.texture_sized_color_renderable_internal_format.IsValid( I am not sure I follow the logic of this block. texture_storage should have nothing to do with these validators.
Thx for reviewing. I answer the question. https://codereview.chromium.org/2117183006/diff/60001/gpu/command_buffer/serv... File gpu/command_buffer/service/feature_info.cc (right): https://codereview.chromium.org/2117183006/diff/60001/gpu/command_buffer/serv... gpu/command_buffer/service/feature_info.cc:1121: if (!validators_.texture_sized_color_renderable_internal_format.IsValid( On 2016/10/10 20:30:29, Zhenyao Mo wrote: > I am not sure I follow the logic of this block. texture_storage should have > nothing to do with these validators. Sorry for insufficient explanation. |texture_sized_xxx_internal_format| contains list of sized formats. GL_BGRA8 should be in the list, if both |enable_texture_storage| and |enable_texture_format_bgra8888| are true, because gpu client can call glTexStorage2D(GL_TEXTURE_2D, 1, GL_BGRA8, 256, 256). CC actually calls it on linux. https://codereview.chromium.org/2117183006/diff/60001/gpu/command_buffer/serv... File gpu/command_buffer/service/gles2_cmd_decoder.cc (left): https://codereview.chromium.org/2117183006/diff/60001/gpu/command_buffer/serv... gpu/command_buffer/service/gles2_cmd_decoder.cc:16145: feature_info_->IsES3Enabled() ? internal_format : format; Here is inside GLES2DecoderImpl::TexStorageImpl(). texture_manager()->SetLevelInfo() checks if the sized format is registered in |texture_sized_xxx_internal_format|
https://codereview.chromium.org/2117183006/diff/60001/gpu/command_buffer/serv... File gpu/command_buffer/service/feature_info.cc (right): https://codereview.chromium.org/2117183006/diff/60001/gpu/command_buffer/serv... gpu/command_buffer/service/feature_info.cc:1121: if (!validators_.texture_sized_color_renderable_internal_format.IsValid( On 2016/10/10 20:57:10, dshwang wrote: > On 2016/10/10 20:30:29, Zhenyao Mo wrote: > > I am not sure I follow the logic of this block. texture_storage should have > > nothing to do with these validators. > > Sorry for insufficient explanation. > |texture_sized_xxx_internal_format| contains list of sized formats. > GL_BGRA8 should be in the list, if both |enable_texture_storage| and > |enable_texture_format_bgra8888| are true, because gpu client can call > glTexStorage2D(GL_TEXTURE_2D, 1, GL_BGRA8, 256, 256). CC actually calls it on > linux. But this also enables TexImage calls to use these sized internalformats, which could be illegal unless it's APPLE_texture_format_BGRA8888 on ES3.
https://codereview.chromium.org/2117183006/diff/60001/gpu/command_buffer/serv... File gpu/command_buffer/service/feature_info.cc (right): https://codereview.chromium.org/2117183006/diff/60001/gpu/command_buffer/serv... gpu/command_buffer/service/feature_info.cc:1121: if (!validators_.texture_sized_color_renderable_internal_format.IsValid( On 2016/10/10 21:00:27, Zhenyao Mo wrote: > On 2016/10/10 20:57:10, dshwang wrote: > > On 2016/10/10 20:30:29, Zhenyao Mo wrote: > > > I am not sure I follow the logic of this block. texture_storage should have > > > nothing to do with these validators. > > > > Sorry for insufficient explanation. > > |texture_sized_xxx_internal_format| contains list of sized formats. > > GL_BGRA8 should be in the list, if both |enable_texture_storage| and > > |enable_texture_format_bgra8888| are true, because gpu client can call > > glTexStorage2D(GL_TEXTURE_2D, 1, GL_BGRA8, 256, 256). CC actually calls it on > > linux. > > But this also enables TexImage calls to use these sized internalformats, which > could be illegal unless it's APPLE_texture_format_BGRA8888 on ES3. Oh, I didn't know. Even if EXT_texture_format_BGRA8888 and EXT_texture_storage are supported, TexImage(GL_BGRA8) is illegal? EXT_texture_storage spec say """ If EXT_texture_format_BGRA8888 or APPLE_texture_format_BGRA8888 is supported: BGRA8_EXT """ https://www.khronos.org/registry/gles/extensions/EXT/EXT_texture_storage.txt If it's illegal, Texture::CanRenderTo() should be implemented differently for storage texture and regular texture. https://cs.chromium.org/chromium/src/gpu/command_buffer/service/texture_manag... If it's illegal, I'll change Texture::CanRenderTo() implementation.
On 2016/10/10 21:18:30, dshwang wrote: > https://codereview.chromium.org/2117183006/diff/60001/gpu/command_buffer/serv... > File gpu/command_buffer/service/feature_info.cc (right): > > https://codereview.chromium.org/2117183006/diff/60001/gpu/command_buffer/serv... > gpu/command_buffer/service/feature_info.cc:1121: if > (!validators_.texture_sized_color_renderable_internal_format.IsValid( > On 2016/10/10 21:00:27, Zhenyao Mo wrote: > > On 2016/10/10 20:57:10, dshwang wrote: > > > On 2016/10/10 20:30:29, Zhenyao Mo wrote: > > > > I am not sure I follow the logic of this block. texture_storage should > have > > > > nothing to do with these validators. > > > > > > Sorry for insufficient explanation. > > > |texture_sized_xxx_internal_format| contains list of sized formats. > > > GL_BGRA8 should be in the list, if both |enable_texture_storage| and > > > |enable_texture_format_bgra8888| are true, because gpu client can call > > > glTexStorage2D(GL_TEXTURE_2D, 1, GL_BGRA8, 256, 256). CC actually calls it > on > > > linux. > > > > But this also enables TexImage calls to use these sized internalformats, which > > could be illegal unless it's APPLE_texture_format_BGRA8888 on ES3. > > Oh, I didn't know. > > Even if EXT_texture_format_BGRA8888 and EXT_texture_storage are supported, > TexImage(GL_BGRA8) is illegal? > > EXT_texture_storage spec say > """ > If EXT_texture_format_BGRA8888 or APPLE_texture_format_BGRA8888 is supported: > BGRA8_EXT > """ > https://www.khronos.org/registry/gles/extensions/EXT/EXT_texture_storage.txt > > If it's illegal, Texture::CanRenderTo() should be implemented differently for > storage texture and regular texture. > https://cs.chromium.org/chromium/src/gpu/command_buffer/service/texture_manag... > > If it's illegal, I'll change Texture::CanRenderTo() implementation. In ES2, internalformat == format for Tex{Sub}Image calls - that is a requirement.
Description was changed from ========== gpu: remove internal_format of texture hack Now GLES2 decoder tracks true internal_format of texture. In addition, it makes CMAA skip redundant copy path. BUG=535198 ========== to ========== gpu: remove internal_format of texture hack Now GLES2 decoder tracks true internal_format of texture. In addition, it makes CMAA skip redundant copy path. BUG=535198 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel ==========
The CQ bit was checked by dongseong.hwang@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== gpu: remove internal_format of texture hack Now GLES2 decoder tracks true internal_format of texture. In addition, it makes CMAA skip redundant copy path. BUG=535198 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel ========== to ========== gpu: Clarify the sized texture format is available only if ES3 context or immutable texture is supported. This clarification makes it possible that gles2 decoder tracks true internal format of immutable texture. So this CL removes internal_format hack in GLES2DecoderImpl::TexStorageImpl() In addition, it makes CMAA skip redundant copy path because CMAA can know true internal format. BUG=535198 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel ==========
Zhenyao, I resolved your concern as well as fixed the existing bug. could you review again? https://codereview.chromium.org/2117183006/diff/80001/gpu/command_buffer/serv... File gpu/command_buffer/service/texture_manager.cc (right): https://codereview.chromium.org/2117183006/diff/80001/gpu/command_buffer/serv... gpu/command_buffer/service/texture_manager.cc:1734: internal_format))); >> If it's illegal, Texture::CanRenderTo() should be implemented differently for >> storage texture and regular texture. >> https://cs.chromium.org/chromium/src/gpu/command_buffer/service/texture_manag... >> >> If it's illegal, I'll change Texture::CanRenderTo() implementation. > In ES2, internalformat == format for Tex{Sub}Image calls - that is a > requirement. There are existing bugs. In ES2, TexImage*(GL_RGBA8) works because autogenerated Validators::Validators() always fill common format into texture_sized_color_renderable_internal_format. Now we check sized format is color renderable only if the context is ES3 context or the texture is immutable texture
The CQ bit was checked by dongseong.hwang@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Exceeded global retry quota
The CQ bit was checked by dongseong.hwang@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was checked by dongseong.hwang@chromium.org to run a CQ dry run
Patchset #6 (id:100001) has been deleted
Patchset #6 (id:120001) has been deleted
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: win_optional_gpu_tests_rel on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_optional_gpu_...)
Mostly looks good to me https://codereview.chromium.org/2117183006/diff/140001/gpu/command_buffer/ser... File gpu/command_buffer/service/feature_info.cc (right): https://codereview.chromium.org/2117183006/diff/140001/gpu/command_buffer/ser... gpu/command_buffer/service/feature_info.cc:689: if (IsES3Capable()) { This should be IsES3Enabled(). Otherwise the command buffer is still ES2 and sized internalformat isn't allowed. https://codereview.chromium.org/2117183006/diff/140001/gpu/command_buffer/ser... gpu/command_buffer/service/feature_info.cc:1118: if (!validators_.texture_sized_color_renderable_internal_format.IsValid( You don't need this at all. AddValue() will do this check anyway, so it's duplicated efforts. https://codereview.chromium.org/2117183006/diff/140001/gpu/command_buffer/ser... File gpu/command_buffer/service/texture_manager.cc (right): https://codereview.chromium.org/2117183006/diff/140001/gpu/command_buffer/ser... gpu/command_buffer/service/texture_manager.cc:314: bool immutable) { nit: wrong indent https://codereview.chromium.org/2117183006/diff/140001/gpu/command_buffer/ser... gpu/command_buffer/service/texture_manager.cc:315: return !feature_info->gl_version_info().is_es2 || immutable; This should be feature_info->IsES3Enabled(), not about the underlying driver version, but more, the current command buffer context version
Also, the webgl2 test failure seems real.
The CQ bit was checked by dongseong.hwang@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
dongseong.hwang@intel.com changed reviewers: + kbr@chromium.org
Zhenyao, I resolved all concerns. could you review again? https://codereview.chromium.org/2117183006/diff/140001/gpu/command_buffer/ser... File gpu/command_buffer/service/feature_info.cc (right): https://codereview.chromium.org/2117183006/diff/140001/gpu/command_buffer/ser... gpu/command_buffer/service/feature_info.cc:689: if (IsES3Capable()) { On 2016/10/13 21:23:38, Zhenyao Mo wrote: > This should be IsES3Enabled(). Otherwise the command buffer is still ES2 and > sized internalformat isn't allowed. Here is inside FeatureInfo::InitializeFeatures() FeatureInfo::EnableES3Validators() sets IsES3Enabled() to true, which is later procedure. Let me move this code to FeatureInfo::EnableES3Validators() https://codereview.chromium.org/2117183006/diff/140001/gpu/command_buffer/ser... gpu/command_buffer/service/feature_info.cc:1118: if (!validators_.texture_sized_color_renderable_internal_format.IsValid( On 2016/10/13 21:23:38, Zhenyao Mo wrote: > You don't need this at all. AddValue() will do this check anyway, so it's > duplicated efforts. Done. https://codereview.chromium.org/2117183006/diff/140001/gpu/command_buffer/ser... File gpu/command_buffer/service/texture_manager.cc (right): https://codereview.chromium.org/2117183006/diff/140001/gpu/command_buffer/ser... gpu/command_buffer/service/texture_manager.cc:314: bool immutable) { On 2016/10/13 21:23:38, Zhenyao Mo wrote: > nit: wrong indent Done. https://codereview.chromium.org/2117183006/diff/140001/gpu/command_buffer/ser... gpu/command_buffer/service/texture_manager.cc:315: return !feature_info->gl_version_info().is_es2 || immutable; On 2016/10/13 21:23:38, Zhenyao Mo wrote: > This should be feature_info->IsES3Enabled(), not about the underlying driver > version, but more, the current command buffer context version Done. https://codereview.chromium.org/2117183006/diff/160001/gpu/command_buffer/ser... File gpu/command_buffer/service/texture_manager.cc (right): https://codereview.chromium.org/2117183006/diff/160001/gpu/command_buffer/ser... gpu/command_buffer/service/texture_manager.cc:326: } > Also, the webgl2 test failure seems real. following test failed. https://www.khronos.org/registry/webgl/sdk/tests/conformance/extensions/oes-t... It's because https://bugs.chromium.org/p/chromium/issues/detail?id=329605 added CHROMIUM_color_buffer_float_rgb extensions; https://cs.chromium.org/chromium/src/gpu/GLES2/extensions/CHROMIUM/CHROMIUM_c... , which allows TexImage*(GL_RGB32F) in ES2 context. I'm not fully understand why it's needed. kbr@, do you have some clue?
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Mostly looks good with one minor concern of losing test coverage. However, we have webgl2 conformance tests, so probably OK to land as is. LGTM https://codereview.chromium.org/2117183006/diff/140001/gpu/command_buffer/ser... File gpu/command_buffer/service/framebuffer_manager_unittest.cc (right): https://codereview.chromium.org/2117183006/diff/140001/gpu/command_buffer/ser... gpu/command_buffer/service/framebuffer_manager_unittest.cc:497: const GLenum kFormat3 = GL_RGB565; Testing this is on purpose to allow ES3 sized format.
The CQ bit was checked by dongseong.hwang@intel.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
thx for reviewing. https://codereview.chromium.org/2117183006/diff/140001/gpu/command_buffer/ser... File gpu/command_buffer/service/framebuffer_manager_unittest.cc (right): https://codereview.chromium.org/2117183006/diff/140001/gpu/command_buffer/ser... gpu/command_buffer/service/framebuffer_manager_unittest.cc:497: const GLenum kFormat3 = GL_RGB565; On 2016/10/14 21:05:09, Zhenyao Mo wrote: > Testing this is on purpose to allow ES3 sized format. Non trivial code is needed to make IsES3Enabled() return true. I submit the recovery CL of test coverage. https://codereview.chromium.org/2424833003 Let's dicuss about it in the next CL.
Message was sent while issue was closed.
Description was changed from ========== gpu: Clarify the sized texture format is available only if ES3 context or immutable texture is supported. This clarification makes it possible that gles2 decoder tracks true internal format of immutable texture. So this CL removes internal_format hack in GLES2DecoderImpl::TexStorageImpl() In addition, it makes CMAA skip redundant copy path because CMAA can know true internal format. BUG=535198 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel ========== to ========== gpu: Clarify the sized texture format is available only if ES3 context or immutable texture is supported. This clarification makes it possible that gles2 decoder tracks true internal format of immutable texture. So this CL removes internal_format hack in GLES2DecoderImpl::TexStorageImpl() In addition, it makes CMAA skip redundant copy path because CMAA can know true internal format. BUG=535198 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel ==========
Message was sent while issue was closed.
Committed patchset #7 (id:160001)
Message was sent while issue was closed.
Description was changed from ========== gpu: Clarify the sized texture format is available only if ES3 context or immutable texture is supported. This clarification makes it possible that gles2 decoder tracks true internal format of immutable texture. So this CL removes internal_format hack in GLES2DecoderImpl::TexStorageImpl() In addition, it makes CMAA skip redundant copy path because CMAA can know true internal format. BUG=535198 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel ========== to ========== gpu: Clarify the sized texture format is available only if ES3 context or immutable texture is supported. This clarification makes it possible that gles2 decoder tracks true internal format of immutable texture. So this CL removes internal_format hack in GLES2DecoderImpl::TexStorageImpl() In addition, it makes CMAA skip redundant copy path because CMAA can know true internal format. BUG=535198 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel Committed: https://crrev.com/730f8089f791fa31a3ee2a7f5059a1052b129d21 Cr-Commit-Position: refs/heads/master@{#425684} ==========
Message was sent while issue was closed.
Patchset 7 (id:??) landed as https://crrev.com/730f8089f791fa31a3ee2a7f5059a1052b129d21 Cr-Commit-Position: refs/heads/master@{#425684}
Message was sent while issue was closed.
https://codereview.chromium.org/2117183006/diff/160001/gpu/command_buffer/ser... File gpu/command_buffer/service/texture_manager.cc (right): https://codereview.chromium.org/2117183006/diff/160001/gpu/command_buffer/ser... gpu/command_buffer/service/texture_manager.cc:326: } On 2016/10/14 12:03:34, dshwang wrote: > > Also, the webgl2 test failure seems real. > following test failed. > https://www.khronos.org/registry/webgl/sdk/tests/conformance/extensions/oes-t... > > It's because https://bugs.chromium.org/p/chromium/issues/detail?id=329605 added > CHROMIUM_color_buffer_float_rgb extensions; > https://cs.chromium.org/chromium/src/gpu/GLES2/extensions/CHROMIUM/CHROMIUM_c... > , which allows TexImage*(GL_RGB32F) in ES2 context. > > I'm not fully understand why it's needed. kbr@, do you have some clue? Sorry, I don't remember. The ultimate goal was to enable rendering to these floating-point textures in WebGL 1.0 via an extension, but I don't remember the reason for this direction.
Message was sent while issue was closed.
A revert of this CL (patchset #7 id:160001) has been created in https://codereview.chromium.org/2443123004/ by mcasas@chromium.org. The reason for reverting is: Broke playing back recorded data as detailed in https://crbug.com/657532. The bug has step-by-step notes on how to repro (see also #4 in there). Locally reverting of this CL restores the functionality.. |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
