Local lights on forward renderer

[ada-tv]

The biggest missing feature from the forward renderer, missing no more

Shaders to be modified:

• model
• sdf_text3D
• simple - Opaque shape entities are unlit because the light buffers aren't set up during the instanced shape step
• simple_procedural (maybe not necessary?)
• polyvox

CPP stuff todo:

• Local lights toggle for both renderers
• Lighting on opaque shapes (the lighting buffers aren't set up for instanced drawing)

Fixes #1485
Fixes #1792

[HifiExperiments]

awesome work, this will be so cool!

to answer your questions:

• simple_procedural doesn't currently have a true forward variant (and there's a FIXME for this in Procedural.cpp). maybe we can leave that for a separate PR since not many people use them anyways. you could try adding local light support to the existing transparent path if you wanted?
• for the toggle:
  • replace the toggle in GraphicsSettings with the commented out one:

    overte/scripts/system/settings/qml/pages/GraphicsSettings.qml

    Lines 107 to 120 in d18bf03

    	SettingBoolean {
    	settingText: "Local Lights";
    	settingEnabledCondition: () => { return Render.renderMethod === 0 }
    	}
    	// NOTE: Once local lights have a proper toggle, the SettingBoolean above can be replaced with this one below.
    	// SettingBoolean {
    	// settingText: "Local Lights";
    	// settingEnabledCondition: () => { return Render.localLightsEnabled }
    	// onSettingEnabledChanged: {
    	// Render.localLightsEnabled = settingEnabled;
    	// }
    	// }

  • add the local lights setting to RenderScriptingInterface, following the existing examples there
  • then I think you have two options:
    • you could have forceLocalLightsEnabled call recursivelyUpdateLightingModel like some of the other settings, and have it set enablePointLight/enableSpotLight. then in fetchClusterInfo you could have it return 0 for numLights if those are both disabled. this is a little bit of a hack, because the clusters will still be getting updated with values, but the shader will just short circuit...but it's definitely simpler
    • more complicated but maybe more "correct", you could have forceLocalLightsEnabled configure the actual LightClusteringPass, more similar to how recursivelyUpdateAntialiasingMode works. this would be like an "enabled" setting in LightClusteringPassConfig and then in LightClusteringPass::run you'd have to figure out how actually clear the clusters + short-circuit everything else to truly disable everything

[HifiExperiments]

weird - it would be great to figure this out because instancing is a big performance boost. when we do instancing, objects render at a different point in the frame, so maybe they need to have the light cluster buffers re-set up?

lightBatchSetter in RenderPipelinesInit sets up the directional light, but maybe it needs to also set the light clusters

[ada-tv]

I remember diagnosing this with Renderdoc, and the lighting buffers are indeed unset in that instanced shape step.

[HifiExperiments]

I think setting them in lightBatchSetter would re-set them though? in renderInstances we call pipeline->prepare(...) which should call the batch setter, and forward pipelines have forceLightBatchSetter = true

[HifiExperiments]

similar to the comment in RenderableShapeEntityItem, wonder if we could just do this in lightBatchSetter and then not have to do either of these here.

is it necessary to set the DeferredFrameTransform too?

[ada-tv]

I think DeferredFrameTransform's name is a bit misleading, it seems to just be generic frame transform info, and it's set up for both the layered (front and hud) rendering and the 2D overlay HUD too.

DeferredFrameTransform was already being used on the forward renderer (TransformCamera being used in web_browser.slf's haze blending is one example), but it wasn't set up, so its buffer was unset or uninitialised (which in OpenGL always returns zeros in shaders, afaik). Haze depends on it for calculating things like vertical depth fog, though even with DeferredFrameTransform set up it still doesn't seem to use the camera height when calculating the vertical depth fog color, so I might be missing something critical to setting it up properly. See #1648

[HifiExperiments]

agreed the naming is confusing, I think it used to only be used for deferred but now we should probably change it

looking at it closer, both deferred and forward paths have GenerateDeferredFrameTransform, but the deferred task passed it to DrawStateSortDeferred, and the forward pass does not pass it to DrawForward, so that could be the problem

[HifiExperiments]

I'd still love to get this working before we merge this.

looking at it again, renderInstances in GeometryCache calls pipeline->prepare which calls _batchSetter. the pipeline it should be using is from GeometryCache::getShapePipelinePointer, which uses the _shapePipelines, which are set up in GeometryCache::initializeShapePipelines.

it looks like GeometryCache::getShapePipeline and getFadingShapePipeline only set the keylight. maybe you just need to call setupLocalLightsBatch there too?

[ada-tv]

I've found the bits you've mentioned, but DeferredLightingEffect::setupLocalLightsBatch needs lightClusters and lightingModel (so it doesn't run when local lights are turned off), and I'm not sure how I'd get those here

[HifiExperiments]

you could store a copy of lightClusters on the light stage itself (have the light cluster setup job set it on the light stage), and look at how the second setupKeyLightBatch that doesn’t accept the light stage looks it up. you could have a second version of setupLocalLightsBatch that does something similar to get the light stage and then the clusters from that. you might not even need the lightingModel; you only use it to conditionally set the buffer, but shouldn’t we always set the buffer, lest it has incorrect data in it from something binding there previously?

[ada-tv]

I've had a shot at keeping a shared_ptr reference of LightClusters to reuse in the pipeline lambdas where it's not easily available in 180ab8f, but it hasn't gotten local lights working on instanced shapes yet. I think I'm doing setupLocalLightsBatch everywhere setupKeyLightBatch is.

[HifiExperiments]

I think I asked this before but I forget the answer - I remember you saying we need to set the frame transform here for the haze + lighting calculation, but why do we need to set the lighting? I think it should be handled by the individual pipelines

[ada-tv]

This was mostly copy-pasted from RenderDeferredTask. setupLocalLightsBatch probably could go into get*Pipeline next to their setupKeyLightBatch calls. If we can get LightClusteringPass and LightingModel from there then it'd be possible.