Support external textures in OSR mode - revisted (issue #1006)

#158 Declined
Repository
wesselsga
Branch
issue-1006
Repository
chromiumembedded
Branch
master
Author
  1. wesselsga
Reviewers
Description

Similar to pull request 144 with a few key differences:

  • Modify CEF to not render to the hidden window - this will cause Chromium to use an off-screen gl FBO vs. the default frame buffer for the native window. Chromium already contains code to render with acceleration off-screen - we just need to modify CEF to use it.
  • Modify Chromium to use a shared texture directly for the offscreen FBO. The color attachment for the FBO will be a gl texture that is mapped to a D3D11 shared texture.
  • Chromium will be responsible for creating the D3D11 shared texture and only associate it with a pbuffer (required for interop) one time.
  • When the off-screen FBO is used, CEF will perform no timing - it simply notifies the client that a frame was rendered.

This pull request still uses much of the ExternalTextureManager code originally created by @Isaac Richards . However, the key difference is there is no separate copy frame request issued by CEF since the off-screen FBO is already directly using the shared texture. Also, the AcquireSync and ReleaseSync were broken up into separate commands since we only associate the shared texture to a pbuffer once.

The changes to CEF include a new method added to the RenderHandler - OnAcceleratedPaint. Applications can enable accelerated OSR with the following options on CefWindowInfo:

CefWindowInfo info;
info.windowless_rendering_enabled = true;
info.shared_texture_enabled = true;

This will tell CEF to use the OffscreenBrowserCompositorSurface in Chromium rather than the default window based output. CEF will then call OnAcceleratedPaint directly when a frame is ready vs. trying to setup a copy frame request. The CefCopyFrameGenerator is not used at all.

Client applications can use the command-line switch disable-gpu-vsync to have Chromium perform unthrottled rendering. Any required throttling for timing can be performed by the client application in the new OnAcceleratedPaint handler.

This PR also supports the ability to issue BeginFrame requests directly to Chromium with the following option:

CefWindowInfo info;
info.windowless_rendering_enabled = true;
info.shared_texture_enabled = true;
info.external_begin_frame_enabled = true;

If external_begin_frame_enabled is set to true, then your application is required to issue a call to SendExternalBeginFrame at the desired interval.

Here is a sample application using the new OnAcceleratedPaint handler to efficiently render a HTML view off-screen and then combine it with other D3D11 layers.

Comments (197)

  1. Isaac Richards

    This is a much more efficient method to get textures out of CEF. I just reworked our client to use the new interfaces, and it's all working great (once I flipped things vertically). I'll withdraw my PR in preference to this one. Nicely done!

    1. wesselsga author

      Awesome. Yeah I should've mentioned that in the pull request that you'll need to flip the image vertically. The vertical flip stuff is a bit buried in the sample app. Thanks for checking it out - and most definitely a big thanks for helping kick off this process. I learned so much from reviewing your initial pull request and the changes by @Emmanuel ROCHE.

    1. wesselsga author

      It currently requires D3D. The shared texture created - is a shared d3d11 texture specifically. You might be able to interop it with D3D9 or even D3D12. This method of accelerated OSR is Windows specific. If you are using Windows and your application is written to use openGL - you can use ANGLE to take advantage of the shared texture in a separate gl context.

      1. Isaac Richards

        I don't think it'll work directly on D3D9 without a way to request it to not use keyed mutexes. Might make sense to make that optional for interop purposes? I don't know if the synchronization is working with them, anyway - I'm seeing it copy the previous frame if I use the texture immediately in the callback. If I wait a vsync or so, it's the right image - quite noticeable for hover over effects on a static webpage and only updating the texture on the callback.

        1. wesselsga author

          Agreed on D3D9; it could work in theory - but it would take a bit of work. Not sure there is any point in messing with the D3D9 interop anymore? There is an open issue on the github project for cefmixer that I'm currently working on to resolve. I need a better callback from Chromium when the gpu SwapBuffers takes place - there is something already in Chromium that I'm trying to take advantage of (OnGpuSwapBuffersCompleted). Currently it is issuing the OnAcceleratedPaint notification in SubmitCompositorFrame within CEF - which is incorrect. To work around it temporarily you can simply get the shared handle from OnAcceleratedPaint and draw it every frame in your application until this is resolved.

        2. wesselsga author

          Just an update - I believe the drawing issue where it is behind a frame is now resolved. I did update the pull request with a fix. It might not be the best way to get the OnGpuBufferSwapCompleted notification back to CEF (hijacking the LatencyInfo) - but that is the function I was targeting. Basically, send a callback/message back to CEF after the glFlush call.

          1. Isaac Richards

            Unless I applied things wrong somehow, that update seems to make things worse - I'm not getting frame updates at all now except for after mouse input to the browser.

            1. Mikael Hermansson

              I'm noticing something similar.

              The initial render never happens, only until something visually changes on the page does the first call to OnAcceleratedPaint happen. Opening a very basic webpage like this one never renders for me.

              The problem is that I can't replicate this behavior in cefmixer, so I hesitate making an issue about it.

            2. wesselsga author

              I'm curious, did you apply the pull request update and then just re-build? I had an issue too with this, and simply did a clean build (painful).

              1. Isaac Richards

                It was a mostly clean rebuild, yeah, since the patch wouldn't apply otherwise. It appears frame.metadata.latency_info is empty in CefRenderWidgetHostViewOSR::SubmitCompositorFrame. Trying to just add that to see if it'll work.

                1. wesselsga author

                  Yeah, I found similar code for that in Chromium ... but the statement I have in SubmitCompositorFrame:

                  if (frame.metadata.latency_info.size() > 0) ...
                  

                  Doesn't make sense thinking about it - we need that marker in there regardless - so we should delete that check.

                  It does make sense that mouse input makes it work on your end ... because there is an INPUT latency component then added

                  1. Isaac Richards

                    This makes it work (and the rest of it does fix the original issue):

                          // we're going to trick Chromium into calling OnGpuSwapBufferCompleted by
                          // registering a TAB_SHOW_COMPONENT latency component with the frame
                          if (shared_texture && render_widget_host_) {
                            if (frame.metadata.latency_info.size() == 0) {
                              frame.metadata.latency_info.push_back(ui::LatencyInfo());
                            }
                    
                            frame.metadata.latency_info.front().AddLatencyNumber(
                                ui::TAB_SHOW_COMPONENT,
                                render_widget_host_->GetLatencyComponentId(), 0);
                          }
                    
                    1. wesselsga author

                      Perfect. I will update the pull request now with your code snippet. Nice work!

                      Luckily that change is just in the CEF layer and we don't have to re-patch

      2. Hugh Bailey

        This is just my personal opinion: D3D9 support is pretty much irrelevant these days. Not worth wasting time on, for anyone. I know that the person you're responding to didn't mention that, but I wanted to throw that out there just simply due to the importance of the pull requests such as these. It would work on D3D11 (and above) just fine, and that's the important thing. These sort of PRs are definitely never worth shelving due to D3D9 (or even OpenGL) intercompatibility.

        However, indeed it would work on OpenGL via interop. If you can use D3D11+ directly however, that's always more ideal.

      3. wesselsga author

        I don't currently have any plans to attempt support for D3D9. If someone does have an issue with using IDXGIKeyedMutex on the shared texture...it can be easily toggled off by locating the following line in OffscreenBrowserCompositorSurface:

        shared_handle_ = gl->CreateSharedTexture(
              shared_texture_, texture_width, texture_height, GL_TRUE);
        

        The GL_TRUE parameter denotes whether the shared texture should be setup for Keyed Mutex - if GL_FALSE is used instead the Lock/Unlock methods will not do anything

  2. Isaac Richards

    I haven't managed to figure out the best fix yet, but this addition to CefRenderWidgetHostViewOSR::CefRenderWidgetHostViewOSR() crashes for popup browsers:

      // Can the application support accelerated Paint?
      bool accelerated_paint = false;
      {
        CefRefPtr<CefRenderHandler> handler =
            browser_impl_->GetClient()->GetRenderHandler();
        if (handler.get())
          accelerated_paint = handler->CanUseAcceleratedPaint(browser_impl_.get());
      }
    

    1. wesselsga author

      Probably related to the code comment at line 232 in render_widge_host_view_osr.cc?

      if (parent_host_view_) {
          browser_impl_ = parent_host_view_->browser_impl();
          DCHECK(browser_impl_);
        } else if (content::RenderViewHost::From(render_widget_host_)) {
          // CefBrowserHostImpl might not be created at this time for popups.
          browser_impl_ = CefBrowserHostImpl::GetBrowserForHost(
              content::RenderViewHost::From(render_widget_host_));
        }
      

      I suppose a quick test would be to do (just force accelerated_paint on):

      // Can the application support accelerated Paint?
        bool accelerated_paint = true;//false;
        /*{
          CefRefPtr<CefRenderHandler> handler =
              browser_impl_->GetClient()->GetRenderHandler();
          if (handler.get())
            accelerated_paint = handler->CanUseAcceleratedPaint(browser_impl_.get());
        }*/
      

      If that works for OSR in a popup - then I guess the resolution would be to eliminate the CanUseAcceleratedPaint() callback and just pass it in as a setting (e.g. like background_color)

    2. wesselsga author

      I did reproduce this here.

      I'm wondering if something like this would be better (control it via CefWindowInfo)

      CefWindowInfo info;
      info.windowless_rendering_enabled = true;
      info.shared_textures_enabled = true;
      

      That way we know if we need to tell Chromium to use a shared texture FBO before the browser_imp_ is created in CefRenderWidgetHostViewOSR::CefRenderWidgetHostViewOSR()

      By having it on CefWindowInfo vs. CefBrowserSettings - you would be able to control whether or not you wanted to use OnAcceleratedPaint() independently for popups?

      CanUseAcceleratedPaint() would be removed.

        1. wesselsga author

          Working on it now, I probably should've have done it that way originally (I considered it). But adding a new value CefWindowInfo required more work in CEF - I was cheating :)

        2. wesselsga author

          Alright, I removed CanUseAcceleratedPaint() from the API and switched to:

          CefWindowInfo info;
          info.windowless_rendering_enabled = true;
          info.shared_textures_enabled = true;
          

          Earlier, I was able to reproduce the crash by starting to add support for Popup Views in cefmixer. I'm no longer seeing any crashes - however, I'm also not getting any updates for OnAcceleratedPaint() for Popups. But it is probably operator error since this popup stuff is pretty new to me. (I usually have ignored them)

          This is a breaking change as CanUseAcceleratedPaint() will need to be removed from your Render Handler.

          I do like this better - the API for Render Handler seems cleaner. Now there is just 1 additional method (OnAcceleratedPaint) for shared textures.

          1. Isaac Richards

            Yup, that fixes things. Popup browsers (and normal popups in popup browers, heh) are now using the fast path, with no crashes.

            You're likely missing the extra call to CefBrowserHost::WasResized() that a popup browser needs to start rendering in OSR mode - I remember that being non-obvious back when adding support for them in our client.

            1. Jakub Głuszkiewicz

              @Isaac Richards or @wesselsga could u be that kind and give a little more info where CefBrowserHost::WasResized() should be added ? On render_widget_host.cc?

              I would like to support popups in acceleratedpaint, i dont have any crashes but popups doesnt generate any onacceleratedpaint calls, probably due to lack of CefBrowserHost::WasResized() mentioned by Isaac,

              i would really appreciate any advice where to add this call,

              thanks

            2. Jakub Głuszkiewicz

              pls can someone help me,

              this pr works reliable and stable but native dropdowns doesnt work,

              for ex https://www.w3schools.com/tags/tryit.asp?filename=tryhtml_select

              when im trying to show dropdown no acceleratedpaint or onpaint is called, no crashes… just silence :(

              i tried douzen ways to make it work but without success,

              without shared textures that link works without isuues

              that dropdown is popup view but pet_popup paint calls doesnt occur. any clue where i can dig to solve it?

  3. Isaac Richards

    One last bug I've found, but it's from my original PR - in external_texture_manager.cc, the entirety of CreateTexture and DeleteTexture probably should be under the OS_WIN ifdef - none of the EGL stuff exists on OSX, so it'll break the compile there.

  4. Michael

    Hello friends! I would like to ask how to work with multi GPU? How to transfer the device for which the share texture is created? Of course, provided that I have several GPU on the computer.

    1. Marcin Lizer
      1. Sharing textures between adapters is not possible in DX11 to my best knowledge. It is though possible in DX12 using D3D12_RESOURCE_FLAG_ALLOW_CROSS_ADAPTER flag, so we would need to wait for DX12 support by ANGLE.
      2. You select which GPU is used by the process using appropriate application (nVidia Control Panel or AMD Catalyst) and selecting the cef sub process. I tried using chromium switches (e.g. --gpu-testing-device-id/--gpu-testing-vendor-id) but it did not work as expected. I faced some issue but do not remember the details. The only method I know is the user's action using gpu software tool.
      3. For mulit GPU scenario I use traditional GPU1-CPU-GPU2 path
    2. wesselsga author

      I believe which gpu device that is used by the gpu process can be specified with the --gpu-active-device-id and --gpu-active-vendor-id. However, spawing multiple gpu processes for multiple renderer processes from 1 browser process (your app) are likely to require changes. What those changes are I'm not sure, but it would be a separate pull request from this one.

      EDIT: I agree with the comment from @Marcin Lizer above - without being able to share memory across devices, you would probably have to fallback to CPU readback

      1. Michael

        I want to use only one GPU to rendering, but computer have several same(vendor-id/device-id) GPU cards. Each process need to use one/another GPU card of the computer. Question: How I can send(enumerate) to process adapter ID of relevant card? Example target: Two browsers use different cards for accelerated OSR, computer have two GPU AMD W8100. No need transfer data between GPUs.

        1. Marcin Lizer

          I do not think what you want is possible as cef has global sub process path, and the selection of the GPU is done by the driver based on the path of the sub process application and here we have single path. Anyway, I suggest you ask the question to the CEF forum as it is not this PR related.

          http://magpcss.org/ceforum/

  5. Marcin Lizer

    This PR works like charm for me. Thank you all guys for moving this forward !

    just a remark:

    The GL_TRUE parameter denotes whether the shared texture should be setup for Keyed Mutex - if GL_FALSE is used instead the Lock/Unlock methods will not do anything

    Should not we have this as an option ?

    1. wesselsga author

      I'm not opposed to an option to control the keyed mutex creation - I just chose initially not to add one waiting to determine necessity.

  6. Mikael Hermansson

    I would love to know if anyone else is having this issue where sometimes a render process will remain after the application has been closed, seemingly just spinning on a single thread (judging by the 25% CPU usage on my 4-core CPU).

  7. Mikael Hermansson

    Has anyone tried resizing at all? We're seeing an issue where after resizing the texture stops updating altogether.

    After calling CefBrowserHost::WasResized the CEF UI thread calls CefRenderHandler::GetViewRect as it should, but then just stops calling CefRenderHandler::OnAcceleratedPaint altogether, leaving the texture "frozen". The CEF UI thread is still running though, and you can inspect changes happening to the DOM from the dev tools, so it seems it's purely the rendering that becomes unresponsive.

    I implemented a crude form of resizing in cefmixer and we were able to reproduce the issue there as well.

    The weird part is that we're only seeing this issue on one of our two test machines. The other one can resize without any issues at all.

    I'm not sure if it's relevant, but the test machine that works is using an AMD GPU with a 4-core Intel CPU and the one that doesn't work is using an Nvidia GPU with a 32-core AMD CPU.

    1. Mikael Hermansson

      Here's a branch I whipped up that resizes the browser texture (and only that) when resizing the window. Since the back buffer and viewport isn't being resized it'll look a bit weird, but it should at least serve the purpose of showing whether the texture freezes up or not, if anyone wants to try it.

      1. wesselsga author

        I merged your branch into master, and modified it a bit to allow resizing the back buffers (swapchain) as well on WM_SIZE. I will do some testing to try and reproduce the issue where the texture stops updating.

          1. wesselsga author

            I would be curious to know if OnPaint is being called rather than OnAcceleratedPaint? The changes I put in CEF will only call OnAcceleratedPaint if Chromium has a valid shared handle for a texture. I'm wondering if its failing to create the shared texture on resize or something? It should revert to OnPaint in that case - it would give a starting point on where to look at least.

            UPDATE: I did reproduce the issue on a Windows 7 machine.

          2. wesselsga author

            Is your test machine running Windows 7? I believe this is related to the use of disable-gpu-vsync on Windows 7. I've been fighting this issue for a while on previous versions of CEF / Chromium (without this pull request). Try commenting out the AppendSwitch("disable-gpu-vsync") in cefmixer and see if that works on your test machine. Removing that switch seemed to fix the issue here.

            I believe this issue is also reproducible in Chrome itself (65.0.3325.181) just by running it with the switch:

            chrome --disable-gpu-vsync
            

            It definitely doesn't paint correctly (usually not at all). I will continue to try and track it down.

            1. Mikael Hermansson

              The test machine that experiences the issue is running Windows 10.

              Removing the --disable-gpu-vsync switch did fix the issue it seems, and just like you said we were able to reproduce the exact same behavior in Chrome (64.0.3282.186) when using that switch.

              When removing that switch I can only get at most 60 FPS though, despite the screen refresh rate being 144hz. Any idea what might be going on there?

              I assumed it had to do with windowless_frame_rate and it being clamped to 60 just before CEF reports the vsync interval to the compositor, so I tried removing the upper clamp and set windowless_frame_rate to 144, but that didn't seem to help with the frame rate cap in any way.

              1. wesselsga author

                Interesting, I haven't seen the issue on Windows 10 yet. I always intended the switch --disable-gpu-vsync to be temporary ... the better solution would be to have the client app actually issue the BeginFrame to chromium at a rate that it determines. In cefmixer, this would likely be in the tick method on HtmlLayer. I guess it might be time to start heading down that path.

                I don't know why its capped to 60 on a 144Hz screen - I have also removed the windowless_frame_rate cap in CEF previously without luck.

      2. wesselsga author

        Just an update - I have modified CEF locally to include a new method SendExternalBeginFrame so the application can control when BeginFrame is sent to the renderer. With initial testing here, these seems to resolve the issue where a resize stops updating the texture. I'm planning on another update to CefWindowInfo so you can enable this option:

        CefWindowInfo window_info;
        window_info.SetAsWindowless(nullptr);
        window_info.shared_textures_enabled = true;
        window_info.external_begin_frame_enabled = true;
        

        If window_info.external_begin_frame_enabled is set to true, your application will be responsible for issuing a call to SendExternalBeginFrame at whatever interval you wish. Chromium already has an option to use an external begin frame source (headless mode). This new option simply turns that feature on and skips the BackToBackBeginFrameSource which is used by default when --disable-gpu-vsync is used. I don't know why yet, but I believe on some machines when vsync is disabled, the BeginFrame message stops being sent to the renderer. My hope is using the external beginframe option will be more reliable.

        @Marcin Lizer - I'm also planning another option on CefWindowInfo called shared_texture_sync_key where the application can set what sync key it wishes to use for the keyed mutex. I'm thinking of something like shared_texture_sync_key=0; would tell Chromium to not to setup the shared texture using a keyed mutex at all (for D3D9 compatibility or whatever). Any non-zero value would cause the shared texture to use a keyed mutex and that value would be passed to OnAcceleratedPaint.

        1. Mikael Hermansson

          This new option simply turns that feature on and skips the BackToBackBeginFrameSource which is used by default when --disable-gpu-vsync is used.

          Does that mean --disable-gpu-vsync will still be needed?

          1. wesselsga author

            Good question. I'm leaving it on currently - because without that flag, CEF is still trying to setup its own BeginFrame timer, which I don't want. I think the goal would be to not have to specify --disable-gpu-vsync at all - but I'm not there yet.

            The code of interest in Chromium can be found in GpuProcessTransportFactory look for the code:

            if (compositor->external_begin_frames_enabled()) {
                external_begin_frame_controller_client =
                    std::make_unique<ExternalBeginFrameControllerClientImpl>(
                        compositor.get());
            

            This new option will enable this branch of code and skip the viz::BackToBackBeginFrameSource it is using currently with --disable-gpu-vsync

            1. Mikael Hermansson

              Gotcha. I'd prefer having it disabled myself, but considering the resizing bug I get the feeling it's not meant to be something your every day Chromium user should use.

              Also, I have no idea if this is even remotely related to the resizing bug, but there is something in the pipeline related to --disable-gpu-vsync. It seems to have more to do with the flag not doing anything at all rather than it breaking stuff. But who knows, maybe it fixes the freezing issue as well.

              1. wesselsga author

                Yeah, I was reading through those posted issues as well. I was having a hard time following if that flag was broken or fixed or broken again. Seems sketchy.

              2. wesselsga author

                I opened a new issue for the updating bug. There is a link in that issue to a test binary of cefmixer, if we can verify that works on machines exhibiting the issue - that would be very helpful.

  8. wesselsga author

    The use of --disable-gpu-vsync should no longer be recommended with this pull request. You can now simply use windowless_frame_rate and set it to a value greater than your refresh rate. The cefmixer example uses the following:

    settings.windowless_frame_rate = 240;
    

    This pull request does not cap the windowless_frame_rate value at 60Hz. If you run the latest version of cefmixer you will see by default it will run at the vsync rate - then if you disable vsync (with Ctrl+V), the HTML demos should report the higher rate (assuming your hardware can handle it).

    The option --disable-gpu-vsync does still work ... but I've seen too many issues with this flag and Chromium on some hardware sets that I wouldn't advise using it other than testing.

  9. Hugh Bailey

    I'm the author of OBS Studio, one of the prime applications where this pull request is probably the most applicable use case. I have tested this pull request with OBS Studio's implementation of CEF, and it works pretty much perfectly, with a few minor (but acceptable) quirks. You've just about hit the nail right on the head with everything needed to make CEF perfect for hardware-accelerated OSR on windows.

    A few important notes:

    1.) The ability to choose whether or not to use keyed mutex is great consideration, thank you for doing that; when coupled with SendExternalBeginFrame, the need for keyed mutex becomes pretty much pointless and more of a burden, so having the ability to choose whether or not to use that functionality is very useful.

    2.) Although I have no need for keyed mutexes, I would like to note that when using keyed mutexes, it doesn't seem to use keyed mutexes in the way the documentation states: the key for both the acquire and the release is always the same key for both CEF's device and the implementer's device, where as the documentation states to alternate between a different key for each device. However, it does appear to work despite sharing the same key, which is honestly baffling; I can't help but feel that the design of keyed mutexes by Microsoft is fundamentally terrible in the first place. Microsoft's keyed mutex API a complete hack of an API and I'm glad I don't need to use it.

    3.) I wish we didn't have to worry about setting the CEF FPS when coupled with using SendExternalBeginFrame. I know that this probably isn't your fault or anything, but the fact that we still have to assign an arbitrary FPS value (like 250 in that example) when using manual frame synchronization feels like an unfortunate but perhaps necessary hack to work around the internals of chromium and/or CEF.

    4.) The parameters of SendExternalBeginFrame are terrible to deal with. It feels as though certain parameters will cause unintended side effects, such as a web page not playing back correctly, or performance being impacted negatively. It also needs documentation, or at least default values. Looking at the PR code for SendExternalBeginFrame, it seems the intended "default" values (0, -1, -1) are linked to the CEF FPS value, which is sort of frustrating because SendExternalBeginFrame is supposed to be used to manually synchronize frames specifically so we don't have to rely on that FPS.

    5.) However, having the ability to synchronize framerate manually with SendExternalBeginFrame is a *highly significant* boon to OSR usage in general, as it for example allows the ability to sync up OBS Studio's framerate and a browser source's framerate, as well as minimize the need for keyed mutexes on the texture sharing. This feature is of highly significant importance despite its quirks.

    So to sum up my feelings on this pull request: It's pretty much everything I ever wanted, except I hate the need that we still have to set an FPS, and I really wish we didn't need the parameters of SendExternalBeginFrame, although I understand why they're there and could be useful.

    I will be experimenting with this more as time passes. For OBS Studio's purposes, this PR is of critical importance.

    Apologies for any redundant discussion. Hopefully in time, this will make its way in to chromium/CEF proper. Well done.

    1. wesselsga author

      Hugh, thanks for your input - I pretty much agree with all your points.

      1) & 2) I did fight the keyed mutex for a while as well, and even attempted to use it properly by acquiring and releasing with different key values as @Mikael Hermansson and myself discussed on this issue. I would consider the keyed mutex option a work in-progress and wouldn't recommend its use until I can get it working in proper cadence.

      3) & 4) Temporary necessary evils? I consider the current parameters just a first crack at exposing SendExternalBeginFrame. I believe you can just pass 0 for all and it will basically ignore what windowless_frame_rate has for a value. I was definitely on-the-fence of trying to leverage the existing windowless_frame_rate in CEF when using accelerated OSR - more testing might result in simply removing them all? That is, if users see no value in being able to specify the deadline time and interval for viz::BeginFrameArgs in Chromium - the method SendExternalBeginFrame wouldn't even need any params.

      5) Could not agree more. I use the SendExternalBeginFrame in both the sample cefmixer application and our commercial product.

      This type of feedback is invaluable and greatly appreciated. Maybe for now I'll just add more documentation and defaults to the parameters for SendExternalBeginFrame - if the consensus here is to simply remove them, I wouldn't be opposed to that either.

      1. Hugh Bailey

        Yea, I can understand that FPS value isn't necessarily your fault or anything, just something worth noting. I'll try out (0, 0, 0), I actually tested about every other set of values but that. Whether or not the parameters of SendExternalBeginFrame would actually be useful or not in some cases, I couldn't say, but if they can be eliminated safely for all use cases (which seems likely), that would of course be ideal. Just having a way to signal CEF/chromium to render a frame by itself is a great thing.

        I'm glad to help. Thank you for writing this PR. If there's anything else I can note I'll do so.

        1. wesselsga author

          I was incorrect. Even when passing 0 for all args to SendExternalBeginFrame it is still capped by windowless_frame_rate. CEF is still calling SetAuthoritativeVSyncInterval with the value in windowless_frame_rate, I would have to dig a bit to see if this can be bypassed.

    2. Julian Waller

      It is good to know that it works well in OBS, I have used your code as reference in the past when trying to figure out the CEF performance issues in CasparCG, but havent had time to try this PR out myself yet.

      What is your plan for cross-platform with this? I assume it doesnt work on anything other than windows currently?

      1. Hugh Bailey

        It's a windows-specific pull request, so it doesn't apply to other operating systems.

        I wouldn't necessarily advise using our current obs-browser source code as a reference, primarily due to its overly complex nature. One of the reasons I was able to test this pull request is because I'm in the middle of a major refactor of the browser source base to prune as much unnecessary code and dependencies as possible. Over the next week or so the 20 or so cpp files that comprise of our browser handling code should be reduced to a max of around 4-5.

    3. wesselsga author

      This pull request has been updated to ignore the value of windowless_frame_rate when using SendExternalBeginFrame. Also, there are now defaults for the arguments to SendExternalBeginFrame.

      Because the keyed mutex is not working as intended yet ... the default setting in CefWindowInfo will be to disable it.

      I updated the cefmixer application as well, so if you run that app and press Ctrl+V to disable vsync - you should see a FPS reported by the HTML that is unthrottled rather than limited by the value of windowless_frame_rate.

      If an application chooses not to use SendExternalBeginFrame, it can still use values > 60 for windowless_frame_rate when using CEF's BeginFrame timer.

  10. Jiří “Fenryxo” Janoušek

    It's a great piece of work. Despite being a Windows-only feature, there seems to be a lot of cross-platform code. Do I understand it correctly that only the two things below are necessary for OpenGL/Linux support? Or am I missing something else?

    • Add (const bool use_shared_texture, const uint64 shared_texture_sync_key, const bool use_external_begin_frame) params to CefBrowserPlatformDelegateNativeLinux::CefBrowserPlatformDelegateNativeLinux.
    • Implement ExternalTextureManager (gpu/command_buffer/service/external_texture_manager.cc) for #if defined(OS_LINUX).
    1. wesselsga author

      The 2 things you have listed should be the only things required to get something to work on Linux/OpenGL. What technique did you have in mind to share the texture efficiently across processes in GL? If there is something that will work - I'm all for it.

      1. Jiří “Fenryxo” Janoušek

        WebKitGTK does share GL textures neither. They use XPixmap and XComposite & XDamage extensions for Xorg. I'm not experienced in this area so I cannot tell whether it is faster then the way CEF transfers pixbuf data from GPU to Browser process.

        https://blogs.igalia.com/carlosgc/2016/09/20/webkitgtk-2-14/ Legend: UI Process ~ CEF Browser process, Web Process ~ CEF Renderer & GPU processes combined.

        The main challenge is compositing in the web process and sending the results to the UI process to be rendered on the actual screen. In X11 we use an offscreen redirected XComposite window to render in the web process, sending the XPixmap ID to the UI process that renders the window offscreen contents in the web view and uses XDamage extension to track the repaints happening in the XWindow. In Wayland we use a nested compositor in the UI process that implements the Wayland surface interface and a private WebKitGTK+ protocol interface to associate surfaces in the UI process to the web pages in the web process. The web process connects to the nested Wayland compositor and creates a new surface for the web page that is used to render accelerated contents. On every swap buffers operation in the web process, the nested compositor in the UI process is automatically notified through the Wayland surface protocol, and new contents are rendered in the web view. The main difference compared to the X11 model, is that Wayland uses EGL in both the web and UI processes, so what we have in the UI process in the end is not a bitmap but a GL texture that can be used to render the contents to the screen using the GPU directly. We use gdk_cairo_draw_from_gl() when available to do that, falling back to using glReadPixels() and a cairo image surface for older versions of GTK+.

      2. Jiří “Fenryxo” Janoušek

        I've asked for advice on GTK+ app devel list and Emmanuele Bassi (a GTK+ developer) recommended:

        You can store the texture data as an X11 Pixmap object, and use the GLX_EXT_texture_from_pixmap extension to get a GL texture object out of a Pixmap XID.

        1. wesselsga author

          In hindsight, I should've added a enum type to the OnAcceleratedPaint callback to describe what type of shared_handle is passed. The default would be TextureTypeD3D11 or something - in the future it could be something else GTK related, or other. I'll be curious what you come up with.

          I'll probably go ahead and add the enum to the callback in the next update.

        2. Sesse_

          I asked NVIDIA, who recommended allocating the shared texture using Vulkan (you just need to initialize the device, you don’t need any command buffers or window system initialization or anything), and then use GL_EXT_memory_object to import it into OpenGL in both processes. This should be both fast and well supported across vendors.

            1. Sesse_

              I don’t have any current plans to work on this, no (neither the XPixmap solution, nor Vulkan). Can you even share X pixmaps reliably across processes? What would you do for synchronization when you don’t have shared semaphores?

              1. Sesse_

                Also, be aware that GLX is legacy; all new software should use EGL wherever possible, which means CEF is likely to be called from an EGL context.

  11. elad bahar

    Hi,

    Did someone maybe was able to integrate this solution with d3d9 ( for example wpf d3d image or any other native d3d9 application)?

    thanks,

  12. Jakub Głuszkiewicz

    Hello wesselsga,

    firstly thank you so much for your work,

    i need to admit its one of my dreams to speed up osr mode in my 2d framwework,

    i manually merged your pr into 3325 branch, build was succeded,

    also i implemented all required changes in my cef client,

    onacceleartedpaint is called without issues and im able to obtain shared handle,

    also im creating d3d11texture with shared handle without issues,

    sadly i dont know why, i see only black screen, no crashes, no output 😞

    i have been working with shared textures many times in my history so im familiar with this technique

    after few hours of investigation i really have no idea why i have black screen without output,

    so i have few questions, could you be that kind and answer them for me?

    1. In your opinion this pr should work with 3325 branch?
    2. I know u posted 3359 redist in post above, could you give me link to libcef_wrapper.lib too? I would like to avoid custom build this wrapper and use yours. If you could give me that lib i can quickly switch to your build and see if i still have this issue, it will help me to investigate my problem,

    thanks so much in advance

    1. wesselsga author

      On the same machine, can you build and run the cef-mixer demo application (even with latest CEF)?

      The link I posted for the sample distribution can be used to build libcef_wrapper.lib just following the instructions for cef-mixer. Just download the older archive, and set CEF_ROOT to the location and run gen_vs2017.bat.

      If that doesn't work let me know and I can build it ... x64 Release?

      These changes should be able to work with 3325

      1. Jakub Głuszkiewicz

        once more thank you, thank you… 1000000x thank you

        thanks to you one of my bigest dev dreams come true! Now my osr mode is almost as fast and fluent as standard rendering by chrome to native window

        i have tears in my eyes 🙂

        i wasnt skill enough to figure out how to make sharing textures, i tried by my own year ago and i failed, you showed me a way, thanks so much

        finally managed to solve and i see so outstanding performance!!

        issue was cuz i used for sharing textures in video decoding that kind of implementation:

        resourceViewDesc.ViewDimension = D3D11_RTV_DIMENSION_TEXTURE2D;

        yours (i analized your sample) is:

        srv_desc.ViewDimension = D3D11_SRV_DIMENSION_TEXTURE2D;

        and according to D3D documentation shared resources must be absolutly equal in parameters which is quite obvious

        now i have to solve one more issue, but this time is on my side 100%

        im using multithreadedmessageloop which cause calling OnAcceleratedPaint in separate thread

        im making copy of shared texture to my rendering texture and sometimes i have crashes caused by unsynced chrome frame rendering

        possible ways to solve it:

        1. make d3d11 threadsafe (i have no crashes anymore but there is some performance penalty)
        2. use semafores on acceleratedpaint and hold chrome till i will make copy of shared texture. But well… it can cause stall of GPU im not sure if it will not have bad impact on smoothness rendering
        3. Try to fool chrome by creating on my side queue of render textures and copy shared texture to different render textures it will minimize occurence of crashes cuz i will swap buffers on different textures than chrome is rendering.

        So more exciting work i have to do 🙂

        thank you and take care!

        I owe you big dinner and beer or even something stronger if u prefer!

  13. Jiří “Fenryxo” Janoušek

    As a very first step, I tried to build this PR on Linux. I believe there needs to be : CefBrowserPlatformDelegateNative(window_info, background_color, false, 0, false), instead of : CefBrowserPlatformDelegateNative(window_info, background_color, false), in libcef/browser/native/browser_platform_delegate_native_linux.cc. The same probably for mac.

  14. wesselsga author

    This pull request has been refactored to build against CEF master (Chromium 69). Due to some changes in Chromium, the following command line arg is required:

    command_line->AppendSwitchWithValue("use-cmd-decoder", "validating");

    This is temporary - just until I finish updating support for External Textures using the pass-through GLES2 command decoder which is now the default in Windows. (update in May 2018)

    1. wesselsga author

      Update: the option command_line->AppendSwitchWithValue("use-cmd-decoder", "validating"); should no longer be required. The Chromium patch now has the passthrough decoder updated.

  15. Jakub Głuszkiewicz

    thank you @wesselsga for your continues work on this awsome great path.

    Before merging it into current master dont forget that dropdowns (in-browser popups) still doesnt work. OnAcceleratedPaint is not fired at all :(.

    Can we investigate it a litte? I think its currently only known issue in this pr,

    once more, thank you very much for all u did for us!

    1. wesselsga author

      I have not tried the External Begin frame call without using shared textures … but there shouldn’t be anything Windows specific about it. If it doesn’t work as is - I see no reason why it couldn’t be made to work on Linux/Mac.

  16. Muazin Mugadr

    I have seen a few times mentioned here that resizing stops the rendering. I have the same issue right now. I am using SendExternalBeginFrameand when I am resizing the window all rendering stops. If the location of the webview changes or if some DOM changes happen (just CSS effects is not enough) it starts rendering again. Do you have any idea what this could be related to?

    1. wesselsga author

      Is this something you can replicate with the cefmixer demo app? Does your app issue a call to WasResized when the window is resized?

      1. Muazin Mugadr

        It is calling WasResized(with the old OnPaint it did not happen). I will try to build cefmixer and see what happens there! Are there any requirements for the SendExternalBeginFrame call? I am currently calling it once per frame of my own render loop.

      2. Muazin Mugadr

        I am suspecting that I am not using the right version of this PR, because cefmixer does not build complaining about argument mismatch in SendExternalBeginFrame

        See below

      3. Muazin Mugadr

        Changing html_layer.cpp#515 from browser->GetHost()->SendExternalBeginFrame(); to browser->GetHost()->SendExternalBeginFrame(0, 0, 0); makes cefmixer build, and the same thing happens when resizing, all rendering stops.

          1. wesselsga author

            Yes, I had forgot to push that change earlier. I found the issue with resizing ... working on a fix now. DidPresentCompositorFrame is only called 1 time and not repeatedly as I was expecting. Thanks for pointing this issue out.

              1. wesselsga author

                If you want a quick fix and are setup to build CEF - you can insert the following at line 610 in chromium/src/cef/libcef/browser/osr/render_widget_host_view_osr.cc:

                frame.metadata.request_presentation_feedback = true;

                Before the call to GetDelegatedFrameHost()->SubmitCompositorFrame(..)

                Basically, when using shared textures - we need to inform the renderer that we want Presentation callbacks fired everytime.

                I believe that should resolve the update issue. I’m in the process of updating the pull request - but it will take a bit since I’ll have to merge with CEF master. Thanks again

  17. Marshall Greenblatt

    @wesselsga I’m using your PR at Chromium version 69.0.3464.0 with the frame.metadata.request_presentation_feedback = true; fix mentioned above. I’m initializing the OSR browser with:

    window_info.shared_texture_enabled = true;
    window_info.external_begin_frame_enabled = false;
    window_info.shared_texture_sync_key = uint64(-1);  // Default value.
    

    I’m seeing calls to CefRenderHandler::OnPaint instead of OnAcceleratedPaint. Is this combination of flags supported?

    I notice in cef-mixer that you set external_begin_frame_enabled = true; so perhaps the false condition hasn’t been tested recently.

    1. Marshall Greenblatt

      Looks like it’s calling SoftwareBrowserCompositorOutputSurface instead of OffscreenBrowserCompositorOutputSurface. That causes shared_texture to be nullptr in CefRenderWidgetHostViewOSR::OnPresentCompositorFrame.

      >   content.dll!content::BrowserCompositorOutputSurface::GetSharedTexture() Line 66 C++
          content.dll!content::GpuProcessTransportFactory::GetSharedTexture(ui::Compositor * compositor) Line 220 C++
          compositor.dll!ui::Compositor::GetSharedTexture() Line 550  C++
          libcef.dll!CefRenderWidgetHostViewOSR::OnPresentCompositorFrame() Line 534  C++
          libcef.dll!CefCompositorFrameSinkClient::DidPresentCompositorFrame(unsigned int presentation_token, const gfx::PresentationFeedback & feedback) Line 222    C++
          content.dll!content::DelegatedFrameHost::DidPresentCompositorFrame(unsigned int presentation_token, const gfx::PresentationFeedback & feedback) Line 323    C++
          service.dll!viz::CompositorFrameSinkSupport::DidPresentCompositorFrame(unsigned int presentation_token, const gfx::PresentationFeedback & feedback) Line 473    C++
      
      1. wesselsga author

        Interesting. Does it work for you with window_info.external_begin_frame_enabled = true;? I can investigate and see what is going on there.

    2. Marshall Greenblatt

      Rendering now appears to be working (I get a blue screen). However, the renderer process is crashing while loading http://www.google.com:

      [0718/161921.032:FATAL:first_meaningful_paint_detector.cc(317)] Check failed: !swap_stamp.is_null(). 
      >   blink_core.dll!blink::FirstMeaningfulPaintDetector::SetFirstMeaningfulPaint(base::TimeTicks stamp, base::TimeTicks swap_stamp) Line 318 C++
          blink_core.dll!blink::FirstMeaningfulPaintDetector::Network2QuietTimerFired(blink::TimerBase *) Line 205    C++
          blink_core.dll!blink::TaskRunnerTimer<blink::FirstMeaningfulPaintDetector>::Fired() Line 158    C++
          blink_platform.dll!blink::TimerBase::RunInternal() Line 161 C++
      
      1. Marshall Greenblatt

        The texture turns from blue to gray after resize, and it never renders any web content. I’m adding support for external_begin_frame_enabled = true now to see if that’s related.

          1. Marshall Greenblatt

            OK, thanks for the confirmation. I’m getting the same behavior with my external_begin_frame_enabled = true implementation. I’ll update to the newest Chromium version and try again after that.

  18. Masako Toda

    I’m observing strange memory usage when opening dev tools. I modified cefmixer a bit to show dev tools https://github.com/masakotoda/cef-mixer/commit/4db95e2d11c617a818e754dc88ecb4c9db99518e

    Run cefmixer.exe https://www.youtube.com/embed/R3AKlscrjmQ?autoplay=1 , maximize the window (1920x1137 in my case), and let it play for 15 minutes.

    • When I’m not opening dev tools, the render process’s memory usage is about 250MB after 15 minutes.
    • When I’m opening dev tools, the render process’s memory usage keeps going up and it’s about 900MB after 15 minutes. At this point, close dev tools and wait for 10 seconds. The memory usage goes down to ~250MB.

    Can anyone reproduce this?

    1. Masako Toda

      I’m afraid my comment is buried… Anyone has issue with devtools? I should also try with cefclient but my environment is a bit hard to do it right now…

        1. Muazin Mugadr

          I get the same issues as @Masako Toda but with the remote debugging dev tools. I have been developing my UI and experimenting with changing the HTML and CSS of my UI (in the remote debugging view). When implementing some changes in the code I noticed the IDE getting very slow. Task manager revealed, that my application was using 26gb RAM. Without using dev tools / remote debugging I can let it run forever without any noticeable change in memory usage. I have not experienced the same with the old non-accelerated painting.

          1. Muazin Mugadr

            After some more tries, devtools are practically unusable. As soon as you inspect elements for a few times memory skyrockets until it blocks your entire system. I will see if I can profile it.

            1. David Morasz

              I can confirm this behavior and it’s also leaking in the VRAM

              this is how it looks like after I hovered couple of DOM elements in devtool for cirka 10-20 seconds (6 Gb RAM, 3 Gb VRAM usage)

              (yellow column is requested system memory, orange is working set, next to that is dedicated video memory)

              P.S.: This is a great Pull Request btw, I’m combining it with the SendTouch for OSR. This be a game-changer, now rendering silky-smooth UI with proper HTML in realtime stuff is actually viable in higher resolution or element count

  19. Marshall Greenblatt

    I’ve uploaded a patch of the proposed cefclient changes to http://magpcss.net/files/cefclient_1006.patch. The new osr_d3d11_win.[cc\|h] files are a copy/paste of composition.[cpp\|h] and d3d11.[cpp\|h] from cef-mixer with some minor fixes related to style and functionality. The rest is mostly refactoring to support both GL and D3D11 implementations on Windows. If you have a local build of this PR you should be able to apply the patch file with git apply cefclient_1006.patch.

    This patch adds --shared-texture-enabled and --external-begin-frame-enabled command-line flags which can be used in combination with --off-screen-rendering-enabled and --off-screen-frame-rate=XX. It’s currently showing blue output for me and crashing the renderer process as described above at Chromium version 69.0.3464.0. I’ll update it soon and test with the newer Chromium version supported by this PR.

    It’s quite possible that I’m doing something wrong with the D3D11 code. Feedback is welcome 🙂

    1. wesselsga author

      Ok. I have applied the patch applied - and I can get it to work (not seeing any crashes with 3489)

      A couple of things I changed to get past the blue screen:

      void OsrRenderHandlerWinD3D11::SetSize(int width, int height) does not seem to be called at all on startup so the composition object has no dimensions (0x0)

      So, at line 84 in osr_render_handler_win_d3d11.cc I hacked some dimensions in:

      // Create the browser layer.
        browser_layer_ = std::make_shared<BrowserLayer>(
            device_, browser, settings.external_begin_frame_enabled,
            true /* flip */);
      
        // Set up the composition.
        composition_ = std::make_shared<d3d11::Composition>(device_);
        composition_->resize(true, 1280, 720);
        composition_->add_layer(browser_layer_);
      
        browser_layer_->move(0.0f, 0.0f, 1.0f, 1.0f);
      

      flip needs to be true going from ANGLE/GL in chromium to D3D11. and move() takes in normalized 0-1.0 coordinates vs. pixel coordinates.

      Also, when the window is resized SetSize() is called: so I changed to normalized coordinates: (the call to move() here isn’t even necessary when working in normalized units)

      void OsrRenderHandlerWinD3D11::SetSize(int width, int height) {
        CEF_REQUIRE_UI_THREAD();
        composition_->resize(!browser_layer_->send_begin_frame(), width, height);
        browser_layer_->move(0, 0, (float)1.0f, (float)1.0f);
      }
      

      With a static page like google.com there does seem to be some repainting issues on resize … that is - it needs to force a repaint

      1. Marshall Greenblatt

        Thanks, I applied your changes and that fixed the initial display issue.

        With a static page like google.com there does seem to be some repainting issues on resize … that is - it needs to force a repaint

        For resizing we call WasResized() and then Chromium asynchronously resizes the texture. When the texture changes OnAcceleratedPaint() will be called with a new \|shared_handle\| value. Should the Composition only be resized after OnAcceleratedPaint() is called with the new (resized) texture? How can we verify that the new \|shared_handle\| is actually the expected size (for example, if WasResized() is called multiple times and GetViewRect returns different values during resizing)?

        Note that we get around this problem with OnPaint() by passing width/height arguments to that method.

        1. wesselsga author

          Once open_shared_texture() is called - you should be able to query the texture dimensions. The Texture2D class from cef-mixer has width()/height() property methods.

      2. Marshall Greenblatt

        I’m still getting the same renderer process crash (Debug build of 69.0.3489.0) when using --external-begin-frame-enabled . Perhaps there’s something wrong with the timing of how I’m calling SendExternalBeginFrame().

        When not using --external-begin-frame-enabled I don’t get regular display updates (for example, I don’t see the blinking cursor on google.com).

        Is it incorrect to rely on OnAcceleratedPaint() being called at regular intervals by the BeginFrame timer in order to trigger rendering? Here’s my code:

        void OsrRenderHandlerWinD3D11::OnAcceleratedPaint(
            CefRefPtr<CefBrowser> browser,
            CefRenderHandler::PaintElementType type,
            const CefRenderHandler::RectList& dirtyRects,
            void* share_handle,
            uint64 sync_key) {
          CEF_REQUIRE_UI_THREAD();
        
          if (type == PET_POPUP) {
            // Popup support is not implemented.
            return;
          }
        
          browser_layer_->on_paint(share_handle, sync_key);
          Render();
        }
        
        void OsrRenderHandlerWinD3D11::Render() {
          if (!browser_layer_->send_begin_frame()) {
            // Chromium is driving the rendering.
            // Update composition + layers based on time.
            const auto t = (d3d11::time_now() - start_time_) / 1000000.0;
            composition_->tick(t);
          }
        
          auto ctx = device_->immedidate_context();
          swap_chain_->bind(ctx);
        
          // Resize the swap chain to match the composition if necessary.
          swap_chain_->resize(composition_->width(), composition_->height());
        
          // Clear the render target.
          swap_chain_->clear(0.0f, 0.0f, 1.0f, 1.0f);
        
          // Render the scene.
          composition_->render(ctx);
        
          // Present to window.
          swap_chain_->present(browser_layer_->send_begin_frame() ? 0 : 1);
        }
        
        1. wesselsga author

          The difference between cefclient and cefmixer is how SendExternalBeginFrame is used. cef-mixer ticks (or updates time) on the main window thread - and part of that update is to tell chromium we want a frame. OnAcceleratedPaint simply catches the shared handle texture and sets up a D3D11 equivalent object in the cef-mixer app. The last updated texture is then composited on the main thread.

          I guess I have not tried using OnAcceleratedPaint() to drive the rendering.

          1. Marshall Greenblatt

            Interesting. Does it matter what thread D3D11 is initialized/rendered on, provided it’s all done on the same thread or access is protected by a mutex? Since OnAcceleratedPaint is not necessarily called at the desired frame rate (for example, if the GPU can’t keep up, or if the page content is static) does it make more sense to only render when OnAcceleratedPaint is called and consequently we know something has changed?

            1. wesselsga author

              Yes, I was just replying about that …. D3D11 calls to ID3D11DeviceContext should only happen from one thread (there are ways around this, but by default its not thread safe). Basic calls to CreateTexture/OpenSharedResource are ok from any thread since they don’t use the device context. Which is why we get away with opening the shared texture handle in OnAcceleratedPaint - but binding it and using on the ID3D11DeviceContext only happens from the main rendering thread.

              Most D3D11 apps just use the context/swapchain from the main window thread.

              1. Marshall Greenblatt

                Thanks. I’ve updated cefclient to retrieve the size from Texture2D and that improves the resizing behavior. Using OnAcceleratedPaint to drive the rendering (and working past the Renderer assertion) it’s now behaving nicely (blinking cursor, resize, etc.) both with and without --external-begin-frame-enabled . I'm only calling D3D11 functions from the UI thread and that seems to work fine both with and without --multi-threaded-message-loop, so I’ve removed the mutex in FrameBuffer.

                1. wesselsga author

                  It was asked above earlier ... did you try SendExternalBeginFrame without using shared textures? That is, on the normal OnPaint code path?

    2. Marshall Greenblatt

      http://magpcss.net/files/cefclient_1006.patch is now updated to support --external-begin-frame-enabled with both the GL and D3D11 implementations on Windows. I’ve tested the following combination of flags and they all work for me:

      • --off-screen-rendering-enabled (software/GL implementation)
      • --off-screen-rendering-enabled --external-begin-frame-enabled (software/GL implementation + external BeginFrame)
      • --off-screen-rendering-enabled --shared-texture-enabled (hardware/D3D11 implementation)
      • --off-screen-rendering-enabled --shared-texture-enabled --external-begin-frame-enabled (hardware/D3D11 implementation + external BeginFrame)
  20. Marshall Greenblatt

    Looks like there are some whitespace errors in external_textures_1006.patch:

    ... successfully applied (with warnings):
            external_textures_1006.patch:166: trailing whitespace.
            external_textures_1006.patch:173: trailing whitespace.
            }
            external_textures_1006.patch:297: trailing whitespace.
    

    Also, tabs in offscreen_browser_compositor_output_surface.cc and build_gles2_cmd_buffer.py should be replaced with spaces.

  21. Marshall Greenblatt

    Thanks for all the great work on this. This is the remaining work that I see before we can merge:

    1. Add/demonstrate support for popups (PET_POPUP). Maybe this is just a matter of creating a Layer in the client code for the popup contents?
    2. Remove the "experimental" functionality like shared_texture_sync_key (Keyed Mutex) and SendExternalBeginFrame arguments. If we want this functionality we can re-add it as new PR(s) after merge with the necessary test application support.
    1. wesselsga author

      The arguments for SendExternalBeginFrame have been removed as well as keyed mutex functionality. I also fixed the whitespace warnings in the patch file and tabs have been removed from chromium source files.

      I'm currently updating cef-mixer to support popups.

    2. wesselsga author

      The cef-mixer app should now support popups. When a popup is opened, it will create a new layer and center it within the composition (screen). The test app will now also forward mouse clicks to web-based layers.

      1. Muazin Mugadr

        For pop ups will it call OnAcceleratedPaint with a seperate texture the size of the current pop up window? Do you have some high level details about that without having to look through the cef-mixer code?

        1. wesselsga author

          The cef-mixer app creates a new CefClient for the popup - so you get another instance of the Render Handler, in which the OnAcceleratedPaint() notifications will have a new texture the size of the popup.

      2. Jakub Głuszkiewicz

        I checked your popups implementation, and its based on creating new rendering layer from OnBeforePopup method. And yes it works for standard popups,

        But still in browser popups like dropdowns doesnt work, so any webpage with select html tags is not usable,

        look at this sample: https://www.w3schools.com/tags/tryit.asp?filename=tryhtml_select

        when you will try to open dropdown, nothing happens in accelerated way, in software rendering that kind of popups works,

        we tested this PR very very heavy, and we are on production currently. Above issue is only one which we detected, everything else seems to be complete.

        thanks

        1. wesselsga author

          I believe I found the issue - I assume you are using SendExternalBeginFrame? I'll have to make a change to forwared that call to the popup_host_view_. I should have this fixed soon.

          If you’re setup to build locally - you can add the following:

          if (!IsPopupWidget() && popup_host_view_){
            popup_host_view_->SendExternalBeginFrame();
          }
          

          At the end of the function CefRenderWidgetHostViewOSR::SendExternalBeginFrame() in render_widget_host_view.cc if you want a quick fix. You should start getting OnAcceleratedPaint() notifications then for popups (PET_POPUP)

          1. Jakub Głuszkiewicz

            Ohh thats great news! Yes im using SendExternalBeginFrame cuz its perfect method to avoid any frame drops and sync to 60fps,

            Yes i have setup to build cef locally so i definitely will try tommorow to see if it works for me,

            many thanks! cheers

              1. Jakub Głuszkiewicz

                Yes i confirm!! Your fix works like a charm!!!! Thank you buddy, you did so awesome work!

                Only html tag which doesnt work is datalist but its not related to this pr, cuz in general cef doesnt support datalists.

                So, your PR is now absolutly full implemented. NICE JOB!!!

                Shoot me a message when u will visit Poland someday, i owe you really nice dinner!

                cheers!

              2. Muazin Mugadr

                Tried to like your comment but it doesnt seem to be possible. Just wanted to tell you that it also works for me 🙂 The only issue that I also have with OnPaint is the slight delay between the time the popup is made visible using the callback and the update of the texture looks a bit odd.

  22. Muazin Mugadr

    For your interest, before the last update there was an issue that on very high FPS (5000+) OnAcceleratedPaint would suddenly stop. This has also been resolved by the last commit to the PR.

    1. Muazin Mugadr

      When running it for a longer period or with I/O heavy stuff it is not solved. Sometimes the renderer freezes for a few seconds and then renders all the web interaction in fast forward

  23. Muazin Mugadr

    With the recent update I fail to build against Chromium 69.0.3489.0 because the following patch fails:

    Apply external_textures_1006.patch in U:\various\cef_branch\cef_download\chromium\src
    ...
    -------------------------------------------------------------------------------
    !!!! ERROR: This patch failed to apply. Your build will not be correct.
    -------------------------------------------------------------------------------
    
    1. wesselsga author
      error: gpu/command_buffer/service/external_texture_manager.cc: already exists in working directory
      error: gpu/command_buffer/service/external_texture_manager.h: already exists in working directory
      

      Seems like you’re trying to re-apply the patch since these files already exist?

      I just did it here locally against latest CEF master and it worked.

  24. Alex Van Camp

    Just wanted to pop in and say I am very grateful for all the hard work going into this PR. My job is making web-based broadcast graphics, so this PR will have a substantial impact on the quality of work that both myself and my peers are able to produce. It is not an understatement to say that my entire career relies on this kind of thing. Thank you very much for making this, and thank you Marshall for maintaining CEF and putting in the work to get this merged. It is very much appreciated by many folks in the broadcast industry.

    1. Jakub Głuszkiewicz

      Its hard to disagree. This PR and touch_osr PR are most valuable pathes ever. Its so hard to say how gratefull i am for your work guys. CEF project itself is amazing, but OSR mode with this path is just a killer!

  25. Muazin Mugadr

    Do you happen to know if this is related to this pull request:
    [0731/194725.280:ERROR:latency_info.cc(144)] Surface::TakeLatencyInfoFromFrame, LatencyInfo vector size 102 is too big.

    Happens when my application is running for a while and then nothing is rendered anymore.

  26. vivien anglesio

    Hi,

    First thanks a lot for that work ! It unlocks tons of possibilities.

    I’m currently using this PR on a project and I’m facing an issue with videos. The issue is reproductible with the cefmixer project too.

    http://dl3.webmfiles.org/big-buck-bunny_trailer.webm
     >>> the video is not rendered unless you stop or start playing it.

    https://www.youtube.com/watch?v=eqihIOSdseY
     >>> the video is not rendered unless you stop or start playing it.
     >>> others elements on the page are rendered correctly

    https://webglsamples.org/video/video.html

    In this page the polygons are 3D objects rendered in a canvas with a video texture applied on it.
     >>> the video texture is not rendered smoothly.
     >>> polygons are moving normaly and smoothly.

    I also tried with CEF built with proprietary codecs enabled and it does the same thing with mp4 videos

    Anyone can reproduce this ?

    1. Hugh Bailey

      Yes – it appears to happen only with off-screen rendering on chromium versions past 3440. Doesn’t appear to happen when not using off-screen rendering. There also appears to be issues with transparency and off-screen rendering. I had to downgrade the chromium version for this PR to 3440 to get it functioning with stability. https://simpl.info/videoalpha/ is another site you can use to reproduce this issue.

      This may be something that should be reported as a normal issue with CEF and/or chromium themselves when using off-screen rendering – not necessarily with the PR itself. (I should really get to that). This should be reproducible with regular CEF as well as long as you’re using off-screen rendering.

      1. vivien anglesio

        Yes sorry I forgot mentioning that it happens in offscreen rendering mode only

        Is it possible to downgrade chromium and to get benefits of this PR at the same time ?

    2. Fred

      Same problem here. On youtube I can ear the audio track of the videos playing, but nothing moves. And when I click to play/pause, it refresh, but immediately freeze again.

  27. wesselsga author

    I created a patch so this pull request can be easily applied to CEF branch 3440. I have reproduced the issues with video identified by @vivien anglesio using chromium 3516 and rolled back to 3440 as recommended by @Hugh Bailey .

    Assuming you’re following the basic build setup from the Master Build QuickStart, modifiy your update.bat to include the 3440 branch of CEF by modifying the args to automate-git.py like the following:

    python ..\automate\automate-git.py <your other arguments> --branch=3440
    

    Then cd chromium_git\chromium\src\cef and apply the shared_textures_3440.patch

    git apply shared_textures_3440.patch
    

    Run your create.bat and build cef as normal

  28. wesselsga author

    FYI, I have ran into the following build issue now on 2 development machines that have been updated to Visual Studio 15.8.1

    C:\Program Files (x86)\Microsoft Visual Studio\2017\Enterprise\VC\Tools\MSVC\14.14.26428\bin\HostX86\x86\pgort140.dll
    

    This error occurs when running create.bat. Looks like vs_toolchain.py in Chromium is scanning the above directory and getting confused because of what looks like remnants from the Visual Studio update installer. I simply work around the issue by manually deleting the 14.15.26428 folder above.

    The new version of that folder is 14.15.26726 for 15.8.1.

    Anyway, just in case someone runs into it.

  29. Fred

    I’m working with the patched 3440 version, and I found out that when working with the release build, if I load https://www.google.com I do not get the first dirtyRect covering the whole page. So I end up with an incomplete page. I do not see this issue with the debug version.

    1. wesselsga author

      Are you doing partial updates based on the dirtyRects? I just tried it in cef-mixer and it worked fine - but that app simply ignores the dirtyRects. I copied the dirty rect calculation from the older frame copy request path … its possible something is off there.

      I would recommend for now just ignoring the rects

      1. Fred

        Actually I’m fetching the pixels of the texture in order to send them to my display device (not a graphic card). This is why I’m intersted in using dirtyRects, it helps speeding up the copy. But for now, as you suggest, I do not use the dirtyRects. Not a big problem, the whole copy is pretty fast anyway.

    2. wesselsga author

      Looking at the code ... it's possible the damage_rect_ member variable is out of sync with the OnAcceleratedPaint callback. For now, I'll probably just pass the full size of the frame and ignore the damage rect.

  30. wesselsga author

    I added another patch for 3497 if anyone is interested.

    For 3497 and 3516 I’ve seen the following errors with DEBUG builds in the First Meaningful Paint Detector (FMP) (this also was reported by @Marshall Greenblatt below)

    Check failed: !swap_stamp.is_null()
    

    I believe the issue should now be resolved by adding feedback.timestamp = base::TimeTicks::Now(); in the OffscreenBrowserCompositorOutputSurface::OnSwapBuffersComplete method.

    The above fix has been made in both the patch for 3497 and 3516 (PR against master). Videos appeared to work in 3497, but are still not working correctly in 3516 (seems like a Chromium issue).

  31. Marshall Greenblatt

    I’ve updated http://magpcss.net/files/cefclient_1006.patch to add support for a PET_POPUP layer in cefclient. I believe this completes the necessary changes to cefclient.

    A few patching issues that I noticed while locally updating my PR build:

    1. external_textures_1006.patch includes files that also exist in compositor_1368.patch (content/browser/compositor/gpu_process_transport_factory.cc and ui/compositor/compositor.h). The compositor_1368 patch file should probably be removed and all the changes included in external_textures_1006.patch instead.
    2. The new files added by external_textures_1006.patch (gpu/command_buffer/service/external_texture_manager.cc and gpu/command_buffer/service/external_texture_manager.h) don’t play nicely with CEF’s patcher tool. We should instead include these files in the CEF repo and modify gpu/command_buffer/service/BUILD.gn to include the files from there. See for example the changes to third_party/blink/renderer/controller/BUILD.gn in component_build_1617.patch. A reasonable directory for the files in CEF might be libcef/browser/gpu.

    From reading above these seem to be the remaining functional issues:

    1. Incorrect |dirtyRects| passed to OnAcceleratedPaint.
    2. Video playback in master not working (possibly a Chromium issue).

    Please add to this list if I’ve missed anything. Thanks.

    1. wesselsga author

      The patch changes for 1 and 2 are now included in the PR. There is also a proposed fix for the |dirtyRects| - currently doing more testing.

  32. vivien anglesio

    Hello, I did some tests with this PR and the patch for 3440.

    The videos are back and playback smoothly in cefmixer. But it seems there is still a big issue with the video rendering pipeline:

    In my case I'm using cef to render a webpage on a video device that is not a Graphic Card and as nothing to do with the display screen rate. This device is outputing frames at it's own rate. When i'm playing videos on this device with cef and not using --disable-gpu-vsync, the video playback seems to have it's own rate based on the screen display of the screen rate rather than my video device rate. The playback of video is very bad and there is a lot of flickering. It's look like the video rendering process doesn't care about the SendExternalBeginFrame calls.

    If there's also animations going along with the video, everything becomes jerky. It's seems that the video process enforce it's rendering rate to the whole page.

    Here is a video I made today of the issue : https://drive.google.com/open?id=1PphjNGuyIdo0idU1Csm_9YgweFSBWu6E

    I hope I'm clear. If you need some other explainations ask me.

    Thanks again for your work, i wish being able to help you but my skills are not good enough...

    1. vivien anglesio

      I think you can reproduce this in cefmixer by setting sync_interval_ to 2 or 3. The videos will start flickering but it’s not obvious because the fps will still be a multiple of your screen refresh rate. In that case the video flickering is not due to a low fps rate, you can compare it with css animations or some webGL rendering to see the difference.

  33. Tammo Hinrichs

    No issue, just a “OMG, thank you guys so very much” from my side. Buttery smooth flawless 60fps @ 4K resolution here, and our clients will be really happy about this :)