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

Respect hell and hellfire color maps for fully lit and fully dark tiles #7166

Merged
merged 2 commits into from
Jun 25, 2024

Conversation

obligaron
Copy link
Contributor

Fixes #6535

Background

  • The renderer has a optimization for fully lit and fully dark tiles.
    • For fully lit tiles (light table index 0) the color map is ignored and a simple copy of the pixels is made
    • For fully dark tiles (light table index 15) the color map is ignored and all pixels are filled with black.
  • This optimization assumes that the color map always maps to the same color (fully lit) or maps always maps to black (fully dark). This is not true.
    • Hell levels have a color map for fully lit tiles to support the ceiling animation.
    • Hellfire levels there do not have a fully dark color map.
  • In [Issue Report]: Flickering graphics issue walking underneath animated ceiling tiles #6535 one ceiling tile has a dLight value of 0 (fully lit).
    • This happens more often in devilutionX, because the light is updated more often since b7424a0.
    • The bug is more visible when + light radius items are equipped.

Changes

  • Added a helper (IsFullyDark/IsFullyLit) to check if the optimization can be used.
  • Don't use incompatible optimizations for the affected levels.
  • Added an assert to check if the optimizations are correctly enabled/disabled.

Notes

  • We can change the way hellfire levels are handled. Currently they mostly use the color 1 for completely dark tiles. We could change this to pure black (0) and still use the optimization.

}

assert((FullyLitLightTable != nullptr) == (LightTables[0][0] == 0 && std::adjacent_find(LightTables[0].begin(), LightTables[0].end() - 1, [](auto x, auto y) { return (x + 1) != y; }) == LightTables[0].end() - 1));
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you please add a comment about what these asserts are testing

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 opened #7168 for it 🙂

Copy link
Member

@AJenbo AJenbo left a comment

Choose a reason for hiding this comment

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

I didn't think that was actually solvable as I though it was related to the pixels which where incorrectly encoded in the tiles. Nice work :)

@AJenbo AJenbo merged commit 4b74249 into diasurgical:master Jun 25, 2024
22 checks passed
@AJenbo
Copy link
Member

AJenbo commented Jun 25, 2024

@glebm oh, sorry I see I overlapped with your comment/review

@obligaron
Copy link
Contributor Author

I didn't think that was actually solvable as I though it was related to the pixels which where incorrectly encoded in the tiles. Nice work :)

That was my first impression too, but then I discovered that the older version didn't have this problem. And so my chase down the rabbit hole began. 😁

@obligaron obligaron deleted the fullylit branch June 26, 2024 16:20
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.

[Issue Report]: Flickering graphics issue walking underneath animated ceiling tiles
3 participants