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

Outline library does not behave well with non-screen-sized render targets #1411

Open
nike4613 opened this issue Feb 13, 2024 · 37 comments
Open
Labels
type/bug Something isn't working
Milestone

Comments

@nike4613
Copy link
Contributor

Your version of TTT2 (mandatory)

Leave one of the following:

  • Workshop version from Steam

Describe the bug (mandatory)

When render.RenderView(...) is called with a non-screen-sized render target (like multiplier zoom TFA scopes do), the only thing that is visible in the output is a sliver of outline and whatever is inside the outline. (This makes the above scopes useless when there is an outline, because the rest of the world will appear black.)

To reproduce

Steps to reproduce the behaviour:

  1. Install TFA Base + TFA Insurgency: Shared Parts + at least one weapon which can use multiplier sights (such as the P90)
  2. Spawn in that weapon in TTT, with at least one entity added to outline (my testing was done with this addon, using a fellow traitor)
  3. Open the TFA customization menu (this gets a bit strange-- you have to bind a key in Sandbox, Q -> Utilities -> Keybinds) and enable the PO 4x24P scope (or any other scope that includes Nx zoom in the description; percentage-zoom scopes don't have the issue)
  4. ADS, and observe a black world except for the outline

Expected behaviour

The world should be fully visible through the scope. It'd be nice if the outline renders in the scope, but not necessary.

Context (please provide as much as you can)

  • Collection link of your Server: https://steamcommunity.com/workshop/filedetails/?id=2168680613
  • No errors, client or server
  • I have several bespoke addons to tweak or fix various parts of the game, none of which are currently available anywhere. (One of them "fixes" this problem, though fairly jankily; it's source is included below)

Without fix:
image

With fix:
image

Fix addon:

lua/terrortown/autorun/shared/fix_ttt2_outline.lua:

local render = render
local hook = hook
local cam = cam

if SERVER then
  AddCSLuaFile()
  return
end

if not TTT2 then return end

local cvDisableOutlineScoped = CreateClientConVar("cl_tfa_outline_disable_scoped", "0", true, false)

local is_reload = false
if FixTfaTTT2ReloadTbl then is_reload = true end
FixTfaTTT2ReloadTbl = FixTfaTTT2ReloadTbl or {}

FixTfaTTT2ReloadTbl.rt = FixTfaTTT2ReloadTbl.rt or {}
local rtTbl = FixTfaTTT2ReloadTbl.rt

local copyMat = Material("pp/copy")

print("loading...")

local function RenderOutlines()
  if cvDisableOutlineScoped:GetBool() and TFA.DrawingRenderTarget then
    return
  end

  local origTex = render.GetRenderTarget()

  local tgt, w, h
  if origTex then
    render.PushRenderTarget(nil)

    w, h = ScrW(), ScrH()
    local size = "sz_" .. w .. "_" .. h

    tgt = rtTbl[size]
    if not tgt then
      tgt = GetRenderTargetEx("TTT2_Outline_RT_" .. size, w, h, RT_SIZE_OFFSCREEN, MATERIAL_RT_DEPTH_SEPARATE, 0, CREATERENDERTARGETFLAGS_UNFILTERABLE_OK, IMAGE_FORMAT_RGBA8888)
      rtTbl[size] = tgt
    end

    render.PopRenderTarget()

    render.PushRenderTarget(tgt)
    copyMat:SetTexture("$basetexture", origTex)
    render.SetMaterial(copyMat)
    render.DrawScreenQuadEx(0, 0, origTex:Width(), origTex:Height())
  end

  FixTfaTTT2ReloadTbl.outlineHook()
  if origTex then
    render.PopRenderTarget()

    copyMat:SetTexture("$basetexture", tgt)
    render.SetMaterial(copyMat)
    render.DrawScreenQuadEx(0, 0, w, h)

  end
end

FixTfaTTT2ReloadTbl.RenderOutlinesReplacement = RenderOutlines

local function UpdateHooks()
  local allHooks = hook.GetTable()

  print("Attempting to fix TTT2 outline scope bug...")

  local outlineHook = allHooks["PostDrawEffects"]["RenderOutlines"]
  if outlineHook then
    print("Replacing RenderOutlines hook")
    PrintTable({outlineHook})
    FixTfaTTT2ReloadTbl.outlineHook = outlineHook
    hook.Remove("PostDrawEffects", "RenderOutlines")
    hook.Add("PostDrawEffects", "RenderOutlines", function()
      FixTfaTTT2ReloadTbl.RenderOutlinesReplacement()
    end)
  end
end

if is_reload then
  hook.Add("PreDrawOutlines", "FixTTT2Outline", function()
    hook.Remove("PreDrawOutlines", "FixTTT2Outline")
    UpdateHooks()
  end)
end

hook.Remove("Initialize", "FixTTT2Outline")
hook.Add("Initialize", "FixTTT3Outline", function()
  UpdateHooks()
end)
@TimGoll
Copy link
Member

TimGoll commented Feb 13, 2024

I see you already put a lot of effort into this issue, thank you a lot! I fear I don't know enough about these rendering shenanigangs to give you a good input here. Maybe someone from our team can chime in here? Or @WardenPotato can give us a bit of his knowledge :D

Would it be possible to put your work into a pullrequest for TTT2 that would be a bit less dependent on TFA? For example, you have a convar to enable the fix, wouldn't it work to have the fix always enabled? Or are there any drawbacks to that?

@nike4613
Copy link
Contributor Author

The convar is, at the moment, to revert to an earlier version of the fix, which disables outlines in scopes altogether.

This is a very hacky fix, that has to work around the fact that I can't modify the outline rendering code. I could PR (not right now, possibly tomorrow) a more proper/deeply integrated fix which does the dynamic GetRenderTargetEx thing for the 2 internal buffers of the outline rendering instead of using the effect buffers. Not sure what the performance impact of that change would be though (but I can't imagine it would be worse than this hack...)

@WardenPotato
Copy link
Contributor

I see you already put a lot of effort into this issue, thank you a lot! I fear I don't know enough about these rendering shenanigangs to give you a good input here. Maybe someone from our team can chime in here? Or @WardenPotato can give us a bit of his knowledge :D

Would it be possible to put your work into a pullrequest for TTT2 that would be a bit less dependent on TFA? For example, you have a convar to enable the fix, wouldn't it work to have the fix always enabled? Or are there any drawbacks to that?

Im wondering if the issue does lie within GetScreenEffectTexture then the gmod halo library should also cause this problem, if not then the outline library should more closesly resemble what the halo library does as theyre very similar in effect and code, in the case that the halo lib also breaks this then i'd be inclined to say either TFA is doing something which wouldnt work with base gmod or the gmod halo lib also needs to be fixed.

@TimGoll
Copy link
Member

TimGoll commented Feb 13, 2024

The convar is, at the moment, to revert to an earlier version of the fix, which disables outlines in scopes altogether.

This is a very hacky fix, that has to work around the fact that I can't modify the outline rendering code. I could PR (not right now, possibly tomorrow) a more proper/deeply integrated fix which does the dynamic GetRenderTargetEx thing for the 2 internal buffers of the outline rendering instead of using the effect buffers. Not sure what the performance impact of that change would be though (but I can't imagine it would be worse than this hack...)

