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

Clarify use of source_color in Shading Language #10119

Merged
merged 1 commit into from
Oct 25, 2024

Conversation

tetrapod00
Copy link
Contributor

@tetrapod00 tetrapod00 commented Oct 20, 2024

  • Rewrite section on using the source_color uniform hint. Content is mostly the same, but is more clear.
  • Rename one instance of "shader hint" to "uniform hint". Both terms are only used in one place, and neither term is used in the engine itself. So this solidifies the name as "uniform hints".
  • Adds a section header for Uniform hints.

The rest of this section (Uniforms) can use some more work. It needs reorganization, more subheaders, and some rewrites for clarity. That will be a separate PR though.

hints for proper sRGB -> linear conversion (i.e. ``source_color``), as Godot's
3D engine renders in linear color space. If this is not done, the texture will
appear washed out.
.. admonition:: Source Color
Copy link
Contributor Author

@tetrapod00 tetrapod00 Oct 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the first use of an admonition note with a custom title in the whole docs repo. I think it makes this more readable, but if this is against some style guide I can change this to use a subheader or standard note block instead.

It renders as something like this:
firefox_gk9XGAXHlj

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll be trying this out whenever a note block could use a title, then. I don't mind reverting such changes back if it proves to be a problem. I think in most cases, a note with a title should be a subsection, though?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think so. I suspect that most notes will be fine without a special title

@AThousandShips AThousandShips requested a review from a team October 22, 2024 11:23
@AThousandShips AThousandShips added enhancement area:manual Issues and PRs related to the Manual/Tutorials section of the documentation topic:shaders labels Oct 22, 2024
Copy link
Member

@clayjohn clayjohn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall this is a great change, I've left a couple small suggestions for your consideration

tutorials/shaders/shader_reference/shading_language.rst Outdated Show resolved Hide resolved
hints for proper sRGB -> linear conversion (i.e. ``source_color``), as Godot's
3D engine renders in linear color space. If this is not done, the texture will
appear washed out.
.. admonition:: Source Color
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me

tutorials/shaders/shader_reference/shading_language.rst Outdated Show resolved Hide resolved
Rewrite section on using the source_color uniform hint. Content is mostly the same,
but is more clear.
Rename one instance of "shader hint" to "uniform hint".
Adds a section header for Uniform hints.
@tetrapod00
Copy link
Contributor Author

Changes applied, plus one minor change to wrap the paragraphs to ~80 characters.

@mhilbrunner mhilbrunner merged commit cb2a53f into godotengine:master Oct 25, 2024
1 check passed
@mhilbrunner
Copy link
Member

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:manual Issues and PRs related to the Manual/Tutorials section of the documentation enhancement topic:shaders
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants