Skip to content

renderer/tr_shader: introduce engine-agnostic normalFormat material stage keyword#307

Merged
illwieckz merged 1 commit intoDaemonEngine:masterfrom
illwieckz:normalformat
Apr 18, 2020
Merged

renderer/tr_shader: introduce engine-agnostic normalFormat material stage keyword#307
illwieckz merged 1 commit intoDaemonEngine:masterfrom
illwieckz:normalformat

Conversation

@illwieckz
Copy link
Copy Markdown
Member

@illwieckz illwieckz commented Apr 7, 2020

Introduce engine-agnostic normalFormat material stage keyword.

There is already a normalScale keyword which allows to revert channel by using a negative value, but this is not a way to describe the format of the file that is loaded. So, people would have to use a negative normalScale Y value in their .shader file if the normal map file is stored in DirectX way and the engine is computing normal maps the OpenGL way, but a positive Y normalScale value if file format and engine format matches.

By introducing the normalFormat stage keyword, artist can describe the normal map file format and don't mind about what the engine does. It also means the engine may switch its internal format without having to mind anything, the coder implementing another normal format would know how to convert the normal map file in engine whatever the format, not requiring him to ask artists to edit every existing .shader material file.

It was possible to make normalScale keyword engine-agnostic but that would be a very bad idea because this keyword exist in other engine of this lineage (idtech3 renderer2, OpenJK…, etc.). Then it's better to keep this keyword behaviour the way it works the same as other engine does.

For making things easier to think of, the engine is currently assuming a default normal map format is OpenGL (+X +Y +Z), because it's easier to multiply every channel with 1.0 by default whatever the channel when there is nothing to do.

For backward-compatibility purposes and to keep compatibility with other engines (idtech3 renderer2, XreaL, ET:Legacy, OpenJK, etc.), the normalMap, normalHeightMap keywords and bumpMap deprecated alias set the normal map format to DirectX one. Historically, the Doom 3 engine is an OpenGL engine using DirectX normal map format convention, and XreaL implemented normal maps in order to be able to load Doom3 formats, then expected DirectX normal maps. Engines like those I just quoted are reusing XreaL material stage keywords like normalMap that were Doom3 keywords at first, then are expecting DirectX format. It looks like we would have to live with it.

While the normalMap keyword is expecting normal map in the DirectX format because of backward compatibility with historical engines and third-party ones, Dæmon-based games developers are free to encourage the use of normal map using OpenGL format, maybe use contribution guidelines asking for normal map using OpenGL format explicitely. In such case contributors would just have to add the required line to tell the normal map is using OpenGL format.

The material stage keyword syntax is:

OpenGL format:

normalMap /textures/castle/brick_n
normalFormat X Y Z

DirectX format (normalFormat keyword is not required since nomalMap keyword already does it):

normalMap /textures/castle/brick_n
normalFormat X -Y Z

Weird format (like the one used in Vega texture set):

normalMap /textures/castle/brick_n
normalFormat -X -Y Z

With this work, we may one day change the normal map format used in GLSL code (see #274) without having to mind anything: the material parser is telling the engine which conversion to do, and the assets would not have to be edited if they use the normalFormat keyword.

The Vega map and texture set is expected to use the normalFormat keyword to fix normal map orientation in upcoming Unvanquished 0.52.

With such design we would be able to add support for alternative material syntax from other games to load normal maps, in such case, the keyword parsing code would just have to set the normal map format this syntax expects. The Darkplaces compatibility layer already does it this way.

Here are the default normal map formats used in engine:

  • C++ engine: OpenGL
  • XreaL normalMap keyword: DirectX
  • Darkplaces _norm sidecar autoloading: OpenGL
  • GLSL renderer as a black box: OpenGL
  • GLSL code itself: DirectX (a hack reverts the Y channel on input, we may edit the code to avoid the Y reversion hack)

@illwieckz
Copy link
Copy Markdown
Member Author

This implementation or another one is required for Unvanquished 0.52 because Vega normal maps have to be fixed in a way we don't have to edit materials in the future again. Editing the pictures themselves must be avoided to not uselessly increase the git repository size.

@illwieckz illwieckz added this to the unv/for-0.52.0 milestone Apr 8, 2020
@illwieckz illwieckz added A-Renderer T-Feature-Request Proposed new feature labels Apr 8, 2020
@illwieckz
Copy link
Copy Markdown
Member Author

illwieckz commented Apr 11, 2020

After some deep thoughts, this is the syntax:

normalFormat X Y Z
normalFormat X -Y Z

Even if I have no plan to implement it in engine for normal maps, this syntax allows to implement swizzling. I would like to implement swizzling for PBR maps (physicalFormat R M O, physicalFormat O R M, etc.) so this syntax would be consistent with other format keywords. We can also imagine heightFormat -H to tell black is high. So I feel confident with this syntax. The remaining alternative syntax I would not be against would be XYZ and X-YZ in one word. I would prefer one word for the whole format, but the - does not make me happy. Material parsing is usually case-insensitive so it may be confusing to make XYZ and XyZ not meaning the same (because that was another nice idea).

@slipher
Copy link
Copy Markdown
Member

slipher commented Apr 12, 2020

For files that provide only X and Y channels and infer Z, what does it mean to put -Z? Does it do anything?

@illwieckz
Copy link
Copy Markdown
Member Author

Hmm, Right! Z must only be modified with normalFormat if Z is loaded from file. But it still makes sense to modify Z with normalScale or r_normalScale. Hmm, that may make code complicated.

@illwieckz
Copy link
Copy Markdown
Member Author

Well, because of all those corner cases requiring a better source code to be targeted properly, I had to do the refactorization I was first hoping to be able to postpone for after the next release. So this PR is now built atop #317.

@illwieckz
Copy link
Copy Markdown
Member Author

New code will not apply custom format transformation on Z if Z is reconstructed from X and Y, but will still apply custom scale on Z.

@illwieckz illwieckz force-pushed the normalformat branch 3 times, most recently from 86854ac to 7041894 Compare April 14, 2020 18:34
@illwieckz
Copy link
Copy Markdown
Member Author

Just rebased.

@illwieckz
Copy link
Copy Markdown
Member Author

I forgot to tell it before, but that new code based on the recent factorization (#317) sets an UNREACHABLE_ASSERT() when there is no normal format set at the end of the processing of a material. This expects future code implementing new syntax sets a normal format explicitely, without assumption.

@illwieckz
Copy link
Copy Markdown
Member Author

I also added a warning when people use negative value with normalScale, telling normalFormat is recommended to swap channels.

@illwieckz
Copy link
Copy Markdown
Member Author

OK, now everything looks fine to me, I think I addressed all the comments.

I'm just waiting for this to be merged to push the normal map format fixes on vega map and texture repositories.

@slipher
Copy link
Copy Markdown
Member

slipher commented Apr 18, 2020

LGTM

@illwieckz illwieckz merged commit be488cb into DaemonEngine:master Apr 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants