Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2623863002:
Support level > 0 for CopyTextureCHROMIUM extension (Closed)
DescriptionSupport level > 0 for CopyTextureCHROMIUM extension
Source level > 0 is not supported in ES 2.0 due to
glFramebufferTexture2D doesn't support level > 0 in ES 2.0. Run into
DRAW_AND_COPY path for dest level > 0 in ES 2.0, i.e. draw the source
texture to a fbo attaching level 0 of an intermediate texture, then use
glCopyTexImage2D to copy intermediate texture to dest texture.
For ES 3.0, we can set base level of source and dest texture to the
specifed level respectively, then do CopyTextureCHROMIUM as before,
restore base level to previous value at the end of CopyTextureCHROMIUM.
But we fall back to DRAW_AND_COPY path due to driver bug of base level
on some scenarios.
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/2623863002
Cr-Commit-Position: refs/heads/master@{#444295}
Committed: https://chromium.googlesource.com/chromium/src/+/cbaeb8aa76be268c5179deb56e67a765303e34cc
Patch Set 1 #
Total comments: 4
Patch Set 2 : restore base level #Patch Set 3 : fix Android bot #
Total comments: 6
Patch Set 4 : rebase only #Patch Set 5 : skip dest_level > 0 on Android #
Total comments: 2
Messages
Total messages: 33 (10 generated)
Description was changed from ========== Support level > 0 for CopyTextureCHROMIUM extension Source level > 0 is not supported in ES 2.0 due to glFramebufferTexture2D doesn't support level > 0 in ES 2.0. Run into DRAW_AND_COPY path for dest level > 0 in ES 2.0, i.e. draw the source texture to a fbo attaching level 0 of an intermediate texture, then use glCopyTexImage2D to copy intermediate texture to dest texture. For ES 3.0, we can set base level of source and dest texture to the specifed level respectively, then do CopyTextureCHROMIUM as before, restore base level to previous value at the end of CopyTextureCHROMIUM. But we fall back to DRAW_AND_COPY path due to driver bug of base level on some scenarios. BUG=612542 ========== to ========== Support level > 0 for CopyTextureCHROMIUM extension Source level > 0 is not supported in ES 2.0 due to glFramebufferTexture2D doesn't support level > 0 in ES 2.0. Run into DRAW_AND_COPY path for dest level > 0 in ES 2.0, i.e. draw the source texture to a fbo attaching level 0 of an intermediate texture, then use glCopyTexImage2D to copy intermediate texture to dest texture. For ES 3.0, we can set base level of source and dest texture to the specifed level respectively, then do CopyTextureCHROMIUM as before, restore base level to previous value at the end of CopyTextureCHROMIUM. But we fall back to DRAW_AND_COPY path due to driver bug of base level on some scenarios. 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.
Mostly look good with a few minor issues and a question. https://codereview.chromium.org/2623863002/diff/1/gpu/command_buffer/service/... File gpu/command_buffer/service/gles2_cmd_copy_texture_chromium.cc (right): https://codereview.chromium.org/2623863002/diff/1/gpu/command_buffer/service/... gpu/command_buffer/service/gles2_cmd_copy_texture_chromium.cc:523: glTexParameteri(target, GL_TEXTURE_BASE_LEVEL, level); Do we recover this parameter afterwards? https://codereview.chromium.org/2623863002/diff/1/gpu/command_buffer/service/... gpu/command_buffer/service/gles2_cmd_copy_texture_chromium.cc:531: DCHECK(glGetError() == GL_NO_ERROR); This is wrong. It might swallow a GL error from previous command in debug mode.
Patchset #2 (id:20001) has been deleted
https://codereview.chromium.org/2623863002/diff/1/gpu/command_buffer/service/... File gpu/command_buffer/service/gles2_cmd_copy_texture_chromium.cc (right): https://codereview.chromium.org/2623863002/diff/1/gpu/command_buffer/service/... gpu/command_buffer/service/gles2_cmd_copy_texture_chromium.cc:523: glTexParameteri(target, GL_TEXTURE_BASE_LEVEL, level); On 2017/01/10 20:11:22, Zhenyao Mo wrote: > Do we recover this parameter afterwards? I added the recover in decoder->RestoreTextureState. I wrongly thought it was there before. https://codereview.chromium.org/2623863002/diff/1/gpu/command_buffer/service/... gpu/command_buffer/service/gles2_cmd_copy_texture_chromium.cc:531: DCHECK(glGetError() == GL_NO_ERROR); On 2017/01/10 20:11:22, Zhenyao Mo wrote: > This is wrong. It might swallow a GL error from previous command in debug mode. Remove this DCHECK. https://codereview.chromium.org/2623863002/diff/60001/gpu/command_buffer/serv... File gpu/command_buffer/service/gles2_cmd_decoder.cc (right): https://codereview.chromium.org/2623863002/diff/60001/gpu/command_buffer/serv... gpu/command_buffer/service/gles2_cmd_decoder.cc:5571: #if defined(OS_ANDROID) It seems android has a bug about base level.
https://codereview.chromium.org/2623863002/diff/60001/gpu/command_buffer/serv... File gpu/command_buffer/service/gles2_cmd_decoder.cc (right): https://codereview.chromium.org/2623863002/diff/60001/gpu/command_buffer/serv... gpu/command_buffer/service/gles2_cmd_decoder.cc:5571: #if defined(OS_ANDROID) On 2017/01/11 10:58:22, qiankun wrote: > It seems android has a bug about base level. I'd like a bug filed with details of the problem on Android. https://codereview.chromium.org/2623863002/diff/60001/gpu/command_buffer/serv... gpu/command_buffer/service/gles2_cmd_decoder.cc:5575: glGetTexParameteriv(target, GL_TEXTURE_BASE_LEVEL, &base_level); Querying the driver seems heavier than necessary. I think we will need a driver bug workaround here, and we should keep track of what's the base level on driver in chromium.
Thanks for working on this Qiankun. I defer review to Mo, but a couple of comments. https://codereview.chromium.org/2623863002/diff/60001/gpu/command_buffer/serv... File gpu/command_buffer/service/gles2_cmd_decoder.cc (right): https://codereview.chromium.org/2623863002/diff/60001/gpu/command_buffer/serv... gpu/command_buffer/service/gles2_cmd_decoder.cc:5571: #if defined(OS_ANDROID) On 2017/01/11 18:41:41, Zhenyao Mo wrote: > On 2017/01/11 10:58:22, qiankun wrote: > > It seems android has a bug about base level. > > I'd like a bug filed with details of the problem on Android. Agree. https://codereview.chromium.org/2623863002/diff/60001/gpu/command_buffer/serv... gpu/command_buffer/service/gles2_cmd_decoder.cc:5575: glGetTexParameteriv(target, GL_TEXTURE_BASE_LEVEL, &base_level); On 2017/01/11 18:41:41, Zhenyao Mo wrote: > Querying the driver seems heavier than necessary. I think we will need a driver > bug workaround here, and we should keep track of what's the base level on driver > in chromium. Yes, please create a driver bug workaround rather than adding an #ifdef.
https://codereview.chromium.org/2623863002/diff/60001/gpu/command_buffer/serv... File gpu/command_buffer/service/gles2_cmd_decoder.cc (right): https://codereview.chromium.org/2623863002/diff/60001/gpu/command_buffer/serv... gpu/command_buffer/service/gles2_cmd_decoder.cc:5571: #if defined(OS_ANDROID) On 2017/01/11 19:53:03, Ken Russell wrote: > On 2017/01/11 18:41:41, Zhenyao Mo wrote: > > On 2017/01/11 10:58:22, qiankun wrote: > > > It seems android has a bug about base level. > > > > I'd like a bug filed with details of the problem on Android. > > Agree. If you look at the failure log, you can find not all of the channels are wrong, see log from Patch Set#2: https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi.... I filed a bug at: https://bugs.chromium.org/p/chromium/issues/detail?id=680460. But I still didn't get very clear the failure on Android. My previous hack didn't work for all Android devices. So, I just skipped test for dest_level > 0 to make bots green currently. I will investigate it in future. Does this make sense?
On 2017/01/12 15:38:16, qiankun wrote: > https://codereview.chromium.org/2623863002/diff/60001/gpu/command_buffer/serv... > File gpu/command_buffer/service/gles2_cmd_decoder.cc (right): > > https://codereview.chromium.org/2623863002/diff/60001/gpu/command_buffer/serv... > gpu/command_buffer/service/gles2_cmd_decoder.cc:5571: #if defined(OS_ANDROID) > On 2017/01/11 19:53:03, Ken Russell wrote: > > On 2017/01/11 18:41:41, Zhenyao Mo wrote: > > > On 2017/01/11 10:58:22, qiankun wrote: > > > > It seems android has a bug about base level. > > > > > > I'd like a bug filed with details of the problem on Android. > > > > Agree. > > If you look at the failure log, you can find not all of the channels are wrong, > see log from Patch Set#2: > https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi.... > > > I filed a bug at: https://bugs.chromium.org/p/chromium/issues/detail?id=680460. > But I still didn't get very clear the failure on Android. My previous hack > didn't work for all Android devices. So, I just skipped test for dest_level > 0 > to make bots green currently. I will investigate it in future. Does this make > sense? lgtm
Thanks for pushing this forward. I still defer to zmo's review, and it's OK to land this given that it's tested on most platforms, but one concern. https://codereview.chromium.org/2623863002/diff/100001/gpu/command_buffer/tes... File gpu/command_buffer/tests/gl_copy_texture_CHROMIUM_unittest.cc (right): https://codereview.chromium.org/2623863002/diff/100001/gpu/command_buffer/tes... gpu/command_buffer/tests/gl_copy_texture_CHROMIUM_unittest.cc:786: // NVIDIA Windows: crbug.com/679639 and Android: crbug.com/680460. Sorry for going back and forth on this (per my suggestion on crbug.com/679639), but I'm concerned that we don't explicitly catch the known error cases in the core code, and are explicitly skipping the tests rather than expecting failure. It would be better to add a driver bug workaround like "disable_copy_texture_chromium_souce_and_dest_levels", and turn on that workaround for Android and Windows/NVIDIA. The "workaround" would be to manually generate a GL error if the client attempts to pass in source_level > 0 or dest_level >0 to Copy{Sub}TextureCHROMIUM on the affected platforms, and abort the command. If that's feasible then let's please follow this CL soon with something like it. Since in the tests we know the platform but not the GPU, we could add a test here just for Android to expect failure of the CopyTextureCHROMIUM command if source_level > 0 or dest_level > 0, and on Windows, skip the test. Thanks.
Thanks your careful review Zhenyao and Ken! After considering this further, I think we can not reply on base level to implement this level > 0 functionality. Framebuffer completeness may depend on driver implementation from each GPU vendor. I saw following descriptions in both ES3 spec and OpenGL spec: In ES, I saw the following description in page 215, ES 3.0.4, https://www.khronos.org/registry/gles/specs/3.0/es_spec_3.0.4.pdf: " If the combination of formats of the images attached to a framebuffer object are not supported by the implementation, then the framebuffer is not complete under the clause labeled FRAMEBUFFER_UNSUPPORTED. " In GL, page 312, https://www.opengl.org/registry/doc/glspec45.core.pdf " The rules of framebuffer completeness are dependent on the properties of the attached images, and on certain implementation-dependent restrictions. " So, I intend to do same implementation for ES3 as ES3, i.e go to DRAW_AND_COPY path for level > 0. In such path, we can only support dest_level > 0 and restrict source_level == 0 always, see the reason in https://codereview.chromium.org/2609393002/#msg6. But we cannot verify results are correct for some non-filterable dest texture because generateMipmap hack doesn't work for non-filterable formats. What do you think?
On 2017/01/13 13:32:09, qiankun wrote: > Thanks your careful review Zhenyao and Ken! > After considering this further, I think we can not reply on base level to > implement this level > 0 functionality. Framebuffer completeness may depend on > driver implementation from each GPU vendor. I saw following descriptions in both > ES3 spec and OpenGL spec: > > In ES, I saw the following description in page 215, ES 3.0.4, > https://www.khronos.org/registry/gles/specs/3.0/es_spec_3.0.4.pdf: > " If the combination of formats of the images attached to a framebuffer object > are not supported by the implementation, then the framebuffer is not complete > under the clause labeled FRAMEBUFFER_UNSUPPORTED. " > In GL, page 312, https://www.opengl.org/registry/doc/glspec45.core.pdf > " The rules of framebuffer completeness are dependent on the properties of the > attached images, and on certain implementation-dependent restrictions. " > > So, I intend to do same implementation for ES3 as ES3, i.e go to DRAW_AND_COPY > path for level > 0. In such path, we can only support dest_level > 0 and > restrict source_level == 0 always, see the reason in > https://codereview.chromium.org/2609393002/#msg6. But we cannot verify results > are correct for some non-filterable dest texture because generateMipmap hack > doesn't work for non-filterable formats. What do you think? I think restricting source_level to 0 is OK, as the intended use cases of this extension all have source_level == 0. I don't think I fully understand the last question. Can you explain a bit?
On 2017/01/13 18:30:57, Zhenyao Mo wrote: > On 2017/01/13 13:32:09, qiankun wrote: > > Thanks your careful review Zhenyao and Ken! > > After considering this further, I think we can not reply on base level to > > implement this level > 0 functionality. Framebuffer completeness may depend on > > driver implementation from each GPU vendor. I saw following descriptions in > both > > ES3 spec and OpenGL spec: > > > > In ES, I saw the following description in page 215, ES 3.0.4, > > https://www.khronos.org/registry/gles/specs/3.0/es_spec_3.0.4.pdf: > > " If the combination of formats of the images attached to a framebuffer object > > are not supported by the implementation, then the framebuffer is not complete > > under the clause labeled FRAMEBUFFER_UNSUPPORTED. " > > In GL, page 312, https://www.opengl.org/registry/doc/glspec45.core.pdf > > " The rules of framebuffer completeness are dependent on the properties of the > > attached images, and on certain implementation-dependent restrictions. " > > > > So, I intend to do same implementation for ES3 as ES3, i.e go to DRAW_AND_COPY > > path for level > 0. In such path, we can only support dest_level > 0 and > > restrict source_level == 0 always, see the reason in > > https://codereview.chromium.org/2609393002/#msg6. But we cannot verify results > > are correct for some non-filterable dest texture because generateMipmap hack > > doesn't work for non-filterable formats. What do you think? > > I think restricting source_level to 0 is OK, as the intended use cases of this > extension all have source_level == 0. Sounds fine to me too. I haven't thought through that comment fully. For the source texture, I would expect to use NEAREST filtering rather than NEAREST_MIPMAP_NEAREST. I haven't been following all the bugs you've run into, but it seems to me that setting the base level of the source texture should be well supported, though I can imagine that attaching such a texture to a framebuffer wouldn't be. On the other hand, FramebufferTexture2D's level argument should work to attach a particular level of a texture, shouldn't it? Also, the new textureLod GLSL function might be useful for sampling a particular level. This is all not that important -- I'm not sure why we were pursuing uploading to levels > 0. Getting uploads to cube map faces working is much more important. > I don't think I fully understand the last question. Can you explain a bit?
On 2017/01/13 18:30:57, Zhenyao Mo wrote: > On 2017/01/13 13:32:09, qiankun wrote: > > Thanks your careful review Zhenyao and Ken! > > After considering this further, I think we can not reply on base level to > > implement this level > 0 functionality. Framebuffer completeness may depend on > > driver implementation from each GPU vendor. I saw following descriptions in > both > > ES3 spec and OpenGL spec: > > > > In ES, I saw the following description in page 215, ES 3.0.4, > > https://www.khronos.org/registry/gles/specs/3.0/es_spec_3.0.4.pdf: > > " If the combination of formats of the images attached to a framebuffer object > > are not supported by the implementation, then the framebuffer is not complete > > under the clause labeled FRAMEBUFFER_UNSUPPORTED. " > > In GL, page 312, https://www.opengl.org/registry/doc/glspec45.core.pdf > > " The rules of framebuffer completeness are dependent on the properties of the > > attached images, and on certain implementation-dependent restrictions. " > > > > So, I intend to do same implementation for ES3 as ES3, i.e go to DRAW_AND_COPY > > path for level > 0. In such path, we can only support dest_level > 0 and > > restrict source_level == 0 always, see the reason in > > https://codereview.chromium.org/2609393002/#msg6. But we cannot verify results > > are correct for some non-filterable dest texture because generateMipmap hack > > doesn't work for non-filterable formats. What do you think? > > I think restricting source_level to 0 is OK, as the intended use cases of this > extension all have source_level == 0. > > I don't think I fully understand the last question. Can you explain a bit? I talked about how to sample for non-zero level of non-filterable texture when verifying result dest texture is correct. Maybe I over-thought about base level issue. It may affect framebuffer completeness, but we still can sample from non-zero base level.
https://codereview.chromium.org/2623863002/diff/100001/gpu/command_buffer/ser... File gpu/command_buffer/service/gles2_cmd_decoder.cc (right): https://codereview.chromium.org/2623863002/diff/100001/gpu/command_buffer/ser... gpu/command_buffer/service/gles2_cmd_decoder.cc:16416: } See the comments here. The current implementation doesn't reply on framebuffer completeness. I just set TEXTURE_BASE_LEVEL and do sampling from that base level. So it should be no problem. Zhenyao, can I land the CL as-is?
On 2017/01/13 19:57:49, Ken Russell wrote:
> > I think restricting source_level to 0 is OK, as the intended use cases of
this
> > extension all have source_level == 0.
>
> Sounds fine to me too.
>
> I haven't thought through that comment fully. For the source texture, I would
> expect to use NEAREST filtering rather than NEAREST_MIPMAP_NEAREST.
NEAREST only works for base level.
> I haven't
> been following all the bugs you've run into, but it seems to me that setting
the
> base level of the source texture should be well supported, though I can
imagine
> that attaching such a texture to a framebuffer wouldn't be. On the other hand,
> FramebufferTexture2D's level argument should work to attach a particular level
> of a texture, shouldn't it?
I also think it should work.
GLint level = 1;
GLTexture tex;
glBindTexture(GL_TEXTURE_2D, tex.get());
glTexImage2D(GL_TEXTURE_2D, level, GL_RGBA, 8, 8, 0, GL_RGBA, GL_UNSIGNED_BYTE,
nullptr);
glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_NEAREST);
glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_NEAREST);
glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_BASE_LEVEL, level);
glBindFramebuffer(GL_FRAMEBUFFER, mFramebuffer);
glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D,
tex.get(), level);
// Verify the framebuffer is complete.
ASSERT_GLENUM_EQ(GL_FRAMEBUFFER_COMPLETE,
glCheckFramebufferStatus(GL_FRAMEBUFFER));
Linux Mesa returns GL_FRAMEBUFFER_COMPLETE, while Linux NVIDIA returns
GL_FRAMEBUFFER_UNSUPPORTED in above test.
It’s expected that texture level is bound to fbo, level is base level, so fbo
should be complete.
The only reason why Linux NVIDIA returned FRAMEBUFFER_UNSUPPORTED is it didn’t
support combinations of formats of the images attached to fbo if the image is
from a non-zero base level. Since spec allows it’s implement-dependent, so I am
not sure if it’s a driver bug.
>
> Also, the new textureLod GLSL function might be useful for sampling a
particular
> level.
I may consider this in future.
> This is all not that important -- I'm not sure why we were pursuing uploading
to
> levels > 0. Getting uploads to cube map faces working is much more important.
I will fix cube map next week.
On 2017/01/14 00:31:18, qiankun wrote: > On 2017/01/13 19:57:49, Ken Russell wrote: > > > I think restricting source_level to 0 is OK, as the intended use cases of > this > > > extension all have source_level == 0. > > > > Sounds fine to me too. > > > > I haven't thought through that comment fully. For the source texture, I would > > expect to use NEAREST filtering rather than NEAREST_MIPMAP_NEAREST. > NEAREST only works for base level. > > > > I haven't > > been following all the bugs you've run into, but it seems to me that setting > the > > base level of the source texture should be well supported, though I can > imagine > > that attaching such a texture to a framebuffer wouldn't be. On the other hand, > > FramebufferTexture2D's level argument should work to attach a particular level > > of a texture, shouldn't it? > I also think it should work. > > GLint level = 1; > GLTexture tex; > glBindTexture(GL_TEXTURE_2D, tex.get()); > glTexImage2D(GL_TEXTURE_2D, level, GL_RGBA, 8, 8, 0, GL_RGBA, GL_UNSIGNED_BYTE, > nullptr); > glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_NEAREST); > glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_NEAREST); > glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_BASE_LEVEL, level); > > glBindFramebuffer(GL_FRAMEBUFFER, mFramebuffer); > glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, > tex.get(), level); > > // Verify the framebuffer is complete. > ASSERT_GLENUM_EQ(GL_FRAMEBUFFER_COMPLETE, > glCheckFramebufferStatus(GL_FRAMEBUFFER)); > > Linux Mesa returns GL_FRAMEBUFFER_COMPLETE, while Linux NVIDIA returns > GL_FRAMEBUFFER_UNSUPPORTED in above test. > It’s expected that texture level is bound to fbo, level is base level, so fbo > should be complete. > > The only reason why Linux NVIDIA returned FRAMEBUFFER_UNSUPPORTED is it didn’t > support combinations of formats of the images attached to fbo if the image is > from a non-zero base level. Since spec allows it’s implement-dependent, so I am > not sure if it’s a driver bug. > > > > > Also, the new textureLod GLSL function might be useful for sampling a > particular > > level. > I may consider this in future. > > > This is all not that important -- I'm not sure why we were pursuing uploading > to > > levels > 0. Getting uploads to cube map faces working is much more important. > I will fix cube map next week. If we draw to framebuffer with texture |level| > 0, we don't have to set the base level of that texture to |level|, right? In that case, we can still do DIRECT_DRAW. If I am correct, can you modify the code to reflect that?
On 2017/01/14 00:56:11, Zhenyao Mo wrote: > On 2017/01/14 00:31:18, qiankun wrote: > > On 2017/01/13 19:57:49, Ken Russell wrote: > > > > I think restricting source_level to 0 is OK, as the intended use cases of > > this > > > > extension all have source_level == 0. > > > > > > Sounds fine to me too. > > > > > > I haven't thought through that comment fully. For the source texture, I > would > > > expect to use NEAREST filtering rather than NEAREST_MIPMAP_NEAREST. > > NEAREST only works for base level. > > > > > > > I haven't > > > been following all the bugs you've run into, but it seems to me that setting > > the > > > base level of the source texture should be well supported, though I can > > imagine > > > that attaching such a texture to a framebuffer wouldn't be. On the other > hand, > > > FramebufferTexture2D's level argument should work to attach a particular > level > > > of a texture, shouldn't it? > > I also think it should work. > > > > GLint level = 1; > > GLTexture tex; > > glBindTexture(GL_TEXTURE_2D, tex.get()); > > glTexImage2D(GL_TEXTURE_2D, level, GL_RGBA, 8, 8, 0, GL_RGBA, > GL_UNSIGNED_BYTE, > > nullptr); > > glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_NEAREST); > > glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_NEAREST); > > glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_BASE_LEVEL, level); > > > > glBindFramebuffer(GL_FRAMEBUFFER, mFramebuffer); > > glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, > > tex.get(), level); > > > > // Verify the framebuffer is complete. > > ASSERT_GLENUM_EQ(GL_FRAMEBUFFER_COMPLETE, > > glCheckFramebufferStatus(GL_FRAMEBUFFER)); > > > > Linux Mesa returns GL_FRAMEBUFFER_COMPLETE, while Linux NVIDIA returns > > GL_FRAMEBUFFER_UNSUPPORTED in above test. > > It’s expected that texture level is bound to fbo, level is base level, so fbo > > should be complete. > > > > The only reason why Linux NVIDIA returned FRAMEBUFFER_UNSUPPORTED is it didn’t > > support combinations of formats of the images attached to fbo if the image is > > from a non-zero base level. Since spec allows it’s implement-dependent, so I > am > > not sure if it’s a driver bug. > > > > > > > > Also, the new textureLod GLSL function might be useful for sampling a > > particular > > > level. > > I may consider this in future. > > > > > This is all not that important -- I'm not sure why we were pursuing > uploading > > to > > > levels > 0. Getting uploads to cube map faces working is much more > important. > > I will fix cube map next week. > > If we draw to framebuffer with texture |level| > 0, we don't have to set the > base level of that texture to |level|, right? In that case, we can still do > DIRECT_DRAW. > > If I am correct, can you modify the code to reflect that? Not right. We should set base level of that texture to |level| to make framebuffer complete since spec requires draw buffer be complete when drawing.
On 2017/01/14 00:31:18, qiankun wrote: > On 2017/01/13 19:57:49, Ken Russell wrote: > > > I think restricting source_level to 0 is OK, as the intended use cases of > this > > > extension all have source_level == 0. > > > > Sounds fine to me too. > > > > I haven't thought through that comment fully. For the source texture, I would > > expect to use NEAREST filtering rather than NEAREST_MIPMAP_NEAREST. > NEAREST only works for base level. Not in ESSL 3.00. The new textureLod function allows a particular level to be sampled explicitly. NEAREST filtering makes sense in this situation. > > I haven't > > been following all the bugs you've run into, but it seems to me that setting > the > > base level of the source texture should be well supported, though I can > imagine > > that attaching such a texture to a framebuffer wouldn't be. On the other hand, > > FramebufferTexture2D's level argument should work to attach a particular level > > of a texture, shouldn't it? > I also think it should work. > > GLint level = 1; > GLTexture tex; > glBindTexture(GL_TEXTURE_2D, tex.get()); > glTexImage2D(GL_TEXTURE_2D, level, GL_RGBA, 8, 8, 0, GL_RGBA, GL_UNSIGNED_BYTE, > nullptr); > glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_NEAREST); > glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_NEAREST); > glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_BASE_LEVEL, level); > > glBindFramebuffer(GL_FRAMEBUFFER, mFramebuffer); > glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, > tex.get(), level); > > // Verify the framebuffer is complete. > ASSERT_GLENUM_EQ(GL_FRAMEBUFFER_COMPLETE, > glCheckFramebufferStatus(GL_FRAMEBUFFER)); > > Linux Mesa returns GL_FRAMEBUFFER_COMPLETE, while Linux NVIDIA returns > GL_FRAMEBUFFER_UNSUPPORTED in above test. > It’s expected that texture level is bound to fbo, level is base level, so fbo > should be complete. > > The only reason why Linux NVIDIA returned FRAMEBUFFER_UNSUPPORTED is it didn’t > support combinations of formats of the images attached to fbo if the image is > from a non-zero base level. Since spec allows it’s implement-dependent, so I am > not sure if it’s a driver bug. I see. It may be. Please note that setting the base level in this situation should not be necessary. Setting the base level of a texture only affects sampling of the texture, not its behavior when a level is attached to a framebuffer for rendering. Maybe there are bugs in drivers when a texture with a nonzero base level has one of its levels attached to an FBO. Could you try changing this sample to not set the base level, and see how that affects framebuffer completeness on Mesa and NVIDIA? Note that this shouldn't block landing this CL; I think it should be landed as is and that we should continue to iterate on it as quickly as possible. > > Also, the new textureLod GLSL function might be useful for sampling a > particular > > level. > I may consider this in future. Very good. > > This is all not that important -- I'm not sure why we were pursuing uploading > to > > levels > 0. Getting uploads to cube map faces working is much more important. > I will fix cube map next week. That's excellent. I think that will fix a lot of WebGL conformance failures on Android and allow many more tests to be re-enabled on the waterfall. Thanks for your continued work on this.
On 2017/01/14 01:07:36, qiankun wrote: > On 2017/01/14 00:56:11, Zhenyao Mo wrote: > > On 2017/01/14 00:31:18, qiankun wrote: > > > On 2017/01/13 19:57:49, Ken Russell wrote: > > > > > I think restricting source_level to 0 is OK, as the intended use cases > of > > > this > > > > > extension all have source_level == 0. > > > > > > > > Sounds fine to me too. > > > > > > > > I haven't thought through that comment fully. For the source texture, I > > would > > > > expect to use NEAREST filtering rather than NEAREST_MIPMAP_NEAREST. > > > NEAREST only works for base level. > > > > > > > > > > I haven't > > > > been following all the bugs you've run into, but it seems to me that > setting > > > the > > > > base level of the source texture should be well supported, though I can > > > imagine > > > > that attaching such a texture to a framebuffer wouldn't be. On the other > > hand, > > > > FramebufferTexture2D's level argument should work to attach a particular > > level > > > > of a texture, shouldn't it? > > > I also think it should work. > > > > > > GLint level = 1; > > > GLTexture tex; > > > glBindTexture(GL_TEXTURE_2D, tex.get()); > > > glTexImage2D(GL_TEXTURE_2D, level, GL_RGBA, 8, 8, 0, GL_RGBA, > > GL_UNSIGNED_BYTE, > > > nullptr); > > > glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_NEAREST); > > > glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_NEAREST); > > > glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_BASE_LEVEL, level); > > > > > > glBindFramebuffer(GL_FRAMEBUFFER, mFramebuffer); > > > glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, > > > tex.get(), level); > > > > > > // Verify the framebuffer is complete. > > > ASSERT_GLENUM_EQ(GL_FRAMEBUFFER_COMPLETE, > > > glCheckFramebufferStatus(GL_FRAMEBUFFER)); > > > > > > Linux Mesa returns GL_FRAMEBUFFER_COMPLETE, while Linux NVIDIA returns > > > GL_FRAMEBUFFER_UNSUPPORTED in above test. > > > It’s expected that texture level is bound to fbo, level is base level, so > fbo > > > should be complete. > > > > > > The only reason why Linux NVIDIA returned FRAMEBUFFER_UNSUPPORTED is it > didn’t > > > support combinations of formats of the images attached to fbo if the image > is > > > from a non-zero base level. Since spec allows it’s implement-dependent, so I > > am > > > not sure if it’s a driver bug. > > > > > > > > > > > Also, the new textureLod GLSL function might be useful for sampling a > > > particular > > > > level. > > > I may consider this in future. > > > > > > > This is all not that important -- I'm not sure why we were pursuing > > uploading > > > to > > > > levels > 0. Getting uploads to cube map faces working is much more > > important. > > > I will fix cube map next week. > > > > If we draw to framebuffer with texture |level| > 0, we don't have to set the > > base level of that texture to |level|, right? In that case, we can still do > > DIRECT_DRAW. > > > > If I am correct, can you modify the code to reflect that? > > Not right. We should set base level of that texture to |level| to make > framebuffer complete since spec requires draw buffer be complete when drawing. I took another look at ES 3.0.4 spec section 4.4.4 Framebuffer Completeness, there is nothing related with the base level, only the image that's attached to the fbo. If I miss something, please point out the spec text to me.
On 2017/01/17 18:16:57, Zhenyao Mo wrote: > On 2017/01/14 01:07:36, qiankun wrote: > > On 2017/01/14 00:56:11, Zhenyao Mo wrote: > > > On 2017/01/14 00:31:18, qiankun wrote: > > > > On 2017/01/13 19:57:49, Ken Russell wrote: > > > > > > I think restricting source_level to 0 is OK, as the intended use cases > > of > > > > this > > > > > > extension all have source_level == 0. > > > > > > > > > > Sounds fine to me too. > > > > > > > > > > I haven't thought through that comment fully. For the source texture, I > > > would > > > > > expect to use NEAREST filtering rather than NEAREST_MIPMAP_NEAREST. > > > > NEAREST only works for base level. > > > > > > > > > > > > > I haven't > > > > > been following all the bugs you've run into, but it seems to me that > > setting > > > > the > > > > > base level of the source texture should be well supported, though I can > > > > imagine > > > > > that attaching such a texture to a framebuffer wouldn't be. On the other > > > hand, > > > > > FramebufferTexture2D's level argument should work to attach a particular > > > level > > > > > of a texture, shouldn't it? > > > > I also think it should work. > > > > > > > > GLint level = 1; > > > > GLTexture tex; > > > > glBindTexture(GL_TEXTURE_2D, tex.get()); > > > > glTexImage2D(GL_TEXTURE_2D, level, GL_RGBA, 8, 8, 0, GL_RGBA, > > > GL_UNSIGNED_BYTE, > > > > nullptr); > > > > glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_NEAREST); > > > > glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_NEAREST); > > > > glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_BASE_LEVEL, level); > > > > > > > > glBindFramebuffer(GL_FRAMEBUFFER, mFramebuffer); > > > > glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, > GL_TEXTURE_2D, > > > > tex.get(), level); > > > > > > > > // Verify the framebuffer is complete. > > > > ASSERT_GLENUM_EQ(GL_FRAMEBUFFER_COMPLETE, > > > > glCheckFramebufferStatus(GL_FRAMEBUFFER)); > > > > > > > > Linux Mesa returns GL_FRAMEBUFFER_COMPLETE, while Linux NVIDIA returns > > > > GL_FRAMEBUFFER_UNSUPPORTED in above test. > > > > It’s expected that texture level is bound to fbo, level is base level, so > > fbo > > > > should be complete. > > > > > > > > The only reason why Linux NVIDIA returned FRAMEBUFFER_UNSUPPORTED is it > > didn’t > > > > support combinations of formats of the images attached to fbo if the image > > is > > > > from a non-zero base level. Since spec allows it’s implement-dependent, so > I > > > am > > > > not sure if it’s a driver bug. > > > > > > > > > > > > > > Also, the new textureLod GLSL function might be useful for sampling a > > > > particular > > > > > level. > > > > I may consider this in future. > > > > > > > > > This is all not that important -- I'm not sure why we were pursuing > > > uploading > > > > to > > > > > levels > 0. Getting uploads to cube map faces working is much more > > > important. > > > > I will fix cube map next week. > > > > > > If we draw to framebuffer with texture |level| > 0, we don't have to set the > > > base level of that texture to |level|, right? In that case, we can still do > > > DIRECT_DRAW. > > > > > > If I am correct, can you modify the code to reflect that? > > > > Not right. We should set base level of that texture to |level| to make > > framebuffer complete since spec requires draw buffer be complete when drawing. > > I took another look at ES 3.0.4 spec section 4.4.4 Framebuffer Completeness, > there is nothing related with the base level, only the image that's attached to > the fbo. > > If I miss something, please point out the spec text to me. Never mind, I found the text: "If the value of FRAMEBUFFER_ATTACHMENT_OBJECT_TYPE is TEXTURE and the value of FRAMEBUFFER_ATTACHMENT_OBJECT_NAME does not name an immutable-format texture and the value of FRAMEBUFFER_- ATTACHMENT_TEXTURE_LEVEL is not levelbase, then the texture must be mipmap complete, and if FRAMEBUFFER_ATTACHMENT_OBJECT_NAME names a cubemap texture, the texture must also be cube complete." This is rather unfortunate. In this case, the CL LGTM
On 2017/01/17 18:21:01, Zhenyao Mo wrote: > On 2017/01/17 18:16:57, Zhenyao Mo wrote: > > On 2017/01/14 01:07:36, qiankun wrote: > > > On 2017/01/14 00:56:11, Zhenyao Mo wrote: > > > > On 2017/01/14 00:31:18, qiankun wrote: > > > > > On 2017/01/13 19:57:49, Ken Russell wrote: > > > > > > > I think restricting source_level to 0 is OK, as the intended use > cases > > > of > > > > > this > > > > > > > extension all have source_level == 0. > > > > > > > > > > > > Sounds fine to me too. > > > > > > > > > > > > I haven't thought through that comment fully. For the source texture, > I > > > > would > > > > > > expect to use NEAREST filtering rather than NEAREST_MIPMAP_NEAREST. > > > > > NEAREST only works for base level. > > > > > > > > > > > > > > > > I haven't > > > > > > been following all the bugs you've run into, but it seems to me that > > > setting > > > > > the > > > > > > base level of the source texture should be well supported, though I > can > > > > > imagine > > > > > > that attaching such a texture to a framebuffer wouldn't be. On the > other > > > > hand, > > > > > > FramebufferTexture2D's level argument should work to attach a > particular > > > > level > > > > > > of a texture, shouldn't it? > > > > > I also think it should work. > > > > > > > > > > GLint level = 1; > > > > > GLTexture tex; > > > > > glBindTexture(GL_TEXTURE_2D, tex.get()); > > > > > glTexImage2D(GL_TEXTURE_2D, level, GL_RGBA, 8, 8, 0, GL_RGBA, > > > > GL_UNSIGNED_BYTE, > > > > > nullptr); > > > > > glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_NEAREST); > > > > > glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_NEAREST); > > > > > glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_BASE_LEVEL, level); > > > > > > > > > > glBindFramebuffer(GL_FRAMEBUFFER, mFramebuffer); > > > > > glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, > > GL_TEXTURE_2D, > > > > > tex.get(), level); > > > > > > > > > > // Verify the framebuffer is complete. > > > > > ASSERT_GLENUM_EQ(GL_FRAMEBUFFER_COMPLETE, > > > > > glCheckFramebufferStatus(GL_FRAMEBUFFER)); > > > > > > > > > > Linux Mesa returns GL_FRAMEBUFFER_COMPLETE, while Linux NVIDIA returns > > > > > GL_FRAMEBUFFER_UNSUPPORTED in above test. > > > > > It’s expected that texture level is bound to fbo, level is base level, > so > > > fbo > > > > > should be complete. > > > > > > > > > > The only reason why Linux NVIDIA returned FRAMEBUFFER_UNSUPPORTED is it > > > didn’t > > > > > support combinations of formats of the images attached to fbo if the > image > > > is > > > > > from a non-zero base level. Since spec allows it’s implement-dependent, > so > > I > > > > am > > > > > not sure if it’s a driver bug. > > > > > > > > > > > > > > > > > Also, the new textureLod GLSL function might be useful for sampling a > > > > > particular > > > > > > level. > > > > > I may consider this in future. > > > > > > > > > > > This is all not that important -- I'm not sure why we were pursuing > > > > uploading > > > > > to > > > > > > levels > 0. Getting uploads to cube map faces working is much more > > > > important. > > > > > I will fix cube map next week. > > > > > > > > If we draw to framebuffer with texture |level| > 0, we don't have to set > the > > > > base level of that texture to |level|, right? In that case, we can still > do > > > > DIRECT_DRAW. > > > > > > > > If I am correct, can you modify the code to reflect that? > > > > > > Not right. We should set base level of that texture to |level| to make > > > framebuffer complete since spec requires draw buffer be complete when > drawing. > > > > I took another look at ES 3.0.4 spec section 4.4.4 Framebuffer Completeness, > > there is nothing related with the base level, only the image that's attached > to > > the fbo. > > > > If I miss something, please point out the spec text to me. > > Never mind, I found the text: "If the value of > FRAMEBUFFER_ATTACHMENT_OBJECT_TYPE is TEXTURE > and the value of FRAMEBUFFER_ATTACHMENT_OBJECT_NAME does not > name an immutable-format texture and the value of FRAMEBUFFER_- > ATTACHMENT_TEXTURE_LEVEL is not levelbase, then the texture must > be mipmap complete, and if FRAMEBUFFER_ATTACHMENT_OBJECT_NAME > names a cubemap texture, the texture must also be cube complete." > > This is rather unfortunate. In this case, the CL LGTM Also, I am wondering if simply setting BASE_LEVEL causes the fbo to be incomplete, what about also set MAX_LEVEL to the same level and see if it makes fbo complete?
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...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
On 2017/01/17 18:21:52, Zhenyao Mo wrote: > On 2017/01/17 18:21:01, Zhenyao Mo wrote: > > On 2017/01/17 18:16:57, Zhenyao Mo wrote: > > > On 2017/01/14 01:07:36, qiankun wrote: > > > > On 2017/01/14 00:56:11, Zhenyao Mo wrote: > > > > > On 2017/01/14 00:31:18, qiankun wrote: > > > > > > On 2017/01/13 19:57:49, Ken Russell wrote: > > > > > > > > I think restricting source_level to 0 is OK, as the intended use > > cases > > > > of > > > > > > this > > > > > > > > extension all have source_level == 0. > > > > > > > > > > > > > > Sounds fine to me too. > > > > > > > > > > > > > > I haven't thought through that comment fully. For the source > texture, > > I > > > > > would > > > > > > > expect to use NEAREST filtering rather than NEAREST_MIPMAP_NEAREST. > > > > > > NEAREST only works for base level. > > > > > > > > > > > > > > > > > > > I haven't > > > > > > > been following all the bugs you've run into, but it seems to me that > > > > setting > > > > > > the > > > > > > > base level of the source texture should be well supported, though I > > can > > > > > > imagine > > > > > > > that attaching such a texture to a framebuffer wouldn't be. On the > > other > > > > > hand, > > > > > > > FramebufferTexture2D's level argument should work to attach a > > particular > > > > > level > > > > > > > of a texture, shouldn't it? > > > > > > I also think it should work. > > > > > > > > > > > > GLint level = 1; > > > > > > GLTexture tex; > > > > > > glBindTexture(GL_TEXTURE_2D, tex.get()); > > > > > > glTexImage2D(GL_TEXTURE_2D, level, GL_RGBA, 8, 8, 0, GL_RGBA, > > > > > GL_UNSIGNED_BYTE, > > > > > > nullptr); > > > > > > glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_NEAREST); > > > > > > glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_NEAREST); > > > > > > glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_BASE_LEVEL, level); > > > > > > > > > > > > glBindFramebuffer(GL_FRAMEBUFFER, mFramebuffer); > > > > > > glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, > > > GL_TEXTURE_2D, > > > > > > tex.get(), level); > > > > > > > > > > > > // Verify the framebuffer is complete. > > > > > > ASSERT_GLENUM_EQ(GL_FRAMEBUFFER_COMPLETE, > > > > > > glCheckFramebufferStatus(GL_FRAMEBUFFER)); > > > > > > > > > > > > Linux Mesa returns GL_FRAMEBUFFER_COMPLETE, while Linux NVIDIA returns > > > > > > GL_FRAMEBUFFER_UNSUPPORTED in above test. > > > > > > It’s expected that texture level is bound to fbo, level is base level, > > so > > > > fbo > > > > > > should be complete. > > > > > > > > > > > > The only reason why Linux NVIDIA returned FRAMEBUFFER_UNSUPPORTED is > it > > > > didn’t > > > > > > support combinations of formats of the images attached to fbo if the > > image > > > > is > > > > > > from a non-zero base level. Since spec allows it’s > implement-dependent, > > so > > > I > > > > > am > > > > > > not sure if it’s a driver bug. > > > > > > > > > > > > > > > > > > > > Also, the new textureLod GLSL function might be useful for sampling > a > > > > > > particular > > > > > > > level. > > > > > > I may consider this in future. > > > > > > > > > > > > > This is all not that important -- I'm not sure why we were pursuing > > > > > uploading > > > > > > to > > > > > > > levels > 0. Getting uploads to cube map faces working is much more > > > > > important. > > > > > > I will fix cube map next week. > > > > > > > > > > If we draw to framebuffer with texture |level| > 0, we don't have to set > > the > > > > > base level of that texture to |level|, right? In that case, we can still > > do > > > > > DIRECT_DRAW. > > > > > > > > > > If I am correct, can you modify the code to reflect that? > > > > > > > > Not right. We should set base level of that texture to |level| to make > > > > framebuffer complete since spec requires draw buffer be complete when > > drawing. > > > > > > I took another look at ES 3.0.4 spec section 4.4.4 Framebuffer Completeness, > > > there is nothing related with the base level, only the image that's attached > > to > > > the fbo. > > > > > > If I miss something, please point out the spec text to me. > > > > Never mind, I found the text: "If the value of > > FRAMEBUFFER_ATTACHMENT_OBJECT_TYPE is TEXTURE > > and the value of FRAMEBUFFER_ATTACHMENT_OBJECT_NAME does not > > name an immutable-format texture and the value of FRAMEBUFFER_- > > ATTACHMENT_TEXTURE_LEVEL is not levelbase, then the texture must > > be mipmap complete, and if FRAMEBUFFER_ATTACHMENT_OBJECT_NAME > > names a cubemap texture, the texture must also be cube complete." > > > > This is rather unfortunate. In this case, the CL LGTM > > Also, I am wondering if simply setting BASE_LEVEL causes the fbo to be > incomplete, what about also set MAX_LEVEL to the same level and see if it makes > fbo complete? Setting MAX_LEVEL doesn't change anything. Filling level 0 will make fbo complete on Intel Mac OSX. Fbo incomplete was only seen in NVIDIA Linux with ANGLE and Intel Mac OSX at my side. See, https://bugs.chromium.org/p/chromium/issues/detail?id=678526. Intel Mesa, NVIDIA linux desktop GL and egl, AMD Mac OSX works well on test added in https://github.com/KhronosGroup/WebGL/pull/2236.
The CQ bit was checked by qiankun.miao@intel.com
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": 100001, "attempt_start_ts": 1484720746824980,
"parent_rev": "837eb230d7f98f155c3181c27054322431aff40e", "commit_rev":
"cbaeb8aa76be268c5179deb56e67a765303e34cc"}
Message was sent while issue was closed.
Description was changed from ========== Support level > 0 for CopyTextureCHROMIUM extension Source level > 0 is not supported in ES 2.0 due to glFramebufferTexture2D doesn't support level > 0 in ES 2.0. Run into DRAW_AND_COPY path for dest level > 0 in ES 2.0, i.e. draw the source texture to a fbo attaching level 0 of an intermediate texture, then use glCopyTexImage2D to copy intermediate texture to dest texture. For ES 3.0, we can set base level of source and dest texture to the specifed level respectively, then do CopyTextureCHROMIUM as before, restore base level to previous value at the end of CopyTextureCHROMIUM. But we fall back to DRAW_AND_COPY path due to driver bug of base level on some scenarios. 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 level > 0 for CopyTextureCHROMIUM extension Source level > 0 is not supported in ES 2.0 due to glFramebufferTexture2D doesn't support level > 0 in ES 2.0. Run into DRAW_AND_COPY path for dest level > 0 in ES 2.0, i.e. draw the source texture to a fbo attaching level 0 of an intermediate texture, then use glCopyTexImage2D to copy intermediate texture to dest texture. For ES 3.0, we can set base level of source and dest texture to the specifed level respectively, then do CopyTextureCHROMIUM as before, restore base level to previous value at the end of CopyTextureCHROMIUM. But we fall back to DRAW_AND_COPY path due to driver bug of base level on some scenarios. 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/2623863002 Cr-Commit-Position: refs/heads/master@{#444295} Committed: https://chromium.googlesource.com/chromium/src/+/cbaeb8aa76be268c5179deb56e67... ==========
Message was sent while issue was closed.
Committed patchset #5 (id:100001) as https://chromium.googlesource.com/chromium/src/+/cbaeb8aa76be268c5179deb56e67... |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