Don't feel pressured. If you have time to work on this, it would be highly appreciated. I just want to add that nobody in our current team is fluid in rendering code. So I fear that no one of us could resolve this issue. You're kind of on your own here.

@nike4613
Copy link
Contributor Author

Inspecting the halo code, I suspect it would suffer from the same (or a similar) problem. That would implicate TFA, but having looked at its implementation I don't know of any other approach it could take... Both outline and halo bake an assumption that they always render to the main render target (by way of GetScreenEffectTexture). This, despite the fact that there exist documented APIs that allow arbitrary render targets and implicitly invoke the hook both libraries use.

@WardenPotato
Copy link
Contributor

Yeah renderview has always been kind of a mess. I think a good course of action would be to highlight this as an issue in https://github.com/Facepunch/garrysmod-issues/issues or to make a pull request in https://github.com/Facepunch/garrysmod for the halo lib issue itself. This way we get it fixed for TFA and anyone using renderview while being able to backport whatever fix is applied to the halo lib to our render lib. Im interested to see what solution this more official route takes as dynamic rt's can cause some pretty nasty memory leaks Facepunch/garrysmod-issues#5011

@Histalek Histalek added the type/bug Something isn't working label Feb 13, 2024
@nike4613
Copy link
Contributor Author

nike4613 commented Feb 14, 2024

