+kbr,vmiura for thoughts as well.

Cross context synchronization should be fine. afaik virtual context is not enabled on nvidia on android, and there aren't any complaints about synchronization there.

Are you sure virtual context is the issue, and not something else in your CL? I tried just removing that blacklist item on trunk, but otherwise followed the same steps. And I'm not seeing any tearing or out of order frames on a pixel.

The blacklist was probably overly broad though, and it's probably worth checking with newer qualcomm chips and see if virtual context is still required (and if it has any perf impact， positive or negative).

@boliu, thank you for following up on this. You're right, I've experimented more and it turns out that the tearing is only triggered when low priority mode is activated for the GL context (via EGL_CONTEXT_PRIORITY_LEVEL_IMG). If I disable that part of the patch, the non-virtualized offscreen context appears to work as expected with no visible tearing, both for plain WebGL and WebVR. Sorry about the red herring.

I'd still like to see if it's possible to narrow the blacklist to allow use of non-virtualized contexts, at least on Daydream-ready devices which presumably have recent drivers, considering that it doesn't seem to cause obvious issues after all.

pixel device represent the best possible android device, but android industry is super fragmented, and has a long tail.. need to test on more popular devices (eg samsung s series) before narrowing the blacklist

Could a first step be to very specifically narrow the blacklist, i.e. just exclude Pixel and/or Android N devices from it?

FWIW, I think it's potentially interesting that the issue shows up when using a low priority context for WebGL rendering. It's possible that this is a driver bug and red herring, but I think it may also be that the changed priority and resulting preemption is highlighting a pre-existing synchronization issue related to non-virtualized contexts that would otherwise be hard to reproduce.

 I'll look into making a more restricted version of the own-surface patch that's off by default and doesn't change priority, so that there's a baseline for further experiments.

> just exclude Pixel and/or Android N devices from it?

generally, we blacklist by OS (or possibly driver version) and chipset, but not any more granular than that. eg "adreno 5xx on M or above".

blacklist by device is way too granular and serves no purpose for an unpopular device. nexus/pixel devices is probably a rounding error in the entire android market

Just FYI, S6 Exynos/Mali produces the same issue, thus it is not a Qualcomm specific issue.

klausw@: Is this still revelvant?

This bug is still relevant, and is blocking an originally intended WebVR optimization. Plan was to enable drawing from WebGL directly to surfaces, but this broke non-WebVR GL content and needed to be rolled back.

I'm fairly confident that this is a Chrome bug and not an issue with the drivers. As far as I understand, the problem is that Chrome's mailbox synchronization via SyncToken is fundamentally just based on sequencing the emitted commands to the underlying actual GL contexts, but is not using any synchronization primitives that would actually enforce this across contexts.

This works fine when using virtualized contexts since there's just one underlying shared GL context, so the emitted commands from the virtual GL contexts all end up in the single GL output command stream in the correct sequence.

However, when not using virtualized contexts, the commands get sent to different underlying GL contexts. This happens in correct temporal sequence, but since the contexts are independent from each other it appears that the driver is technically free to reorder operations among them. This is especially apparent when I experimented with changing GL context priority  which made tearing due to bad synchronization very obvious.

As supporting evidence, MailboxManagerSync seems to have attempted to solve a similar issue by adding EGLFence objects, but this appears to not have had the desired effect, and virtualized contexts needed to be enabled to work around issues.

See also comment  http://crbug.com/510243#c22  by gold@nvidia.com, Oct 28 2015 :
> I found that performing a glFinish on LoseCurrent() resolves the corruption.  This should not be necessary if the app is performing proper synchronization (GL/EGL make no guarantees about serialization across context switches, even within the same thread).  Tegra4 has a channel per context whereas many implementations (including T124 and newer) use a channel per thread which implicitly forces serialization.  If the new code depends on per-thread serialization, that could explain why it fails only on Tegra4.  Of course its also possible we have a bug, but conceptually a fence is pretty simple to implement and I can't find a problem in our implementation.
>
> We're still looking at this as I'd rather not submit this workaround - it has performance consequences and may hide application errors.  I emailed boliu more detail offline as I'm hoping he can take a look from the webview side.
 
