chromiumembedded / cef

Chromium Embedded Framework (CEF). A simple framework for embedding Chromium-based browsers in other applications.
https://bitbucket.org/chromiumembedded/cef/
Other
3.28k stars 458 forks source link

OSR mode with shared textures: pass out partial update information in "OnAcceleratedPaint" #3730

Open markus-neff-bl opened 3 months ago

markus-neff-bl commented 3 months ago

Is your feature request related to a problem? Please describe. We have an application based on CEF that uses accelerated offscreen rendering to combine a CEF rendered GUI with 3D views of the application. To keep performance as high as possible (latency low, fps high), we try to make sure to only render the changed parts. It seems that although respective information (the "damage_rect") is present in "CefVideoConsumerOSR::OnFrameCaptured" and used for the non-accelerated case in "CefRenderHandler::OnPaint", it is currently not handed out in the accelerated case.

Describe the solution you'd like I would like the partial update information to be delivered in the 3rd parameter "dirtyRects" of the "CefRenderHandler::OnAcceleratedPaint" method.

Describe alternatives you've considered Currently I am patching CEF but I assume that others might also be interested in this information.

magreenblatt commented 3 months ago

Currently I am patching CEF but I assume that others might also be interested in this information.

Are you planning to submit your changes as a PR? Thanks.

markus-neff-bl commented 3 months ago

Absolutely - I just wanted to check here first if there is interest in a PR.

reitowo commented 2 months ago

There's no partial rendering when you use GPU accelerated OSR in chromium yet. The dirty rect is just same as full frame size.

However I'm working on electron's port and this time I passed more info. But you can see that there's no difference between these rects.

image

markus-neff-bl commented 2 months ago

@reitowo - I looked at it again and I can't confirm your statement.

For me it clearly gives out reasonable sub-screen damage-rects - in the screenshot you see my debugger attached to a cefclient (built with my small patch) dumping out the "damage_rect" and started with the following params: --off-screen-rendering-enabled --shared-texture-enabled --enable-chrome-runtime --remote-debugging-port=9222. You can see that it shows different sub-screen damage_rects:

image

As far as I can tell they are exactly as expected - for the test I had a fullscreen cefclient window on a 4K display.

But maybe you mean something different than me - for example that chrome itself does keep and render into a full-sized texture. But for me that does not matter. I have to copy updates from the chromium texture to another composition texture and with the proper dirty rects, I can save quite some GPU mem bandwidth (I measured that).

Or on your machine, chromium takes some other rendering path e.g. due to a different GPU setup or you have some other config/flag set that changes the situation - e.g. I noticed that when enabling paint flashing in devtools to see the update regions, the damage_rect is fullscreen.

But as I see real sub-screen damage_rects in a real use-case, I think that the patch still would makes sense.

reitowo commented 2 months ago

I think you're right. Despite you don't show the full code, I assume you use metadata.capture_update_rect as the damage_rect.

image

Since the callback is already passed with an struct, you can try pass everything out if you want. I suggest leave the content_rect untouched because it changes the API behavior silently, an extra structure field would be nice.

It will be nice if you can submit a PR and give everything out like I'm trying to do in electron port. I forgot to do so when I was writing original patch.

OffscreenSharedTextureValue texture;
texture.pixel_format = info->pixel_format;
texture.coded_size = info->coded_size;
texture.visible_rect = info->visible_rect;
texture.content_rect = content_rect;
texture.timestamp = info->timestamp.InMicroseconds();
texture.frame_count = info->metadata.capture_counter.value_or(0);
texture.capture_update_rect = info->metadata.capture_update_rect;
texture.source_size = info->metadata.source_size;
texture.region_capture_rect = info->metadata.region_capture_rect;
texture.widget_type = view_->GetWidgetType();
reitowo commented 1 month ago

Done it for you: https://bitbucket.org/chromiumembedded/cef/pull-requests/801

markus-neff-bl commented 1 month ago

@reitowo

Very cool - thanks a lot! However I have a further comment. I added this as a comment in your pull request. In my eyes the partial update rect should also be sent out in the first parameter of "OnAcceleratedPaint".

Here is my patch - as I only needed the partial update rectangle, I did not add any extra-info. As this still might be needed for other use-cases, I think it is very good that you expose it now, but I really think that the partial update rect belongs also into the first parameter as this is the primary spot in the interface for it.

video_consumer_osr_partial_update_info.patch

reitowo commented 1 month ago

I think thats very true. I'll change that later :)

reitowo commented 1 month ago

I applied your patch, thanks!