Issue metadata
Sign in to add a comment
|
WASM Rendering broken
Reported by
cfakhrud...@zynga.com,
Mar 21 2018
|
||||||||||||||||||||||||
Issue descriptionUserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/66.0.3359.33 Safari/537.36 Steps to reproduce the problem: 1. Open https://skypebot.farm2.zynga.com/ 2. On Mac, on Google Chrome 66.0.3359.33 the page does not show a loader bar What is the expected behavior? On Chrome Stable 65 the page loads with a loader bar below What went wrong? Rendering is broken with WASM Did this work before? Yes Stable 65.0.3325.181 Does this work in other browsers? Yes Chrome version: 66.0.3359.33 Channel: beta OS Version: OS X 10.12.6 Flash Version: 29.0.0.113 This is issue is reproducible only on Mac and not on Windows.
,
Mar 21 2018
This is the behavior I see on Stable, Canary, and Chromium latest build: Stable: hangs with white screen which has some kind of green/white loading bar at the bottom. Canary and Chromium: black screen, then after a few seconds a blue screen with rendered clouds of some kind. I didn't test Beta, since it running it side-by-side with stable isn't a supported workflow. What exactly do you see with Beta on Mac?
,
Mar 21 2018
Ok, on Chromium Beta I see the blue screen with white clouds. Is this what you are seeing?
,
Mar 21 2018
Seeing the white screen and loading bar (green) at the bottom is the right behavior. The sky and clouds are actually a layer behind and they aren't visible. This behavior is correct on the stable version. On Beta, i can see only the background (ie the clouds and sky screen), the white screen and the loading bar has disappeared on Beta on Mac. I haven't checked on Canary and Chrome Dev build.
,
Mar 21 2018
@titzer, your observations are same as mine.
,
Mar 21 2018
This bug in the Beta version breaks our production application completely. Certain things dont render at all due to this.
,
Mar 21 2018
,
Mar 21 2018
I tried with --nowasm-jit-to-native on Chromium Dev66, the most likely culprit, and that has no effect, so I suspect that this is not a WASM bug but a rendering bug.
,
Mar 21 2018
,
Mar 22 2018
titzer@ My application is running on WASM and when i run the application on the Flash Player (instead of WASM), it seems to render correctly. Hence, i marked the bug thinking it might be related to WASM.
,
Mar 22 2018
c10: No worries. Thanks for the report. I hope we can get it worked out soon.
,
Mar 28 2018
Hi guys, any update on this? This issue still exists.
,
Mar 28 2018
CC'ing a few Blink>Loader owners for further triaging.
,
Mar 29 2018
I'm not sure if this is a loading problem. It would be great if someone having WASM knowledge can look into the issue.
,
Mar 29 2018
-mtrofin, +ahaas
,
Apr 3 2018
Getting a bisect here would be valuable. Adding ReleaseBlock-Stable, since there may be a issue w/ layout/ rendering (should revisit once the bisect is complete).
,
Apr 3 2018
,
Apr 3 2018
Rajshree, could you please triage and bisect.
,
Apr 4 2018
we tried bisecting using the public tool, got the below commit Downloading revision 535065... Received 78776576 of 78776576 bytes, 100.00% Bisecting range [530377 (good), 540271 (bad)], roughly 11 steps left. Trying revision 535065... Revision 535065 is [(g)ood/(b)ad/(r)etry/(u)nknown/(s)tdout/(q)uit]: b Downloading revision 532443... Bisecting range [530377 (good), 535065 (bad)], roughly 10 steps left. Trying revision 532443... Revision 532443 is [(g)ood/(b)ad/(r)etry/(u)nknown/(s)tdout/(q)uit]: g Downloading revision 534102... Bisecting range [532443 (good), 535065 (bad)], roughly 9 steps left. Trying revision 534102... Revision 534102 is [(g)ood/(b)ad/(r)etry/(u)nknown/(s)tdout/(q)uit]: g Downloading revision 534402... Bisecting range [534102 (good), 535065 (bad)], roughly 8 steps left. Trying revision 534402... Revision 534402 is [(g)ood/(b)ad/(r)etry/(u)nknown/(s)tdout/(q)uit]: g Downloading revision 534700... Bisecting range [534402 (good), 535065 (bad)], roughly 7 steps left. Trying revision 534700... Revision 534700 is [(g)ood/(b)ad/(r)etry/(u)nknown/(s)tdout/(q)uit]: g Downloading revision 534931... Bisecting range [534700 (good), 535065 (bad)], roughly 6 steps left. Trying revision 534931... Revision 534931 is [(g)ood/(b)ad/(r)etry/(u)nknown/(s)tdout/(q)uit]: b Downloading revision 534828... Bisecting range [534700 (good), 534931 (bad)], roughly 5 steps left. Trying revision 534828... Revision 534828 is [(g)ood/(b)ad/(r)etry/(u)nknown/(s)tdout/(q)uit]: g Downloading revision 534884... Bisecting range [534828 (good), 534931 (bad)], roughly 4 steps left. Trying revision 534884... Revision 534884 is [(g)ood/(b)ad/(r)etry/(u)nknown/(s)tdout/(q)uit]: b Downloading revision 534855... Bisecting range [534828 (good), 534884 (bad)], roughly 3 steps left. Trying revision 534855... Revision 534855 is [(g)ood/(b)ad/(r)etry/(u)nknown/(s)tdout/(q)uit]: g Downloading revision 534864... Bisecting range [534855 (good), 534884 (bad)], roughly 2 steps left. Trying revision 534864... Revision 534864 is [(g)ood/(b)ad/(r)etry/(u)nknown/(s)tdout/(q)uit]: g Downloading revision 534871... Bisecting range [534864 (good), 534884 (bad)], roughly 2 steps left. Trying revision 534871... Revision 534871 is [(g)ood/(b)ad/(r)etry/(u)nknown/(s)tdout/(q)uit]: g You are probably looking for a change made after 534871 (known good), but no later than 534884 (first known bad). CHANGELOG URL: https://chromium.googlesource.com/chromium/src/+log/7adfb7752591c843ef6c90d15b9449d698d8cdea..a02af9067611c9dbe93ace2826eef7e9cd0adb05
,
Apr 4 2018
^^^^^ Loading bar visible is GOOD Blue clouds visible is BAD also note that the stuff is visible once the Preloader.swf gets downloaded(which takes sometime ...roughly 10-15 seconds...you can monitor on devtools network console)
,
Apr 4 2018
There are some Chromium ANGLE related commits in the above change log....could they be causing the issue ? https://chromium.googlesource.com/chromium/src/+/3f5caf560324799038d66714118322ef1b238036 CHANGELOG URL: https://chromium.googlesource.com/chromium/src/+log/7adfb7752591c843ef6c90d15b9449d698d8cdea..a02af9067611c9dbe93ace2826eef7e9cd0adb05
,
Apr 4 2018
Able to reproduce the issue on mac 10.13.3 using chrome reported version #66.0.3359.33 and latest canary #67.0.3387.0. Bisect Information: ===================== Good build: 66.0.3341.0 Bad Build : 66.0.3342.0 Change Log URL: https://chromium.googlesource.com/chromium/src/+log/6df8a50cc64b41c7018b0415561e2ef9eba085c0..3f5caf560324799038d66714118322ef1b238036 Skia change log: https://chromium.googlesource.com/angle/angle.git/+log/bfeed4ddfcd6..57dd97aaaf09 From the above change log suspecting below change Change-Id: I49cbd77328c01f945d66f92e6ec4ba7c552abeff Reviewed-on: https://chromium-review.googlesource.com/904684 jmadill@ - Could you please check whether this is caused with respect to your change, if not please help us in assigning it to the right owner. Note: Adding stable blocker for M-66 as it seems to be a recent regression. Please feel free to remove the same if not appropriate. Thanks...!!
,
Apr 4 2018
Correction: Angle CL in place of skia CL Change Log URL: https://chromium.googlesource.com/chromium/src/+log/6df8a50cc64b41c7018b0415561e2ef9eba085c0..3f5caf560324799038d66714118322ef1b238036 Angle change log: https://chromium.googlesource.com/angle/angle.git/+log/bfeed4ddfcd6..57dd97aaaf09
,
Apr 4 2018
Change almost certainly couldn't be implicated, it only changes functions in the Vulkan back-end that isn't shipped. The bisect was probably done incorrectly.
,
Apr 4 2018
I have bisected again and got the same CHANGE LOG (which is different from krajshree) So the bisect was indeed wrong Can you please use my results 05003A-SNAVELKAR:CHROME-BISECT snavelkar$ python bisect-builds.py -a mac64 -g 534871 -b 534950 --use-local-cache Downloading list of known revisions... Loaded revisions 15734-547993 from /Users/snavelkar/Documents/CHROME-BISECT/.bisect-builds-cache.json Downloading revision 534921... Received 78790868 of 78790868 bytes, 100.00% Bisecting range [534871 (good), 534949 (bad)], roughly 4 steps left. Trying revision 534921... Revision 534921 is [(g)ood/(b)ad/(r)etry/(u)nknown/(s)tdout/(q)uit]: b Downloading revision 534900... Bisecting range [534871 (good), 534921 (bad)], roughly 3 steps left. Trying revision 534900... Revision 534900 is [(g)ood/(b)ad/(r)etry/(u)nknown/(s)tdout/(q)uit]: b Downloading revision 534884... Bisecting range [534871 (good), 534900 (bad)], roughly 2 steps left. Trying revision 534884... Revision 534884 is [(g)ood/(b)ad/(r)etry/(u)nknown/(s)tdout/(q)uit]: b You are probably looking for a change made after 534871 (known good), but no later than 534884 (first known bad). python bisect-builds.py -a mac64 -g 534864 -b 534888 --use-local-cache Downloading list of known revisions... Loaded revisions 15734-547993 from /Users/snavelkar/Documents/CHROME-BISECT/.bisect-builds-cache.json Downloading revision 534871... Received 78793974 of 78793974 bytes, 100.00% Bisecting range [534864 (good), 534884 (bad)], roughly 2 steps left. Trying revision 534871... Revision 534871 is [(g)ood/(b)ad/(r)etry/(u)nknown/(s)tdout/(q)uit]: g You are probably looking for a change made after 534871 (known good), but no later than 534884 (first known bad). CHANGELOG URL: https://chromium.googlesource.com/chromium/src/+log/7adfb7752591c843ef6c90d15b9449d698d8cdea..a02af9067611c9dbe93ace2826eef7e9cd0adb05 ^^^^^^This is the same CHANGE LOG url I had got earlier as well It has an ANGLE Chromium change at https://chromium.googlesource.com/chromium/src/+/3f5caf560324799038d66714118322ef1b238036
,
Apr 4 2018
@Ken - Could this be related to the Angle roll (not necessarily Jamie's CL)? If it was a generalized rendering issue I'd expect it to be present on Windows too. @Sarvesh - Thank you for the bisect. To confirm, this issue is only apparent on MacOS?
,
Apr 4 2018
Seems unlikely but could be for example https://chromium.googlesource.com/angle/angle.git/+/fbb1c792150a8d304f20b84a4bf002599be7d97f , which changed the shader translator, which is used on all platforms.
,
Apr 5 2018
We did check on Windows 10, the issue is not reproducible there
,
Apr 5 2018
As an additional data point: all Chrome versions, including M65, fail on Windows when I try them out on BrowserStack (the clouds are always shown instead of the progress bar), but I get the reported results (i.e. all versions work fine) when I try things out on my Windows 10 laptop (I've tried stable, beta and canary). The difference's that BrowserStack uses SwiftShader with no hardware acceleration features whatsoever. I'm able to reproduce the issue at will on Windows and Linux if I run chrome with --use-gl=swiftshader.
,
Apr 5 2018
Kai, could you please confirm the bisect and see whether reverting https://chromium.googlesource.com/angle/angle.git/+/fbb1c792150a8d304f20b84a4bf002599be7d97f locally fixes the problem? Thanks.
,
Apr 5 2018
You probably need to roll back ANGLE completely to be able to test whether the ImmutableString patch is at fault - there's heaps of code built on top of it now. I doubt that it's the root cause of the issue though.
,
Apr 5 2018
Re-ran the per-revision bisect script considering good and bad builds as in comment #22. Good behavior is a progressive green bar seen at the bottom of page when loading. Bad behavior is a blue background with white clouds with no progressive green bar. Again after running the per-revision bisect script for two times got the same Angle CL as in comment #22 i.e https://chromium.googlesource.com/angle/angle.git/+log/bfeed4ddfcd6..57dd97aaaf09. Hence, removing Needs-Bisect label. Thanks...!!
,
Apr 5 2018
I noticed that this patch in the bisect result may have actually introduced a bug: https://chromium.googlesource.com/angle/angle.git/+/ca5c105982c0423a80fd1d2f8b73f633ba34f811%5E%21/#F0 According to http://en.cppreference.com/w/cpp/string/multibyte/mbsrtowcs , mbsrtowcs returns static_cast<std::size_t>(-1) on error. The ANGLE code changed in the patch on the other hand seems to treat 0 as an error value. The utility function is only used on D3D in current ANGLE though, so unless the function was used more widely before, this may not be behind the regression.
,
Apr 5 2018
,
Apr 5 2018
Investigating.
,
Apr 5 2018
My per-revision ANGLE bisect does in fact point to https://chromium.googlesource.com/angle/angle.git/+/fbb1c792150a8d304f20b84a4bf002599be7d97f specifically. I don't have any ideas so far as to what the specific issue is. Tried --disable-webgl-image-chromium and --disable-gpu-compositing but neither works around the issue.
,
Apr 6 2018
Wow, mind blown. We could rebuild Chromium to dump all of the translated shaders and see what changed before and after that patch.
,
Apr 6 2018
I looked over the patch, paying particular attention to some of the trickier parts like the changes in glsang.l and glslang.y, and could only find one issue: Instead of writing hex values >= 10 using letters a to f, the character codes were accidentally offset by 10, so it was using letters k to p. This does result in different shader output. I don't know any reason why this would cause any content to misbehave, but I'll fix it either way. Here's a patch: https://chromium-review.googlesource.com/c/angle/angle/+/999480 I don't have a Mac at hand to test this, so Kai could you continue helping with this one? Agree with kbr's suggested debugging approach if the first patch doesn't do the trick.
,
Apr 7 2018
The following revision refers to this bug: https://chromium.googlesource.com/angle/angle/+/5ae64c94dbf1766d0c9f233942dcfb38c83229ac commit 5ae64c94dbf1766d0c9f233942dcfb38c83229ac Author: Olli Etuaho <oetuaho@nvidia.com> Date: Sat Apr 07 08:48:15 2018 Fix writing hex values in ImmutableStringBuilder The old code was accidentally using letters offset by 10 when writing out hex values >= 10. Now the letters a to f are used as they should. This is one issue that changed shader output when ImmutableString was introduced, so it is a potential cause for a regression detailed in bug 824062 , though this has not been verified. BUG= chromium:824062 TEST=angle_unittests Change-Id: Idb871dffba32a3ab20df0fe17b4b1a98ec00b7fa Reviewed-on: https://chromium-review.googlesource.com/999480 Commit-Queue: Olli Etuaho <oetuaho@nvidia.com> Reviewed-by: Jamie Madill <jmadill@chromium.org> [modify] https://crrev.com/5ae64c94dbf1766d0c9f233942dcfb38c83229ac/src/compiler/translator/ImmutableStringBuilder.h [modify] https://crrev.com/5ae64c94dbf1766d0c9f233942dcfb38c83229ac/src/tests/angle_unittests.gypi [add] https://crrev.com/5ae64c94dbf1766d0c9f233942dcfb38c83229ac/src/tests/compiler_tests/ImmutableString_test.cpp
,
Apr 7 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/37fb7006c59877f6f5bb0772af7385b1999691c0 commit 37fb7006c59877f6f5bb0772af7385b1999691c0 Author: angle-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com <angle-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com> Date: Sat Apr 07 11:33:09 2018 Roll src/third_party/angle/ 8957e8328..5ae64c94d (2 commits) https://chromium.googlesource.com/angle/angle.git/+log/8957e832818b..5ae64c94dbf1 $ git log 8957e8328..5ae64c94d --date=short --no-merges --format='%ad %ae %s' 2018-04-06 oetuaho Fix writing hex values in ImmutableStringBuilder 2018-04-05 lfy GLES1: gl(Push|Pop)Matrix Created with: roll-dep src/third_party/angle BUG= chromium:824062 The AutoRoll server is located here: https://angle-chromium-roll.skia.org Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md If the roll is causing failures, please contact the current sheriff, who should be CC'd on the roll, and stop the roller if necessary. CQ_INCLUDE_TRYBOTS=luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel TBR=fjhenigman@chromium.org Change-Id: Ia974e589af897c773f9b94355b80d48287e260d1 Reviewed-on: https://chromium-review.googlesource.com/1000554 Reviewed-by: angle-chromium-autoroll <angle-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com> Commit-Queue: angle-chromium-autoroll <angle-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com> Cr-Commit-Position: refs/heads/master@{#549056} [modify] https://crrev.com/37fb7006c59877f6f5bb0772af7385b1999691c0/DEPS
,
Apr 9 2018
Kai, could you check if the patch fixing the hex output affected the issue on Mac or not? Can you also try with --disable-gpu-shader-disk-cache cmd line option?
,
Apr 9 2018
The Canary builds unfortunately didn't push this weekend. We'll need a release >= 67.0.3392.0 to confirm.
,
Apr 9 2018
Without the fix, with --disable-gpu-shader-disk-cache, I still see the bug. Updating my build to include the patch, will report back.
,
Apr 9 2018
Confirming fixed by your patch (#39), thanks Olli!
Though we are close to the M66 stable cut, I'm going to request merge to 66. The impact of this regression (introduced in 66) could be large. (Keeping myself assigned so I can take care of the merge; will reassign it when I close the bug.)
Reviewers: This change (#39) is very small and safe. In fact, this is the full change (excluding new tests):
- char digitChar = digit < 10 ? digit + '0' : digit + 'a';
+ char digitChar = (digit < 10) ? (digit + '0') : (digit + ('a' - 10));
Unfortunately as pointed out by laforge@, it has not made it to Canary yet.
,
Apr 9 2018
This bug requires manual review: We are only 7 days from stable. Please contact the milestone owner if you have questions. Owners: cmasso@(Android), cmasso@(iOS), josafat@(ChromeOS), abdulsyed@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Apr 9 2018
Reminder: Please note that M66 Stable is only 7 days away. This bug has been marked as ReleaseBlock Stable for M66. So please take a look and appropriately address this bug.
,
Apr 10 2018
I'm a bit surprised that the small difference in output shader variable naming could cause this type of an issue - are you aware of any code where only using the characters 0 to f is required, or which might throw a tantrum whenever shader output changes? We ruled out the Chromium disk shader cache, but is there some other caching mechanism that might be Mac-specific? Thanks for looking into still getting the change into M66, I agree it's very low risk.
,
Apr 10 2018
I checked on Chrome 67.0.3393.0, and looks like the issue is fixed
,
Apr 10 2018
Approving merge to M66. Branch:3359
,
Apr 10 2018
The following revision refers to this bug: https://chromium.googlesource.com/angle/angle/+/22c768fbda545bbba2de6a0728b545bfa7e2ea94 commit 22c768fbda545bbba2de6a0728b545bfa7e2ea94 Author: Olli Etuaho <oetuaho@nvidia.com> Date: Tue Apr 10 19:29:53 2018 Fix writing hex values in ImmutableStringBuilder The old code was accidentally using letters offset by 10 when writing out hex values >= 10. Now the letters a to f are used as they should. This is one issue that changed shader output when ImmutableString was introduced, so it is a potential cause for a regression detailed in bug 824062 , though this has not been verified. BUG= chromium:824062 TEST=angle_unittests Change-Id: Idb871dffba32a3ab20df0fe17b4b1a98ec00b7fa Reviewed-on: https://chromium-review.googlesource.com/999480 Commit-Queue: Olli Etuaho <oetuaho@nvidia.com> Reviewed-by: Jamie Madill <jmadill@chromium.org> (cherry picked from commit 5ae64c94dbf1766d0c9f233942dcfb38c83229ac) Reviewed-on: https://chromium-review.googlesource.com/1005800 Reviewed-by: Kai Ninomiya <kainino@chromium.org> [modify] https://crrev.com/22c768fbda545bbba2de6a0728b545bfa7e2ea94/src/compiler/translator/ImmutableStringBuilder.h [modify] https://crrev.com/22c768fbda545bbba2de6a0728b545bfa7e2ea94/src/tests/angle_unittests.gypi [add] https://crrev.com/22c768fbda545bbba2de6a0728b545bfa7e2ea94/src/tests/compiler_tests/ImmutableString_test.cpp
,
Apr 10 2018
|
|||||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||||
Comment 1 by raphael....@intel.com
, Mar 21 2018