A few little fix ups #81
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The DirectVboReader class has its own setup_draw() method, and it should replicate the logic to enable texture arrays sequentially.
We forgot to update the glEnableClientState() and glDisableClientState() functions to operate on the active texture unit.
We previously added GL_MAX_TEXTURE_IMAGE_UNITS, which has about the same meaning, but it's for meaningful for fragment shaders only: programs using OpenGL in immediate mode use GL_MAX_TEXTURE_UNITS.
Extension names need to be separated by a space.
We could indeed add this library to our pkg-config file, but we don't use any other of its functions, and using malloc() and free() is more efficient anyway.
Small refactoring of the CoordVertexReader class to support vertex elements consisting of a single float coordinate (that's the case normally handled by GX_TexCoord1f32()).
For some reason, Neverball passes the same array where vertex position coordinates are stored as texture coordinates, but GX does not support more than 2 texture coordinates per elements. It may be that we can do something smarter here (detect that the vertex data pointer is the same as the vertex coordinates, and don't pass it twice to GX, instead setup a texgen matrix to process it), but for the time being just limit the number of components in order not to send wrong data into the FIFO.
Ths OpenGL spec says that the texture index 0 must be silently ignored by glDeleteTextures(). Failure to ignore this value was causing weird artifacts with text labels in Neverball. We also invert the logic of the "if" condition, to make it easier to read.
How this did not break all other applications is a mystery.
Avoid performing a matrix multiplication if the angle is zero, but also if the rotation axis is null: in that case, the rotation matrix would get filled with some NaN values, therefore corrupting the resulting matrix.
There are subtle differences between these, but for the time being just map them to GX_CLAMP, which the most appropriate match.
We should check all the four TEV inputs for the "one" constant, and we need a way to tell it apart from GX_CA_KONST (which could be used when an arbitrary constant value is desired).
The alpha should not get summed, but multiplied.
We should not hardcode the input to be GX_CC_CPREV or GX_CA_APREV, because this would not work for the first stage. Instead, check if we are computing inputs for the first stage, and in that case use the raster color.
In the second color channel, the alpha component of the diffuse material should not be added, but multiplied with the input from the previous stage, because the OpenGL spec says that the value of alpha should be taken from the alpha component of the color material's diffuse value. This fixes the rendering of glass parts in Neverball.
The client code might have called glTexParameteri() and set a specific wrapping mode before loading the texture data, and we should preserve it. This effectively reverts 2853d62, but now the GX_REVERT wrapping mode is set automatically in glGenTextures and glBindTexture if the texture was previously unused.
It turns out that while this works on Dolphin, the GX_TEVPREV register cannot be written to, on a real Wii. Therefore, write on a GX_TEVREGx register, and allocate them dynamically.
The GX_CC_RASC and GX_CA_RASA constants do not refer to the untextured vertex color, but only to the color being generated by the color channel associated with the current stage: while in some situation these two entities coincide, in the case of a lit object we do have two active color channels, and these constants can only be used to refer to one of them at a time. Furthermore, when we enter the texturing stage on a lit geometry, we pass GX_COLORNULL, meaning that no rasterizer color channel is associated, and here GX_CC_RASC and GX_CA_RASA only render black. Yet, the texture environment operations in OpenGL sometimes need to operate on the color emitted on the vertex by the rasterizer. The solution is to emit the result of the lighting operations not on GX_TEVPREV (which gets changed at each stage), but on a well-named register (one of GX_TEVREG0 - GX_TEVREG2), which later stages can then refer to.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Please review commit by commit, as the fixes are unrelated to each other.