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

Manual docs edits - HLMS block reference guides #233

Merged
merged 6 commits into from Apr 3, 2022

Conversation

oldmanauz
Copy link
Contributor

Start of a PR to add reference documentation for all of the HLMS blocks. Not sure I understand how the manual links work so this commit is just to make sure I'm on the right track.

Start of a PR to add reference documentation for all of the HLMS blocks. Not sure I understand how the manual links work so this commit is just to make sure I'm on the right track.
@oldmanauz
Copy link
Contributor Author

@darksylinc

  1. This PR isn't finalised yet. I just wanted clarification that the changes I made will work as intended. Specifically, adding a new file "HlmsSamplerReference.md" and linking it with "- @subpage hlmssamplerref"
  2. In ogre-next\Docs\src\SettingUpOgre\SettingUpOgreWindows.md, where does the line: @copydoc DownloadingOgreCommon get its data from?
  3. In a sampler block, what do the parameters min_lod and max_lod do?
    Thanks.

@darksylinc
Copy link
Member

darksylinc commented Oct 12, 2021

2. where does the line: @copydoc DownloadingOgreCommon get its data from?

It comes from

@addtogroup DownloadingOgreCommon
@ingroup MdInternal @{
@}

in https://github.com/OGRECave/ogre-next/blob/02d1cc5d21a01f693507ad710d0aed10dfe8b404/Docs/src/SettingUpOgre/SettingUpOgreWindows.md

Later gets reused by the Linux doc.

The exact syntax I don't remember. I always have to consult the Doxygen docs on how it works
I prefer not to use it but it was necessary there to avoid copy paste multiple times for each platform

3. In a sampler block, what do the parameters min_lod and max_lod do?

If the GPU samples the texture and determines it should use e.g. LOD 1 but min_lod is 3, then it will use LOD 3.

Likewise if it wants to use LOD 8 but max lod is 6, then it will use 6

I.e. it gets clamped.

I can't think of any use case for it other than when loading textures progressively, you can limit which LODs are available.

Some Antialiasing techniques may want to limit max lod

This setting gets sent to the API directly. For more info see D3D11 docs (ir Vulkan, Metal's, OpenGL's; it's pretty much the same everywhere).

@oldmanauz
Copy link
Contributor Author

oldmanauz commented Oct 13, 2021

That makes sense. I was reading the Ogre 13 manual to help my understanding and from what I saw, it used "lod" in reference to mesh lods. In the OgreNext context, this didn't make sense to me becuase a sampler block was about defining texture usage.

Few more clarifactions:

  1. Datablock: The roughness value is only used for certain BRDF implementations, not ALL implentations?
  2. Datablock: "Transparency" block > "mode" parameter. In the documentation it references a possible "Refractive" value but in the source code it doesn't seem to allow that to be used?

Added reference guides for
- blendblocks
- datablocks
- macroblocks
- samplerblock
@oldmanauz
Copy link
Contributor Author

@darksylinc How do these commits look to you? Thats all I was going to add for the moment.

@oldmanauz
Copy link
Contributor Author

Added reference guides for Terra and Unlit datablocks.
Some more clarifications:

  1. In the unlit datablock, is diffuse_map0 treated differently to diffuse_map1 > diffuse_map15?
  2. In the unlit datablock, just wanted to confirm that the unlit datablocks will use "blendblock" and "macroblock"?
  3. Just to confirm that the following is an inconsistancy, not a bug:
    The code here in OgreHlmsUnlitDatablock.cpp > Line 62 is used when reading a V1 material file or "HlmsParamVec"? In this code the 0 index is "diffuse_map"
const String c_diffuseMap[NUM_UNLIT_TEXTURE_TYPES] =
{
    "diffuse_map", <-- NO "0" ON THE END
    "diffuse_map1",
    ...
    "diffuse_map15"
};

OgreHlmsJsonUnlit.cpp > line 177: Reads "diffuse_map0" to "diffuse_map15" from the material file. (0 index is "diffuse_map0")
4. In the Terra terrain sample material file, it reads a normalize and autogenerate parameters from the datablock. These aren't read in the HlmsJsonTerra::loadMaterial function. Are they future or depreciated features?

@darksylinc
Copy link
Member

Added reference guides for Terra and Unlit datablocks. Some more clarifications:

1. In the unlit datablock, is `diffuse_map0` treated differently to `diffuse_map1` > `diffuse_map15`?

Yes. It's due to both backwards compatibility and user friendliness.
Originally there was one "diffuse_map" (later came 15 more), and also some users would expect this little change to work out of the box:

// Old
hlms myMaterial pbs
{
  diffuse_map myDiffuse.png
}

// New
hlms myMaterial unlit
{
  diffuse_map myDiffuse.png
}

However with JSON:

  • There is no such backwards compatibility (JSON came much later)
  • Turning a PBS material into Unlit material isn't as straightforward
  • JSON materials are half of the time edited via tools rather than hand text thus this no longer matters to them
2. In the unlit datablock, just wanted to confirm that the unlit datablocks will use "blendblock" and "macroblock"?

Yes. That's the idea: all types of datablocks should share macro- and blendblocks (and samplerblocks too)

OgreHlmsJsonUnlit.cpp > line 177: Reads "diffuse_map0" to "diffuse_map15" from the material file. (0 index is "diffuse_map0") 4. In the Terra terrain sample material file, it reads a normalize and autogenerate parameters from the datablock. These aren't read in the HlmsJsonTerra::loadMaterial function. Are they future or depreciated features?

Looks like a bug. Unlit is supposed to accept both diffuse_map (backwards compatible) diffuse_map0 (new). in HlmsUnlitDatablock::HlmsUnlitDatablock

@oldmanauz oldmanauz changed the title Manual edits - HLMS block references Manual docs edits - HLMS block reference guides Oct 16, 2021
@darksylinc
Copy link
Member

Is this ready for review and merge?

@oldmanauz
Copy link
Contributor Author

oldmanauz commented Oct 27, 2021

Yep. Am I supposed to mark them as such?
Also I think I probably misunderstood you and the commit: "Handle 'diffuse_map0' case in unlit datablock" is unnecessary.

@darksylinc
Copy link
Member

Ah ok!

No need. I wasn't sure if there was more to be done.
I'll take a look and merge it when I get some time

@oldmanauz
Copy link
Contributor Author

Actually, if you have anything to say about these:

  1. Datablock: "Transparency" block > "mode" parameter. In the documentation it references a possible "Refractive" value but in the source code it doesn't seem to allow that to be used. Is "Refractive" a valid mode parameter?
  2. In the Terra terrain sample material file, it has a normalize and autogenerate parameters in the datablock. These aren't read in the HlmsJsonTerra::loadMaterial function. Are they future or depreciated features?

@darksylinc
Copy link
Member

darksylinc commented Nov 12, 2021

In api/html/hlmspbsdatablockref.html it appears malformed Markdown causes everything to become bold:

mode: Type string. Blend mode to use for each detail map. This parameter only affects the diffuse detail map. DEFAULT=NormalNonPremul.** Valid values:
NormalNonPremul: Regular alpha blending.

image

Likewise # Example Samplerblock Definition: {#sbExample} + # Example Macroblock Definition: and co. somehow aren't interpreted as a header/title:

image

@darksylinc
Copy link
Member

darksylinc commented Nov 12, 2021

  1. Datablock: "Transparency" block > "mode" parameter. In the documentation it references a possible "Refractive" value but in the source code it doesn't seem to allow that to be used. Is "Refractive" a valid mode parameter?

That's a bug. We support refractions (although requires special Compositor setup).Samples/2.0/ApiUsage/Refractions shows it in action

  1. In the Terra terrain sample material file, it has a normalize and autogenerate parameters in the datablock. These aren't read in the HlmsJsonTerra::loadMaterial function. Are they future or depreciated features?

I have a lot of code I worked for a client for auto-generating beautiful materials; but I haven't been approved to release it yet (hasn't been in a while... there was a plan but...).
I wanted to reimplement it from scratch but I didn't have time for that either. So it's basically "TODO"

