Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1465223010:
IsGL_REDSupportedOnFBOs: return true only if FBO is complete (Closed)
|
Created:
5 years ago by Julien Isorce Samsung Modified:
5 years ago CC:
chromium-reviews, piman+watch_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionIsGL_REDSupportedOnFBOs: return true only if FBO is complete
In general the framebuffer has to be complete to claim
supporting RED on FBO.
If textureId is invalid glFramebufferTexture2DEXT returns
GL_FRAMEBUFFER_INCOMPLETE_ATTACHMENT.
So before this patch, IsGL_REDSupportedOnFBOs would have
returned true.
This leads to green frames when playing video.
This CL does fix bug 545904 but it is an indirect consequence.
We still need a new gpu driver bug workaround because this
CL does not avoid the DCHECK(glGetError() == GL_NO_ERROR);
at the end of IsGL_REDSupportedOnFBOs. Indeed the error is
set from the glTexImage2D call and glFramebufferTexture2DEXT
does not reset this gl error.
BUG=545904
R=bajones@chromium.org, hendrikw@chromium.org, kbr@chromium.org, zmo@chromium.org
TEST= --use-gl=egl on Mesa drivers
Committed: https://crrev.com/78acd34a5a46c8398dbe1e1b1d3168551ba8b6ff
Cr-Commit-Position: refs/heads/master@{#362996}
Patch Set 1 #
Messages
Total messages: 22 (8 generated)
The CQ bit was checked by j.isorce@samsung.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1465223010/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1465223010/1
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: No L-G-T-M from a valid reviewer yet. Only full committers are accepted. Even if an L-G-T-M may have been provided, it was from a non-committer, _not_ a full super star committer. See http://www.chromium.org/getting-involved/become-a-committer Note that this has nothing to do with OWNERS files.
ping :) ?
On 2015/12/02 12:59:10, j.isorce wrote: > ping :) ? Change makes sense, LGTM, but needs an owner review.
zmo@ should review this. It looks OK to me.
LGTM
The CQ bit was checked by j.isorce@samsung.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1465223010/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1465223010/1
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_chromium_compile_dbg_32_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by zmo@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1465223010/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1465223010/1
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
On 2015/12/03 00:29:04, commit-bot: I haz the power wrote: > Try jobs failed on following builders: > mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED, > http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...) Here is the failure: BrowserActionButtonUiTest.AddExtensionWithMenuOpen (run #1): [ RUN ] BrowserActionButtonUiTest.AddExtensionWithMenuOpen [4637:1799:1202/144234:WARNING:vt_video_decode_accelerator.cc(189)] Failed to initialize VideoToolbox framework. Hardware accelerated video decoding will be disabled. BrowserTestBase signal handler received SIGTERM. Backtrace: (full backtrace http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...) CHROME_DEVEL_SANDBOX=out/Release/chrome_sandbox ./out/Release/interactive_ui_tests --allow-file-access --gtest_also_run_disabled_tests --gtest_filter=BrowserActionButtonUiTest.AddExtensionWithMenuOpen --single_process Note: Google Test filter = BrowserActionButtonUiTest.AddExtensionWithMenuOpen [==========] Running 0 tests from 0 test cases. [==========] 0 tests from 0 test cases ran. (0 ms total) [ PASSED ] 0 tests. So it seems I cannot test it on Linux. Also I see "Patch download failed" and Compilation failed without patch on Windows. So I'll re-run the commit bot without updating the CL.
The CQ bit was checked by j.isorce@samsung.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1465223010/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1465223010/1
Message was sent while issue was closed.
Committed patchset #1 (id:1)
Message was sent while issue was closed.
Description was changed from ========== IsGL_REDSupportedOnFBOs: return true only if FBO is complete In general the framebuffer has to be complete to claim supporting RED on FBO. If textureId is invalid glFramebufferTexture2DEXT returns GL_FRAMEBUFFER_INCOMPLETE_ATTACHMENT. So before this patch, IsGL_REDSupportedOnFBOs would have returned true. This leads to green frames when playing video. This CL does fix bug 545904 but it is an indirect consequence. We still need a new gpu driver bug workaround because this CL does not avoid the DCHECK(glGetError() == GL_NO_ERROR); at the end of IsGL_REDSupportedOnFBOs. Indeed the error is set from the glTexImage2D call and glFramebufferTexture2DEXT does not reset this gl error. BUG=545904 R=bajones@chromium.org, hendrikw@chromium.org, kbr@chromium.org, zmo@chromium.org TEST= --use-gl=egl on Mesa drivers ========== to ========== IsGL_REDSupportedOnFBOs: return true only if FBO is complete In general the framebuffer has to be complete to claim supporting RED on FBO. If textureId is invalid glFramebufferTexture2DEXT returns GL_FRAMEBUFFER_INCOMPLETE_ATTACHMENT. So before this patch, IsGL_REDSupportedOnFBOs would have returned true. This leads to green frames when playing video. This CL does fix bug 545904 but it is an indirect consequence. We still need a new gpu driver bug workaround because this CL does not avoid the DCHECK(glGetError() == GL_NO_ERROR); at the end of IsGL_REDSupportedOnFBOs. Indeed the error is set from the glTexImage2D call and glFramebufferTexture2DEXT does not reset this gl error. BUG=545904 R=bajones@chromium.org, hendrikw@chromium.org, kbr@chromium.org, zmo@chromium.org TEST= --use-gl=egl on Mesa drivers Committed: https://crrev.com/78acd34a5a46c8398dbe1e1b1d3168551ba8b6ff Cr-Commit-Position: refs/heads/master@{#362996} ==========
Message was sent while issue was closed.
Patchset 1 (id:??) landed as https://crrev.com/78acd34a5a46c8398dbe1e1b1d3168551ba8b6ff Cr-Commit-Position: refs/heads/master@{#362996} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
