Accelerated env baking on the GPU #1165

riccardobl · 2019-09-01T15:40:35Z

This PR improves the performances and user friendliness of environment baking.

Accelerated Baking

This PR adds 3 accelerated bakers that can be used in place of the current CPU baker.
The enhancements brought by these bakers is very remarkable, as they allow the environment baking process to be completed in <1s, effectively rendering the need for prebaked environments obsolete.

IBLGLEnvBaker

This is the most high-performing baker. It run entirely on the GPU, and can generate prefiltered, irradiance, and BRDF maps.
This is the classic env baker implementation that is currently not used in jme.

IBLHybridEnvBakerLight and IBLGLEnvBakerLight

These bakers generate the prefiltered map and spherical harmonics currently used in the latest PBR shader in jme.
Although this method reduces the VRAM consumption, this approach doesn't allow for as much optimization as the IBLGLEnvBaker. Nonetheless, the following accelerated baking methods are available:

IBLHybridEnvBakerLight: A hybrid baker that creates the prefiltered environmental map using GPU while performing spheric harmonics calculations on the CPU.
IBLGLEnvBakerLight: Similar to the IBLHybridEnvBakerLight, but it leverages the GPU to handle spheric harmonics generation as well.

New APIs

The LightProbeFactory has now been deprecated in favor of the LightProbeFactory2, which seamlessly integrates the accelerated baking methods. This transition serves as a dedicated migration path for legacy code.

The new API EnvironmentProbeControl is designed to streamline and simplify the entire baking and rebaking process.
Here's how it works:

Integration: Developers are only required to instantiate an EnvironmentProbeControl and attach it to the root node of their targeted scene (e.g., the rootNode).
Geometry Tagging: By employing the EnvironmentProbeControl.tag(Spatial) method, you can mark which geometries are part of the environment and should be baked (e.g., the skybox).
Profit: Once the tagging is in place, the control takes care of the rest. On the next frame, it automatically initiates baking for the tagged geometries within its node and completes it before the scene is rendered. No more waiting for the processes to conclude or resorting to convoluted culling tricks.
Update the baking: Every time the baking needs to be updated (eg. day-night cycles) the rebake() method of the control can be invoked.

This new system operates directly within the same scene. It uses RenderManager.setRenderFilter() (added by this PR) to filter out non tagged geometries, avoiding cloning, extra viewports, or other convoluted approaces.

Example

This is a fully functional remake of TestPBRLighting using the new API

package jme3test.light.pbr;


import com.jme3.app.SimpleApplication;
import com.jme3.environment.EnvironmentProbeControl;
import com.jme3.input.ChaseCamera;
import com.jme3.material.Material;
import com.jme3.math.FastMath;
import com.jme3.scene.Geometry;
import com.jme3.scene.Spatial;
import com.jme3.util.SkyFactory;
import com.jme3.util.mikktspace.MikktspaceTangentGenerator;

/**
 * TestPBRSimple
 */
public class TestPBRSimple extends SimpleApplication {

    public static void main(String[] args) {
        new TestPBRSimple().start();
    }

    @Override
    public void simpleInitApp() {

        Geometry model = (Geometry) assetManager.loadModel("Models/Tank/tank.j3o");
        MikktspaceTangentGenerator.generate(model);

        Material pbrMat = assetManager.loadMaterial("Models/Tank/tank.j3m");
        model.setMaterial(pbrMat);
        rootNode.attachChild(model);

        ChaseCamera chaseCam = new ChaseCamera(cam, model, inputManager);
        chaseCam.setDragToRotate(true);
        chaseCam.setMinVerticalRotation(-FastMath.HALF_PI);
        chaseCam.setMaxDistance(1000);
        chaseCam.setSmoothMotion(true);
        chaseCam.setRotationSensitivity(10);
        chaseCam.setZoomSensitivity(5);
        flyCam.setEnabled(false);

        Spatial sky = SkyFactory.createSky(assetManager, "Textures/Sky/Path.hdr", SkyFactory.EnvMapType.EquirectMap);
        rootNode.attachChild(sky);

        // Create baker control
        EnvironmentProbeControl envProbe = new EnvironmentProbeControl( assetManager, 256);
        rootNode.addControl(envProbe);

        // Tag the sky, only the tagged spatials will be rendered in the env map
        EnvironmentProbeControl.tag(sky);

    }

}

As you can see, much simpler and cleaner.

Knowing issues

The difference in roughness levels in the prefiltered envmap generated by the accelerated baker is less prominent than the current one, i am not sure why, you can test it out by toggling USE_ACCELERATED_BAKING in the TestPBRLighting class included in this PR.
I am not sure if this is an issue or which one is the correct behavior.

