Closed magreenblatt closed 4 months ago
Original comment by Alexander Guettler (Bitbucket: xforce, GitHub: xforce).
So a quick update on this, I have to say this turns out to be for me a lot more complicated than I initially expected.
It could be just I am just missing something but oh well.
The best explaination of the changes involved was this document [1] when using Viz Compositor, CompositorFrames are now longer sent from the render to the browser, but instead sent directly to the GPU process and then compositing happens directly onto the accelerated widget inside the render process.
This basically just completely prevents the current OSR approach, where we intercept the SubmitCompositorFrame calls. What I then tried was to add a new mojo IPC call that sents compositor frame metadata (also changed to get damage rect information) on every SubmitCompositorFrame call in the renderer, but right now I am facing issues of very high latency of frames being copied from the GPU to the browser process (talking sometimes multiple seconds here), I have yet to identify what is really going on there, but oh well.
This then kind of made me realize (or let's just say I am pretty sure) that we can't really do the current approach to shared texture support, but I will think about it, as all the output device stuff including the viz::Display no longer lives in the browser process but rather the render process.
If anyone has some input on this it would be greatly appreciated, but I will continue my endeavors regardless.
My current approach can be seen here [2]
[1] https://docs.google.com/document/d/1tFdX9StXn9do31hddfLuZd0KJ_dBFgtYmxgvGKxd0rY/edit
[2] https://bitbucket.org/xforce_dev/cef/commits/1e4bda98e58ca83c12176be33b8ee1a98d54fce9
@xforce_dev Thanks for the update. I'll try to give feedback next week.
I'm not sure what the best solution is here, especially with regards to shared textures. I've asked @wesselsga (the shared textures author) for feedback as well.
Original comment by Alexander Guettler (Bitbucket: xforce, GitHub: xforce).
@magreenblatt thanks, I might just a bit in over my head with this here, but it's a great learning opportunity, digging through a lot of the chromium code and changes that are involved here.
I was however thinking about this a bit more and I think I have an idea on how to make it work, at least roughly, which I will try hopefully sometime this week.
Just want to finally finish my tests for the media access PR first (hopefully some time tonight), then I can devote more time for this.
At the end this is not immediately required, but I would personally prefer to have the Viz based version sooner rather than later, considering that the default for all desktop chrome platforms is now to use Viz.
From kylechar@ on the Chromium/viz team:
If you want to do something similar to [having a custom SoftwareOutputDevice that owns an SkBitmap] then look at
SoftwareOutputDeviceWinProxy
andLayeredWindowUpdaterImpl
.SoftwareOutputDeviceWinProxy
draws into shared memory and then sends an IPC back to the browser process.LayeredWindowUpdaterImpl
in the browser copies from shared memory to the HWND. We would definitely accept patches to makeSoftwareOutputDeviceWinProxy
andLayeredWindowUpdaterImpl
more general base classes with windows specific implementation built on top. That way embedders could just extend the base class.
FrameSinkVideoCapturer
is also potentially an option when using hardware accelerated compositing. It does readback from GL and transports the pixels to the browser process. DevTools uses it to capture tab contents in a similar way.
Also, it's worth noting that Ozone does not support software compositing but instead has the GPU process rendering directly to the target surface. That's similar to the shared surface use case in CEF.
Original comment by Alexander Guettler (Bitbucket: xforce, GitHub: xforce).
Thanks, that is actually quite helpful, I came across those before, but didn't quite understand how they specifically relate to some things, I am currently working on the Chromium 74 branch update, but will continue work on this again very soon (If all goes well sometime this weekend actually)
@xforce_dev Thanks! I think it would be nice to trial the SoftwareOutputDeviceWinProxy and LayeredWindowUpdaterImpl generalization approach in CEF, and then submit those changes back to Chromium if it works out.
Original comment by Alexander Guettler (Bitbucket: xforce, GitHub: xforce).
PR: https://bitbucket.org/chromiumembedded/cef/pull-requests/226
Although that is still WIP and just the finished stuff I had locally everything seems to be working fine,
except for shared texture, which I removed temporarily until my local new implementation is ready.
Issue #2682 was marked as a duplicate of this issue.
Issue #2722 was marked as a duplicate of this issue.
Update to Chromium version 76.0.3809.0 (#665002)
OSR tests will be fixed by a follow-up merge of Viz support (see issue #2575).
→ <<cset cc0db5f166b6 (bb)>>
Add initial Viz implementation for OSR (see issue #2575).
The old shared surface implementation has been removed and will need to be re-implemented using the Viz code path.
→ <<cset ac2cc54e13ff (bb)>>
Remove unused GetCompositor method and fix macOS compile error (see issue #2575)
→ <<cset cf87c88b0567 (bb)>>
Original comment by Rob Sussman (Bitbucket: Rob Sussman).
I just tried building the 3809 branch but it failed with the error below (on mac). Should the above changeset cf87c88b0567 be applied to 3809 in addition to master?
../../cef/libcef/browser/osr/render_widget_host_view_osr.cc:1361:5: error: use of undeclared identifier 'browser_compositor_'
browser_compositor_->UpdateVSyncParameters(
^
Remove unused GetCompositor method and fix macOS compile error (see issue #2575)
→ <<cset d98abbb7927b (bb)>>
Original comment by Isaac Richards (Bitbucket: irichardsnv).
Hey, hopefully this is useful - I've basically just gotten it all working, but the attached is a re-implementation of the shared texture bits for the new viz osr approach. Compared to the earlier shared texture patch, this simplifies the chrome parts of the change quite a bit. Chrome now has enough shared texture code to handle a large portion of that earlier patch. Unfortunately, this means that cef is passing the textures back to the client slightly differently - chromium is internally using the dx11.1 nthandle/keyed mutex style textures, so the client needs to deal with the handles properly and lock/unlock the textures when used.
Patch should apply to the 3865 branch, and I should have it tested enough to submit a proper PR sometime next week.
Original comment by Alexander Guettler (Bitbucket: xforce, GitHub: xforce).
@{557058:a2ba0474-dc8c-446a-a7ba-63ea519588f3}
Thanks for the patch, I didn’t have the time yet to finish what I was working on which was trying to use the SharedImage work that has been going in chromium for quite a while [1] to maybe get a cross-platform implementation working but due to not having enough time available it’s not in a working state.
So thanks for doing this and hopefully we can get this merged so that we are back to the previous feature set of OSR.
Original comment by Isaac Richards (Bitbucket: irichardsnv).
I actually tried for a quite a while to get the SharedImage stuff working first, but wasn’t able to. GLImageDXGI locks the texture when it’s bound, and I couldn’t figure out how to get the SharedImage interface to unbind the texture (and unlock it for the client to use) without destroying it.
The patch I posted is just using the GpuMemoryBuffer bits, though, and passing that to the OSR code in libcef, so there’s a good chance it’ll work with slight massaging on other platforms. The only windows specific code in there is to deal a bit with the handles in libcef and a small workaround for the brokenness of the dx11 keyed mutex design..
Original comment by pkv (Bitbucket: pkvstream).
@{557058:a2ba0474-dc8c-446a-a7ba-63ea519588f3} do you have a version of this patch for branch 3809 ? we’re quite interested in having shared textures reimplemented (we = obsproject) ; 3865 unfortunately is missing another feature which interests us (https://bitbucket.org/chromiumembedded/cef/commits/58e1149c7127314072903d3d45b9ba8b9fd2fc92)
Original comment by Isaac Richards (Bitbucket: irichardsnv).
Hey, bit of a delay on the accelerated texture stuff - I’ve got one last issue I’m trying to work out first with some rare flickering after a resize. In the meantime, though, I was able to get the accelerated path working on OSX as well, with very minor plumbing changes in CEF (basically getting the shared texture flag passed to RenderWidgetHostViewOSR). I’ll need to port the changes over to cefclient for that, though - need to change the GL code to use GL_TEXTURE_RECTANGLE instead of 2D, use the CGL/IOSurface APIs to get access to the shared texture, a couple other things...
I wasn’t planning on backporting this to 3809, but can take a look if I find the time.
Original comment by pkv (Bitbucket: pkvstream).
@{557058:a2ba0474-dc8c-446a-a7ba-63ea519588f3} great news for the support of macOS :clap: .
I tried to build your patch against 3865 but unfortunately the patch doesn’t apply against current branch. Do you have a more recent version of the patch ? Tried looking in your repo but it hasn’t been updated for a while.
edit: made a mistake in applying the patch. Discard
Original comment by pkv (Bitbucket: pkvstream).
@{557058:a2ba0474-dc8c-446a-a7ba-63ea519588f3} on further testings, I found some issues: I tested this URL https://devnook.github.io/OffscreenCanvasDemo/index.html with and without shared textures on cef 3865 + your patch on cefclient :
.\cefclient.exe --off-screen-rendering-enabled --shared-texture-enabled --enable-gpu
without shared textures, no issues. But with shared textures, although it works at first, at some point it can freeze. That’s on windows 10 pro 1903 using a release build x64 of vanilla 3865 with your patch applied.
Original comment by Romain Caire (Bitbucket: Romain Caire).
Hi, did anybody tested this on Linux ? Would be super happy if this would work on all three major platforms :slight_smile:
Original comment by Scott Maxwell (Bitbucket: scottmax, GitHub: scottmax).
Hi @{557058:a2ba0474-dc8c-446a-a7ba-63ea519588f3} ,
Would it be possible for you to post another .patch
file with your latest? I’d love to get the OSX support you mentioned. I’m working on building for Mac and Windows. I may have some bandwidth to move this forward if necessary but I’d like to start with the latest code you have.
Cheers,
Scott
Original comment by Isaac Richards (Bitbucket: irichardsnv).
Sure - here's my latest. Should be more stable than the last revision. I'm hoping to find the time to finish clean up and update to the latest branch and then submit a proper pull request in the next week or two.
It's still against the 3904 branch, and does not contain the cefclient changes required to use the hardware texture sharing on Mac.. To work on mac, the opengl code needs to change from GL_TEXTURE_2D to GL_TEXTURE_RECTANGLE, and once that's done, you can bind to the shared handles with something like:
#!c++
mach_port_t port = (mach_port_t)(uintptr_t)(shared_handle);
IOSurfaceRef m_cef_surface = IOSurfaceLookupFromMachPort(port);
if (!m_cef_surface) {
LOG(WARNING) << "failed to get io surface from: " << shared_handle;
return;
}
CGLContextObj cgl_context = CGLGetCurrentContext();
GLsizei surfw = IOSurfaceGetWidth(m_cef_surface);
GLsizei surfh = IOSurfaceGetHeight(m_cef_surface);
// bind to the texture we've created already.
glBindTexture(GL_TEXTURE_RECTANGLE, m_texture_id);
CGLError cgl_error = CGLTexImageIOSurface2D(cgl_context, GL_TEXTURE_RECTANGLE,
GL_RGBA, surfw, surfh, GL_BGRA, GL_UNSIGNED_INT_8_8_8_8_REV, m_cef_surface, 0);
if (cgl_error != kCGLNoError) {
LOG(WARNING) << "CGLTexImageIOSurface2D: " << cgl_error;
}
glBindTexture(GL_TEXTURE_RECTANGLE, 0);
Original comment by Romain Caire (Bitbucket: Romain Caire).
Thanks a lot @Issac Richards. Could this be (with minimal changes) be also used in an CEF OSR app running on Linux ?
Original comment by pkv (Bitbucket: pkvstream).
the bug i Mentioned is fixed in the new patch @{557058:a2ba0474-dc8c-446a-a7ba-63ea519588f3} ! Hope you can PR soon. Thanks for this.
Original comment by Isaac Richards (Bitbucket: irichardsnv).
@{557058:1abc75c9-ed35-467b-9d29-aa6fd2ae20fb} - Theoretically, yes, it should work with minimal modifications to the cef/chromium bits on Linux. You’d have to modify the libcef/browser/native/browser_platform_delegate files as I did for the Mac support to plumb through the use_shared_texture_flag on linux, and then in libcef/browser/osr/host_display_client_osr.cc’s OnAfterFlip() function, add stuff to unwrap the gfx::GpuMemoryBufferHandle to get the platform specific handle of the shared texture/data. Then on the client, your starting point for the changes would be somewhere around chromium/src/ui/gl/gl_image_native_pixmap.cc - would need to figure out how to go from the NativePixmapHandle you get in the GpuMemoryBufferHandle to something usable in opengl land..
Original comment by Romain Caire (Bitbucket: Romain Caire).
Cool, thanks a lot @Issac Richards . I’ll try that as soon as I have some spare time. Would be very interesting to see it working :slight_smile:
Original comment by David Morasz (Bitbucket: microdee, GitHub: microdee).
I’ve tested out @{557058:a2ba0474-dc8c-446a-a7ba-63ea519588f3} 's patch on windows on branch 3904 (with Release x64 config), unfortunately cefclient now only displaying white, instead of the proper content and has painting artifacts when resizing :disappointed: . Cefclient was run with --off-screen-rendering-enabled --shared-texture-enabled
arguments, but even without shared textures it’s not displaying anything. Can I miss something?
As an extra I was generating projects with
set GN_DEFINES=use_jumbo_build=true is_component_build=true proprietary_codecs=true ffmpeg_branding=Chrome
set GN_ARGUMENTS=--ide=vs2017 --sln=cef --filters=//cef/*
set GYP_DEFINES=proprietary_codecs=1 ffmpeg_branding=Chrome
call cef_create_projects.bat
Original comment by Riku Palomäki (Bitbucket: riku_palomaki).
I updated the patch by @{557058:a2ba0474-dc8c-446a-a7ba-63ea519588f3} on 3945 branch: https://bitbucket.org/riku_palomaki/cef/commits/137be9414bc47ba4351d927e20046335480e475a. I’m using this in our OpenGL based app with WGL_NV_DX_interop extension to use the shared D3D11 texture in an OpenGL context, which kind of works. However, I noticed couple of issues:
AcquireSync
call in GLImageDXGI::BindTexImage
will fail which seems to break everything on CEF side and eventually gets acquire/release calls out of sync and deadlocks the application. I fixed this by forcing the BindTexImage
call to wait infinitely: https://bitbucket.org/riku_palomaki/cef/commits/98880f6988aff76616e60dbfb498fdcf239c321a - this also makes sure the browser doesn’t run any faster than the actual client application.filter: blur(10px);
don’t work at all and render as white windows, maybe this is what @{557058:839a108b-3556-4440-a35f-de23075d080f} saw as well. Easy way to reproduce this: cefclient.exe --off-screen-rendering-enabled --shared-texture-enabled --enable-gpu --url=https://www.autodraw.com/
wglDXRegisterObjectNV
always fails without a call to wglDXSetResourceShareHandleNV which shouldn’t be needed at all. Also after ~10 mins nvidia drivers crash when calling wglDXSetResourceShareHandleNV
. These GL DX interop issues are probably unrelated to this CEF patch.
Original comment by Isaac Richards (Bitbucket: irichardsnv).
Thanks! - I forgot to comment here, but there’s an open pull request https://bitbucket.org/chromiumembedded/cef/pull-requests/285/reimplement-shared-texture-support-for-viz/diff with it updated to 3945 as well.
I wasn’t sure what to do about the acquiresync in gl_image_dxgi having an immediate timeout, like you’ve mentioned - since that code’s used for a number of other things inside of chromium, I couldn’t easily determine the side-effects of making it an infinite wait. One potential safe workaround would be to lock/unlock the handle inside of the new gl_output_surface_external with an infinite timeout after the texture comes back from the client, but before it’s handed back to the queue for chromium to render to..
I’ll take a look at the post-process thing.
Original comment by Riku Palomäki (Bitbucket: riku_palomaki).
:man_facepalming: Ah, I didn’t notice that merge request. It’s great that you are working on it!
It seems that GLImageDXGI uses triple buffering, I think in normal cases inside Chromium it’s not possible to get into a situation where AcquireSync wouldn’t just immediately succeed, so I would like to believe that the change to infinite timeout is safe.
Original comment by David Morasz (Bitbucket: microdee, GitHub: microdee).
I can confirm Isaac Richards’ pull request works fine except that autodraw website was glitching as mentioned before, however it might not be caused solely by the CSS filters as Riku Palomäki said, because they worked perfectly well for me in the w3school live examples (blur and dropshadow or any combination of multiple ones as well).
Original comment by Jon S (Bitbucket: sirbrialliance, GitHub: sirbrialliance).
Issue #2960 was marked as a duplicate of this issue.
Original comment by Felix Faust (Bitbucket: Felix Faust).
any news on this?
Original comment by Qiang Li (Bitbucket: Qiang Li).
Now I am using the latest version 86.0.241(Cefsharp.Wpf), facing the same problem, expected this version can resolve but i am wrong.
And I am search a lot of same issue but the result was end up with nothing conclusive.
Finally I step by step debuged and found that when closing the window which browser is in, sometime the page stacked( process is killed), so I thinked if window's close method caused the irregular killing? And I override the closing method as follows:
First, My page's image is below:
1: Window1View -> Window1Model -> browserModel
2: Window2View -> Window2Model -> browserModel
the browserModel is moving from one to another bettwen window1 and 2
Use this way I haven't see the above process killed problem (cef's log was no error even if page stacked)
private void Window1_OnClosing(object sender, CancelEventArgs e)
{
// when click close button not to close, just hide the window1, and browserModel is reused in window2
if (sender.As<Window1View>().Visibility == Visibility.Visible)
{
sender.As<Window1View>().Visibility = Visibility.Hidden;
e.Cancel = true;
Window2Model.browserModel = Window1Model.browserModel;
Window2View.Show();
}
}
But I don't know if it is the right way to resolve the problem, anyone can give same advice ?
Original changes by Isaac Richards (Bitbucket: irichardsnv).
Related PR for OnAcceleratedPaint support: https://bitbucket.org/chromiumembedded/cef/pull-requests/734
Original report by Alexander Guettler (Bitbucket: xforce, GitHub: xforce).
Chromium is currently replacing much of the old compositor code with a new system called Viz [1].
Right now we can still use the old path, but I expect them to remove that in a not too distant future (already disabled by default on desktop platforms [2]).
So we need a 'new' OSR implementation that is based on Viz.
[1] https://chromium.googlesource.com/chromium/src/+/master/services/viz/
[2] https://crrev.com/c/1374207