It looks like halo does not suffer from this problem:
image
That same view with outline instead of halo:
image

It has the same "thinning" effect that outline has with my fix enabled, which is most likely due to the way that TFA scales the render target when drawing the material, but it does not black out the rest of the world like outline does by default.

@nike4613
Copy link
Contributor Author

It seems as though, in outline specifically, the stencil is being respected when copying the full texture back into the effect buffer, and the stencil is not being correctly inverted when drawing the outline itself. My gut tells me that this has to do with TFA allocating its render targets as MATERIAL_RT_DEPTH_SHARED, which uses the "default" depth and stencil buffers, but that doesn't explain why halo is well behaved...

@nike4613
Copy link
Contributor Author

nike4613 commented Feb 14, 2024

More interesting behavior I'm noticing: When MSAA is enabled (set to 8x at the moment) outlines behave even more strangely in scopes with or without my fix, and have some even more interesting stencil artifacts.

Without fix:
image

With fix:
image

Fixing my fix requires the final DrawScreenQuadEx to get its width multiplied by 2, but only when some form of MSAA is enabled (which of course there isn't a function to check). It might be time to bust open RenderDoc.

E: I was able to work around the MSAA problem by reading from the mat_antialias convar.

     render.SetMaterial(copyMat)
-    render.DrawScreenQuadEx(0, 0, w, h)
+    local drawWidth = tgt:Width()
+    if cvAntialias:GetInt() ~= 0 then
+      drawWidth = drawWidth * 2
+    end
+    render.DrawScreenQuadEx(0, 0, drawWidth, tgt:Height())

@nike4613
Copy link
Contributor Author

Some more updates: I've started playing with modifying outline.lua, and it seems like the actual issue is that TFA (sometimes) uses an opaque render target (rather than one with alpha, as the default render target is), and that causes the black-world effect. Forcing TFA to use a non-opaque render target half-fixes the issue with no outline changes required. However, the outlines through the scope are painfully thin, and MSAA still breaks things.

The fix script continues to work with both transparent and non-transparent RTs, and in fact ends up producing slightly thicker outlines.

No MSAA, Non-opaque RT, fix script disabled:
image
No MSAA, Non-opaque RT, fix script enabled:
image
2x MSAA, Non-opaque RT, fix script disabled:
image
2x MSAA, Non-opaque RT, fix script enabled:
image
No MSAA, Opaque RT, fix script disabled:
image
No MSAA, Opaque RT, fix script enabled:
image

For comparison, halo:

No MSAA, Opaque RT:
image
2x MSAA, Opaque RT:
image
No MSAA, Non-opaque RT:
image
2x MSAA, Non-opaque RT:
image

@WardenPotato
Copy link
Contributor

I think i might know whats wrong here, i'll test it a bit later today.
Nice and clear findings btw.

@WardenPotato
Copy link
Contributor

Mhm im not able to reproduce the blackening effect, only the out of place/warped outlines.

@nike4613
Copy link
Contributor Author

Are you rendering to an opaque render target? That's the only place I'm seeing black-world.

TFA uses one of these two RT creation calls, depending on some settings on the weapon:

tgt = GetRenderTargetEx("TFA_RT_ScreenO_" .. size, size, size, RT_SIZE_NO_CHANGE, MATERIAL_RT_DEPTH_SHARED, 0, CREATERENDERTARGETFLAGS_UNFILTERABLE_OK, IMAGE_FORMAT_RGB888)
tgt = GetRenderTargetEx("TFA_RT_Screen_" .. size, size, size, RT_SIZE_NO_CHANGE, MATERIAL_RT_DEPTH_SHARED, 0, CREATERENDERTARGETFLAGS_UNFILTERABLE_OK, IMAGE_FORMAT_RGBA8888)

Only the first one (the one without an alpha channel) exhibits the issue.

@WardenPotato
Copy link
Contributor

TFA_RT_ScreenO_

im using the P90 with the PO 4x24P scope

@WardenPotato
Copy link
Contributor

It would be nice if you could provide a minimal reproduction, the code for these weapons is an absolute mess.

@nike4613
Copy link
Contributor Author

Yeah... I don't have time today, but I'll see if I can get a simple repro tomorrow.

@nike4613
Copy link
Contributor Author

nike4613 commented Feb 16, 2024

@WardenPotato What OS/build are you on? I'm quite consistently seeing the issue on Windows x86-64 (and have a friend who sees the issue on x86) on an Nvidia GPU, even with a custom opaque RT unrelated to TFA.

In the same fixer script I've been using:

FixTfaTTT2ReloadTbl.oprt = FixTfaTTT2ReloadTbl.oprt or GetRenderTargetEx("text_scr_512_opaque", 512, 512, RT_SIZE_NO_CHANGE, MATERIAL_RT_DEPTH_SHARED, 0, CREATERENDERTARGETFLAGS_UNFILTERABLE_OK, IMAGE_FORMAT_RGB888)

local tfaRtMat = CreateMaterial("tfa_rtmaterial", "UnLitGeneric", {["$translucent"] = 1})

local isRenderingSelf = false
hook.Remove("PostDrawEffects", "test render")
hook.Add("PostDrawEffects", "test render", function()
  if isRenderingSelf then return end

  isRenderingSelf = true

  local tgt = FixTfaTTT2ReloadTbl.oprt
  render.PushRenderTarget(tgt)
  render.Clear(0, 0, 0, 255, true, true)

  render.PopRenderTarget()

  copyMat:SetTexture("$basetexture", tgt)
  render.SetMaterial(copyMat)
  render.DrawScreenQuadEx(0, 0, tgt:Width(), tgt:Height())

  render.PushRenderTarget(tgt)

  render.CullMode(MATERIAL_CULLMODE_CCW)

  local cd = {
    ["x"] = 0,
    ["y"] = 0,
    ["w"] = ScrW(),
    ["h"] = ScrH(),
    ["fov"] = 100,
    ["drawviewmodel"] = false,
    ["drawhud"] = false,
    ["znear"] = 0.1,
    ["origin"] = LocalPlayer():GetShootPos() + LocalPlayer():GetViewOffset(),
--    ["angles"] = LocalPlayer():GetRenderAngles()
    -- are there any other settins that are useful?
  }
  --render.Clear(0, 0, 0, 255, true, true)
  render.RenderView(cd)


  render.SetScissorRect(0, 0, 0, 0, false)
  render.PopRenderTarget()

  copyMat:SetTexture("$basetexture", tgt)
  render.SetMaterial(copyMat)
  render.DrawScreenQuadEx(0, 0, tgt:Width(), tgt:Height())

  render.SetMaterial(tfaRtMat)
  render.DrawScreenQuadEx(0, 512, 512, 512)

  isRenderingSelf = false
end)

This creates a custom opaque render target which demonstrates the issue, and also presents the TFA scope texture. Obviously if you are actually using my whole script, make sure to disable the fix.

(Interestingly, this snippet also exposes that the colored overlay the F1 menu puts up only applies to the first view to complete for some reason.)

@WardenPotato
Copy link
Contributor

@WardenPotato What OS/build are you on? I'm quite consistently seeing the issue on Windows x86-64 (and have a friend who sees the issue on x86) on an Nvidia GPU, even with a custom opaque RT unrelated to TFA.

In the same fixer script I've been using:

FixTfaTTT2ReloadTbl.oprt = FixTfaTTT2ReloadTbl.oprt or GetRenderTargetEx("text_scr_512_opaque", 512, 512, RT_SIZE_NO_CHANGE, MATERIAL_RT_DEPTH_SHARED, 0, CREATERENDERTARGETFLAGS_UNFILTERABLE_OK, IMAGE_FORMAT_RGB888)

local tfaRtMat = CreateMaterial("tfa_rtmaterial", "UnLitGeneric", {["$translucent"] = 1})

local isRenderingSelf = false
hook.Remove("PostDrawEffects", "test render")
hook.Add("PostDrawEffects", "test render", function()
  if isRenderingSelf then return end

  isRenderingSelf = true

  local tgt = FixTfaTTT2ReloadTbl.oprt
  render.PushRenderTarget(tgt)
  render.Clear(0, 0, 0, 255, true, true)

  render.PopRenderTarget()

  copyMat:SetTexture("$basetexture", tgt)
  render.SetMaterial(copyMat)
  render.DrawScreenQuadEx(0, 0, tgt:Width(), tgt:Height())

  render.PushRenderTarget(tgt)

  render.CullMode(MATERIAL_CULLMODE_CCW)

  local cd = {
    ["x"] = 0,
    ["y"] = 0,
    ["w"] = ScrW(),
    ["h"] = ScrH(),
    ["fov"] = 100,
    ["drawviewmodel"] = false,
    ["drawhud"] = false,
    ["znear"] = 0.1,
    ["origin"] = LocalPlayer():GetShootPos() + LocalPlayer():GetViewOffset(),
--    ["angles"] = LocalPlayer():GetRenderAngles()
    -- are there any other settins that are useful?
  }
  --render.Clear(0, 0, 0, 255, true, true)
  render.RenderView(cd)


  render.SetScissorRect(0, 0, 0, 0, false)
  render.PopRenderTarget()

  copyMat:SetTexture("$basetexture", tgt)
  render.SetMaterial(copyMat)
  render.DrawScreenQuadEx(0, 0, tgt:Width(), tgt:Height())

  render.SetMaterial(tfaRtMat)
  render.DrawScreenQuadEx(0, 512, 512, 512)

  isRenderingSelf = false
end)

This creates a custom opaque render target which demonstrates the issue, and also presents the TFA scope texture. Obviously if you are actually using my whole script, make sure to disable the fix.

(Interestingly, this snippet also exposes that the colored overlay the F1 menu puts up only applies to the first view to complete for some reason.)

I am on Linux with a 7900XTX but i also have a virtual machine with windows 10 and a GTX 1070, im gonna test this repro on both now.

@TimGoll
Copy link
Member

TimGoll commented Feb 16, 2024

Just wanted to note that is is awesome that you two are tinkering so much with this issue. Thank you!

@WardenPotato
Copy link
Contributor

Alright i can now reproduce the issue on windows and the inconsistencies seem quite similar to previous bugs ive worked on for TTT2, im doing some major maintenance on the outline lib right now to see if that fixes it. Found atleast 1 unreported bug already and fixed it.

@WardenPotato
Copy link
Contributor

After a full day of extensive testing i could sadly not get both linux and windows to play nicely with the same stuff, it keeps being either or and its not suitable for it being conditional on the platform. I will most likely be creating a pull that implements this RT based fix together with some neat optimizations, extra features and rendering fixes i found along the way though, so it wasnt all for nothing :). Thanks again for the clear comparisons. If any of the maintainers could weigh in now what i did was backport the most recent version of the outlines library into TTT2 and made it backwards compatible, this gives improvements like replacing IsLineOfSightClear with just using the depth buffer and being able to define render types and thickness per outline aswell.

