Issue metadata
Sign in to add a comment
|
Antialiasing accidentally disabled on Android 7.1.1 |
||||||||||||||||||||||
Issue descriptionChrome Version: 57.0.2987.74 http://crbug.com/663811 introduced a blacklist entry that disabled antialiasing on Adreno 5xx GPUs on Android 7.1. It (correctly) added an OS check with the following format: "os": { "type": "android", "version": { "op": "=", "value": "7.1" } } As it turns out, however, our version comparison logic inadvertently also makes this match "7.1.1", a version of Android that has the bug fixed. This effectively causes a regression in Antialiasing between M56 and M57. The bug is that in the version compare (https://cs.chromium.org/chromium/src/gpu/config/gpu_control_list.cc?type=cs&l=193) we compare each of the sections of the version string only up to the last element of the shortest string, at which point we return that the versions match. (So 1.2 would match against 1.2.3.500.42.6) It's hard to tell if this was intentional or not, since there's valid reasons for wanting a prefix-only match, but it's definitely not giving the desired results in this case. We probably want to investigate adding a more explicit "*" wildcard here to intentionally invoke that behavior and allow for truly exact matching. In the meantime to allow M57 to work correctly the blacklist entry can be changed to "7.1.0", which will match "7.1" as intended but not "7.1.1"
,
Feb 25 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5dcf3589e69f5511ded5a0180da2ddb00db23234 commit 5dcf3589e69f5511ded5a0180da2ddb00db23234 Author: bajones <bajones@chromium.org> Date: Sat Feb 25 00:50:51 2017 Made Adreno multisampling blacklist entry more explicit This prevents the blacklist entry from inadvertently matching Android 7.1.1 due to a version comparison bug, which will be handled separately to keep this CL slim for easier cherry-picking to M57. BUG= 696059 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/2714973003 Cr-Commit-Position: refs/heads/master@{#453027} [modify] https://crrev.com/5dcf3589e69f5511ded5a0180da2ddb00db23234/gpu/config/gpu_driver_bug_list_json.cc
,
Feb 25 2017
,
Feb 25 2017
,
Feb 25 2017
Your change meets the bar and is auto-approved for M57. Please go ahead and merge the CL to branch 2987 manually. Please contact milestone owner if you have questions. Owners: amineer@(clank), cmasso@(bling), ketakid@(cros), govind@(desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Feb 25 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/143b76cd51f52702eb5424c53feadaef7f40d4ee commit 143b76cd51f52702eb5424c53feadaef7f40d4ee Author: Brandon Jones <bajones@chromium.org> Date: Sat Feb 25 17:51:23 2017 Made Adreno multisampling blacklist entry more explicit This prevents the blacklist entry from inadvertently matching Android 7.1.1 due to a version comparison bug, which will be handled separately to keep this CL slim for easier cherry-picking to M57. BUG= 696059 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/2714973003 Cr-Commit-Position: refs/heads/master@{#453027} (cherry picked from commit 5dcf3589e69f5511ded5a0180da2ddb00db23234) Review-Url: https://codereview.chromium.org/2712213003 . Cr-Commit-Position: refs/branch-heads/2987@{#687} Cr-Branched-From: ad51088c0e8776e8dcd963dbe752c4035ba6dab6-refs/heads/master@{#444943} [modify] https://crrev.com/143b76cd51f52702eb5424c53feadaef7f40d4ee/gpu/config/gpu_driver_bug_list_json.cc
,
Apr 5 2017
This should be fixed, please reopen if not
,
Apr 6 2017
I forgot to clarify - initially I think we left this open because we thought we needed to fix the version comparison logic, but I think we decided that the current logic (or something similar) was necessary? If we still need to update the version comparison logic, we should reopen or open a new issue.
,
Jun 20 2017
|
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by kbr@chromium.org
, Feb 25 2017Cc: ericrk@chromium.org