VL.StandardLibs icon indicating copy to clipboard operation
VL.StandardLibs copied to clipboard

[VL.ImGui.Stride.Viewports]

Open kopffarben opened this issue 1 year ago • 4 comments

PR Details

Viewports support for VL.Imgui.Stride

Description

This is an attempt to support ImGui viewports in VL.

It already works quite well, there are a few small things that do not work satisfactorily yet.

IssueTracker: https://github.com/kopffarben/VL.StandardLibs/issues

The previous features should all still work. Not 100 percent sure.

The problem with the Skia input should also be fixed. I think Kairos is still working, but I'm not quite sure as I don't know exactly how it should behave.

In general, it could be cleaned up again, I'm also not completely satisfied with the naming of some classes ... but in principle it's good for now. I'm also not quite sure if this is the best solution for some things.

I would be very happy about a code review.

Types of changes

  • [ ] Docs change / refactoring / dependency upgrade
  • [ ] Bug fix (non-breaking change which fixes an issue)
  • [x] New feature (non-breaking change which adds functionality)
  • [ ] Breaking change (fix or feature that would cause existing functionality to change)

kopffarben avatar Apr 16 '24 11:04 kopffarben

Because the window out and in again bug, i.e. losing the input handling, I will also take another close look at it. I'm just relatively busy at the moment. But hopefully I'll get back to it tonight or in the next few days. I think it also needs another cleanup

kopffarben avatar May 23 '24 11:05 kopffarben

@kopffarben My last commit (linked above) seems to have fixed #672 as well. However there was also that boolean flag I had to add as a workaround back in April which I now managed to get rid of by using the same code path for notifications as the normal Skia render path takes (see linked commit in #672, 2b92219b68aa61730b06b8ac38ca5ab6399caaab). I didn't merge it back to main yet because of our discussion further up which is all about that line. You said in docking branch it expects the screen space coordinates in AddMousePosEvent - but then I wonder how this will work together with current approach / approach taken in my new branch where we feed the PositionInWorldSpace which to my understanding takes those Skia viewports into account.

azeno avatar May 23 '24 13:05 azeno

the AddMousePosEvent is only set if ImGui is not in viewport mode. If ImGui is in viewport mode, the screen space mouse position is set every frame in ImGuiWindows.

private void SetPerFrameImGuiData()
{
    unsafe
    { 
        int x, y;
        uint buttons = global::Stride.Graphics.SDL.Window.SDL.GetGlobalMouseState(&x, &y);
        _strideDeviceContext.IO.MouseDown[0] = (buttons & 0b0001) != 0;
        _strideDeviceContext.IO.MouseDown[1] = (buttons & 0b0010) != 0;
        _strideDeviceContext.IO.MouseDown[2] = (buttons & 0b0100) != 0;
        _strideDeviceContext.IO.MousePos = new Vector2(x, y);
    }

    _strideDeviceContext.IO.DisplaySize = new Vector2(mainViewportWindow.Size.X, mainViewportWindow.Size.Y);
    _strideDeviceContext.IO.DisplayFramebufferScale = new Vector2(1.0f, 1.0f);
    
    
    ImGui.GetPlatformIO().Viewports[0].Pos = new Vector2(mainViewportWindow.Position.X, mainViewportWindow.Position.Y);    
    ImGui.GetPlatformIO().Viewports[0].Size = new Vector2(mainViewportWindow.Size.X, mainViewportWindow.Size.Y);
}

So it should not break the current approach

kopffarben avatar May 23 '24 15:05 kopffarben

SO I fixed most of it for now, merged main and the bugfix. The problem with pulling the window in and out still exists.

I lose the input source or the appropriate viewport after dragging in and out a few times. Sometimes it goes faster ... but I don't have enough time right now to investigate it more closely. But I will keep at it.

kopffarben avatar May 23 '24 16:05 kopffarben

SO finally I have it ... Yeahhh

I also had to use the SourceGenerator to get the layout saving of ImGui to work. Does nothing else than bringing the NodeContext in the Window Widget into the Constructor ... so I have a UniqueID for the names of the windows that I can add to the name name##uniqueID

The InputHandling was a really difficult matter ... but now it works completely ... I always capture the correct InputSource ... and the mapping to the SubInputSources always works.

Apart from that, I also handle the commands, perfmeters and so on ... Even the EditMode works, oh yes, I also save the bounds of the main window in the InputPin ... Copy from RenderWindow

Just take a look at Helppatch ... it's almost a full-blown application ... Windows, save layout, edit layout ... , LightMode/DarkMode ... etc.

kopffarben avatar Dec 18 '24 17:12 kopffarben

Finally found some time to have a look. Two observations:

  • Text input doesn't work. Tried in the help patch in the StyleEditor/Colors tab or Log/Filters.
  • When opening the help patch parts of the main window are initially not visible on my screen. Moving the window to the left in order to make them visible seems to trigger some render issue, content is drawn out of place. See screenshot: image

azeno avatar Jan 07 '25 17:01 azeno

Ahh, thought I'd found all the bugs.

The keyboard input doesn't seem to work at all, doesn't work with the LogUI helppatch either. Short debugging shows that the events from Stride arrive but are somehow not passed on to ImGui. I have relatively little time at the moment, I'm at mBox. I'll be able to take a closer look on Friday.

I can't understand the second bug at all, I haven't seen it like this before and unfortunately I can't reproduce it.

kopffarben avatar Jan 08 '25 14:01 kopffarben

mhh don't fix it in Viewport/Docking mode, but fix it in VL.ImGui.Stride Helppatch LogView

kopffarben avatar Jan 13 '25 17:01 kopffarben

Good news, you have fixed the input bug ... I just merged with main and finally it works. But I also saw in the changelog that you did something and wanted to try it.

kopffarben avatar Feb 28 '25 11:02 kopffarben

Ok, managed to merge this PR. Basically everything up to 831f878426ebc6a3f540d24fe860995459d9865f - that one was some sort of merge commit which reverted changes from main. And the two commits later on were only merges as well.

Thanks a lot for this contribution @kopffarben

Edit: For reference, this is the final merge: https://github.com/vvvv/VL.StandardLibs/commit/902b09e3209f0be619d00556a9836daa048f9da2

azeno avatar Mar 03 '25 16:03 azeno