JetBrains / JetBrainsRuntime

Runtime environment based on OpenJDK for running IntelliJ Platform-based products on Windows, macOS, and Linux
GNU General Public License v2.0
1.32k stars 196 forks source link

JBR-5973 Vulkan update #447

Closed YaaZ closed 2 months ago

YaaZ commented 2 months ago
bourgesl commented 2 months ago

Will review it tomorrow

YaaZ commented 2 months ago

OOM strategy implemented as follows:

avu commented 2 months ago

Concerning dynamic rendering, did you check what particular hardware does not support it? For example, this feature is available only in 470.62.07 NVidia drivers.

avu commented 2 months ago

Please have a look at this link https://vulkan.gpuinfo.org/listdevicescoverage.php?extension=VK_KHR_dynamic_rendering Looks like the support for this extension may not be present in some user hardware, so I think we should remove support for dynamic rendering from this pull request. It can be added later as possible optimisation.

YaaZ commented 2 months ago

@avu I just checked the statistics: https://vulkan.gpuinfo.org/displayextensiondetail.php?extension=VK_KHR_dynamic_rendering On Linux it's 47%, which means "well supported on modern HW/drivers"

And this extension is optional, we can work with core render passes as well. I well tested both configurations.

The neccesity for this extension is motivated by dynamic nature of our J2D rendering. We can dynamically specify render pass parameters, like clearing op, which can not be known in advance, as well as removing the need to create separate render pass and framebuffer instances.

YaaZ commented 2 months ago

The same thing with VK_EXT_extended_dynamic_state3: without it we need to compile pipelines multiple times for each alpha composite mode, and this also implies changing pipelines when composite mode is changed. With dynamic states we need much less pipelines and state changes, which is good. But both scenarios are supported and tested, of course.

YaaZ commented 2 months ago

If you are concerned about the increased code complexity these extensions bring, it's in fact very small. Dynamic rendering and blending extensions are meant to replace heavy and verbose core concepts with simpler yet more effective ones. Therefore main burden comes not from supporting the optional extensions, but supporting the default implementation. You can Ctrl+F the "dynamicRendering" and "dynamicBlending" and see it yourself: if we remove these extensions, we would literally get rid of around 25 lines of code for each extension, and also get us rid of more optimal paths of execution.

YaaZ commented 2 months ago

I split the changes into multiple smaller reviews, so I'm closing this one. See https://github.com/JetBrains/JetBrainsRuntime/pull/452