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

pointlight: Implement point light. #3580

Closed
wants to merge 0 commits into from
Closed

Conversation

vlj
Copy link
Contributor

@vlj vlj commented Dec 30, 2023

This PR adds point lights support to OpenGL and Vulkan backend for terrain high settings and instanced lit object.

Description

This adds support for per pixel (not lightmap based) point lights that behaves "correctly" with normal data in the scene.

This is a rough implementation of the "Clustered Forward Shading" technic used in forward renderer like warzone renderer.
The idea is to split the view frustrum in small bucket and build for every bucket a list of all lights intersecting this bucket. Then in the fragment shaders, the fragment will query the right bucket depending on the screenspace location.

The assumption is that point lights are often very local, and thus it doesn't make sense to run the light computations on all visible lights for every pixel in the scene.

Due to the demanding nature of the technic it is enabled only for instanced meshes rendering and high details terrain.

For now there is a limit of 128 different "frame" lights, and the accumulated size of bucket light is 512, and there are 64 buckets.

Help needed

Coding

  • I don't know how to handle settings menu. I created 2 lightManagers, a "legacy" one that rely on lightmap like before and a "new" one that do the frustum calculations for consumption by the terrain_combined_high pipeline and the tcmask_instanced pipeline.
  • I have trouble with the CI, it looks like some builders don't see that I added an extra shader file for vulkan, and I don't know how to fix this atm.

Testing

The best map for quick test is the beginning of the beta campaign, with a lot of lights. Note that the first time when the VTOL are bombing the base in this mission, some point light disappear for a brief moment due to the amount of light.

  • After a toggle is introduced in the settings menu (currently I only tested by recompilation), I need tests to see if the legacy codepath still works.
  • I need tests to see if it works on various config, with and without the toggle enabled.

Screenshots

image

@@ -46,8 +46,8 @@
# endif
# if defined(DEBUG)
# define strdup(s) \
strdup2(s,__FILE__,__LINE__)
static inline char *strdup2(const char *s, char *fileName, int line)
strdup2(static_cast<const char*>(s),__LINE__)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

when changing the C++ version of the standard, I discovered that it didn't compile (because type is const char[] vs const char* or something), I reverted back to C++14 but I think this is a safe change

src/input/context.h Outdated Show resolved Hide resolved
@@ -25,6 +25,7 @@
#include <algorithm>
#include <cinttypes>
#include <limits>
#include <cassert>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

it didn't build in debug for me (we use assert in the code below)

@@ -747,6 +753,13 @@ namespace gfx_api
float fogBegin;
float timeState; // graphicsCycle
int fogEnabled;
int viewportWidth;
int viewportheight;
float unused2;
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 don't really know how to elegantly handle std140 layout, I added an extra padding.

};


struct LightMap
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was part of the MAP_TILE struct before, I extracted this part so that it's easier to determine what happens on the lightmap (and surprisingly, not a lot)

@@ -1351,6 +1357,10 @@ static void drawTiles(iView *player)
const Vector3f theSun = (viewMatrix * glm::vec4(getTheSun(), 0.f)).xyz();
pie_BeginLighting(theSun);

// Reset all lighting data
getCurrentLightingData().lights.clear();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since lights have a lifetime of a single frame in warzone engine, I reset the lists of light at the beginning of a frame.

data/base/shaders/pointlights.frag Outdated Show resolved Hide resolved
data/base/shaders/pointlights.frag Outdated Show resolved Hide resolved
data/base/shaders/vk/pointlights.glsl Outdated Show resolved Hide resolved
data/base/shaders/tcmask_instanced.frag Outdated Show resolved Hide resolved
@@ -0,0 +1,40 @@
// See https://lisyarus.github.io/blog/graphics/2022/07/30/point-light-attenuation.html for explanation
// we want something that looks somewhat physically correct, but must absolutely be 0 past range
float pointLightEnergyAtPosition(vec3 position, vec3 pointLightWorldPosition, float range)
Copy link
Member

Choose a reason for hiding this comment

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

This pointLightEnergyAtPosition and processPointLight is copy-pasted in the data/base/shaders/pointlights.frag, is there a way to re-use the implementation?

Copy link
Member

@past-due past-due Dec 31, 2023

Choose a reason for hiding this comment

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

Vulkan shaders (inside shaders/vk) are kept as separate files from the OpenGL shaders (inside shaders/), so in this case we want to maintain the pattern and keep them separate.

(Despite them both nominally using GLSL, the Vulkan shaders often have to diverge - and can take advantage of newer GLSL features - than the OpenGL shaders.)

Copy link
Member

Choose a reason for hiding this comment

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

Ah, OK. Let's keep them separate then.

lib/ivis_opengl/pielighting.cpp Outdated Show resolved Hide resolved
lib/ivis_opengl/pielighting.cpp Outdated Show resolved Hide resolved
lib/ivis_opengl/pielighting.cpp Outdated Show resolved Hide resolved
lib/ivis_opengl/pielighting.h Outdated Show resolved Hide resolved
src/display3d.cpp Outdated Show resolved Hide resolved
src/lighting.cpp Outdated
@@ -380,26 +388,6 @@ void calcDroidIllumination(DROID *psDroid)
psDroid->illumination = retVal;
}

void doBuildingLights()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

was not used

@vlj vlj marked this pull request as ready for review January 1, 2024 16:40
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.

3 participants