vvvv / VL.StandardLibs

A collection of standard libraries for vvvv including VL.Stride, VL.Skia, VL.ImGui, msgpack.org[VL]
https://visualprogramming.net
GNU Lesser General Public License v3.0
35 stars 14 forks source link

Feature/VL.ImGui.Stride #667

Closed kopffarben closed 3 months ago

kopffarben commented 4 months ago

PR Details

ImGui.Renderer for Stride

Description

VL.ImGui.Stride.Renderer works exactly like the Skia version (ImGui Region), it only renders the buffers in Stride.

Known problems:

Motivation and Context

Missing so far

Types of changes

Checklist

I moved a couple of functions that are the same in Skia and Stride to VL.ImGui.RenderHelper to have a common codebase.

azeno commented 4 months ago

Great work! Thank you!

Regarding wrong colors: On the CPU side our colors (like stored in pins and IO boxes) are in gamma space. When feeding them to the GPU we need to convert them to the device color space. For that we should add a virtual method virtual System.Numerics.Vector4 ToDeviceColorSpace(Color4) on the internal Context class, which both the Skia and Stride backends can override as needed. We then need to identify where colors get fed into the ImGui API and call that method accordingly beforehand. Once we identified all those places we can be sure that the vertex buffers generated by ImGui will contain colors in the proper color space. No need now to do anything special in the shader.

I'll try to put those thoughts as comments in your changes.

azeno commented 4 months ago

SkiaLayerWidget does not work. That's ok for now. If we figure out a way to connect the existing bridge with it - cool. But would see this is a separate task. What should be doable is a RenderWidget, but again, it's ok if it comes in later.

azeno commented 4 months ago

Just tested your branch myself and it seems my solution also doesn't work - for example the color picker will look completely wrong. Guess it needs to be solved in the shader like you tried initially. See https://github.com/ocornut/imgui/issues/578#issuecomment-1585432977

kopffarben commented 4 months ago

So I found some time to continue working here.

I got quite far. Helppatches and so on have to be made pretty again.

Apart from that I now have a TextureWidget, a RenderWidget and the LayerWidget also works.

I also had to adjust ToSkiaLayer.cs, SkiaWidget.cs and SkiaContext.cs a bit to get the LayerWidget working.

I changed the somewhat strange workaround with the small texture IDs. Now use drawList.AddCallback to pass ILayer from SkiaWidget to ToSkiaLayer SkiaContextet is now used just for Notification So get rid off spooky low IntPtr workaround

known issus: the notification in the layer widget does not work yet,

kopffarben commented 4 months ago

Would be great if you could do a code review

azeno commented 4 months ago

Will have a look at this today. Is it ready to get merged from your end?

kopffarben commented 4 months ago

Most Thinks working and well testet. EntityWidget is WIP ... and Helppatches need to be touched.