bajones@ thought it is it plausible that this effect wasn't visible on other GPUs because the drivers chose to respect the command order even if they technically don't have to, for example because this helped avoid issues due to erroneous assumptions in other software.

Does this sound plausible? If I'm misunderstanding things, I'd appreciate feedback.

https://cs.chromium.org/chromium/src/gpu/GLES2/extensions/CHROMIUM/CHROMIUM_sync_point.txt

hMore specifically, the issue may be that Chrome is emitting GL commands to multiple contexts from within a single thread which I assume is somewhat unusual application behavior, and as gold@nvidia.com mentioned many driver implementations use a per-thread channel which would keep commands synchronized implicitly.

I don't know if Qualcomm/Adreno or the S6 Exynos/Mali chipsets Artem mentioned in comment 7 use per-context channels. I think it would also seem likely that activating a non-default context priority would force it out of sharing a per-thread channel.

I am taking my comment about S6 Mali back, I just seen judder which periodically happen with or without virtualized context, assuming something's wrong with timings / vsyncs.
In fact, I've seen the issue Klaus is describing only on a Pixel phone and very old Samsung Note 4 (SD805 / Adreno 420). We use this way to optimize WebVR for Oculus Browser on S6, S7 Mali, S7 Adreno (SD820/A530), S8 Mali, S8 Adreno, Note 5 (Mali) and I haven't seen this issue on any of them. (the issue on note 4 could be seen here: https://youtu.be/tiQhhj1QlL8 )
One more detail: Mali (all - S6, S7 and S8) work fine with this optimization even with virtualized contexts (while, with non-virtualized contexts it produces artifacts, like lost textures).

And just FYI: Oculus is currently on M57 and we are planning to switch to M60 in nearest future. We are planning to use 'own offscreen surface' path as a default one, but keep the mailbox path as a fallback for phones with known issues (the good candidate for that is Note 4, for example). Is this a way what you may be interested in as well? I know that S8 is going (already?) to support Daydream and there is no reason to use mailbox way on it (IMO), since the 'own offscreen surface' works just fine.

@piman or @kbr, do you think my interpretation from comment #10 makes sense? TL;DR: Chrome makes assumptions about cross-context GL synchronization that are out of spec, and this was worked around by forcing virtualized GL contexts on Qualcomm.

If I'm misunderstanding things, and it's really the driver's fault, is there an open issue with Qualcomm to get this resolved?

sunnyps@ and I talked about this issue a couple of days ago. I don't know the MailboxManager code well (and am working on other bugs; can't study it in depth right now). Sunny thinks we can make the synchronization cheaper. Glancing at MailboxManagerSync it uses EGL fences because there's no guarantee that the contexts are in the same share group, and these apparently are costly. If it were changed to use GL fences and server-side waits (perhaps only when we know we're sending mailboxes between contexts in the same share group), and assuming the VR context and the other context are in the same EGL share group, that might make the correct synchronization cheap enough to use.

I don't think we'll get away without using EGLFences. Android doesn't provide flush ordering between contexts like Mac does so we'll still need proper synchronization.

However, we won't need EGLImage based sharing if the (native) contexts are in the same (native) share group. Also, we may not need the snapshotting functionality in MailboxManagerSync if we can define the right semantics. In webview's case we aren't in control of android UI context so we have to snapshot each mailbox at every InsertFenceSync.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/409774e95d22cffc51877e176045a82ef6f65907

commit 409774e95d22cffc51877e176045a82ef6f65907
Author: Klaus Weidner <klausw@chromium.org>
Date: Mon Dec 04 23:57:15 2017

Disable SupportOwnOffscreenSurface for WebGL

The WebVRExperimentalRendering flag is being repurposed for other
rendering optimizations, and activating the own offscreen surface was
causing glitches.

BUG= 761432 ,691102, 791755 

Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;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
Change-Id: I15ab8ee3c3370968ce5ea2ea6a170f6282858ad1
Reviewed-on: https://chromium-review.googlesource.com/802631
Commit-Queue: Klaus Weidner <klausw@chromium.org>
Reviewed-by: Brandon Jones <bajones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#521535}
[modify] https://crrev.com/409774e95d22cffc51877e176045a82ef6f65907/third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.cpp

Removing Blink>WebVR component and assigning to Blink>WebXR