Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rewrite ImGuiScene using TerraFX + Add DX12 backend #1542

Closed
wants to merge 16 commits into from

Conversation

Soreepeong
Copy link
Contributor

@Soreepeong Soreepeong commented Nov 23, 2023

image

Changes

Effective

  • Frequency of heap allocation has been hugely reduced.
  • Memory leak in ImGuiScene has been addressed.
  • Number of supported image file types has been increased.

Code

  • Integrated deps/ImGuiScene into Dalamud in a namespace Dalamud.ImGuiScene, after a rewrite to avoid allocations and use TerraFX instead of SharpDX.
  • Changed image loader from stb_image to WindowsCodecs.
  • Duct taping into InterfaceManager to avoid modifying ImGuiScene submodule can stop.
  • Dx12Win32Scene has been added.
    • You can use Dx12OnDx11Win32Scene to test it out.
  • Incorporates: Add ImDrawList UserCallback support #1526
    • IImGuiScene.SetTexturePipeline, GetTexturePipeline, and ReleaseTexturePipeline has been added. CreateTexturePipeline is on the concrete class for the interface.

Possible future

  • Remove PInvoke.User32, PInvoke.Kernel32, and NativeFunctions, and replace them with TerraFX.Interop.Windows.Windows.

Obsoleting

  • SharpDX.Direct3D11 is pending for deletion; it has been [Obsolete]d in UiBuilder.
    • Expose DevicePtr instead, so that consumer plugins can wrap the native ID3D11Device* into their wrapper library of choice.
  • SharpDX.Mathematics is pending for deletion; it has been [Obsolete]d in VectorExtensions.
    • That's one less autocomplete candidate when you're trying to write Vector3 and stuff..

Notes

ImGui Native Backend cannot be used as-is. ImGui's default DX11 implementation does not push and pop HS/DS/CS in ID3D11DeviceContext, and Win32 implementation deals with IME, which probably will conflict with the game's handling of IME.

@Soreepeong Soreepeong requested a review from a team as a code owner November 23, 2023 09:05
@goaaats
Copy link
Member

goaaats commented Nov 23, 2023

Curious about the note regarding the native DX11 backend, I tried this a while ago and it seemed to be working fine. Was I missing something?

@Soreepeong
Copy link
Contributor Author

Soreepeong commented Nov 23, 2023

image

IME completion window would not show, and when multi-viewport is enabled, outside-the-game windows would get considered as not a valid target for IME input; only type English to those.

For HS/DS/CS, it's more of foolproofing. Realistically the game would bind shaders every frame so it won't matter but just in case.

@Soreepeong Soreepeong force-pushed the ex/terrafx branch 2 times, most recently from fc269d7 to 1f45ea9 Compare November 25, 2023 11:17
@Soreepeong Soreepeong changed the title Rewrite ImGuiScene using TerraFX Rewrite ImGuiScene using TerraFX + Add DX12 backend Nov 25, 2023
@Soreepeong Soreepeong force-pushed the ex/terrafx branch 7 times, most recently from f4e0f66 to 989bb7d Compare November 27, 2023 17:48
@Soreepeong Soreepeong marked this pull request as draft November 29, 2023 03:28
@Soreepeong
Copy link
Contributor Author

Soreepeong commented Nov 29, 2023

DX11 part works fine; it's restructuring of what has been working so far after all. DX12 part, on the other hand, needs more work. On a moderate load of loading/unloading textures, it causes AVE accessing nullptr during Create(Whatever)Resource calls.

Can't reproduce when DX debug layer is turned off. Will test more. Can't get it to crash anymore without the debug layer.

GPU Based Validation (Preview) enabled from DirectX Control Panel was causing the crash. Disabling that made the crash stop happening. Tested with a sample app that repeatedly creates and frees reserved resources.

@Soreepeong Soreepeong force-pushed the ex/terrafx branch 2 times, most recently from 0ea2395 to 9af2b91 Compare November 30, 2023 10:55
@Soreepeong Soreepeong marked this pull request as ready for review November 30, 2023 10:56
@Soreepeong Soreepeong mentioned this pull request Dec 5, 2023
Dalamud.Boot/dllmain.cpp Outdated Show resolved Hide resolved
@Soreepeong Soreepeong force-pushed the ex/terrafx branch 3 times, most recently from a8d2e88 to a86f6f9 Compare December 17, 2023 18:52
@goaaats
Copy link
Member

goaaats commented Jun 6, 2024

@Soreepeong Did you do any tests on how this interacts with reshade while working on it? Same as before?

@Soreepeong
Copy link
Contributor Author

This makes reshade unaware of windows created from using viewports.

@Soreepeong Soreepeong marked this pull request as draft June 7, 2024 04:01
@Soreepeong Soreepeong changed the base branch from master to apiX June 7, 2024 04:02
@goaaats goaaats deleted the branch goatcorp:master July 1, 2024 18:48
@goaaats goaaats closed this Jul 1, 2024
@KazWolfe KazWolfe reopened this Jul 1, 2024
@KazWolfe KazWolfe changed the base branch from apiX to master July 1, 2024 18:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants