Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2717953003:
Support RGB32F and RGBA32F for CopyTextureCHROMIUM in ES2 with related extensions (Closed)
DescriptionSupport RGB32F and RGBA32F for CopyTextureCHROMIUM in ES2 with related extensions
CHROMIUM_color_buffer_float_rgb and CHROMIUM_color_buffer_float_rgba
support GL_RGB32F and GL_RGBA32F formats in ES2 context.
CopyTextureCHROMIUM should also support RGB32F/RGBA32F when the above
two extensions are enabled.
Blink WebGL will convert format and type combinations RGBA/FLOAT and
RGB/FLOAT to RGBA32F/FLOAT to RGB32F/FLOAT if the above two extensions
is supported.
BUG=612542
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
Review-Url: https://codereview.chromium.org/2717953003
Cr-Commit-Position: refs/heads/master@{#454556}
Committed: https://chromium.googlesource.com/chromium/src/+/83c9185edb57a156c2c52270448dacef9d0b7cc1
Patch Set 1 #Patch Set 2 : fix android bots #
Total comments: 2
Patch Set 3 : add more logging message #
Messages
Total messages: 20 (10 generated)
Description was changed from ========== Support RGB32F and RGBA32F for CopyTextureCHROMIUM in ES2 with related extensions CHROMIUM_color_buffer_float_rgb and CHROMIUM_color_buffer_float_rgba support GL_RGB32F and GL_RGBA32F formats in ES2 context. CopyTextureCHROMIUM should also support RGB32F/RGBA32F when the above two extensions are enabled. Blink WebGL will convert format and type combinations RGBA/FLOAT and RGB/FLOAT to RGBA32F/FLOAT to RGB32F/FLOAT if the above two extensions is supported. BUG=612542 ========== to ========== Support RGB32F and RGBA32F for CopyTextureCHROMIUM in ES2 with related extensions CHROMIUM_color_buffer_float_rgb and CHROMIUM_color_buffer_float_rgba support GL_RGB32F and GL_RGBA32F formats in ES2 context. CopyTextureCHROMIUM should also support RGB32F/RGBA32F when the above two extensions are enabled. Blink WebGL will convert format and type combinations RGBA/FLOAT and RGB/FLOAT to RGBA32F/FLOAT to RGB32F/FLOAT if the above two extensions is supported. BUG=612542 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 ==========
qiankun.miao@intel.com changed reviewers: + kbr@chromium.org, yunchao.he@intel.com, zmo@chromium.org
PTAL I met this issue when I enabled copyTextureCHROMIUM in blink.
Thanks Qiankun for this patch. It's failing on Android; can you please check why? I have a feeling that the constants being used assume that the code's running either on desktop OpenGL, or ES 3.0.
On 2017/02/28 06:44:14, Ken Russell wrote: > Thanks Qiankun for this patch. > > It's failing on Android; can you please check why? I have a feeling that the > constants being used assume that the code's running either on desktop OpenGL, or > ES 3.0. In ES2 the internalformat should be unsized and equal to format. Maybe that's the cause?
On 2017/02/28 15:52:40, Zhenyao Mo wrote: > On 2017/02/28 06:44:14, Ken Russell wrote: > > Thanks Qiankun for this patch. > > > > It's failing on Android; can you please check why? I have a feeling that the > > constants being used assume that the code's running either on desktop OpenGL, > or > > ES 3.0. > > In ES2 the internalformat should be unsized and equal to format. Maybe that's > the cause? The failure reason of linux_android_rel_ng bot is doing generateMipmap on RGBA32F texture which is not filterable in the testing code. texture_float_linear extension is not supported on the platform. I need to figure out a better testing method. I skipped these two tests if texture_float_linear is not enabled now. Please take another look.
https://codereview.chromium.org/2717953003/diff/20001/gpu/command_buffer/test... File gpu/command_buffer/tests/gl_copy_texture_CHROMIUM_unittest.cc (right): https://codereview.chromium.org/2717953003/diff/20001/gpu/command_buffer/test... gpu/command_buffer/tests/gl_copy_texture_CHROMIUM_unittest.cc:789: .enable_texture_float_linear) This is always false by default unless you explicitly enable it, so in that sense, it seems to me that you are skipping these two tests everywhere, not just where the extension is missing.
https://codereview.chromium.org/2717953003/diff/20001/gpu/command_buffer/test... File gpu/command_buffer/tests/gl_copy_texture_CHROMIUM_unittest.cc (right): https://codereview.chromium.org/2717953003/diff/20001/gpu/command_buffer/test... gpu/command_buffer/tests/gl_copy_texture_CHROMIUM_unittest.cc:789: .enable_texture_float_linear) On 2017/03/02 17:31:53, Zhenyao Mo wrote: > This is always false by default unless you explicitly enable it, so in that > sense, it seems to me that you are skipping these two tests everywhere, not just > where the extension is missing. Is that accurate? It looks like GLManager::InitializeWithCommandLine uses an empty DisallowedFeatures, and that causes FeatureInfo::InitializeFloatAndHalfFloatFeatures to automatically enable these features. Either way, Qiankun, please add some logging in this early-return case so it is obvious from the logs what is going on. Thanks.
On 2017/03/02 18:26:06, Ken Russell wrote: > https://codereview.chromium.org/2717953003/diff/20001/gpu/command_buffer/test... > File gpu/command_buffer/tests/gl_copy_texture_CHROMIUM_unittest.cc (right): > > https://codereview.chromium.org/2717953003/diff/20001/gpu/command_buffer/test... > gpu/command_buffer/tests/gl_copy_texture_CHROMIUM_unittest.cc:789: > .enable_texture_float_linear) > On 2017/03/02 17:31:53, Zhenyao Mo wrote: > > This is always false by default unless you explicitly enable it, so in that > > sense, it seems to me that you are skipping these two tests everywhere, not > just > > where the extension is missing. > > Is that accurate? It looks like GLManager::InitializeWithCommandLine uses an > empty DisallowedFeatures, and that causes > FeatureInfo::InitializeFloatAndHalfFloatFeatures to automatically enable these > features. > > Either way, Qiankun, please add some logging in this early-return case so it is > obvious from the logs what is going on. Thanks. Ah, I didn't realize testing configs it differently from product code path. LGTM then.
The CQ bit was checked by qiankun.miao@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...
On 2017/03/02 18:26:06, Ken Russell wrote: > https://codereview.chromium.org/2717953003/diff/20001/gpu/command_buffer/test... > File gpu/command_buffer/tests/gl_copy_texture_CHROMIUM_unittest.cc (right): > > https://codereview.chromium.org/2717953003/diff/20001/gpu/command_buffer/test... > gpu/command_buffer/tests/gl_copy_texture_CHROMIUM_unittest.cc:789: > .enable_texture_float_linear) > On 2017/03/02 17:31:53, Zhenyao Mo wrote: > > This is always false by default unless you explicitly enable it, so in that > > sense, it seems to me that you are skipping these two tests everywhere, not > just > > where the extension is missing. > > Is that accurate? It looks like GLManager::InitializeWithCommandLine uses an > empty DisallowedFeatures, and that causes > FeatureInfo::InitializeFloatAndHalfFloatFeatures to automatically enable these > features. This is true. If "GL_ARB_texture_float" extension is existed or desktop core profile is used, enable_texture_float_linear is set as true. > > Either way, Qiankun, please add some logging in this early-return case so it is > obvious from the logs what is going on. Thanks. Add logs already.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by qiankun.miao@intel.com
The patchset sent to the CQ was uploaded after l-g-t-m from zmo@chromium.org Link to the patchset: https://codereview.chromium.org/2717953003/#ps40001 (title: "add more logging message")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 40001, "attempt_start_ts": 1488535442326340,
"parent_rev": "e9dbe4d1324caf643ae7808a875c6fad254555a3", "commit_rev":
"83c9185edb57a156c2c52270448dacef9d0b7cc1"}
Message was sent while issue was closed.
Description was changed from ========== Support RGB32F and RGBA32F for CopyTextureCHROMIUM in ES2 with related extensions CHROMIUM_color_buffer_float_rgb and CHROMIUM_color_buffer_float_rgba support GL_RGB32F and GL_RGBA32F formats in ES2 context. CopyTextureCHROMIUM should also support RGB32F/RGBA32F when the above two extensions are enabled. Blink WebGL will convert format and type combinations RGBA/FLOAT and RGB/FLOAT to RGBA32F/FLOAT to RGB32F/FLOAT if the above two extensions is supported. BUG=612542 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 ========== Support RGB32F and RGBA32F for CopyTextureCHROMIUM in ES2 with related extensions CHROMIUM_color_buffer_float_rgb and CHROMIUM_color_buffer_float_rgba support GL_RGB32F and GL_RGBA32F formats in ES2 context. CopyTextureCHROMIUM should also support RGB32F/RGBA32F when the above two extensions are enabled. Blink WebGL will convert format and type combinations RGBA/FLOAT and RGB/FLOAT to RGBA32F/FLOAT to RGB32F/FLOAT if the above two extensions is supported. BUG=612542 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 Review-Url: https://codereview.chromium.org/2717953003 Cr-Commit-Position: refs/heads/master@{#454556} Committed: https://chromium.googlesource.com/chromium/src/+/83c9185edb57a156c2c52270448d... ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as https://chromium.googlesource.com/chromium/src/+/83c9185edb57a156c2c52270448d... |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
