-
Notifications
You must be signed in to change notification settings - Fork 15.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: GPU shared texture offscreen rendering #42001
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
# OffscreenSharedTexture Object | ||
|
||
* `textureInfo` Object - The shared texture info. | ||
* `widgetType` string - The widget type of the texture, could be `popup` or `frame`. | ||
* `pixelFormat` string - The pixel format of the texture, could be `rgba` or `bgra`. | ||
* `sharedTextureHandle` string - _Windows_ _macOS_ The handle to the shared texture. | ||
* `planes` Object[] - _Linux_ Each plane's info of the shared texture. | ||
* `stride` number - The strides and offsets in bytes to be used when accessing the buffers via a memory mapping. One per plane per entry. | ||
* `offset` number - The strides and offsets in bytes to be used when accessing the buffers via a memory mapping. One per plane per entry. | ||
* `size` number - Size in bytes of the plane. This is necessary to map the buffers. | ||
* `fd` number - File descriptor for the underlying memory object (usually dmabuf). | ||
* `modifier` string - _Linux_ The modifier is retrieved from GBM library and passed to EGL driver. | ||
nornagon marked this conversation as resolved.
Show resolved
Hide resolved
|
||
* `release` Function - Release the resources. The `texture` cannot be directly passed to another process, users need to maintain texture lifecycles in | ||
main process, but it is safe to pass the `textureInfo` to another process. |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -871,19 +871,33 @@ app.whenReady().then(() => { | |||||
|
||||||
Returns: | ||||||
|
||||||
* `event` Event | ||||||
* `details` Event\<\> | ||||||
* `texture` [OffscreenSharedTexture](structures/offscreen-shared-texture.md) - The GPU shared texture of the frame, when `webPreferences.offscreenUseSharedTexture` is `true`. | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It might make sense to mark this API as I think this is fine for specialist use cases, but may be difficult to guarantee API stability. And marking it as Experimental would give the Electron Upgrades team the ability to move forward without getting blocked on this API potentially breaking in the future. Curious how other @electron/wg-api folks feel about this. |
||||||
* `dirtyRect` [Rectangle](structures/rectangle.md) | ||||||
* `image` [NativeImage](native-image.md) - The image data of the whole frame. | ||||||
|
||||||
Emitted when a new frame is generated. Only the dirty area is passed in the | ||||||
buffer. | ||||||
Emitted when a new frame is generated. Only the dirty area is passed in the buffer. | ||||||
|
||||||
When using shared texture, it is possible to pass texture to other processes through IPC, or handle the event in async handler. | ||||||
It is important to call `texture.release()` when the texture is no longer needed as soon as possible, before the underlying | ||||||
frame pool is drained. By managing the lifecycle by yourself, you can safely pass the `texture.textureInfo` to other processes. | ||||||
|
||||||
```js | ||||||
const { BrowserWindow } = require('electron') | ||||||
|
||||||
const win = new BrowserWindow({ webPreferences: { offscreen: true } }) | ||||||
win.webContents.on('paint', (event, dirty, image) => { | ||||||
// updateBitmap(dirty, image.getBitmap()) | ||||||
const win = new BrowserWindow({ webPreferences: { offscreen: true, offscreenUseSharedTexture: true } }) | ||||||
win.webContents.on('paint', async (e, dirty, image) => { | ||||||
if (e.texture) { | ||||||
// You can handle the event in async handler | ||||||
await new Promise(resolve => setTimeout(resolve, 50)) | ||||||
// importTextureHandle(dirty, e.texture.textureInfo) | ||||||
// You can also pass the `textureInfo` to other processes (not `texture`, the `release` function is not passable) | ||||||
// You have to release the texture at this process when you are done with it | ||||||
e.texture.release() | ||||||
} else { | ||||||
// details.texture will be null when offscreenUseSharedTexture is false | ||||||
// importBitmap(dirty, image.getBitmap()) | ||||||
} | ||||||
}) | ||||||
win.loadURL('https://github.com') | ||||||
``` | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,13 +9,30 @@ function createWindow () { | |
width: 800, | ||
height: 600, | ||
webPreferences: { | ||
offscreen: true | ||
offscreen: true, | ||
offscreenUseSharedTexture: true // or false | ||
} | ||
}) | ||
|
||
win.loadURL('https://github.com') | ||
win.webContents.on('paint', (event, dirty, image) => { | ||
fs.writeFileSync('ex.png', image.toPNG()) | ||
win.webContents.on('paint', async (event, dirty, image) => { | ||
if (event.texture) { | ||
// Import the shared texture handle to your own rendering world. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you're changing the example to use shared textures by default, it should actually use them :) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good suggestion! But like I described above, there's not a valid use purely inside electron, unless we managed to make it work with WebGPU. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there a simple native module we could use (or write) that would do something straightforward like read all the pixels out of the texture? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Of course, if you want to write it for testing I could write a testing node native module to copy the texture to a staging texture and read out the pixel data. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Testing would be the more important path, and I think it's OK to just test one platform at first. For documentation, I think it would be better to leave this example to use the non-shared-texture path by default, perhaps with a comment about shared textures. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK, I will take a look about how to add a test, considering we need to compile (or just use precompiled binary?) it maybe complicated than other tests? It would be best if you could give some suggestions. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We have a couple of native modules in the repo already for testing this sort of thing. see spec/fixtures/native-addon/echo There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How do you suggest referencing a native-addon in spec/fixture to fiddles/feature? How about a link on github indicating how to write a plugin to support this. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think if there's no clear way to demonstrate the feature in a fiddle, then it shouldn't have a fiddle as documentation. It's okay for the feature to be documented in the docs and not have an accompanying fiddle. |
||
// importSharedHandle(event.texture.textureInfo) | ||
|
||
// Example plugin to import shared texture by native addon: | ||
// https://github.com/electron/electron/tree/main/spec/fixtures/native-addon/osr-gpu | ||
|
||
// You can handle the event in async handler | ||
await new Promise(resolve => setTimeout(resolve, 50)) | ||
|
||
// You can also pass the `textureInfo` to other processes (not `texture`, the `release` function is not passable) | ||
// You have to release the texture at this process when you are done with it | ||
event.texture.release() | ||
} else { | ||
// texture will be null when `offscreenUseSharedTexture` is false. | ||
fs.writeFileSync('ex.png', image.toPNG()) | ||
} | ||
}) | ||
win.webContents.setFrameRate(60) | ||
console.log(`The screenshot has been successfully saved to ${path.join(process.cwd(), 'ex.png')}`) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,85 @@ | ||
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 | ||
From: Reito <[email protected]> | ||
Date: Fri, 19 Apr 2024 11:46:47 +0800 | ||
Subject: Remove DXGI GMB keyed-mutex | ||
|
||
This CL is only for reference for a vendor specific change. It is kept for rebasing. | ||
|
||
On CEF/electron, FrameSinkVideoCapturer is used in GPU accelerated OSR, and it can be faster | ||
if we remove the mutex on the shared resource for Windows. | ||
|
||
Maintain this patch in https://crrev.com/c/5465148 | ||
|
||
Change-Id: Id90acd587f17acd228ae7cb5ef93005eb8388b80 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this patch required for this feature? Why is this mutex there in the first place? Why can't this CL be merged upstream? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is not mandatory, but can provide an extra performance gain. Also as commented by OBS teams, texture mutex have performance spikes when acquiring them, and it may cause driver failure in extreme condition or old systems. Also according to the use case by OSR, the texture will not be written to again so a mutex is not requried. It is a tiny patch and I think it can be maintained easily, in general I think is worth to patch to eliminate performance spikes. CEF already accepts this additional patch. Sadly it is the only one patch that not possible to merge into upstream, as OSR is not the only use case in chromium, it was used as GMB (GpuMemoryBuffer) so a mutex is needed in some other code paths. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Okay, that makes sense. Would you mind expanding the patch description with some of the info from this comment? |
||
|
||
diff --git a/gpu/ipc/service/gpu_memory_buffer_factory_dxgi.cc b/gpu/ipc/service/gpu_memory_buffer_factory_dxgi.cc | ||
index 58df3e9ad67b2b7835fe64835f9e589a563ee073..d08d4611361fc1387e94f553ba88c20a8cd87741 100644 | ||
--- a/gpu/ipc/service/gpu_memory_buffer_factory_dxgi.cc | ||
+++ b/gpu/ipc/service/gpu_memory_buffer_factory_dxgi.cc | ||
@@ -180,7 +180,8 @@ gfx::GpuMemoryBufferHandle GpuMemoryBufferFactoryDXGI::CreateGpuMemoryBuffer( | ||
// so make sure that the usage is one that we support. | ||
DCHECK(usage == gfx::BufferUsage::GPU_READ || | ||
usage == gfx::BufferUsage::SCANOUT || | ||
- usage == gfx::BufferUsage::SCANOUT_CPU_READ_WRITE) | ||
+ usage == gfx::BufferUsage::SCANOUT_CPU_READ_WRITE || | ||
+ usage == gfx::BufferUsage::SCANOUT_VEA_CPU_READ) | ||
<< "Incorrect usage, usage=" << gfx::BufferUsageToString(usage); | ||
|
||
D3D11_TEXTURE2D_DESC desc = { | ||
@@ -194,7 +195,9 @@ gfx::GpuMemoryBufferHandle GpuMemoryBufferFactoryDXGI::CreateGpuMemoryBuffer( | ||
D3D11_BIND_SHADER_RESOURCE | D3D11_BIND_RENDER_TARGET, | ||
0, | ||
D3D11_RESOURCE_MISC_SHARED_NTHANDLE | | ||
- D3D11_RESOURCE_MISC_SHARED_KEYEDMUTEX}; | ||
+ static_cast<UINT>(usage == gfx::BufferUsage::SCANOUT_VEA_CPU_READ | ||
+ ? D3D11_RESOURCE_MISC_SHARED | ||
+ : D3D11_RESOURCE_MISC_SHARED_KEYEDMUTEX)}; | ||
|
||
Microsoft::WRL::ComPtr<ID3D11Texture2D> d3d11_texture; | ||
|
||
diff --git a/media/video/renderable_gpu_memory_buffer_video_frame_pool.cc b/media/video/renderable_gpu_memory_buffer_video_frame_pool.cc | ||
index c007238f1c7392042ae91a82f3d70751ba258422..0a65f20f65ed1d7cc140d0a54042fc4a41138774 100644 | ||
--- a/media/video/renderable_gpu_memory_buffer_video_frame_pool.cc | ||
+++ b/media/video/renderable_gpu_memory_buffer_video_frame_pool.cc | ||
@@ -198,7 +198,7 @@ gfx::Size GetBufferSizeInPixelsForVideoPixelFormat( | ||
bool FrameResources::Initialize() { | ||
auto* context = pool_->GetContext(); | ||
|
||
- constexpr gfx::BufferUsage kBufferUsage = | ||
+ gfx::BufferUsage buffer_usage = | ||
#if BUILDFLAG(IS_MAC) || BUILDFLAG(IS_CHROMEOS) | ||
gfx::BufferUsage::SCANOUT_VEA_CPU_READ | ||
#else | ||
@@ -212,13 +212,30 @@ bool FrameResources::Initialize() { | ||
const gfx::Size buffer_size_in_pixels = | ||
GetBufferSizeInPixelsForVideoPixelFormat(format_, coded_size_); | ||
|
||
+#if BUILDFLAG(IS_WIN) | ||
+ // For CEF OSR feature, currently there's no other place in chromium use RGBA. | ||
+ // If the format is RGBA, currently CEF do not write to the texture anymore | ||
+ // once the GMB is returned from CopyRequest. So there will be no race | ||
+ // condition on that texture. We can request a GMB without a keyed mutex to | ||
+ // accelerate and probably prevent some driver deadlock. | ||
+ if (format_ == PIXEL_FORMAT_ARGB || format_ == PIXEL_FORMAT_ABGR) { | ||
+ // This value is 'borrowed', SCANOUT_VEA_CPU_READ is probably invalid | ||
+ // cause there's no real SCANOUT on Windows. We simply use this enum as a | ||
+ // flag to disable mutex in the GMBFactoryDXGI because this enum is also | ||
+ // used above in macOS and CrOS for similar usage (claim no other one will | ||
+ // concurrently use the resource). | ||
+ // https://chromium-review.googlesource.com/c/chromium/src/+/5302103 | ||
+ buffer_usage = gfx::BufferUsage::SCANOUT_VEA_CPU_READ; | ||
+ } | ||
+#endif | ||
+ | ||
// Create the GpuMemoryBuffer. | ||
gpu_memory_buffer_ = context->CreateGpuMemoryBuffer( | ||
- buffer_size_in_pixels, buffer_format, kBufferUsage); | ||
+ buffer_size_in_pixels, buffer_format, buffer_usage); | ||
if (!gpu_memory_buffer_) { | ||
DLOG(ERROR) << "Failed to allocate GpuMemoryBuffer for frame: coded_size=" | ||
<< coded_size_.ToString() | ||
- << ", usage=" << static_cast<int>(kBufferUsage); | ||
+ << ", usage=" << static_cast<int>(buffer_usage); | ||
return false; | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These seem to be placed to the left of the hyphen in other parts of the docs.