And the issue i found with the current outline code is that one of the DrawScreenQuadEx calls is accidentally a double, one of the values was not made negative on accident resulting in the right side of a model having a slightly thinner outline.

Another thing i fixed was the possibility of pp/copy color leaking over and glitching out the entire screen.

And also i found a way to severely minimize that RT memory leak problem to the point its a non issue.

So expect a pull on it soon (the pull will have a ton of details, i just wanna get a grasp now of what the maintainers think while im still working on the outline library).

@Histalek
Copy link
Member

Histalek commented Feb 16, 2024

After a full day of extensive testing i could sadly not get both linux and windows to play nicely with the same stuff, it keeps being either or and its not suitable for it being conditional on the platform.

That's a pity :/ Do you use Proton on linux for gmod or do you run the native linux build? If you are using the native version i'm inclined to sway towards the windows-favoured changes given that the linux users are the minority and on top of that have the option to use Proton. Although this somewhat hurts to say...
Not saying that you need to include whatever you've done in this regard, but rather if you want to contribute (some of) these changes, i would favour the windows ones.

I will most likely be creating a pull that implements this RT based fix together with some neat optimizations, extra features and rendering fixes i found along the way though, so it wasnt all for nothing :). Thanks again for the clear comparisons. If any of the maintainers could weigh in now what i did was backport the most recent version of the outlines library into TTT2 and made it backwards compatible, this gives improvements like replacing IsLineOfSightClear with just using the depth buffer and being able to define render types and thickness per outline aswell.

