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

[3.2] Add new CPU lightmapper #40035

Closed
wants to merge 12 commits into from
Closed

Conversation

JFonS
Copy link
Contributor

@JFonS JFonS commented Jul 1, 2020

Implement a new path-traced CPU lightmapper.

Partially sponsored by Google (GSoC).
Partially sponsored by IMVU.
Partially sponsored by my own free time.

This new lightmapper should give far better results than the current one. See the exact same scene fully lightmapped (direct and indirect light) using the current lightmapper in both modes and the new path-traced one:

lm

I tried to keep the BakedLightmap properties as similar to the 4.0 ones as possible, to keep compatibility as much as possible.

Also added caching to mesh unwrapping to speed up import time and keep unwraps consistent across imports.

Still missing some work on documenting the Embree version used and the changes made to the official version.

Bugsquad edit: This closes #15055, closes #16820, closes #16398, closes #20321, closes #21587, closes #30789, closes #30929, closes #32746, closes #38685, closes #39532.

@JFonS JFonS changed the title Cpu lightmapper 3.2 [3.2] Add new CPU lightmapper Jul 1, 2020
@JFonS JFonS added this to the 3.2 milestone Jul 1, 2020
@reduz
Copy link
Member

reduz commented Jul 1, 2020

Looks great! did you get to take a look to the probe code in 4.0? I would just backport that directly and not keep compatibility

@Calinou
Copy link
Member

Calinou commented Jul 2, 2020

Does this new lightmapper take environment lighting into account? If so, this would fix #16398.

@JFonS
Copy link
Contributor Author

JFonS commented Jul 2, 2020

@Calinou It can be enabled, yes :)

@lawnjelly
Copy link
Member

Preliminary look (and after discussing a bit with jfons):

  • User interface looks very good, I particularly liked multiple progress bars and time to completion
  • For mesh UV2 mapping, we need a way of specifying the padding. Without padding there will be artifacts
  • Does it do dilate afterwards to expand into the padding?
  • Lights at the moment don't have volume, which is giving only harsh shadows. It would be nice to be able to specify light volume or area, as soft shadows is one of the advantages of lightmaps. Or perhaps this can be done to some extent with an emissive material?
  • It might be possible to get rid of the need for manual selection of the voxel volume. This is a bit tedious in a lot of cases. Maybe we can have an option in the baked lightmap to auto select the geometry.

Overall I liked it. Sure there were a number of features I'd like to see added, but there is good reason to get it available as soon as possible, then more features can be added as needed. Once the basic framework is working it should be fairly easy to tweak / add new features.

Some ideas:

  • Forward tracing option (from the light) as an alternative to the current backward method.
  • Ambient occlusion

@Calinou
Copy link
Member

Calinou commented Jul 24, 2020

Lights at the moment don't have volume, which is giving only harsh shadows. It would be nice to be able to specify light volume or area, as soft shadows is one of the advantages of lightmaps. Or perhaps this can be done to some extent with an emissive material?

Unlike in the master branch, lights don't have a size property in 3.2. This can be faked by placing several less intense lights close together, but it's slower to bake.

It might be possible to backport the light size property, but I don't know how difficult it is to simulate its light attenuation pattern for real-time direct lights in GLES3 (or worse, GLES2). (Simulating real-time soft shadows is probably out of the question due to the cost.)

In the worst case, we could support the light size property only in BakedLightmap - it'd have no effect on real-time lights.

Forward tracing option (from the light) as an alternative to the current backward method.

Out of curiosity, what are the tradeoffs of this method compared to the current backward method?

@lawnjelly
Copy link
Member

lawnjelly commented Jul 24, 2020

Lights at the moment don't have volume, which is giving only harsh shadows. It would be nice to be able to specify light volume or area, as soft shadows is one of the advantages of lightmaps. Or perhaps this can be done to some extent with an emissive material?

Unlike in the master branch, lights don't have a size property in 3.2. This can be faked by placing several less intense lights close together, but it's slower to bake.

Using several lights is a pretty messy solution - as it is better to have something that is easy to edit, and lightmaps can give high quality results, there's no need to restrict them to something similar to PCF shadows. In my toy lightmapper at the moment I'm simply using the light scale to determine light volume, simple with omnis, and also for spotlights for the source volume. I don't know whether this interferes with anything else light wise (can't check this as I'm away from home this week).

I'm also planning to simply use light names to distinguish between some extra light types such as orientated bounding box or sphere / ellipse, light from volume or just from the shape surface. For core we could consider some special extra tags for lights to specify all this kind of stuff instead of bodging it. There was also a proposal about simulating complex light shapes on godot proposals (can't look up at the moment) that might be worth considering.