Minimum requirements

OpenGL 3.1 or GL ES 3.0

tlf30 · 2019-10-29T21:29:23Z

@riccardobl this looks really cool. Is there more info on this?

riccardobl · 2019-10-30T11:23:54Z

You can see it working in
TestPBRLighting.java
and
TestPBRSimple.java

The code is based on this https://learnopengl.com/PBR/IBL/Specular-IBL (and following articles).

The PR works but as you can see in the two examples, it exposes two different apis, one that is similar to what is used now in the master branch (LightProbeFactory2) and one that use a control. I'm not sure which one should go in the final pr. Also it requires modifications to the framebuffer class, that i will have to extract to a dedicated PR.

riccardobl · 2023-08-08T16:09:08Z

I've updated the PR and added a description. This is ready to be reviewed

jme3-core/src/main/java/com/jme3/environment/EnvironmentProbeControl.java

jme3-core/src/main/java/com/jme3/environment/baker/GenericEnvBaker.java

jme3-core/src/main/java/com/jme3/environment/baker/IBLGLEnvBaker.java

jme3-core/src/main/java/com/jme3/environment/baker/IBLHybridEnvBakerLight.java

jme3-core/src/main/java/com/jme3/renderer/RenderManager.java

riccardobl · 2023-08-08T17:29:30Z

Thank you for your review @danielperano

…aking. Make them serializable by default if created with LightProbeFactory2. Accelerate Spherical Harmonics generator.

…the engine

… baker

riccardobl · 2023-10-14T09:26:40Z

Rebased to current master

riccardobl · 2023-10-17T13:01:55Z

Are there any objections to merging this? It's worth noting that this PR doesn't break the current baking process.
@jMonkeyEngine/core

jme3-core/src/main/java/com/jme3/environment/EnvironmentProbeControl.java

jme3-core/src/main/java/com/jme3/environment/baker/IBLGLEnvBaker.java

jme3-core/src/main/java/com/jme3/renderer/RenderManager.java

jme3-core/src/main/java/com/jme3/environment/EnvironmentProbeControl.java

capdevon · 2023-11-07T10:05:13Z

I would suggest adding to the EnvironmentProbeControl.java class also the enabled variable and all the functionality of the AbstractControl class lost with the implementation of the Control class.

getSpatial()
setEnabled()
isEnabled()
write/read spatial and enabled fields.
execution of the update() and render() methods only if the Controller is enabled.

public abstract class AbstractControl implements Control, JmeCloneable {
...
}

Edit: done, thanks @riccardobl

jme3-core/src/main/java/com/jme3/environment/EnvironmentProbeControl.java

capdevon · 2023-11-08T14:19:14Z

jme3-core/src/main/java/com/jme3/environment/EnvironmentProbeControl.java

+     * 
+     * @param assetManager
+     */
+    public void setAssetManager(AssetManager assetManager) {


The setAssetManager() method is not necessary, since it is a mandatory parameter of the public constructor

public EnvironmentProbeControl(AssetManager assetManager, int size) {}

I would suggest removing it.

i can be used to change the asset manager used internally by the control that might not be the same used to deserialize it in some circumstances i supppose

Okay, but if you are not sure about this feature, you could change the visibility of the AssetManager variable from private to protected by letting the developers decide whether to add a set method or not. You also benefited from this trick by extending the LigthProbe class. More freedom and less code to maintain ;)

protected fields are evil. They become part of the public API and provide no recourse for changing.
Always always always better to provide protected methods because at least then you can change implementations, provide backwards compatibility, etc.. With protected fields you are stuck forever.

JME technically supports multiple instances of AssetManager, so setAssetManager was there to allow to change the internal asset manager after the control is deserialized, but on second thought, maybe it only adds confusion.

@riccardobl Ok, I suggest removing it.

jme3-core/src/main/java/com/jme3/environment/EnvironmentProbeControl.java

jme3-core/src/main/java/com/jme3/environment/baker/GenericEnvBaker.java

jme3-core/src/main/java/com/jme3/environment/baker/IBLGLEnvBaker.java

jme3-core/src/main/java/com/jme3/environment/baker/IBLGLEnvBakerLight.java

jme3-core/src/main/java/com/jme3/environment/baker/IBLHybridEnvBakerLight.java

jme3-core/src/main/java/com/jme3/environment/EnvironmentProbeControl.java

stephengold · 2023-11-09T23:11:38Z

jme3-core/src/main/java/com/jme3/environment/EnvironmentProbeControl.java

