Why is Client_AddFontTexture done from a modified back-end and not from user app?

[ocornut]

Is your feature request related to a problem? Please describe.
In your sample, back-ends like imgui_impl_dx11.cpp were modified to add a call to Client_AddFontTexture().

Describe the solution you'd like
It seems like the SampleBasic could just call Client_AddFontTexture() itself, given the data provided by GetTexDataAsRGBA32()?
Not only this would make the call more visible, it would allow not modifying back-ends.
IHMO back-ends should never be modified.
Ditto for the const ImDrawData* why not casting on your end?

[ocornut]

If anything, it seems that the main atlas texture could automatically be updated in NetImgui::EndFrame() the first time, so user doesn't have to upload it manually.

[sammyfreg]

I designed this with the idea that people are already using Dear Imgui and needing few changes to their codebase and I consider imgui_impl_dx11.cpp to be a normal engine sourcefile. I would like to avoid taking ownership of generating the main atlas texture and leave it to the user to notify me of its existence, at the original creation location. Same with any other textures they might want to us.

Its true that you have to know where I added the call though, this is why I use an obvious function name, that's visible in each Sample code, to make it easier to know what's going on, (instead of calling the NetImgui::SendDataTexture directly). But this could be improved, but would still require me to modify the original imgui_impl_dx11.cpp to let the sample handle font creation. My main goal is to make it as easy to use as possible.

I am not sure which change you are refering to, in regard to the const ImDrawData* unless you mean how the GetDrawData returns a const value in the API. This was actually changed in my current code, while I working on integrating NetImgui to Unreal 4. I initially thought there wasn't a need to let user modify the drawdata, but in the end had to leave it non-const. Will be updated later, once I wrap up my Unreal 4 plugin (it is also added to the existing ImGui Unreal plugin, mine just allow remote connection, without local display)

Thank you for your comments, I welcome any suggestions to improve this library, particularly from the author who created the library I'm adding functionality to.

[ocornut]

I consider imgui_impl_dx11.cpp to be a normal engine sourcefile.

It's not, people are encouraged to reuse those back-ends, without modifying them.
NetImgui::EndFrame() could check if Fonts->TexId is set and upload it the first time, you don't need to be triggering the atlas build yourself ?

Texture registration
Generally, for this and other textures, I think the model of explicit registration ahead is flawed. Because large application will have a large number of textures and the ImGui:: end-user shouldn't have to bother about that registration. So maybe it should be done lazily, after the fact (which may requires client to pull them back from GPU first).

My suggestion is that the server should iterate the draw commands, list all unique TexId used and then the server can request them to the emitter.

Emitter could have a single function to perform upload given an ID (find out the pixels etc.).

The client could still:

• explicitly register a texture ahead of time (optional)
• use functions to register a refresh policy (first time, every frame, every second, only when window is focused)
  The server could also have ui to tweak refresh policy.
  Both client and server could also do hit-testing of vertex data to detect when e.g. mouse is hovering at texture, and have refresh policy adapt dynamically (so one policy could be : "only refresh when hovered").

Partial texture update
Another thing to ponder for the mid-term future:
For shadows and dynamic font atlas, we will (I don't know when) transition to a system where dear imgui will be able to notify the back-end that part of a texture needs to upload. So when we'll rasterize new glyphs ideally only a few squares can be reuploaded to GPU instead of the full texture. Similarly ideally we'd only upload part of the texture to the remote NetImgui server.
I don't know when this will be in place but letting you know ahead in case it can have an effect on design.
(as a bonus, high-bar optimization: server can perform partial texture request based on UV coordinates/coverage, so people using mega-textures could still see the part displayed on screen).

Texture id type
Why isn't SendDataTexture() prototype using ImTextureId instead of uint64_t ?

const ImDrawData
I was referring to the fact that I noticed your copy of imgui_impl_dx11 was modified to add const qualifiers to ImDrawData calls.

Not all fully formed thoughts but I hope it can be useful somehow. Great job! :)

[sammyfreg]

I consider imgui_impl_dx11.cpp to be a normal engine sourcefile.

It's not, people are encouraged to reuse those back-ends, without modifying them.

