Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2521803002:
Re-enable GPU raster on AMD (Closed)
|
Created:
4 years, 1 month ago by ericrk Modified:
4 years ago Reviewers:
Ken Russell (switch to Gerrit) CC:
chromium-reviews, piman+watch_chromium.org, Zhenyao Mo, Kai Ninomiya, Jamie Madill, Geoff Lang Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionRe-enable GPU raster on AMD
AMD drivers do not correctly handle the pattern of DX calls generated
by ANGLE for glTexStorage2DExt. We've worked around the known
problematic case in crrev.com/2507963003, so this change removes the
GPU raster blacklist.
It's possible that this issue is still hittable from Skia, and likely
from WebGL2, so it makes sense to try to work around this in ANGLE or
to blacklist glTexStorage2DExt.
Unfortunately, the ANGLE code in this area is quite complex, and a
workaround is a ways out. Additionally, there's no way to disable
glTexStorage2DExt without losing all of WebGL2, which seems non-ideal.
BUG=660897
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/2f7ca486c59879ba9f1396390b5a99c5a1356489
Cr-Commit-Position: refs/heads/master@{#434824}
Patch Set 1 #
Total comments: 2
Patch Set 2 : update versoin # #Messages
Total messages: 16 (9 generated)
Description was changed from ========== Re-enable GPU raster on AMD AMD drivers do not correctly handle the pattern of DX calls generated by ANGLE for glTexStorage2DExt. We've worked around the known problematic case in crrev.com/2507963003, so this change removes the GPU raster blacklist. It's possible that this issue is hittable from Skia, and likely from WebGL2, so it makes sense to try to work around this in ANGLE or to blacklist glTexStorage2DExt. Unfortunately, the ANGLE code in this area is quite complex, and a workaround is a ways out. Additionally, there's no way to disable glTexStorage2DExt without losing all of WebGL2, which seems non-ideal. Given this, we should continue to keep an eye out for any WebGL2 or Skia issues on AMD hardware. BUG=660897 ========== to ========== Re-enable GPU raster on AMD AMD drivers do not correctly handle the pattern of DX calls generated by ANGLE for glTexStorage2DExt. We've worked around the known problematic case in crrev.com/2507963003, so this change removes the GPU raster blacklist. It's possible that this issue is hittable from Skia, and likely from WebGL2, so it makes sense to try to work around this in ANGLE or to blacklist glTexStorage2DExt. Unfortunately, the ANGLE code in this area is quite complex, and a workaround is a ways out. Additionally, there's no way to disable glTexStorage2DExt without losing all of WebGL2, which seems non-ideal. Given this, we should continue to keep an eye out for any WebGL2 or Skia issues on AMD hardware. BUG=660897 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 ==========
Description was changed from ========== Re-enable GPU raster on AMD AMD drivers do not correctly handle the pattern of DX calls generated by ANGLE for glTexStorage2DExt. We've worked around the known problematic case in crrev.com/2507963003, so this change removes the GPU raster blacklist. It's possible that this issue is hittable from Skia, and likely from WebGL2, so it makes sense to try to work around this in ANGLE or to blacklist glTexStorage2DExt. Unfortunately, the ANGLE code in this area is quite complex, and a workaround is a ways out. Additionally, there's no way to disable glTexStorage2DExt without losing all of WebGL2, which seems non-ideal. Given this, we should continue to keep an eye out for any WebGL2 or Skia issues on AMD hardware. BUG=660897 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 ========== Re-enable GPU raster on AMD AMD drivers do not correctly handle the pattern of DX calls generated by ANGLE for glTexStorage2DExt. We've worked around the known problematic case in crrev.com/2507963003, so this change removes the GPU raster blacklist. It's possible that this issue is still hittable from Skia, and likely from WebGL2, so it makes sense to try to work around this in ANGLE or to blacklist glTexStorage2DExt. Unfortunately, the ANGLE code in this area is quite complex, and a workaround is a ways out. Additionally, there's no way to disable glTexStorage2DExt without losing all of WebGL2, which seems non-ideal. Given this, we should continue to keep an eye out for any WebGL2 or Skia issues on AMD hardware. BUG=660897 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 ==========
Description was changed from ========== Re-enable GPU raster on AMD AMD drivers do not correctly handle the pattern of DX calls generated by ANGLE for glTexStorage2DExt. We've worked around the known problematic case in crrev.com/2507963003, so this change removes the GPU raster blacklist. It's possible that this issue is still hittable from Skia, and likely from WebGL2, so it makes sense to try to work around this in ANGLE or to blacklist glTexStorage2DExt. Unfortunately, the ANGLE code in this area is quite complex, and a workaround is a ways out. Additionally, there's no way to disable glTexStorage2DExt without losing all of WebGL2, which seems non-ideal. Given this, we should continue to keep an eye out for any WebGL2 or Skia issues on AMD hardware. BUG=660897 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 ========== Re-enable GPU raster on AMD AMD drivers do not correctly handle the pattern of DX calls generated by ANGLE for glTexStorage2DExt. We've worked around the known problematic case in crrev.com/2507963003, so this change removes the GPU raster blacklist. It's possible that this issue is still hittable from Skia, and likely from WebGL2, so it makes sense to try to work around this in ANGLE or to blacklist glTexStorage2DExt. Unfortunately, the ANGLE code in this area is quite complex, and a workaround is a ways out. Additionally, there's no way to disable glTexStorage2DExt without losing all of WebGL2, which seems non-ideal. BUG=660897 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 ==========
ericrk@chromium.org changed reviewers: + kbr@chromium.org
Hey Ken, does the explanation in the issue description make sense? It seemed too aggressive to blacklist TexStorage2D (and thereby WebGL2) in order to work around an unlikely issue. If we wanted to avoid this and still handle potential Skia uses, we could be a little more strict by adding the ability to blacklist TexStorage2D on non-webgl contexts, but not sure it's worth the effort. WDYT?
On 2016/11/21 23:58:44, ericrk wrote: > Hey Ken, does the explanation in the issue description make sense? > > It seemed too aggressive to blacklist TexStorage2D (and thereby WebGL2) in order > to work around an unlikely issue. If we wanted to avoid this and still handle > potential Skia uses, we could be a little more strict by adding the ability to > blacklist TexStorage2D on non-webgl contexts, but not sure it's worth the > effort. WDYT? Re-enabling GPU raster on these configurations LGTM. Please see the comment about updating the blacklist version. Without knowing more about the sequence of calls which provoke the bug, I can't advise whether WebGL 2.0 should be blacklisted on this configuration -- it's a tradeoff between allowing mysterious application bugs, and having end users complain that WebGL 2.0 doesn't work on their setup. Can you provide more details on http://crbug.com/660897 ? We should write a WebGL 2.0 conformance test exposing the issue so that it's known, tracked, and is less likely to regress in drivers once fixed. Could you find the time to contribute one to sdk/tests/conformance2/textures/misc/ in https://github.com/KhronosGroup/WebGL ? https://codereview.chromium.org/2521803002/diff/1/gpu/config/software_renderi... File gpu/config/software_rendering_list_json.cc (right): https://codereview.chromium.org/2521803002/diff/1/gpu/config/software_renderi... gpu/config/software_rendering_list_json.cc:21: "version": "12.03", Please increment the version number. This is needed for various bookkeeping reasons.
On 2016/11/22 01:08:37, Ken Russell OOO-till-Nov-28 wrote: > On 2016/11/21 23:58:44, ericrk wrote: > > Hey Ken, does the explanation in the issue description make sense? > > > > It seemed too aggressive to blacklist TexStorage2D (and thereby WebGL2) in > order > > to work around an unlikely issue. If we wanted to avoid this and still handle > > potential Skia uses, we could be a little more strict by adding the ability to > > blacklist TexStorage2D on non-webgl contexts, but not sure it's worth the > > effort. WDYT? > > Re-enabling GPU raster on these configurations LGTM. Please see the comment > about updating the blacklist version. > > Without knowing more about the sequence of calls which provoke the bug, I can't > advise whether WebGL 2.0 should be blacklisted on this configuration -- it's a > tradeoff between allowing mysterious application bugs, and having end users > complain that WebGL 2.0 doesn't work on their setup. Can you provide more > details on http://crbug.com/660897 ? > > We should write a WebGL 2.0 conformance test exposing the issue so that it's > known, tracked, and is less likely to regress in drivers once fixed. Could you > find the time to contribute one to sdk/tests/conformance2/textures/misc/ in > https://github.com/KhronosGroup/WebGL ? > > https://codereview.chromium.org/2521803002/diff/1/gpu/config/software_renderi... > File gpu/config/software_rendering_list_json.cc (right): > > https://codereview.chromium.org/2521803002/diff/1/gpu/config/software_renderi... > gpu/config/software_rendering_list_json.cc:21: "version": "12.03", > Please increment the version number. This is needed for various bookkeeping > reasons. Unfortunately, I'm not 100% sure of the exact pattern of calls which will trigger the bug (from the GL or DX side). I know that a certain pattern of calls is needed to hit the bug, but that isn't actually enough... there appears to be a certain amount of additional state needed for these calls to trigger the issue. I'll investigate more and see if I can isolate the calls and put together a conformance test as part of the work on crbug.com/660897.
https://codereview.chromium.org/2521803002/diff/1/gpu/config/software_renderi... File gpu/config/software_rendering_list_json.cc (right): https://codereview.chromium.org/2521803002/diff/1/gpu/config/software_renderi... gpu/config/software_rendering_list_json.cc:21: "version": "12.03", On 2016/11/22 01:08:36, Ken Russell OOO-till-Nov-28 wrote: > Please increment the version number. This is needed for various bookkeeping > reasons. Done.
The CQ bit was checked by ericrk@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from kbr@chromium.org Link to the patchset: https://codereview.chromium.org/2521803002/#ps20001 (title: "update versoin #")
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": 20001, "attempt_start_ts": 1480375217435380,
"parent_rev": "176b8c5428a47d40623d521740e4f414591591a7", "commit_rev":
"6eb07be50f97b8ca19473c9f3ad4d129b2e923d2"}
Message was sent while issue was closed.
Description was changed from ========== Re-enable GPU raster on AMD AMD drivers do not correctly handle the pattern of DX calls generated by ANGLE for glTexStorage2DExt. We've worked around the known problematic case in crrev.com/2507963003, so this change removes the GPU raster blacklist. It's possible that this issue is still hittable from Skia, and likely from WebGL2, so it makes sense to try to work around this in ANGLE or to blacklist glTexStorage2DExt. Unfortunately, the ANGLE code in this area is quite complex, and a workaround is a ways out. Additionally, there's no way to disable glTexStorage2DExt without losing all of WebGL2, which seems non-ideal. BUG=660897 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 ========== Re-enable GPU raster on AMD AMD drivers do not correctly handle the pattern of DX calls generated by ANGLE for glTexStorage2DExt. We've worked around the known problematic case in crrev.com/2507963003, so this change removes the GPU raster blacklist. It's possible that this issue is still hittable from Skia, and likely from WebGL2, so it makes sense to try to work around this in ANGLE or to blacklist glTexStorage2DExt. Unfortunately, the ANGLE code in this area is quite complex, and a workaround is a ways out. Additionally, there's no way to disable glTexStorage2DExt without losing all of WebGL2, which seems non-ideal. BUG=660897 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== Re-enable GPU raster on AMD AMD drivers do not correctly handle the pattern of DX calls generated by ANGLE for glTexStorage2DExt. We've worked around the known problematic case in crrev.com/2507963003, so this change removes the GPU raster blacklist. It's possible that this issue is still hittable from Skia, and likely from WebGL2, so it makes sense to try to work around this in ANGLE or to blacklist glTexStorage2DExt. Unfortunately, the ANGLE code in this area is quite complex, and a workaround is a ways out. Additionally, there's no way to disable glTexStorage2DExt without losing all of WebGL2, which seems non-ideal. BUG=660897 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 ========== Re-enable GPU raster on AMD AMD drivers do not correctly handle the pattern of DX calls generated by ANGLE for glTexStorage2DExt. We've worked around the known problematic case in crrev.com/2507963003, so this change removes the GPU raster blacklist. It's possible that this issue is still hittable from Skia, and likely from WebGL2, so it makes sense to try to work around this in ANGLE or to blacklist glTexStorage2DExt. Unfortunately, the ANGLE code in this area is quite complex, and a workaround is a ways out. Additionally, there's no way to disable glTexStorage2DExt without losing all of WebGL2, which seems non-ideal. BUG=660897 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/2f7ca486c59879ba9f1396390b5a99c5a1356489 Cr-Commit-Position: refs/heads/master@{#434824} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/2f7ca486c59879ba9f1396390b5a99c5a1356489 Cr-Commit-Position: refs/heads/master@{#434824} |