Sounds awesome! Although i'm not exactly knowledgeable about the outlines library. @TimGoll can probably assess this a lot better than me.

And the issue i found with the current outline code is that one of the DrawScreenQuadEx calls is accidentally a double, one of the values was not made negative on accident resulting in the right side of a model having a slightly thinner outline.

Sounds good to me.

Another thing i fixed was the possibility of pp/copy color leaking over and glitching out the entire screen.

Is this what happens in #1385 ?

And also i found a way to severely minimize that RT memory leak problem to the point its a non issue.

nice!

So expect a pull on it soon (the pull will have a ton of details, i just wanna get a grasp now of what the maintainers think while im still working on the outline library).

Great, thank you very much for your dedication and work on this ❤️

@nike4613
Copy link
Contributor Author

Would it not be reasonable to hide the Windows/Linux differences behind a convar, defaulting to the Windows-compatible behavior?

@WardenPotato
Copy link
Contributor

WardenPotato commented Feb 16, 2024

After a full day of extensive testing i could sadly not get both linux and windows to play nicely with the same stuff, it keeps being either or and its not suitable for it being conditional on the platform.

That's a pity :/ Do you use Proton on linux for gmod or do you run the native linux build? If you are using the native version i'm inclined to sway towards the windows-favoured changes given that the linux users are the minority and on top of that have the option to use Proton. Although this somewhat hurts to say... Not saying that you need to include whatever you've done in this regard, but rather if you want to contribute (some of) these changes, i would favour the windows ones.

