segross / UnrealImGui

Unreal plug-in that integrates Dear ImGui framework into Unreal Engine 4.
MIT License
683 stars 216 forks source link

Support for more than just UTexture2D with RegisterTexture() #35

Open Froyok opened 4 years ago

Froyok commented 4 years ago

The TextureManager only handle UTexture2D objects and not Render Target textures (UTextureRenderTarget2D or UCanvasRenderTarget2D for example). Do you think it could be possible to add that support ? Those two class inherit from UTexture and not UTexture2D.

I have plenty of render targets that handle custom effects and I often display them on screen for debug. So being able to display them via ImGui would be very handy.

A workaround could be to use CreateTexture() to convert the render target resource, but that would require to do a copy (and an expensive one).

segross commented 4 years ago

You might be the first person using it... at least the first that I know about. Other then a simple example, I don't use it much myself, just thought it would be good to support it ;) And yes, copying would be a nasty workaround.

Right of the bat, I cannot tell you what exactly will be possible or necessary. If I find a bit of time this week, I will try it. But I can already tell you a few lines that will be deciding.

If you look in the ImGuiWidget.cpp line 664:

const FSlateResourceHandle& Handle = ModuleManager->GetTextureManager().GetTextureHandle(DrawCommand.TextureId);

The resource handle is what I need to pass to Slate API to draw that texture. The texture id from draw command is the same one that you get in a handle after you register your texture, and the same one that you later pass to ImGui functions. That id is used to find a texture entry (FTextureEntry) that was created for your texture.

If you look at the FTextureEntry(const FName& InName, UTexture2D* InTexture, bool bAddToRoot), it creates brush and resource handle. If that resource can be created using other types then I expect it should be an easy modification.

For external functions this constructor is called with bAddToRoot = false through the FTextureManager::CreateTextureResources(const FName& Name, UTexture2D* Texture) function.

segross commented 4 years ago

After a quick look, you could try this:

  1. In the FTextureEntry change UTexture2D to UTexture (constructor and member variable).
  2. Modify FTextureManager::CreateTextureResources(const FName& Name, UTexture2D* Texture) to take UTexture.
  3. Modify FImGuiModule::RegisterTexture(const FName& Name, class UTexture2D* Texture, bool bMakeUnique = false) accordingly (see @ ImGuiModule.h and ImGuiModule.cpp).

If everything compiles and works then this is it.

Froyok commented 4 years ago

So I tried your suggestion (second post) but it's not working, all I'm getting is a #FF00FF colored image.

My guess is because the Slate brush creation fail, I recall having similar issues with UMG/Slate which I solved by creating a rather simple dynamic material instance and feeding it my texture. I then created the brush from the material with the right constructor.

I'm wondering what would be the best course of action here, maybe being able to feed my own Slate brush to the TextureManager instead of letting it create it for me ?

segross commented 4 years ago

How would you create those brushes? Forgive me ignorance but other than in this plugin and occasional customization details, I don't use Slate that often (even less UMG). If all we need are different brushes or creation methods then maybe we could switch that based on a texture type. But ultimately, feeding your own brush could also be an option.

P.S. If you could point me to a simple repro of one of your use cases then I could try it. Are there some engine assets that I could use for test?

Froyok commented 4 years ago

I think a solution could be to simply offer an overload with a UObject as an input instead of Texture2D. This way we could feed any type of resources to the slate brush that you manage internally without having to create it ourselves or be limited to Texture2D resources (thus allowing to suppor UTexture, UMaterialInstanceDynamic, etc).

You can take a look at the SetBrushFromXXX() constructors in the UImage class for some examples.


For the pink image, the problem was on my side : I'm releasing my image handles before ImGui has the time to draw them on screen, leading to the pink result. So my render targets work fine in the end with just the UTexture2D to UTexture change. :)

segross commented 4 years ago

OK, I'll look at those functions.

It seems I'm releasing them before ImGui has the time to draw them on screen.

Oh yes, you cannot do that according to the contract ;)

When it comes to copying resources (I somehow thought that you asked about that but I read your comment again and apparently you did not), it would be something to consider but in general, I'd prefer to avoid that. One reason is that when you have to register and unregister then we are in agreement what is the lifetime of those resources. By managing texture and handle you are in control of those resources. On the other hand, it could be possible to give an option to copy and it would be kept until you ask to remove it by unregistering a handle... so maybe I am over-restrictive here. Just came to that conclusion while writing this reply ;) So this might be something to think about.

I'm not sure I like the idea of using UObject as I hate type-unsafe code, but definitely a few overloads would make sense. And in the worst case, if there is no better option UObject can also do.

segross commented 4 years ago

I tried to pass different objects, like materials, and while they got registered and resource created, the resulting image in ImGui was invisible. Not sure why that happens, I may need to debug it and maybe take a deeper look into the UImage. Did you get it working maybe?

Froyok commented 4 years ago

I confirm that materials work for me, but I made sure the material domain was set to User Interface and not Surface, so maybe that's the reason they don't work for you ?

Also I'm using Material Instance Dynamic objects like this:

// Setup Texture Handle
FImGuiTextureHandle TextureHandle = FImGuiModule::Get().FindTextureHandle( *TextureName );

if( !TextureHandle.IsValid() )
{
    FString Path = "MaterialInstanceConstant'/Game/Editor/Material/INST_RenderTarget_Display.INST_RenderTarget_Display'";

    UMaterialInstanceConstant* Reference = Cast<UMaterialInstanceConstant>(
        StaticLoadObject(
            UMaterialInstanceConstant::StaticClass(),
            nullptr,
            *Path )
        );

    UMaterialInstanceDynamic* Material =
        UMaterialInstanceDynamic::Create( 
            Reference,
            this 
        );

    Material->SetTextureParameterValue( *FString("Texture"), ScriptedTextures[i]->GetTexture() );
    Material->SetScalarParameterValue( *FString("UseAlpha"), 0.0f );

    TextureHandle = FImGuiModule::Get().RegisterTexture( *TextureName, Material );
}