@oldmanauz
Copy link
Contributor Author

@darksylinc I just rebuilt the documents from my PR and I don't have the issues you describe above:

  1. HlmsPBSDatablockRef: The bold tag is closed:
- Block definition:
    - `mode`: Type string. Blend mode to use for each detail map. This parameter only affects the diffuse detail map. **DEFAULT=`NormalNonPremul`.** Valid values:
        - `NormalNonPremul`: Regular alpha blending. 

Picture as built on my machine:
image

  1. I don't have header issue either:

Likewise # Example Samplerblock Definition: {#sbExample} + # Example Macroblock Definition: and co. somehow aren't interpreted as a header/title:

Picture as built on my machine:
image

  1. Possible causes:
  • Doxygen version missmatch? I'm on 1.9.2
  • Your first screen shot has different formating to mine. Is this a browser/OS difference?

@oldmanauz
Copy link
Contributor Author

@eugenegff, @jwwalker, sorry to annoy you both. Could someone test this commit and see if your getting the formatting issues that darksylinc was getting above? Just trying to track down where the problem is.
Thanks.

@jwwalker
Copy link
Contributor

@oldmanauz I'm not sure why you named me, I'm kind of a newbie to Ogre, but I gave it a try... I pulled your changes and ran doxygen, but I can't find the page you're looking at, or even any "Reference Guide" section.

@oldmanauz
Copy link
Contributor Author

@jwwalker I saw that you had been one of the recent active people contributing a PR. In the documentation > Manual > HLMS: High level material system > Blocks menu > The related menu items are circled in red below:

  • HLMS PBS datablock reference > Heading: Parameter: detail_diffuse[X] >Is the bold tag for the text DEFAULT=NormalNonPremul. closing properly?
  • HLMS Macroblock reference > Heading: Example Macroblock Definition: > Is this markdown for the heading being interpreted properly?
    Thanks for looking.
    image

@jwwalker
Copy link
Contributor

@oldmanauz Yes, the 2 things you ask about look OK for me. (I couldn't find them before because my html.cfg was out of date.)

@oldmanauz
Copy link
Contributor Author

@jwwalker thanks a lot for taking a look. I appreciate it.

@oldmanauz
Copy link
Contributor Author

@darksylinc are you interested in this PR still or want me to remove it?

@darksylinc
Copy link
Member

I noticed today this PR hadn't been merged yet. I'll be taking a look tomorrow.

I think I'm gonna merge it and then fix whatever inaccuracy or problem it has as we go.

@oldmanauz
Copy link
Contributor Author

@darksylinc Thanks. I'll look for problems after the merge. Can you please merge #246 as well, but please merge this one first as it will conflict otherwise.

@darksylinc darksylinc merged commit 2d31b5c into OGRECave:master Apr 3, 2022
@oldmanauz oldmanauz deleted the documentation_edits branch April 3, 2022 21:33
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.

None yet

3 participants