Or alternatively just have it support emissive material so that any shape can be a light source.. maybe it can do this already I didn't test. That might be more tricky to mix lightmaps with realtime lighting though.

It might be possible to backport the light size property, but I don't know how difficult it is to simulate its light attenuation pattern for real-time direct lights in GLES3 (or worse, GLES2). (Simulating real-time soft shadows is probably out of the question due to the cost.)

In the worst case, we could support the light size property only in BakedLightmap - it'd have no effect on real-time lights.

This is kind of what I'm doing at the moment and it seems to work fine (haven't tried any huge area lights though).

Forward tracing option (from the light) as an alternative to the current backward method.

Out of curiosity, what are the tradeoffs of this method compared to the current backward method?

Backward tracing is often used because it tends to be more efficient - however that advantage is partly based on the idea of a view being taken from a camera. A lot of rays won't make it into the camera view so it makes a lot of sense to start by only considering things in view. With a lightmap this is less relevant, in a lot of scenes nearly all the rays from lights will hit something.

However with backward tracing you have the advantage that each texel can get equal consideration in terms of number of samples, which tends towards less noise (particularly with later bounces maybe?).

Forward tracing also has the possibility of scaling a lot better in large levels, or complex levels. With backward tracing, there is a very real possibility that a lot of texels will spend a large proportion of time tracing to lights that cannot possibly influence them. With forward tracing areas in shadow are not considered. Also areas at a large distance from the light will automatically spend proportionately less of the processing time.

I'm not an expert by any means, I found that forward tracing was easier to make artifact free and is closer to 'physically correct' out of the box. With backward tracing you have to make assumptions based on the area of a texel in view of a light, whereas with forward tracing, this happen automagically. With forward tracing there's no need to consider distance to light source, or angle, as falloff and variation depending on surface normal happen automatically.

Also with backward tracing there's a number of situations that can give rise to artifacts, consider when a texel has one or multiple cutting planes that may prevent light hitting part of the texel.

Some other methods are bidirectional ray tracing and metropolis light transport.

These kinds of things are easy to play around with when the main framework is setup for ray tracing / exporting lightmaps etc.

@Calinou
Copy link
Member

Calinou commented Jul 24, 2020

Or alternatively just have it support emissive material so that any shape can be a light source.. maybe it can do this already I didn't test.

Judging by the screenshots, this is already supported 🙂

