Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2369313002:
Work around CopyTexImage2D issue on Intel Mac OSX 10.12 (Closed)
DescriptionWork around CopyTexImage2D issue on Intel Mac OSX 10.12
CopyTexImage2D for cube map target has bug on Intel Mac OSX 10.12. If
positive x face of the first two levels are not set, CopyTexImage2D will
copy incorrect pixel data to cube map texture. This bug is similar to
the bug fixed by https://codereview.chromium.org/2348243002/. But
solutions are different. This bug can be worked around by setting
positive x face of level 0 and 1.
BUG=648197
TEST=deqp/functional/gles3/texturespecification/basic_copyteximage2d.html
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/45c5754736e2dffd0bcd34c5499c07a80acae58b
Cr-Commit-Position: refs/heads/master@{#424422}
Patch Set 1 #Patch Set 2 : add workaround flag #Patch Set 3 : rebase only #
Total comments: 8
Patch Set 4 : rename workarouns #
Total comments: 4
Patch Set 5 : rebase only #Patch Set 6 : rebase only #
Total comments: 2
Patch Set 7 : rebase only #
Messages
Total messages: 32 (6 generated)
Description was changed from ========== Work around CopyTexImage2D issue on Intel Mac OSX 10.12 CopyTexImage2D for cube map target has bug on Intel Mac OSX 10.12. If positive x face of the first two levels are not set, CopyTexImage2D will copy incorrect pixel data to cube map texture. This bug is similar to the bug fixed by https://codereview.chromium.org/2348243002/. But solutions are different. This bug can be worked around by setting positive x face of level 0 and 1. BUG=648197 TEST=deqp/functional/gles3/texturespecification/basic_copyteximage2d.html ========== to ========== Work around CopyTexImage2D issue on Intel Mac OSX 10.12 CopyTexImage2D for cube map target has bug on Intel Mac OSX 10.12. If positive x face of the first two levels are not set, CopyTexImage2D will copy incorrect pixel data to cube map texture. This bug is similar to the bug fixed by https://codereview.chromium.org/2348243002/. But solutions are different. This bug can be worked around by setting positive x face of level 0 and 1. BUG=648197 TEST=deqp/functional/gles3/texturespecification/basic_copyteximage2d.html 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 ==========
Patchset #2 (id:20001) has been deleted
qiankun.miao@intel.com changed reviewers: + kbr@chromium.org, piman@chromium.org, yunchao.he@intel.com, zmo@chromium.org
PTAL.
lgtm https://codereview.chromium.org/2369313002/diff/60001/gpu/command_buffer/serv... File gpu/command_buffer/service/gles2_cmd_decoder.cc (right): https://codereview.chromium.org/2369313002/diff/60001/gpu/command_buffer/serv... gpu/command_buffer/service/gles2_cmd_decoder.cc:13327: } This gets complicated when we also need to emulate for LUMINANCE_ALPHA formats. Could you at least add a TODO and create a bug to track it?
https://codereview.chromium.org/2369313002/diff/60001/gpu/command_buffer/serv... File gpu/command_buffer/service/gles2_cmd_decoder.cc (right): https://codereview.chromium.org/2369313002/diff/60001/gpu/command_buffer/serv... gpu/command_buffer/service/gles2_cmd_decoder.cc:13327: } On 2016/09/28 21:14:23, Zhenyao Mo wrote: > This gets complicated when we also need to emulate for LUMINANCE_ALPHA formats. > > Could you at least add a TODO and create a bug to track it? What kinds of TODO, something like to handle this workaround and LUMINANCE_ALPHA emulation correctly? https://codereview.chromium.org/2369313002/diff/60001/gpu/command_buffer/serv... gpu/command_buffer/service/gles2_cmd_decoder.cc:13342: GetBoundReadFramebufferInternalFormat()); FYI: This path requires the init_cube_map_before_copyteximage workaround. Otherwise, luminance related dEQP tests will fail.
I defer to zmo's review but one comment. https://codereview.chromium.org/2369313002/diff/60001/gpu/config/gpu_driver_b... File gpu/config/gpu_driver_bug_list_json.cc (right): https://codereview.chromium.org/2369313002/diff/60001/gpu/config/gpu_driver_b... gpu/config/gpu_driver_bug_list_json.cc:2064: "init_cube_map_before_copyteximage" Since this is so similar to the do_teximage_before_copyteximage_to_cube_map workaround, could you try to name them consistently? init_one_cube_map_level_before_copyteximage / init_two_cube_map_levels_before_copyteximage?
https://codereview.chromium.org/2369313002/diff/60001/gpu/config/gpu_driver_b... File gpu/config/gpu_driver_bug_list_json.cc (right): https://codereview.chromium.org/2369313002/diff/60001/gpu/config/gpu_driver_b... gpu/config/gpu_driver_bug_list_json.cc:2064: "init_cube_map_before_copyteximage" On 2016/09/29 00:52:13, Ken Russell wrote: > Since this is so similar to the do_teximage_before_copyteximage_to_cube_map > workaround, could you try to name them consistently? > > init_one_cube_map_level_before_copyteximage / > init_two_cube_map_levels_before_copyteximage? Modified names of these two workarounds.
https://codereview.chromium.org/2369313002/diff/60001/gpu/command_buffer/serv... File gpu/command_buffer/service/gles2_cmd_decoder.cc (right): https://codereview.chromium.org/2369313002/diff/60001/gpu/command_buffer/serv... gpu/command_buffer/service/gles2_cmd_decoder.cc:13327: } On 2016/09/29 00:51:27, qiankun wrote: > On 2016/09/28 21:14:23, Zhenyao Mo wrote: > > This gets complicated when we also need to emulate for LUMINANCE_ALPHA > formats. > > > > Could you at least add a TODO and create a bug to track it? > > What kinds of TODO, something like to handle this workaround and LUMINANCE_ALPHA > emulation correctly? What I mean is, if the src texture format is LUMINANCE/ALPHA, then WorkaroundCopyTexImageCubeMap() will generate GL errors on platforms where these formats are not supported.
Thanks. LGTM once Mo's comments about the TODO are addressed.
https://codereview.chromium.org/2369313002/diff/60001/gpu/command_buffer/serv... File gpu/command_buffer/service/gles2_cmd_decoder.cc (right): https://codereview.chromium.org/2369313002/diff/60001/gpu/command_buffer/serv... gpu/command_buffer/service/gles2_cmd_decoder.cc:13327: } On 2016/09/29 20:30:16, Zhenyao Mo wrote: > On 2016/09/29 00:51:27, qiankun wrote: > > On 2016/09/28 21:14:23, Zhenyao Mo wrote: > > > This gets complicated when we also need to emulate for LUMINANCE_ALPHA > > formats. > > > > > > Could you at least add a TODO and create a bug to track it? > > > > What kinds of TODO, something like to handle this workaround and > LUMINANCE_ALPHA > > emulation correctly? > > What I mean is, if the src texture format is LUMINANCE/ALPHA, then > WorkaroundCopyTexImageCubeMap() will generate GL errors on platforms where these > formats are not supported. Sorry, I didn't get this. Can you explain it further? Where does the GL errors come from? I think WorkaroundCopyTexImageCubeMap doens't use src texture format. Does LUMINANCE/ALPHA emulation patch land chromium already?
https://codereview.chromium.org/2369313002/diff/80001/gpu/command_buffer/serv... File gpu/command_buffer/service/gles2_cmd_decoder.cc (right): https://codereview.chromium.org/2369313002/diff/80001/gpu/command_buffer/serv... gpu/command_buffer/service/gles2_cmd_decoder.cc:13338: copy_tex_image_blit_->DoCopyTexImage2DToLUMACompatibilityTexture( Here is the luminance/alpha format emulation. Of course the WorkaroundCopyTexImageCubeMap() will fail because it calls TexImage*() functions if the formats are luminance/alpha, which isn't supported on Mac Core profile.
https://codereview.chromium.org/2369313002/diff/80001/gpu/command_buffer/serv... File gpu/command_buffer/service/gles2_cmd_decoder.cc (right): https://codereview.chromium.org/2369313002/diff/80001/gpu/command_buffer/serv... gpu/command_buffer/service/gles2_cmd_decoder.cc:13338: copy_tex_image_blit_->DoCopyTexImage2DToLUMACompatibilityTexture( On 2016/09/30 16:24:20, Zhenyao Mo wrote: > Here is the luminance/alpha format emulation. Of course the > WorkaroundCopyTexImageCubeMap() will fail because it calls TexImage*() functions > if the formats are luminance/alpha, which isn't supported on Mac Core profile. Won't line 13315 do LUMINANCE/ALPHA mapping?
https://codereview.chromium.org/2369313002/diff/80001/gpu/command_buffer/serv... File gpu/command_buffer/service/gles2_cmd_decoder.cc (right): https://codereview.chromium.org/2369313002/diff/80001/gpu/command_buffer/serv... gpu/command_buffer/service/gles2_cmd_decoder.cc:13338: copy_tex_image_blit_->DoCopyTexImage2DToLUMACompatibilityTexture( On 2016/09/30 17:44:42, qiankun wrote: > On 2016/09/30 16:24:20, Zhenyao Mo wrote: > > Here is the luminance/alpha format emulation. Of course the > > WorkaroundCopyTexImageCubeMap() will fail because it calls TexImage*() > functions > > if the formats are luminance/alpha, which isn't supported on Mac Core profile. > > Won't line 13315 do LUMINANCE/ALPHA mapping? That's not enough. The format will still cause an GL error.
https://codereview.chromium.org/2369313002/diff/80001/gpu/command_buffer/serv... File gpu/command_buffer/service/gles2_cmd_decoder.cc (right): https://codereview.chromium.org/2369313002/diff/80001/gpu/command_buffer/serv... gpu/command_buffer/service/gles2_cmd_decoder.cc:13338: copy_tex_image_blit_->DoCopyTexImage2DToLUMACompatibilityTexture( On 2016/09/30 18:02:11, Zhenyao Mo wrote: > On 2016/09/30 17:44:42, qiankun wrote: > > On 2016/09/30 16:24:20, Zhenyao Mo wrote: > > > Here is the luminance/alpha format emulation. Of course the > > > WorkaroundCopyTexImageCubeMap() will fail because it calls TexImage*() > > functions > > > if the formats are luminance/alpha, which isn't supported on Mac Core > profile. > > > > Won't line 13315 do LUMINANCE/ALPHA mapping? > > That's not enough. The format will still cause an GL error. Mac OSX should be use core profile now. But I didn't see the GL errors with this CL for luminance cases. Do you know what situation will trigger the errors?
On 2016/10/01 01:25:06, qiankun wrote: > https://codereview.chromium.org/2369313002/diff/80001/gpu/command_buffer/serv... > File gpu/command_buffer/service/gles2_cmd_decoder.cc (right): > > https://codereview.chromium.org/2369313002/diff/80001/gpu/command_buffer/serv... > gpu/command_buffer/service/gles2_cmd_decoder.cc:13338: > copy_tex_image_blit_->DoCopyTexImage2DToLUMACompatibilityTexture( > On 2016/09/30 18:02:11, Zhenyao Mo wrote: > > On 2016/09/30 17:44:42, qiankun wrote: > > > On 2016/09/30 16:24:20, Zhenyao Mo wrote: > > > > Here is the luminance/alpha format emulation. Of course the > > > > WorkaroundCopyTexImageCubeMap() will fail because it calls TexImage*() > > > functions > > > > if the formats are luminance/alpha, which isn't supported on Mac Core > > profile. > > > > > > Won't line 13315 do LUMINANCE/ALPHA mapping? > > > > That's not enough. The format will still cause an GL error. > > Mac OSX should be use core profile now. But I didn't see the GL errors with this > CL for luminance cases. Do you know what situation will trigger the errors? That's weird. You have to debug into the WorkaroundCopyTexImageCubeMap() where the glTexImage2D is called, verifying internalformat is mapped (LUMINANCE->R8), format isn't (still LUMINANCE). And right after that, there has to be a GL error.
On 2016/10/01 01:34:13, Zhenyao Mo wrote: > On 2016/10/01 01:25:06, qiankun wrote: > > > https://codereview.chromium.org/2369313002/diff/80001/gpu/command_buffer/serv... > > File gpu/command_buffer/service/gles2_cmd_decoder.cc (right): > > > > > https://codereview.chromium.org/2369313002/diff/80001/gpu/command_buffer/serv... > > gpu/command_buffer/service/gles2_cmd_decoder.cc:13338: > > copy_tex_image_blit_->DoCopyTexImage2DToLUMACompatibilityTexture( > > On 2016/09/30 18:02:11, Zhenyao Mo wrote: > > > On 2016/09/30 17:44:42, qiankun wrote: > > > > On 2016/09/30 16:24:20, Zhenyao Mo wrote: > > > > > Here is the luminance/alpha format emulation. Of course the > > > > > WorkaroundCopyTexImageCubeMap() will fail because it calls TexImage*() > > > > functions > > > > > if the formats are luminance/alpha, which isn't supported on Mac Core > > > profile. > > > > > > > > Won't line 13315 do LUMINANCE/ALPHA mapping? > > > > > > That's not enough. The format will still cause an GL error. > > > > Mac OSX should be use core profile now. But I didn't see the GL errors with > this > > CL for luminance cases. Do you know what situation will trigger the errors? > > That's weird. You have to debug into the WorkaroundCopyTexImageCubeMap() where > the glTexImage2D is called, verifying internalformat is mapped (LUMINANCE->R8), > format isn't (still LUMINANCE). And right after that, there has to be a GL > error. Thanks for pointing it. I will try that after holiday if I don't have time soon.
On 2016/10/01 01:34:13, Zhenyao Mo wrote: > On 2016/10/01 01:25:06, qiankun wrote: > > > https://codereview.chromium.org/2369313002/diff/80001/gpu/command_buffer/serv... > > File gpu/command_buffer/service/gles2_cmd_decoder.cc (right): > > > > > https://codereview.chromium.org/2369313002/diff/80001/gpu/command_buffer/serv... > > gpu/command_buffer/service/gles2_cmd_decoder.cc:13338: > > copy_tex_image_blit_->DoCopyTexImage2DToLUMACompatibilityTexture( > > On 2016/09/30 18:02:11, Zhenyao Mo wrote: > > > On 2016/09/30 17:44:42, qiankun wrote: > > > > On 2016/09/30 16:24:20, Zhenyao Mo wrote: > > > > > Here is the luminance/alpha format emulation. Of course the > > > > > WorkaroundCopyTexImageCubeMap() will fail because it calls TexImage*() > > > > functions > > > > > if the formats are luminance/alpha, which isn't supported on Mac Core > > > profile. > > > > > > > > Won't line 13315 do LUMINANCE/ALPHA mapping? > > > > > > That's not enough. The format will still cause an GL error. > > > > Mac OSX should be use core profile now. But I didn't see the GL errors with > this > > CL for luminance cases. Do you know what situation will trigger the errors? > > That's weird. You have to debug into the WorkaroundCopyTexImageCubeMap() where > the glTexImage2D is called, verifying internalformat is mapped (LUMINANCE->R8), > format isn't (still LUMINANCE). And right after that, there has to be a GL > error. I verified internalformat mapping on Mac: LUMINANCE->RED LUMINANCE_ALPHA->RG. These formats are supported by GL on Mac. So, there are no gl errors for this CL.
On 2016/10/01 01:34:13, Zhenyao Mo wrote: > On 2016/10/01 01:25:06, qiankun wrote: > > > https://codereview.chromium.org/2369313002/diff/80001/gpu/command_buffer/serv... > > File gpu/command_buffer/service/gles2_cmd_decoder.cc (right): > > > > > https://codereview.chromium.org/2369313002/diff/80001/gpu/command_buffer/serv... > > gpu/command_buffer/service/gles2_cmd_decoder.cc:13338: > > copy_tex_image_blit_->DoCopyTexImage2DToLUMACompatibilityTexture( > > On 2016/09/30 18:02:11, Zhenyao Mo wrote: > > > On 2016/09/30 17:44:42, qiankun wrote: > > > > On 2016/09/30 16:24:20, Zhenyao Mo wrote: > > > > > Here is the luminance/alpha format emulation. Of course the > > > > > WorkaroundCopyTexImageCubeMap() will fail because it calls TexImage*() > > > > functions > > > > > if the formats are luminance/alpha, which isn't supported on Mac Core > > > profile. > > > > > > > > Won't line 13315 do LUMINANCE/ALPHA mapping? > > > > > > That's not enough. The format will still cause an GL error. > > > > Mac OSX should be use core profile now. But I didn't see the GL errors with > this > > CL for luminance cases. Do you know what situation will trigger the errors? > > That's weird. You have to debug into the WorkaroundCopyTexImageCubeMap() where > the glTexImage2D is called, verifying internalformat is mapped (LUMINANCE->R8), > format isn't (still LUMINANCE). And right after that, there has to be a GL > error. Format is also mapped when doing glTexImage2D, see https://cs.chromium.org/chromium/src/gpu/command_buffer/service/texture_manag...
https://codereview.chromium.org/2369313002/diff/60001/gpu/command_buffer/serv... File gpu/command_buffer/service/gles2_cmd_decoder.cc (right): https://codereview.chromium.org/2369313002/diff/60001/gpu/command_buffer/serv... gpu/command_buffer/service/gles2_cmd_decoder.cc:13327: } On 2016/09/28 21:14:23, Zhenyao Mo wrote: > This gets complicated when we also need to emulate for LUMINANCE_ALPHA formats. > > Could you at least add a TODO and create a bug to track it? Zhenyao&Ken, do I still need to add this TODO according my last comment as below: Format is also mapped when doing glTexImage2D, see: https://cs.chromium.org/chromium/src/gpu/command_buffer/service/texture_manag...
On 2016/10/08 07:58:49, qiankun wrote: > On 2016/10/01 01:34:13, Zhenyao Mo wrote: > > On 2016/10/01 01:25:06, qiankun wrote: > > > > > > https://codereview.chromium.org/2369313002/diff/80001/gpu/command_buffer/serv... > > > File gpu/command_buffer/service/gles2_cmd_decoder.cc (right): > > > > > > > > > https://codereview.chromium.org/2369313002/diff/80001/gpu/command_buffer/serv... > > > gpu/command_buffer/service/gles2_cmd_decoder.cc:13338: > > > copy_tex_image_blit_->DoCopyTexImage2DToLUMACompatibilityTexture( > > > On 2016/09/30 18:02:11, Zhenyao Mo wrote: > > > > On 2016/09/30 17:44:42, qiankun wrote: > > > > > On 2016/09/30 16:24:20, Zhenyao Mo wrote: > > > > > > Here is the luminance/alpha format emulation. Of course the > > > > > > WorkaroundCopyTexImageCubeMap() will fail because it calls TexImage*() > > > > > functions > > > > > > if the formats are luminance/alpha, which isn't supported on Mac Core > > > > profile. > > > > > > > > > > Won't line 13315 do LUMINANCE/ALPHA mapping? > > > > > > > > That's not enough. The format will still cause an GL error. > > > > > > Mac OSX should be use core profile now. But I didn't see the GL errors with > > this > > > CL for luminance cases. Do you know what situation will trigger the errors? > > > > That's weird. You have to debug into the WorkaroundCopyTexImageCubeMap() > where > > the glTexImage2D is called, verifying internalformat is mapped > (LUMINANCE->R8), > > format isn't (still LUMINANCE). And right after that, there has to be a GL > > error. > > I verified internalformat mapping on Mac: > LUMINANCE->RED > LUMINANCE_ALPHA->RG. > > These formats are supported by GL on Mac. So, there are no gl errors for this > CL. What do you mean by mapping on Mac? Are you talking about Mac Core profile will accept LUMINANCE and LUMINANCE_ALPHA and do the mapping by itself? That's not possible.
On 2016/10/11 02:02:55, Zhenyao Mo wrote: > On 2016/10/08 07:58:49, qiankun wrote: > > On 2016/10/01 01:34:13, Zhenyao Mo wrote: > > > On 2016/10/01 01:25:06, qiankun wrote: > > > > > > > > > > https://codereview.chromium.org/2369313002/diff/80001/gpu/command_buffer/serv... > > > > File gpu/command_buffer/service/gles2_cmd_decoder.cc (right): > > > > > > > > > > > > > > https://codereview.chromium.org/2369313002/diff/80001/gpu/command_buffer/serv... > > > > gpu/command_buffer/service/gles2_cmd_decoder.cc:13338: > > > > copy_tex_image_blit_->DoCopyTexImage2DToLUMACompatibilityTexture( > > > > On 2016/09/30 18:02:11, Zhenyao Mo wrote: > > > > > On 2016/09/30 17:44:42, qiankun wrote: > > > > > > On 2016/09/30 16:24:20, Zhenyao Mo wrote: > > > > > > > Here is the luminance/alpha format emulation. Of course the > > > > > > > WorkaroundCopyTexImageCubeMap() will fail because it calls > TexImage*() > > > > > > functions > > > > > > > if the formats are luminance/alpha, which isn't supported on Mac > Core > > > > > profile. > > > > > > > > > > > > Won't line 13315 do LUMINANCE/ALPHA mapping? > > > > > > > > > > That's not enough. The format will still cause an GL error. > > > > > > > > Mac OSX should be use core profile now. But I didn't see the GL errors > with > > > this > > > > CL for luminance cases. Do you know what situation will trigger the > errors? > > > > > > That's weird. You have to debug into the WorkaroundCopyTexImageCubeMap() > > where > > > the glTexImage2D is called, verifying internalformat is mapped > > (LUMINANCE->R8), > > > format isn't (still LUMINANCE). And right after that, there has to be a GL > > > error. > > > > I verified internalformat mapping on Mac: > > LUMINANCE->RED > > LUMINANCE_ALPHA->RG. > > > > These formats are supported by GL on Mac. So, there are no gl errors for this > > CL. > > What do you mean by mapping on Mac? Are you talking about Mac Core profile will > accept LUMINANCE and LUMINANCE_ALPHA and do the mapping by itself? That's not > possible. Chrome do the internalformat and format mapping for LUMINANCE and LUMINANCE_ALPHA on Mac: internalformat: https://cs.chromium.org/chromium/src/gpu/command_buffer/service/gles2_cmd_dec... format: https://cs.chromium.org/chromium/src/gpu/command_buffer/service/texture_manag...
https://codereview.chromium.org/2369313002/diff/120001/gpu/command_buffer/ser... File gpu/command_buffer/service/gles2_cmd_decoder.cc (right): https://codereview.chromium.org/2369313002/diff/120001/gpu/command_buffer/ser... gpu/command_buffer/service/gles2_cmd_decoder.cc:13388: texture_manager()->WorkaroundCopyTexImageCubeMap(&texture_state_, Here is my question: WorkaroundCopyTexImageCubeMap() call TextureManager::DoTexImage(), with internal_format (LUMINANCE_ALPHA) already correctly mapped to final_internal_format (RG), but format is still LUMINANCE_ALPHA. That's why I don't understand how DoTexImage() could succeed? Or is there a place format is also mapped and I missed it?
https://codereview.chromium.org/2369313002/diff/120001/gpu/command_buffer/ser... File gpu/command_buffer/service/gles2_cmd_decoder.cc (right): https://codereview.chromium.org/2369313002/diff/120001/gpu/command_buffer/ser... gpu/command_buffer/service/gles2_cmd_decoder.cc:13388: texture_manager()->WorkaroundCopyTexImageCubeMap(&texture_state_, On 2016/10/11 03:10:18, Zhenyao Mo wrote: > Here is my question: > > WorkaroundCopyTexImageCubeMap() call TextureManager::DoTexImage(), with > internal_format (LUMINANCE_ALPHA) already correctly mapped to > final_internal_format (RG), but format is still LUMINANCE_ALPHA. > > That's why I don't understand how DoTexImage() could succeed? Or is there a > place format is also mapped and I missed it? In TextureManager::DoTexImage, format is translated (i.e. AdjustTexFormat(feature_info_.get(), args.format)) when calling glTexImage2D so DoTexImage could succeed. See: https://cs.chromium.org/chromium/src/gpu/command_buffer/service/texture_manag...
On 2016/10/11 03:18:19, qiankun wrote: > https://codereview.chromium.org/2369313002/diff/120001/gpu/command_buffer/ser... > File gpu/command_buffer/service/gles2_cmd_decoder.cc (right): > > https://codereview.chromium.org/2369313002/diff/120001/gpu/command_buffer/ser... > gpu/command_buffer/service/gles2_cmd_decoder.cc:13388: > texture_manager()->WorkaroundCopyTexImageCubeMap(&texture_state_, > On 2016/10/11 03:10:18, Zhenyao Mo wrote: > > Here is my question: > > > > WorkaroundCopyTexImageCubeMap() call TextureManager::DoTexImage(), with > > internal_format (LUMINANCE_ALPHA) already correctly mapped to > > final_internal_format (RG), but format is still LUMINANCE_ALPHA. > > > > That's why I don't understand how DoTexImage() could succeed? Or is there a > > place format is also mapped and I missed it? > > In TextureManager::DoTexImage, format is translated (i.e. > AdjustTexFormat(feature_info_.get(), args.format)) when calling glTexImage2D so > DoTexImage could succeed. > See: > https://cs.chromium.org/chromium/src/gpu/command_buffer/service/texture_manag... Zhenyao, I think the code is correct now. I have discussed with qiankun offline about the format conversion before call into glTexImage2D in TextureManager::DoTexImage. You can see the code snippet qiankun linked to.
On 2016/10/11 03:21:09, yunchao wrote: > On 2016/10/11 03:18:19, qiankun wrote: > > > https://codereview.chromium.org/2369313002/diff/120001/gpu/command_buffer/ser... > > File gpu/command_buffer/service/gles2_cmd_decoder.cc (right): > > > > > https://codereview.chromium.org/2369313002/diff/120001/gpu/command_buffer/ser... > > gpu/command_buffer/service/gles2_cmd_decoder.cc:13388: > > texture_manager()->WorkaroundCopyTexImageCubeMap(&texture_state_, > > On 2016/10/11 03:10:18, Zhenyao Mo wrote: > > > Here is my question: > > > > > > WorkaroundCopyTexImageCubeMap() call TextureManager::DoTexImage(), with > > > internal_format (LUMINANCE_ALPHA) already correctly mapped to > > > final_internal_format (RG), but format is still LUMINANCE_ALPHA. > > > > > > That's why I don't understand how DoTexImage() could succeed? Or is there a > > > place format is also mapped and I missed it? > > > > In TextureManager::DoTexImage, format is translated (i.e. > > AdjustTexFormat(feature_info_.get(), args.format)) when calling glTexImage2D > so > > DoTexImage could succeed. > > See: > > > https://cs.chromium.org/chromium/src/gpu/command_buffer/service/texture_manag... > > Zhenyao, I think the code is correct now. I have discussed with qiankun offline > about the format conversion before call into glTexImage2D in > TextureManager::DoTexImage. > You can see the code snippet qiankun linked to. Ah I missed that. Thank you and sorry about the noise. LGTM
The CQ bit was checked by qiankun.miao@intel.com
The patchset sent to the CQ was uploaded after l-g-t-m from kbr@chromium.org, zmo@chromium.org Link to the patchset: https://codereview.chromium.org/2369313002/#ps140001 (title: "rebase only")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Committed patchset #7 (id:140001)
Message was sent while issue was closed.
Description was changed from ========== Work around CopyTexImage2D issue on Intel Mac OSX 10.12 CopyTexImage2D for cube map target has bug on Intel Mac OSX 10.12. If positive x face of the first two levels are not set, CopyTexImage2D will copy incorrect pixel data to cube map texture. This bug is similar to the bug fixed by https://codereview.chromium.org/2348243002/. But solutions are different. This bug can be worked around by setting positive x face of level 0 and 1. BUG=648197 TEST=deqp/functional/gles3/texturespecification/basic_copyteximage2d.html 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 ========== Work around CopyTexImage2D issue on Intel Mac OSX 10.12 CopyTexImage2D for cube map target has bug on Intel Mac OSX 10.12. If positive x face of the first two levels are not set, CopyTexImage2D will copy incorrect pixel data to cube map texture. This bug is similar to the bug fixed by https://codereview.chromium.org/2348243002/. But solutions are different. This bug can be worked around by setting positive x face of level 0 and 1. BUG=648197 TEST=deqp/functional/gles3/texturespecification/basic_copyteximage2d.html 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/45c5754736e2dffd0bcd34c5499c07a80acae58b Cr-Commit-Position: refs/heads/master@{#424422} ==========
Message was sent while issue was closed.
Patchset 7 (id:??) landed as https://crrev.com/45c5754736e2dffd0bcd34c5499c07a80acae58b Cr-Commit-Position: refs/heads/master@{#424422} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
