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

Ensure MovieWriter output is in gamma space when using HDR 2D #92496

Merged
merged 1 commit into from
Aug 19, 2024

Conversation

clayjohn
Copy link
Member

Fixes: #81301
Fixes: #82351 (this is a documentation fix)

Right now when writing out movies they have to be in 8-bit sRGB space. So when using a linear render target we need to convert to gamma space before writing.

To support doing that I had to:

  1. Expose viewport_is_using_hdr_2d. This is not exposed to users as its not a good idea to call this every frame (for Movie Writer we can't avoid constant GPU stalls anyway, so it's fine)
  2. Add Image::linear_to_srgb() this is a direct copy of the existing Image::srgb_to_linear() function. (I generated the table of values using Color::linear_to_srgb())

Since this adds a new function let's wait until 4.4 to merge

@Calinou
Copy link
Member

Calinou commented May 30, 2024

Expose viewport_is_using_hdr_2d. This is not exposed to users as its not a good idea to call this every frame (for Movie Writer we can't avoid constant GPU stalls anyway, so it's fine)

In the long term, we should investigate using some kind of ring buffer to avoid GPU stalls: godotengine/godot-proposals#4751
This could easily make Movie Maker mode twice as fast, particularly once #91263 is merged.

Perhaps HDR 2D status could be checked only once at startup to avoid constant stalls every frame. It's unlikely to change at runtime after all.

@clayjohn
Copy link
Member Author

Perhaps HDR 2D status could be checked only once at startup to avoid constant stalls every frame. It's unlikely to change at runtime after all.

Unfortunately, the three function calls above it also force a rendering thread sync. So the addition of this check doesn't change anything

Copy link
Member

@Calinou Calinou left a comment

Choose a reason for hiding this comment

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

Tested locally, it mostly works as expected.

There's a visible precision loss in the movie output compared to the window output (i.e. what I see in the window). Notice the glow's appearance in the top-left corner having noticeable banding, or even the lighting on the cube.

There's also a slight difference in overall brightness. The project uses Forward+ for reference.

Testing project: test_hdr_2d_movie.zip

Window output

Screenshot_20240531_095522

Movie output before

input00000000

Movie output after this PR

input00000000

@clayjohn
Copy link
Member Author

@Calinou I suspect the difference in output comes from the code we use to convert the RGBA16 texture to RGBA8 (it could be a rounding issue)

@akien-mga akien-mga merged commit f4037d6 into godotengine:master Aug 19, 2024
16 checks passed
@akien-mga
Copy link
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Wrong Colors When 2D HDR Enabled Movie mode recording in 2D HDR is not as good as expected.
4 participants