For core we could consider some special extra tags for lights to specify all this kind of stuff instead of bodging it. There was also a proposal about simulating complex light shapes on godot proposals (can't look up at the moment) that might be worth considering.

reduz once mentioned adding the ability to specify a "start angle" for SpotLight3D, which would make it possible to create cylinder-shaped lights (as opposed to cone-shaped lights).

@JFonS
Copy link
Contributor Author

JFonS commented Jul 29, 2020

@RandomShaper and I are working on an update to this PR.

@RandomShaper added texture atlassing and I added some fixes to the light transport computation.

Here's a test scene in case someone wants to try it out :)

LightmapperTest.zip

@lawnjelly
Copy link
Member

lawnjelly commented Jul 29, 2020

Got the emission working, and I can confirm you can get soft shadows with emissive materials 👍 . So just not with the lights at the moment it seems.

For emission to work BTW, you need 1 or more bounces, it won't light anything in the first pass, that caught me out, and the emitting mesh needs to have secondary UVs. Also the emitting mesh has to be in the baked lightmap volume.

jfons_emission

There are some slight artifacts here but these might be due to padding or not enough texel density.

Edit: It looks like the artifacts were from not enough samples. Turning off the denoiser showed this. I think realistically to get rid of these you need high or ultra high quality set. This is with ultra high, 1 bounce, took a couple of mins, looking a lot nicer: 😄

ultrahigh

These are jaggies from normal lights, which are rather objectionable so casting multiple rays from a light volume would probably be good to get working:

jaggies_small

@Calinou
Copy link
Member

Calinou commented Jul 29, 2020

@lawnjelly We should look at adding bicubic sampling support when using GLES3. For those using GLES2 (where bicubic sampling might not be possible), we could add an option to supersample the lightmap baking by a 2× factor. This would make lightmaps much slower to bake (so you'd only enable it for final bakes), but this would avoid adding any run-time performance cost.

Bicubic sampling is already supported in the master branch, it's just not supported in the 3.2 branch.

@lawnjelly
Copy link
Member

@lawnjelly We should look at adding bicubic sampling support when using GLES3. For those using GLES2 (where bicubic sampling might not be possible), we could add an option to supersample the lightmap baking by a 2× factor.

The jaggies here aren't a filtering issue I don't think. There is a limit to the resolution of the lightmap, so I think it's much easier (and cheaper) to fix it in the lightmap.

@JFonS
Copy link
Contributor Author

JFonS commented Jul 29, 2020

Updated the PR:

  • Includes atlassing and other fixes by @RandomShaper
  • Implemented importance sampling, much better looking results.
  • Fixed an issue in the unwrap code.
  • Got rid of a vector push_back() call that made buffer generation slower than it should've been.
  • Reduced the amount of samples in "Ultra" quality from 1024 to 512.

Here's an updated test scene, the one I posted before had weird normals on the walls and ceiling:
LightmapperTest2.zip

@Wavesonics
Copy link
Contributor

I just tried to build the branch on windows but got the following compilation error:

[Initial build] Compiling ==> scene\3d\collision_polygon.cpp
collision_object.cpp
collision_polygon.cpp
[Initial build] Compiling ==> scene\3d\collision_shape.cpp
collision_shape.cpp
scene\3d\baked_lightmap.cpp(776): error C2065: 'M_PI': undeclared identifier
scons: *** [scene\3d\baked_lightmap.windows.tools.64.obj] Error 2
scons: building terminated because of errors.

Is there anything special I should be doing for this branch?

@akien-mga
Copy link
Member

scene\3d\baked_lightmap.cpp(776): error C2065: 'M_PI': undeclared identifier

@JFonS you should use Math_PI from core/math/math_defs.h.

madmiraal and others added 9 commits July 31, 2020 11:53
They will be deleted when the faces are merged, but their edges are
needed for merging faces.
- Fixed file not closed after loading (made future imports in the same session fail)
- Fixed saving of lossless (if format not suiable for PNG it forcefully sets it to uncompressed, but that was being done too late, after the value had already been written)
- Fixed reload-from-file (involved refactoring loading code from its resource loader class to the class iself, much like how StreamTexture works)
- Use of M_PI.
- Better texels-per-unit default and range.
- Prevent potential crashes.
- Fix unwrap cache file kept open.
Bonus:
- On new bakes of the same mesh, touch the import settings file intact only to change what is needed for the new bake setup (namely, don't overwrite texture compression mode).
@Wavesonics
Copy link
Contributor

Just tried the latest and it does build for me now.

I loaded up a project I have the relies heavily on the existing lightmapper implementation (but has many problems with it) and tried to bake my scenes using the new light mapper:

It appears that Gridmaps are currently not being considered in the new baking process?

Most of my scene's geometry lives inside grid maps, none of which gets baked it appears, however a few things in my scenes are just instanced geometry, and those do indeed get baked (and they look good so far!)

@Wavesonics
Copy link
Contributor

Wavesonics commented Aug 2, 2020

I like that you can now enable/disable the capture data, however it does seem to be similar to the old lightmapper in that the capture data is HUGE!

Is one of the goals here to replace the capture data with something like the light probe system in 4.0? I assume that would greatly reduce the binary sizes required, but I suppose that is mostly just an assumption.

My test scene (TSCN) went from 5MB to 50MB by enabling capture data with the new Lightmapper. SCN does shrink this by a lot, but this test scene is small. My actual scenes which are all SCNs still come in at almost 100MB (with the old light mapper), the majority of that coming from the Lightmap capture data I believe.

If that's not part of this though rewrite, that's totally understandable!

@Calinou
Copy link
Member

Calinou commented Aug 3, 2020

My test scene (TSCN) went from 5MB to 50MB by enabling capture data with the new Lightmapper. SCN does shrink this by a lot, but this test scene is small. My actual scenes which are all SCNs still come in at almost 100MB (with the old light mapper), the majority of that coming from the Lightmap capture data I believe.

Never save BakedLightmapData or GIProbe data to a text-based file, as it'll be converted to Base64. You should always save it to a .res (binary resource) file. You can do this while keeping the rest of your scene text-based.

Godot 4.0 will do this for you automatically, but I'm not sure if we can backport that behavior.

@Wavesonics
Copy link
Contributor

I did some digging, and it looks like a big part of my file size bloat is from using Baked Lightmaps in conjunction with GridMaps:
#41009

Not sure if a different approach can be used for this new Light mapper when dealing with Gridmaps, or even if a very different approach is even possible for gridmaps, but thought I'd highlight it here in case it could be addressed as part of this.

@Wavesonics
Copy link
Contributor

It would be pretty great to have an option to save the actual lightmap images on disk as WEBP in order to save disk space. @Calinou mentioned that there might be some load time impact to this, but it might be worth it for some use cases.

PNG file size: 118.5 KB
WebP-lossless file size: 80.1 KB
WebP-lossy (with alpha) file size: 18.4 KB

@lawnjelly
Copy link
Member

lawnjelly commented Aug 23, 2020

In the worst case, we could support the light size property only in BakedLightmap - it'd have no effect on real-time lights.

I've just discovered something which is a giant pain as far as using light scale to determine light volume in the light mapping. Lights have set_disable_scale set to true in their constructor.

So strangely you can set scale on a light, and use it to determine volume, but the moment you move the light in the editor, it resets the scale to 1.0. This may have been to avoid some problem in regular light use (the direction does become invalid at 0.0 scale, presumably because the basis is zero length).

So either re-enabling scale for lights might be needed , or perhaps better still some extra light parameters specifically for lightmapping (and perhaps other GI?). Scale was handy because being an x, y, z it enables you to quickly create long thin lights for example. Blender I seem to remember can use light scale for volume.

Emissive materials gives the most versatility in terms of shape, but has the major disadvantage it doesn't work as a realtime shadow mapped light (although it might work with e.g. GI probe, but that is defeating the object of using baked lighting). It can show the light with light probes, but no shadows.

Perhaps a dedicated section in the inspector for lights for the baking properties might be an idea then. You could then as well as specifying e.g. an xyz orientated bounding box volume that rotated with the light, you could have more info for the type of light, such as box or sphere, solid, hollow etc.

@EzraT
Copy link

EzraT commented Nov 2, 2020

@JFonS
Apologies if I missed this but, is this new lightmapper still planned to be backported to 3.2.x?
If so, any idea on when?
A better lightmapping solution in Godot 3 would really help flesh it out more, especially considering the limitations it has when it comes to dynamic lighting.

@NHodgesVFX
Copy link
Contributor

@JFonS
Apologies if I missed this but, is this new lightmapper still planned to be backported to 3.2.x?
If so, any idea on when?
A better lightmapping solution in Godot 3 would really help flesh it out more, especially considering the limitations it has when it comes to dynamic lighting.

I was talking to jfons about this the other day, he is hoping to get it in time for Godot 3.2.4 Beta 2. there isnt much progress being reflected here as he is working on a separate branch.

@EzraT
Copy link

EzraT commented Nov 2, 2020

I was talking to jfons about this the other day, he is hoping to get it in time for Godot 3.2.4 Beta 2. there isnt much progress being reflected here as he is working on a separate branch.

Got it, thanks for the clarification.

@Calinou
Copy link
Member

Calinou commented Nov 17, 2020

Note that the CPU lightmapper merge is being pushed back to 3.2.4beta3 to allow jfons to make it more polished. IRC log:

<jfons> Akien: I've been working on the lightmapper on the weekends, but I keep finding things to fix or improve :/ When do you plan to release the new beta? I can try to have a mergeable PR by then and then improve the little details in future betas.
<Akien> jfons: I plan to build today and release tomorrow-ish
<Akien> jfons: I can wait a bit if you're very close to ready, but it might be better to have it merged, get some initial testing from people who do custom builds, and broader testing in beta 3.
<Akien> Merging big changes right before a release typically means a broken release :P
<jfons> yeah... I could get the branch in a usable state today, but it would still need to be reviewed and all
<jfons> then beta 3 it is... I am in a "perfect is the enemy of good" situation. The more I work on it the more possible improvements I find.
<Akien> In theory beta 3 could be as early as in one week. Beta 2 was delayed as I waited for some important regression fixes (+ had to fix iOS C# stuff), but my initial idea was to make one build per week.
<Akien> It's much easier to "bisect" regressions when you have a lot of builds available to test against.
<jfons> ok, I'll aim for beta3 then

@dedm0zaj
Copy link

dedm0zaj commented Dec 9, 2020

There is a problem with the lighting of dynamic objects. They come out too dark.
image

@Calinou
Copy link
Member

Calinou commented Dec 9, 2020

@dedm0zaj Please upload a minimal reproduction project to make this easier to troubleshoot.

@hoontee
Copy link
Contributor

hoontee commented Dec 9, 2020

@dedm0zaj Is #25393 the issue you're having?

@dedm0zaj
Copy link

dedm0zaj commented Dec 9, 2020

@hoontee I don't use GIProbe for two reasons:

  1. GIProbe uses more resources than BakedLightmap.
  2. Dynamic objects at the junction with static objects turn black.

@JFonS
Copy link
Contributor Author

JFonS commented Dec 23, 2020

Closing as superseded by #44628.

@JFonS JFonS closed this Dec 23, 2020
@Calinou Calinou removed this from the 3.2 milestone Dec 23, 2020
@JFonS JFonS deleted the cpu_lightmapper_3.2 branch May 4, 2021 07:45
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.