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

[rtextures/rlgl] Load mipmaps for cubemaps #4429

Merged
merged 4 commits into from
Oct 26, 2024

Conversation

Not-Nik
Copy link
Contributor

@Not-Nik Not-Nik commented Oct 24, 2024

Load all mipmaps for cubemaps to the GPU and enable the proper filters for them. This is especially important for image based lighting (IBL), where for different roughness levels, different pre-filtered environment maps are stored in mipmaps. Calculating these filters is rather expensive, so it is often done offline and the pre-filtered cubemaps are shipped with the game.

The implementation did require some reworking; notably rlLoadTextureCubemap gains a new mipmapCount parameter, and ImageMipmaps no longer regenerates already existing mipmaps.

I structured the PR to be able to just be rebased onto master if you want all the different changes to reflect in the tree.

@raysan5 raysan5 merged commit 7fedf9e into raysan5:master Oct 26, 2024
14 checks passed
@raysan5
Copy link
Owner

raysan5 commented Oct 26, 2024

@Not-Nik Thanks for the improvement!

@raysan5
Copy link
Owner

raysan5 commented Oct 26, 2024

@Not-Nik I'm afraid this PR breaks models_skybox example, it crashes on execution at line 91: UnloadImage(img);. Please, can you review it?

raysan5 added a commit that referenced this pull request Oct 26, 2024
@raysan5
Copy link
Owner

raysan5 commented Oct 26, 2024

I just put a quick fix to avoid the crash in the models_skybox example but I noticed this change generates artifacts in the texture.

@Not-Nik
Copy link
Contributor Author

Not-Nik commented Oct 26, 2024

The fix should comment out both mipmap generations, other wise OpenGL will try to read mipmaps that aren't properly generated

@Not-Nik
Copy link
Contributor Author

Not-Nik commented Oct 26, 2024

I found the bug, I'll create a fixup PR in 20 minutes

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.

2 participants