I will most likely be creating a pull that implements this RT based fix together with some neat optimizations, extra features and rendering fixes i found along the way though, so it wasnt all for nothing :). Thanks again for the clear comparisons. If any of the maintainers could weigh in now what i did was backport the most recent version of the outlines library into TTT2 and made it backwards compatible, this gives improvements like replacing IsLineOfSightClear with just using the depth buffer and being able to define render types and thickness per outline aswell.

That sounds awesome! Although i'm not exactly knowledgeable about the outlines library. @TimGoll can probably assess this a lot better than me.

And the issue i found with the current outline code is that one of the DrawScreenQuadEx calls is accidentally a double, one of the values was not made negative on accident resulting in the right side of a model having a slightly thinner outline.

That sounds good to me.

Another thing i fixed was the possibility of pp/copy color leaking over and glitching out the entire screen.

Is this what happens in #1385 ?

And also i found a way to severely minimize that RT memory leak problem to the point its a non issue.

nice!

So expect a pull on it soon (the pull will have a ton of details, i just wanna get a grasp now of what the maintainers think while im still working on the outline library).

Great, thank you very much for your dedication and work on this ❤️

On the win/linux thing, i can get it 100% working on linux, and about 90% on windows, so sadly having it be windows only isnt possible, could have been a bit clearer but was keeping it compact as i'll be explaining it in more detail in the pull. Theres some weirdness going on in how the RT's react with eachother on windows that i can only get to happen in linux by deliberately breaking it. Although taking all the changes combined it will be a net positive on all fronts because of the optimisation that will be included in it. I am on native linux, its full of rendering bugs sadly haha a pain ive had to deal with for a long time, though comically 70% of the time its rendering inconsistency was a positive (Many edge cases exist like render.SetLightingMode being instantly changeable on linux while remaining locked during the hook for windows and carrying over to sprites permanently)

