Skip to content

Fix black artifacts on models with Nvidia driver#479

Merged
slipher merged 4 commits intoDaemonEngine:masterfrom
slipher:nvidiabone
Jun 17, 2021
Merged

Fix black artifacts on models with Nvidia driver#479
slipher merged 4 commits intoDaemonEngine:masterfrom
slipher:nvidiabone

Conversation

@slipher
Copy link
Copy Markdown
Member

@slipher slipher commented Jun 7, 2021

Fixes #472

slipher added 4 commits June 4, 2021 22:03
The `weights` values calculated by VertexFetch in GLSL seem to end
up being different somehow in the generic shader and the
lightMapping shader under the Nvidia proprietrary driver for
Linux (DaemonEngine#472). Arbitrarily change how the values are extracted in
order to avoid an unfortunate GLSL compilation.
@illwieckz
Copy link
Copy Markdown
Member

I confirm it fixes the bug on Quadro K1100M with Nvidia 340 drivers.

@illwieckz
Copy link
Copy Markdown
Member

illwieckz commented Jun 7, 2021

I'm a bit concerned about how much black magic it is, it may be useful to add comments around the precious working code to prevent unwise modification in the future? and maybe somewhat explain what it does (or provide a less obscure theoretical alternative in a comment) ?

@slipher
Copy link
Copy Markdown
Member Author

slipher commented Jun 7, 2021

how much black magic it is

But it isn't. Really, it's slightly simpler than the previous version. I see no reason that someone would get the idea to change it to exactly what it was before.

@illwieckz
Copy link
Copy Markdown
Member

OK, then.

Maybe we can at least put a comment saying modifying this code may break even with theoretically equivalent algorithms? Something like:

/* Better not change this code without strong reason,
slight variants can lead to unfortunate computation mismatch.
See https://github.com/DaemonEngine/Daemon/issues/472 */

@slipher
Copy link
Copy Markdown
Member Author

slipher commented Jun 15, 2021

Maybe we can at least put a comment saying modifying this code may break even with theoretically equivalent algorithms?

That a theoretically correct change could trigger a compiler bug or something, causing it to not work in practice on some implementations, is true of literally any part of the code. A comment takes a toll on the reader's time, and hence should be worthwhile. I don't think this sort of comment meets that bar, unless there is a single obvious way to do something which is being conspicuously avoided.

@illwieckz
Copy link
Copy Markdown
Member

OK then, I prefer having comments around code that can break just by sneezing but well, that's an opinion.

LGTM.

@slipher slipher merged commit d71c8d0 into DaemonEngine:master Jun 17, 2021
@slipher slipher deleted the nvidiabone branch June 17, 2021 20:32
illwieckz added a commit to illwieckz/Daemon that referenced this pull request Jul 1, 2021
…ne#490 ref DaemonEngine#470 DaemonEngine#479

Code introduced in DaemonEngine#479 fixes bug described in DaemonEngine#470 but broke compatibility
with GLSL 1.20 as described in DaemonEngine#490. This commit brings back the legacy weight
calculation code as a fallback when running GLSL 1.20 GPUs (OpenGL 2.1). The new
code still being used with newer GPUs.
illwieckz added a commit to illwieckz/Daemon that referenced this pull request Jul 1, 2021
…ne#490 ref DaemonEngine#470 DaemonEngine#479

Code introduced in DaemonEngine#479 fixes bug described in DaemonEngine#470 but broke compatibility
with GLSL 1.20 as described in DaemonEngine#490. This commit brings back the legacy weight
calculation code as a fallback when running GLSL 1.20 GPUs (OpenGL 2.1). The new
code is still used with newer GPUs as expected.
illwieckz added a commit to illwieckz/Daemon that referenced this pull request Jul 1, 2021
…ne#490 ref DaemonEngine#470 DaemonEngine#479

Code introduced in DaemonEngine#479 fixes bug described in DaemonEngine#470 but broke compatibility
with GLSL 1.20 as described in DaemonEngine#490. This commit brings back the legacy weight
calculation code as a fallback when running GLSL 1.20 GPUs (OpenGL 2.1). The new
code is still used with newer GPUs as expected.
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.

z-fighting black artifacts on models using an nvidia gpu with nvidia's closed-source driver

2 participants