Skip to content

Conversation

@mardy
Copy link
Collaborator

@mardy mardy commented Nov 23, 2024

To be reviewed/merged after #82, since it's based on top of it.

@WinterMute
Copy link
Member

some conflicts after merging #82 sorry. Did I mess something up?

This implementation comes with a big simplification: while the OpenGL
specifications say that the texture coordinates should be computed as a
reflection from the point of view of the camera, we do it as if the
camera rays were all parallel to each other (that is, as if the camera
was located infinitely far away). This approximation is acceptable for
objects like spheres or with other curved surfaces, it works pretty
badly with flat surfaces where all the normals have the same value,
because in that case the same texel from the reflection texture gets
applied over to the whole object.

Also, GL_SPHERE_MAP should not be the same as GL_REFLECTION_MAP, because
the resulting reflection vector should also be scaled down according to
the distance from the viewer.
This implementation complies with the OpenGL specification. In a
follow-up commit we will give the client the possibility to choose
between this software implementation and the faster (but wrong) GPU
implementation.
We might have more cases in the future, where we can choose between an
accurate software implemenation and a faster but non-standard GPU
implementation. Let's create a mechanism through which the client can
select the features which can be sped up at the cost of correct
rendering.

This is currently exposed as an environment variable, and not as an API,
because if it turns out to be a bad idea we can remove it without
breaking the public API.
Implement the read_{pos,norm}() functions for the DirectVboReader class
too, because they are needed when generating texture coordinates.
@mardy
Copy link
Collaborator Author

mardy commented Nov 24, 2024

some conflicts after merging #82 sorry. Did I mess something up?

No, you didn't. It's weird, because now that I rebased the branch locally, I had no conflicts at all. Anyway, I pushed the rebased branch.

@WinterMute WinterMute merged commit 084698d into devkitPro:master Nov 25, 2024
2 checks passed
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