Viz-OSR might be causing some graphic glitches

Issue #2733 open
Masako Toda created an issue

I’m using a custom build - 3770 + viz-osr patch on Windows 10.

  1. Run cefclient with --enable-gpu --off-screen-rendering-enabled
  2. Try to resize the window very slowly.
  3. Notice the entire window gets transformed briefly at some moment. In the attached video, it happens around 00:05, 00:14, and 00:23

Comments (48)

  1. Alex Maitland

    Testing with a 32bit version of cefclient version CEF 76.1.5+gd8a577c+chromium-76.0.3809.87 from spotify and I can reproduce this issue on Windows 10 x64

    Additionally, after further resizing cefclient basically froze, the render process is still active, I can click the Store link in the top corner and navigation happens and rendering resumes. Image attached, log entries below

    [0802/193923.266:WARNING:angle_platform_impl.cc(52)] handleError(8343): Error: 0x00000505, in ../../third_party/angle/src/libANGLE/renderer/d3d/d3d11/Buffer11.cpp, resize:1530. Internal error: 0x00000505: Failed to allocate host memory
    [0802/193923.267:WARNING:angle_platform_impl.cc(52)] handleError(8343): Error: 0x00000505, in ../../third_party/angle/src/libANGLE/Thread.cpp, getValidContext:104. Context has been lost.
    [0802/193923.267:WARNING:angle_platform_impl.cc(52)] handleError(8343): Error: 0x00000505, in ../../third_party/angle/src/libANGLE/Thread.cpp, getValidContext:104. Context has been lost.
    [0802/193923.267:WARNING:angle_platform_impl.cc(52)] handleError(8343): Error: 0x00000505, in ../../third_party/angle/src/libANGLE/Thread.cpp, getValidContext:104. Context has been lost.
    [0802/193923.306:ERROR:gles2_cmd_decoder.cc(17031)] Offscreen context lost via ARB/EXT_robustness. Reset status = GL_UNKNOWN_CONTEXT_RESET_KHR
    [0802/193923.312:ERROR:gles2_cmd_decoder.cc(4822)]   GLES2DecoderImpl: Context reset detected after MakeCurrent.
    [0802/193923.312:ERROR:gpu_service_impl.cc(835)] Exiting GPU process because some drivers can't recover from errors. GPU process will restart shortly.
    [0802/193923.349:ERROR:command_buffer_proxy_impl.cc(107)] ContextResult::kTransientFailure: Shared memory region is not valid
    [0802/194101.375:ERROR:latency_info.cc(149)] Surface::TakeLatencyInfoFromFrame, LatencyInfo vector size 103 is too big.
    

    I'm seeing similar freeze issues in CefSharp when transitioning between browsers in tabs (browsers going from hidden to visible, eventually OnPaint stops being called), might be related.

  2. Alexander Guettler

    Thanks for the info. I am currently busy with the next chromium update, but I will investigate this as soon as I am further done with that, should be this weekend.

  3. Alexander Guettler

    Probably works fine, if we don’t have to deal with strange buffer formats, which as far as I know we don’t. I was working on using content_rect.size() correctly which would handle all those cases (then realized it’s way too much work for what we do here).

    coded_size is the full video frame pixel data, which may depending on a couple of things include non-valid pixels, but I can't think of a case for cef that would result in us running into that.

  4. Alex Maitland

    Testing with cef_binary_78.3.9+gc7345f2+chromium-78.0.3904.108_windows64_client and I’m still seeing very similar problems with GPU Acceleration Enabled, with GPU Compositing disabled everything appears to be rendering ok with the very quick checks I’ve done so far.

    Testing with the following and I see similar issues on resize

    cefclient.exe --cefclient --multi-threaded-message-loop --off-screen-rendering-enabled
    

    Testing with the following and everything appears to be working (not a conclusive test yet)

    cefclient.exe --cefclient --multi-threaded-message-loop --off-screen-rendering-enabled --enable-gpu --disable-gpu-compositing
    

    I’m testing on Windows 10 1093 x64 with DPI 150%

  5. Dmitry Azaraev

    @Marshall Greenblatt @Alexander Guettler

    Unfortunately, I guess, all proposed fixes are still very broken.

    1. Fundamental: CEF API doesn’t differentiate between view size and surface size nor provide size changed event nor image rendered event. We just get OnPaint callback and originally it eventually should have same size as GetViewRect provided. I know this behavior is not set in stone, but it crucial to understand what browser begin resizing and start to painting (with possibility of multiple painting events over different dirty rects, especially during loading).
    2. This change just pass real underlying surface size… but it doesn’t adjust dirty rects. Dirty rects are originated from view, not from surface, as result if you shrink window horizontally - you should see black lines on left side, because underlying surface will not be recreated and view on surface aligned to right side. (Also I guess it may be aligned to bottom, unlike you may think). Even if you will adjust dirty rects - client code still must know where view rect on given surface.
    3. CEF API should work equally (as much as possible) with GPU enabled or disabled. Now this broken completely.

    Related: https://bitbucket.org/chromiumembedded/cef/issues/2833/osr-gpu-consume-cpu-and-may-not-draw

    PS: (However trick with view_size != info->visible_rect.size() anyway looks somehow better, even may be enough.)

  6. Marshall Greenblatt

    The above change causes slightly different, but still incorrect, resize behavior with GPU enabled in 3945 branch. Resize behavior is also currently wrong with GPU disabled in 3945 branch. As mentioned in issue #2869 this is easiest to reproduce with a complex website like amazon.com.

  7. Marshall Greenblatt

    Fix OSR surface sizing on browser resize (fixes issue #2733).

    This includes the following changes: - Update usage of surface IDs to match the Aura implementation from the RWHVAura/Window classes. - Batch CefBrowserHost::WasResized calls to avoid excessive/unnecessary calls to SynchronizeVisualProperties. - Cache the results of CefRenderHandler::GetViewRect after resize and make RWHVOSR::GetViewBounds the source of truth for all size calculations. - Fix bounds calculations in CefVideoConsumerOSR with GPU enabled.

    Known issues: - The size passed to OnPaint may be off by 1 pixel in cases where the device scale factor is not 1 and does not divide evenly into the pixel size. This is due to the inexact conversion from integer pixel size to integer logical size for GetViewRect.

    → <<cset 8c6cc302d066>>

  8. Marshall Greenblatt

    Fix OSR surface sizing on browser resize (fixes issue #2733).

    This includes the following changes: - Update usage of surface IDs to match the Aura implementation from the RWHVAura/Window classes. - Batch CefBrowserHost::WasResized calls to avoid excessive/unnecessary calls to SynchronizeVisualProperties. - Cache the results of CefRenderHandler::GetViewRect after resize and make RWHVOSR::GetViewBounds the source of truth for all size calculations. - Fix bounds calculations in CefVideoConsumerOSR with GPU enabled.

    Known issues: - The size passed to OnPaint may be off by 1 pixel in cases where the device scale factor is not 1 and does not divide evenly into the pixel size. This is due to the inexact conversion from integer pixel size to integer logical size for GetViewRect.

    → <<cset b05ffc446b3a>>

  9. Marshall Greenblatt

    Fix OSR surface sizing on browser resize (fixes issue #2733).

    This includes the following changes: - Update usage of surface IDs to match the Aura implementation from the RWHVAura/Window classes. - Batch CefBrowserHost::WasResized calls to avoid excessive/unnecessary calls to SynchronizeVisualProperties. - Cache the results of CefRenderHandler::GetViewRect after resize and make RWHVOSR::GetViewBounds the source of truth for all size calculations. - Fix bounds calculations in CefVideoConsumerOSR with GPU enabled.

    Known issues: - The size passed to OnPaint may be off by 1 pixel in cases where the device scale factor is not 1 and does not divide evenly into the pixel size. This is due to the inexact conversion from integer pixel size to integer logical size for GetViewRect.

    → <<cset b440f4c9496a>>

  10. Marshall Greenblatt

    The client-facing behavior of the above fix can be summarized as follows:

    1. Client calls WasResized. Additional calls to WasResized are flagged but otherwise ignored until step 3(a).
    2. CEF calls GetViewRect to get the requested size.

      1. If the size has changed then resize the surface. This (eventually) generates a call to OnPaint at the requested size.
      2. If the size has not changed then do nothing.
    3. CEF calls OnPaint.

      1. If we’ve reached the requested size, and if additional calls to WasResized were flagged, then repeat at step 2.

    Performance could potentially be improved further in the future by canceling in-flight frames that we know are incorrectly sized.

  11. Vladislav

    I got an error while trying to build CEF 3945 (b05ffc446b3a).

    ../../cef/libcef/browser/osr/render_widget_host_view_osr.cc(1529,21): error: use of undeclared identifier 'current_view_bounds_'
      if (new_bounds == current_view_bounds_)
    

  12. Czarek Tomczak

    @Vladislaw In “render_widget_host_view_osr.h” add after line 390: gfx::Rect current_view_bounds_;

    Looks like a bug ate it 🙂

  13. Alex Maitland

    The size passed to OnPaint may be off by 1 pixel in cases where the device scale factor is not 1 and does not divide evenly into the pixel size. This is due to the inexact conversion from integer pixel size to integer logical size for

    How is the rounding performed? Up? Down? Nearest integer? In cases where you are trying to detect if the bitmap has rendered at a set size it would be handy if the values were consistently rounded up or down.

    A real world example would be resizing to take a full page screenshot where the bitmap is saved to the clipboard and not displayed directly, detecting when a bitmap of the desired size has been rendered and when resized back to its original display size so updates to the user display can resume.

    Obviously it’s possible to check if the width/height is one less, equal or one greater. It would be nice to just check two values for simplicity.

  14. Marshall Greenblatt

    How is the rounding performed? Up? Down? Nearest integer?

    The rounding is performed by the client when returning a value from GetViewRect. You can see the cefclient implementation as an example (e.g. OsrWindowWin::GetViewRect).

  15. Alex Maitland

    Thanks for the clarification. I read the comment as CEF was performing an internal calculation that resulted in the OnPaint call having a different width/height to what was passed in GetViewRect.

  16. Czarek Tomczak

    Unfortunately this still doesn’t work correctly for me. The width/height in OnPaint is not synchronized with the values provided in GetViewRect. Below are my logs:

    GetViewRect. width=976, height=646
    OnPaint. width=896 height=647
    OnPaint. width=950 height=647
    GetViewRect. width=976, height=646
    GetViewRect. width=976, height=646
    GetViewRect. width=976, height=646
    OnPaint. width=950 height=647

    Testing latest fix with GPU enabled.

  17. Czarek Tomczak

    Also this case:

    GetViewRect. width=1024, height=684
    GetViewRect. width=1024, height=684
    GetViewRect. width=1024, height=684
    OnPaint. width=1 height=1
    OnPaint. width=1024 height=684

    Why is OnPaint called with width=1 height=1 when I’ve never ever provided such size in GetViewRect?

  18. Alex Maitland

    Testing with

    cef_binary_80.0.4+g74f7b0c+chromium-80.0.3987.122_windows64_client
    

    Using the following args

    cefclient --off-screen-rendering-enabled --enable-gpu
    

    Windows 10 x64 1903 with DPI 150%

    Case 1

    • Resize browser window increasing the window size
    • Black border eventually appears
    • Resize browser window decreasing the window size
    • Black border eventually dissapears

    Case 2

    • Maximise window
    • Restore window to normal size
    • Scrollbars are no longer present, content size wasn’t adjusted correctly

  19. Marshall Greenblatt

    @Czarek Tomczak What OS and CEF version are you testing with? Does the problem reproduce with the cefclient sample app?

  20. Denis Declara

    I have had similar issues now, where in my case the application would resize and the page content would however retain the old dimension. The last two commits regarding this bug seems to have made things better (up and including ecefb59) since the app is now correctly resized.

    However, I did noticed that in javascript window.outerWidth and window.outerHeight will now misreport the size in these cases.

    EDIT:
    I have reported this in issue #2889

  21. Czarek Tomczak

    @Marshall Greenblatt Testing on Windows 10. CEF branch 3945 with latest commits. Yes, I can reproduce the issue with cefclient. When resizing I can see black background in the resized area for a noticable periods of time (0.5 - 1.5 sec). As seen in my logs pasted previously, the OnPaint events have the wrong size for some time. Eventually OnPaint event with correct size appears, but it is causing the 0.5-1.5 sec delay that is visually noticable. When running with --enable-gpu the delays are 2-3x longer than when running with gpu disabled.

    Also I want to seperately report completely different resize issues when testing with shared texture support. Please see this comment with screenshots: https://bitbucket.org/chromiumembedded/cef/pull-requests/285/reimplement-shared-texture-support-for-viz/diff#comment-139503269

  22. Czarek Tomczak

    I have tested with latest cef_binary_80.0.7+gd9fd476+chromium-80.0.3987.132 from Spotify and it has the same issues with OnPaint size delayed.

  23. Marshall Greenblatt

    @Czarek Tomczak Some delay during resize is expected because it takes time for the async Chromium surface resize to catch up with the new requested client size. The amount of time required seems to scale with the complexity of the rendered content (e.g. pages that require more layers to render take longer to resize). Are you testing with a Release build?

  24. elad bahar

    we notice 2 issues,
    1 .browser stop filling the view rect in High dpi - this caused due to:

      // Release the resize hold when we reach the desired size.
      if (hold_resize_ && pixel_size == SizeInPixels())
        ReleaseResizeHold();
    

    on high dpi there might be difference of 1 px .. so hold_resize_ never reset. not sure whats is
    the best solution for that.

    2. when calling wasHidden(true) and wasHidden(false) .. on some cases OnPaint stop fire.
    to workaround it, we change to code so when calling wasResize() we force = true.
    and after calling wasHidden(false) we add wasResize() call.

    void CefRenderWidgetHostViewOSR::SynchronizeVisualProperties(
        const cc::DeadlinePolicy& deadline_policy,
        const base::Optional<viz::LocalSurfaceIdAllocation>&
            child_local_surface_id_allocation,
        bool force /*= false*/) {
      SetFrameRate();
    
      const bool resized = ResizeRootLayer(force );
      bool surface_id_updated = false;
     ....
    }
    
    bool CefRenderWidgetHostViewOSR::ResizeRootLayer(bool force /*= false*/) {
      if (!hold_resize_) {
        // The resize hold is not currently active.
        if (SetRootLayerSize(force /* force */)) {
          // The size has changed. Avoid resizing again until ReleaseResizeHold() is
          // called.
          hold_resize_ = true;
          return true;
        }
      } else if (!pending_resize_) {
        // The resize hold is currently active. Another resize will be triggered
        // from ReleaseResizeHold().
        pending_resize_ = true;
      }
      return false;
    }
    

  25. Czarek Tomczak

    @Marshall Greenblatt Tested with Release build. Is it a correct approach to call OnPaint with wrong size when GetViewRect was called previously and provided a different size?

    In my case browser eventually fills area with the correct size, but after some time. Not tested with High DPI.

  26. Vladislav

    The browser stops filling the rectangle in some cases. For example, if I set the size to 346x249 with 150% DPI. It seems this fix solved this issue:

     libcef/browser/osr/render_widget_host_view_osr.cc | 4 ++--
     1 file changed, 2 insertions(+), 2 deletions(-)
    
    diff --git a/libcef/browser/osr/render_widget_host_view_osr.cc b/libcef/browser/osr/render_widget_host_view_osr.cc
    index aa65ab80..6813b61f 100644
    --- a/libcef/browser/osr/render_widget_host_view_osr.cc
    +++ b/libcef/browser/osr/render_widget_host_view_osr.cc
    @@ -1394,8 +1394,8 @@ void CefRenderWidgetHostViewOSR::UpdateFrameRate() {
     }
    
     gfx::Size CefRenderWidgetHostViewOSR::SizeInPixels() {
    -  return gfx::ConvertSizeToPixel(current_device_scale_factor_,
    -                                 GetViewBounds().size());
    +  return gfx::ScaleToCeiledSize(GetViewBounds().size(),
    +                                current_device_scale_factor_);
     }
    
     #if defined(OS_MACOSX)
    

  27. Alex Maitland

    @Marshall Greenblatt Would you prefer to reopen this issue or create a new one for the DPI resizing problem?

  28. Log in to comment