+    }
+
+    /**
+     * Create a new environment probe control.


The Javadoc summary fragment should not be a complete sentence.
I suggest "Create" -> "Creates"

stephengold · 2023-11-09T23:12:29Z

jme3-core/src/main/java/com/jme3/environment/EnvironmentProbeControl.java

+    }
+
+    /**
+     * Tag spatial as part of the environment for this EnvironmentProbeControl.


The Javadoc summary fragment should not be a complete sentence.
I suggest "Tag" -> "Tags the specified"

stephengold · 2023-11-09T23:13:29Z

jme3-core/src/main/java/com/jme3/environment/EnvironmentProbeControl.java

+    }
+
+    /**
+     * Untag spatial as part of the environment for this


The Javadoc summary fragment should not be a complete sentence.
I suggest "Untag" -> "Untags the specified"

stephengold · 2023-11-09T23:13:56Z

jme3-core/src/main/java/com/jme3/environment/EnvironmentProbeControl.java

+    }
+
+    /**
+     * Tag spatial as part of the environment for every EnvironmentProbeControl.


The Javadoc summary fragment should not be a complete sentence.
I suggest "Tag" -> "Tags the specified"

jme3-core/src/main/java/com/jme3/environment/baker/IBLGLEnvBaker.java

jme3-core/src/main/java/com/jme3/environment/baker/IBLHybridEnvBakerLight.java

jme3-examples/src/main/java/jme3test/light/pbr/TestPBRSimple.java

riccardobl · 2023-11-10T08:27:13Z

Apologies, but going through all the Javadoc to insert 's' and '.' seems a bit like nitpicking at this stage. If you check any file in the JME repo, you'll notice that these 'rules' were never enforced before.
I hope you can still appreciate this PR, since it introduces an important feature.
@stephengold

tonihele · 2023-11-10T09:50:25Z

Apologies, but going through all the Javadoc to insert 's' and '.' seems a bit like nitpicking at this stage. If you check any file in the JME repo, you'll notice that these 'rules' were never enforced before. I hope you can still appreciate this PR, since it introduces an important feature. @stephengold

I kinda agree that with Javadoc, good enough is good enough. Enforcing strict rules on Javadoc doesn't serve much purpose to me at least. If it is understandable English and gives good enough information, that would be good enough. Good enough, did I mention this term? Difficult to measure, subjective, maybe you understand what I mean.

And thanks @riccardobl for the feature and taking all the feedback maturely!

capdevon

Thank you for your patience @riccardobl . I am curious to try this new feature! Just a couple of finishing touches and I think it's good enough for me ;)

capdevon · 2023-11-10T13:56:53Z

jme3-core/src/main/java/com/jme3/environment/baker/GenericEnvBaker.java

+        tx.getImage().clearUpdateNeeded();
+    }
+
+    protected int limitMips(int nbMipMaps, int baseW, int baseH, RenderManager rm) {


There are unused parameters in this method (int baseW, int baseH, RenderManager rm). I would suggest to remove them ;)

capdevon · 2023-11-10T14:09:39Z

jme3-core/src/main/java/com/jme3/environment/EnvironmentProbeControl.java

+     * 
+     * @param assetManager
+     */
+    public void setAssetManager(AssetManager assetManager) {


Okay, but if you are not sure about this feature, you could change the visibility of the AssetManager variable from private to protected by letting the developers decide whether to add a set method or not. You also benefited from this trick by extending the LigthProbe class. More freedom and less code to maintain ;)

capdevon · 2023-11-10T14:27:22Z

jme3-core/src/main/java/com/jme3/environment/FastLightProbeFactory.java

+     *            the asset manager
+     * @return a debug node
+     */
+    public static Node getDebugGui(AssetManager manager, LightProbe probe) {


The default settings of this method may not satisfy users who want to decide the name to assign to the debug node or its initial position in their tests. Since this is not typically a core, but a debugging feature, I would suggest moving this method to a test class (eg. TestPBRSimple or TestPBRLighting). That way you have more freedom to explain to users how to debug the data properly without raising exceptions ;) I too reading this method can't figure out when is the right time to test whether the LightProbe is ready or not ;)
Solution: test class with example.

This was copied from LightProbeFactory, so while i agree with you, the purpose of this class is to migrate the code using LightProbeFactory to the new baker, so it should expose the same api

@riccardobl Ok, I don't particularly like it, but I can accept it.

capdevon · 2023-11-10T14:32:46Z

jme3-core/src/main/java/com/jme3/environment/EnvironmentProbeControl.java