An on the #1385 issue, while i dont think its related i am 99% sure i know the cause already, i'll be looking at that after this

PS: the fix i intend on using will work 100% on all platforms! (just wanna make that clear)

@TimGoll
Copy link
Member

TimGoll commented Feb 16, 2024

Sounds awesome! Although i'm not exactly knowledgeable about the outlines library. @TimGoll can probably assess this a lot better than me.

Maybe better that you, but I'm still not really knowledgeable with this. I will try my best though. Really looking forward to this PR!

@WardenPotato
Copy link
Contributor

Alright small update, its almost done, the concern of leaking RT's is now gone thanks to Rubat fixing my issue report Facepunch/garrysmod-issues#5744

Been a bit busy lately but i should have time in the weekend.

@WardenPotato
Copy link
Contributor

Well typing this for the second time as my pc decided it had enough after this journey, but good news, after more debugging than is healthy for your average person ive finally gotten it to work very nicely on windows, linux, nvidia, amd and wine. Getting it to work on any one of these was trivial, getting a universal solution was probably the worst challenge there was, took me debugging individual d3d9 calls to figure it all out. Just got some small tweaks left to do.

20240226231803_1

20240226231803_1

20240226231721_1

@saibotk
Copy link
Member

saibotk commented Feb 26, 2024

Wow, you are insane. Thank you for going above and beyond into debugging this.

Amazing how tracing the actual render calls helped you out here.
We really appreciate your work on this!

Crazy stuff! <3

@EntranceJew
Copy link
Contributor

I'm amazed, I've wasn't expecting anyone to have to bust out RenderDoc, but next time I see something I can't explain it'll be the first thing I do. Thank you so much for dedicating the time to solve this stuff, I know I'd be hopelessly lost trying to take stabs at this myself.

@WardenPotato
Copy link
Contributor

Well my debugging is complete now, and sadly we have to make a decision now because of extremely specific gmod API limitations, these limitations are:

  1. Depth buffers cannot be copied
  2. Depth buffers can only be shared if Anti Alias is off
  3. Opaque render targets can only be additively or subtractively rendered as an overlay
  4. Renderviews have no guarentee of position
  5. The source depth buffer is limited to 192 units, the SSAO depth buffer is not, neither can be used as a standalone depth buffer anyways
  6. Rendering outlines between views will result in them creating an ugly overlap
  7. Requesting a fully resolved SSAO depth texture is a bit expensive

At this points ive written 30 major versions of the outline library each taking a different approach but they always get stuck at 1 of these limitations, so sadly i will have to present the following options to the maintainers:

  1. Disable outline in scopes
  2. Have outlines work for everything but remove the ability to use OUTLINE_MODE_VISIBLE and OUTLINE_MODE_NOTVISIBLE because of depth buffer sharing and copying limitations
  3. Try and get depth buffer copying into gmod by making a request on garrysmod-requests (as i have already done for the rendertarget leaking issue)
  4. Dont use depth buffers negating the performance and visual improvements that would benefit 99.9% of TTT2 users

At this point having written 30 versions of the outline library it makes the most sense to me to just disable outlines in scopes as it will be the neatest solution with current limitations, if you wondered why halo works its because it uses additive and subtractive rendering(this will not work for us as halo doesnt rely on the depth buffer) but that changes the look of outlines entirely.
Given how rare renderview based opaque scopes are and the now insane amount of work that went into debugging this it has come to an end for every conceivable approach out there, sadly i was cheering too soon last message because i was testing in OUTLINE_MODE_BOTH

