Rework the outline library and fix the outstanding issues with it #1628
Conversation
| --FIXME: You can decide if you want this to be a option for users, should be adressed before merge | ||
| local enable_thin_line_workaround = false |
There was a problem hiding this comment.
we do have a performance section in the appearance menu. You could add it there if you like, enabled by default probably
There was a problem hiding this comment.
It really hinges on whether you find the appearance of them being slighty thicker fine, its super subjective but it does look better than the thin line scenarios where it can get cutoff
There was a problem hiding this comment.
you know what, if the performance is mostly similar, chose whatever you believe looks best and set it as default
There was a problem hiding this comment.
no perf difference, it just adjusts the offset by 1 pixel, theres 1 edge case where this looks worse and thats if you have a 1 pixel thick line of geometry on a model being outlined but thats exceptionally rare and even then you'd have to zoom in to notice it, the problem with lines being too thin is a lot more common and noticeable, i'll leave what looks best to you guys in this case because my opinion keeps flipping dependant on which ive been looking at longer so im kinda neutral on it
There was a problem hiding this comment.
to me your discription sounds like it should be enabled
| InitializeCurrentListIfNeeded() | ||
|
|
||
| -- Get List and ListSize for this render type and outline thickness | ||
| local List, ListSize = GetCurrentList(), GetCurrentListSize() |
There was a problem hiding this comment.
variables in our lua style are always lower case - or is here a reason for this decision?
There was a problem hiding this comment.
because of https://wiki.facepunch.com/gmod/list
though we could find another name
There was a problem hiding this comment.
Ah, I see. I'd prefer a different name then. Something like entsList, renderTargetList or so maybe?
There was a problem hiding this comment.
Ah i remember why i did that, theres also a global Lists and ListSize, i used shadowing locals there to make development easier keeping it portable between different parts of the module, but it does break the no uppercase for non globals rule, can adjust that if you still want it
TimGoll
added
type/bug
…ng i to j as thats weird
|
Okay, I tried tackling some of the warnings here and there is one thing I'd like you to change: Once you get that done and the workflow errors are fixed, I can merge this PR |
|
Performance impact should be zero because in the worst case i can localise the global anyways, ill get to changing the names then |
Its finally here, the rework of the outline library which started because of #1411
Ive spent quite some time on this trying over 50 different approaches, running into various setbacks each time.
Some of them got solved: Facepunch/garrysmod-issues#5744
And some didnt: Facepunch/garrysmod-requests#2308
But this is what i consider the best thats possible in gmod today in respect to functionality and performance.
This rework includes(i may have forgotten some, its been a while):
The comments in the code explain a lot so i hope those answer most questions.