Accelerated env baking on the GPU

[riccardobl]

[tlf30]

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

[riccardobl]

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]

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

[riccardobl]

Thank you for your review @danielperano

[riccardobl]

Rebased to current master

[riccardobl]

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

[stephengold]

Please don't mark a conversation as "resolved" unless you've participated in it and/or acted on its advice.

[riccardobl]

Please don't mark a conversation as "resolved" unless you've participated in it and/or acted on its advice.

I was just tracking my progress, since they were so many, it was hard to keep track otherwise. The new commits should address all the requested changes, i've also improved the documentation and some methods and class names (LightProbeFactory2 -> FastLightProbeFactory), and made some addition to support frustum, position and local tagging (eg. one probe can tag some spatials as environment and another probe can tag others) for probes.

[capdevon]

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

[riccardobl]

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]

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!

[stephengold]

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]

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]

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]

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

[scenemax3d]

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

You submitted changes request, therefore I asked