However this journey was not a waste, i have learned a lot about the source engine in the process having had to debug the entire rendering pipeline twice, once from render target creation down to d3d calls and once from model drawing down to d3d calls.
If theres any questions as to why an approach wouldnt work i can answer them. I'd like to hear from maintainers what solution they'd like to see.

And heres a list of the improvements we're taking with approach 1, which is what i'd prefer:

  1. Performance improvement by no longer tracing every outlined entity
  2. Configurable outline thickness
  3. Way neater rendering of OUTLINE_MODE_VISIBLE and OUTLINE_MODE_NOTVISIBLE by having partial obstruction being smoothly shown instead instead of popping in or out
  4. A fix of the outline being slightly thinner in the bottom
  5. Renderview scopes atleast wont be broken

And well atleast the users of gmod have profited from this aswell by all the wiki edits this has resulted in and the fix of the RT leaks.

@nike4613
Copy link
Contributor Author

I personally think that no. 3 is a good idea regardless of the approach taken for the outline library.

It would be neat to keep the outlines viewable in scopes, but it's not essential. (I actually had a thought that it would be cool to mask the world outlines so that they don't get double outlines, but I don't think that's really viable...)

@WardenPotato
Copy link
Contributor

WardenPotato commented Feb 29, 2024

I personally think that no. 3 is a good idea regardless of the approach taken for the outline library.

It would be neat to keep the outlines viewable in scopes, but it's not essential. (I actually had a thought that it would be cool to mask the world outlines so that they don't get double outlines, but I don't think that's really viable...)

Ive went ahead and created the request, lets hope its picked up as it would give us by far the neatest and best solution, if accepted the outline library could be made into a proper pull request for gmod itself too if no further roadblocks come in the way considering that ive already implemented Rubats feedback into the upcoming version.

@WardenPotato
Copy link
Contributor

Alright so its been a bit, i asked rubat a while ago what the status on accepting new requests is which went try next month, my focus right now will be crash fixes only, and then the stupid fucking server drama reports that have been piling on we should probably do something in the meantime to move along the graphical issues in TTT2.

I think the best course of action for me is to make a PR that includes all the outline improvements i made and to just accept for a bit that its not 100% perfect and go with approach 1 until the request is accepted, at that point i can do another PR to make it compatible with that.

Im requesting input from the maintainers here to make sure this is not gonna result in a closed pr.

@saibotk
Copy link
Member

saibotk commented Apr 5, 2024

That sounds very reasonable!
If we get another opinion eg from @TimGoll then its fine for us.
Thanks for making sure this issue is not getting forgotten ❤️

@TimGoll
Copy link
Member

TimGoll commented Apr 5, 2024

Sounds great to me, thank you! You somewhere said that your improvements include a width parameter for the outline. Did I get that right? If you did that, it would be awesome to be exposed in the outline draw API so it can be used to make lines thicker for higher resolution screens

@WardenPotato
Copy link
Contributor

Sounds great to me, thank you! You somewhere said that your improvements include a width parameter for the outline. Did I get that right? If you did that, it would be awesome to be exposed in the outline draw API so it can be used to make lines thicker for higher resolution screens

Yeah it does, its still a pixel shifted outline though so the higher you put it the worse it will look, but double thickness looks fine.

If gmod had better shader support i would have implemented the Jump Flood Algorithm for outlines.
I also opted for now to not add extra passes for thicker outlines as the complexity scales horribly.
both can be observed in this neat medium article about efficient outlines: https://bgolus.medium.com/the-quest-for-very-wide-outlines-ba82ed442cd9

The current API i have locally looks like function outline.Add(ents, color, mode, render_type, outline_thickness)

@TimGoll
Copy link
Member

TimGoll commented Apr 6, 2024

Ok, I only skimmed through that article and it is quite fascinating! Impressive stuff that I really have no clue about.

And the API looks good. I'm looking forward to playing around with it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/bug Something isn't working
Projects
None yet
Development

No branches or pull requests

6 participants