Ah, yes, I can understand your point. I have been coming at it from a game developper perpective, where every engine I have seen use the Dear ImGui source code but not the sample code.

NetImgui::EndFrame() could check if Fonts->TexId is set and upload it the first time, you don't need to be triggering the atlas build yourself ?

I thought that data wasn't available anymore, because the user was expected to call a method to free up that data (once texture has been created). It seems it is no longuer the case. For sure, if I can just look up the data and auto upload it, it would make things more transparent to user. I'll look into it. Thank you.

Texture id type
Why isn't SendDataTexture() prototype using ImTextureId instead of uint64_t ?

An excellent question without a proper awnser. I'll update that :)

const ImDrawData
I was referring to the fact that I noticed your copy of imgui_impl_dx11 was modified to add const qualifiers to ImDrawData calls.

That must have been to fit with my api returning a const ImDrawData object, not liking const_cast and not realizing that other people re-used imgui_impl_dx11.cpp from your source. Once I'm done with my other changes, I'll look into re-importing that file, without any change.

Thank you for the feedbacks.

[ocornut]

On the topic of backends, see: ocornut/imgui#3513

[sammyfreg]

Texture registration
Generally, for this and other textures, I think the model of explicit registration ahead is flawed. Because large application will have a large number of textures and the ImGui:: end-user shouldn't have to bother about that registration. So maybe it should be done lazily, after the fact (which may requires client to pull them back from GPU first).

My suggestion is that the server should iterate the draw commands, list all unique TexId used and then the server can request them to the emitter.

Emitter could have a single function to perform upload given an ID (find out the pixels etc.).

The client could still:

* explicitly register a texture ahead of time (optional)

* use functions to register a refresh policy (first time, every frame, every second, only when window is focused)
  The server could also have ui to tweak refresh policy.
  Both client and server could also do hit-testing of vertex data to detect when e.g. mouse is hovering at texture, and have refresh policy adapt dynamically (so one policy could be : "only refresh when hovered").

How netImgui is made aware of texture is the part I am not satisfied with too, it is not seamless. Reading textures from GPU is a speed bottleneck and Graphic API specific, so non-trivial which is why I took the approach of letting the user handle it for now. In my gaming industry experience, I haven't seen people using textures much (outside RenderTargets preview), which is also why I had not made it a bigger priority. I do like your callback suggestion. Would still have to figure out to handle async update of the data.

[ocornut]

I thought that data wasn't available anymore, because the user was expected to call a method to free up that data (once texture has been created). It seems it is no longer the case.

We do indeed have this function ClearTexData() around but none of the examples have been promoting using it for a few years. So perhaps a few people have called it manually because they dig into the atlas, but I think if we documented it those people wouldn't mind removing that call.

More-over, we can detect that TexID != 0 && TexPixels == NULL and provide an assert/error message that explicitly suggest to not call ClearTexData() if it was called.

More-over, you could still technically support the user manually submitting the texture themselves, like they do it now. The auto-submission would only happens if you know that atlas->TexID was never submitted as a texture.

The expectation is that as we transition to dynamic font atlas support, we will need to preserve the CPU copy of the texture anyway.

Why isn't SendDataTexture() prototype using ImTextureId instead of uint64_t ?
An excellent question without a proper awnser. I'll update that :)

Hmm, maybe there is an answer! If the client is 64-bit and server is 32-bit, or vice-versa, it's probably simpler to ensure the transmitted data are always 64-bits. But I would assume it's possible for the client to always see ImTextureID and internally it is casted to uint64 if needed.

Reading textures from GPU is a speed bottleneck and Graphic API specific, so non-trivial which is why I took the approach of letting the user handle it for now

That's right it's a little tricky. I guess ideally we can support both methods (explicit submission "ahead" if it's easier to read from CPU RAM, and later submission with whichever mechanism the engine has access to).

[sammyfreg]

It'll start with this idea, for auto font upload, I like it.

[sammyfreg]

The good news is that I was able to change the uint64_t -> ImTextureID and implement easily the auto font upload as suggested. Bad news is that my laptop harddrive failed, so I will have to redo the work once I have a working personal PC again :P.

Thankfully the changes are fairly simples.

[sammyfreg]

Submitted changes to dev branch

[ocornut]

Excellent, thank you!