+ * A control that automatically handles environment bake and rebake including
+ * only tagged spatials.
+ * 
+ * Simple usage example: <code>


I don't have a strong opinion on this, but I would suggest using this long explanation in a useful way by removing it from here and putting it in a test class. That way you simplify your life and don't have to worry about updating the javadoc in case of future changes or fixes ;)

stephengold · 2023-11-10T19:10:56Z

Apologies, but going through all the Javadoc to insert 's' and '.' seems a bit like nitpicking at this stage. If you check any file in the JME repo, you'll notice that these 'rules' were never enforced before. I hope you can still appreciate this PR, since it introduces an important feature. @stephengold

@riccardobl I agree it's nitpicking, and I do appreciate the importance of your work.

Are you implying you don't want me to review yourPRs? If so, just say so. In this instance, however, you specifically requested my review, so I felt obligated to do my best, poor though it is.

Prior to PR #1655 (November 2021) this project had no official coding style. With nothing to enforce, there was of course no enforcement. Since then, I've tried to follow and enforce our preferred style, at least on new Java files. This involves plenty of nitpicking. I'm sure I sometimes make mistakes.

If you don't like the "rules", you're in a good position to change them.

riccardobl · 2023-11-10T19:38:43Z

Are you implying you don't want me to review yourPRs?

Not at all. I appreciate your dedication to the project, and I value your input. However, I'd like to suggest that we focus more on the content during the reviews, rather than getting caught up in minor styling issues or comments. I think this adjustment also helps prevent contributors from feeling discouraged due to the strict rules.

scenemax3d · 2024-01-22T19:29:34Z

Apologies, but going through all the Javadoc to insert 's' and '.' seems a bit like nitpicking at this stage. If you check any file in the JME repo, you'll notice that these 'rules' were never enforced before. I hope you can still appreciate this PR, since it introduces an important feature. @stephengold

@riccardobl I agree it's nitpicking, and I do appreciate the importance of your work.

Are you implying you don't want me to review yourPRs? If so, just say so. In this instance, however, you specifically requested my review, so I felt obligated to do my best, poor though it is.

Prior to PR #1655 (November 2021) this project had no official coding style. With nothing to enforce, there was of course no enforcement. Since then, I've tried to follow and enforce our preferred style, at least on new Java files. This involves plenty of nitpicking. I'm sure I sometimes make mistakes.

If you don't like the "rules", you're in a good position to change them.

@stephengold ,
Thanks for posting the detailed JavaDocs comments. I fixed all of them (I hope). Can you please re-review?
10X

stephengold · 2024-01-23T18:12:16Z

I'm not interested in re-reviewing this PR.

scenemax3d · 2024-01-23T18:22:13Z

I'm not interested in re-reviewing this PR.

You submitted changes request, therefore I asked

stephengold added this to the Future Release milestone Mar 13, 2021

riccardobl force-pushed the accellbaker branch from ec01833 to edf7e05 Compare August 8, 2023 08:14

riccardobl marked this pull request as ready for review August 8, 2023 16:09

riccardobl added the enhancement label Aug 8, 2023

danielperano reviewed Aug 8, 2023

View reviewed changes

danielperano approved these changes Aug 8, 2023

View reviewed changes

riccardobl added 15 commits October 14, 2023 11:16

Accelerated env baker that runs on the GPU.

b88be51

Add render filter

0c8801f

EnvironmentProbeControl, for simpler env baking

616c052

Add copyright and javadoc, fix some merging issues

67a3702

Make lightprobes serializable on demand when baked with accelerated b…

be89b88

…aking. Make them serializable by default if created with LightProbeFactory2. Accelerate Spherical Harmonics generator.

Add javadoc fix typo

5261a98

Make EnvironmentProbeControl serializable

7bc7670

Fix TestPBRSimple to use new api

29ffa68

do not dispose the env image

33e7f34

Add default constructor

8b59c31

Set default EnvironmentProbeControl area to Float.MAX_VALUE

9ad84f1

Do not call Spatial.updateModelBound() in env baker

6c5322c

Fix compatibility with GL ES. Ensure mipmaps are never recomputed by …

e648d6c

…the engine

Refactor accelerated baker code to map more closely with the original…

d756e17

… baker

Drivers workaround: do not crash if lower mips cannot be computed.

0804ecd

riccardobl force-pushed the accellbaker branch from 4d75d0d to 0804ecd Compare October 14, 2023 09:25

stephengold requested changes Oct 17, 2023

View reviewed changes

Remove new line

a77eb2c

riccardobl requested a review from stephengold November 3, 2023 19:37

capdevon reviewed Nov 7, 2023

View reviewed changes

jme3-core/src/main/java/com/jme3/environment/EnvironmentProbeControl.java Show resolved Hide resolved

jme3-core/src/main/java/com/jme3/environment/EnvironmentProbeControl.java Show resolved Hide resolved

riccardobl added 3 commits November 8, 2023 12:25

Add AbstractControl functionalities to EnvironmentProbeControl

da2e3b3

Rename serializable to requiredSavableResults

a846a63

Replace Function with Predicate

8314499

riccardobl requested a review from capdevon November 8, 2023 11:31

capdevon suggested changes Nov 8, 2023

View reviewed changes

stephengold requested changes Nov 9, 2023

View reviewed changes

riccardobl added 2 commits November 10, 2023 09:15

improvements

cbb2166

Rename env -> envMap

7b87ff5

capdevon reviewed Nov 10, 2023

View reviewed changes

scenemax3d added 4 commits January 21, 2024 22:09

JavaDoc corrections

4665aaa

JavaDoc fixes

aae61d3

JavaDoc fixes

8663388

JavaDocs Fixes

d4ff52c

scenemax3d requested a review from stephengold January 22, 2024 16:51

JavaDocs fixes

7a77c14

stephengold removed their request for review January 23, 2024 18:12

scenemax3d merged commit e452b16 into master Jan 23, 2024
14 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Accelerated env baking on the GPU #1165

Accelerated env baking on the GPU #1165

riccardobl commented Sep 1, 2019 •

edited

Loading

tlf30 commented Oct 29, 2019

riccardobl commented Oct 30, 2019

riccardobl commented Aug 8, 2023

riccardobl commented Aug 8, 2023

riccardobl commented Oct 14, 2023

riccardobl commented Oct 17, 2023

capdevon commented Nov 7, 2023 •

edited

Loading

capdevon Nov 8, 2023

riccardobl Nov 10, 2023 •

edited

Loading

capdevon Nov 10, 2023 •

edited

Loading

pspeed42 Nov 10, 2023

riccardobl Nov 10, 2023

capdevon Nov 11, 2023

stephengold Nov 9, 2023

stephengold Nov 9, 2023

stephengold Nov 9, 2023

stephengold Nov 9, 2023

riccardobl commented Nov 10, 2023 •

edited

Loading

tonihele commented Nov 10, 2023

capdevon left a comment •

edited

Loading

capdevon Nov 10, 2023

capdevon Nov 10, 2023 •

edited

Loading

capdevon Nov 10, 2023 •

edited

Loading

riccardobl Nov 10, 2023

capdevon Nov 11, 2023

capdevon Nov 10, 2023

stephengold commented Nov 10, 2023

riccardobl commented Nov 10, 2023

scenemax3d commented Jan 22, 2024

stephengold commented Jan 23, 2024

scenemax3d commented Jan 23, 2024

Accelerated env baking on the GPU #1165

Accelerated env baking on the GPU #1165

Conversation

riccardobl commented Sep 1, 2019 • edited Loading

Accelerated Baking

IBLGLEnvBaker

IBLHybridEnvBakerLight and IBLGLEnvBakerLight

New APIs

Example

Knowing issues

Minimum requirements

tlf30 commented Oct 29, 2019

riccardobl commented Oct 30, 2019

riccardobl commented Aug 8, 2023

riccardobl commented Aug 8, 2023

riccardobl commented Oct 14, 2023

riccardobl commented Oct 17, 2023

capdevon commented Nov 7, 2023 • edited Loading

Choose a reason for hiding this comment

riccardobl Nov 10, 2023 • edited Loading

Choose a reason for hiding this comment

capdevon Nov 10, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

riccardobl commented Nov 10, 2023 • edited Loading

tonihele commented Nov 10, 2023

capdevon left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

capdevon Nov 10, 2023 • edited Loading

Choose a reason for hiding this comment

capdevon Nov 10, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stephengold commented Nov 10, 2023

riccardobl commented Nov 10, 2023

scenemax3d commented Jan 22, 2024

stephengold commented Jan 23, 2024

scenemax3d commented Jan 23, 2024

riccardobl commented Sep 1, 2019 •

edited

Loading

capdevon commented Nov 7, 2023 •

edited

Loading

riccardobl Nov 10, 2023 •

edited

Loading

capdevon Nov 10, 2023 •

edited

Loading

riccardobl commented Nov 10, 2023 •

edited

Loading

capdevon left a comment •

edited

Loading

capdevon Nov 10, 2023 •

edited

Loading

capdevon Nov 10, 2023 •

edited

Loading