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

Tiled renderer improvements and fixes #1333

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

VReaperV
Copy link
Contributor

@VReaperV VReaperV commented Oct 1, 2024

  • Removed the non-UBO and non-integer texture code paths. The non-UBO and non-integer texture code was supposed to be used when those extensions are not available, however:
  1. These extnesions are available on nearly all hardware nowadays. These extensions became core in 3.1 as well.
  2. Hardware that doesn't support those extensions porbably won't be using r_realtimeighting on anyway due to performance reasons.
  3. These old code paths were broken.
  • Fixed an issue that resulted in the same light being assigned multiple times to the same tile and some lights not being added.
  • Made the way light IDs are stored in the lighttile texture sane: this was a mess of bitwise operations and strange ways of storing the ID by putting different bits into different parts of a uvec4. Now this will just store each 8-bit ID consecutively.
  • Fixed an issue that resulted in the same light being added multiple times in lightMapping_fp. Some projectile shaders and such might need to be adjusted due to this.
  • Cleaned up the code in lighttile_fp and computeLight_fp.

The non-UBO and non-integer texture code was supposed to be used when those extensions are not available, however:
1. These extnesions are available on nearly all hardware nowadays. These extensions became core in 3.1 as well.
2. Hardware that doesn't support those extensions porbably won't be using `r_realtimeighting on` anyway due to performance reasons.
3. These old code paths were broken.
@illwieckz
Copy link
Member

illwieckz commented Oct 1, 2024

I confirm that the alternative code for missing EXT_texture_integer is working, while the alternative code for missing ARB_uniform_buffer_object doesn't.

In my database of collected glxinfo dumps, I see ATI RV500 and RV400 cards that are capable of compiling the tiled renderer, but don't support either EXT_texture_integer and ARB_uniform_buffer_object. Olders ATI cards like RV300 are not able to compile the tiled renderer shaders anyway. I also see Nvidia NV40 cards that miss both (but may be able to compile the shaders). So actually having an alternate code working for one extension is useless if the alternate code for the other extension is not working.

The glxinfo dumps for those cards are from 2020 so I may re-run them with latest mesa.

Intel gma 3 misses them but is not able to compile the tiled renderer shaders to begin with. The gma 4 can probably compile them but I have no dump and it would probably be super slow to run dynamic lights (I'll have to restart the related computer to check).

@VReaperV
Copy link
Contributor Author

VReaperV commented Oct 1, 2024

Yeah, both extensions seem to really be supported by the same set of hardware.

@slipher
Copy link
Member

slipher commented Oct 2, 2024

In the commit Cleanup lighttile_fp, there's a suspicious change for( int i = u_lightLayer; i < u_numLights; i += numLayers ) { -> for( int i = u_lightLayer; i < u_numLights; i++ ) { (counting up by 1 instead of 4). Is that what you mean by lights being added multiple times? This would make "most" lights be processed 4 times.

@@ -48,6 +48,16 @@ static void EnableAvailableFeatures()
glConfig2.realtimeLighting = false;
}

if ( !glConfig2.uniformBufferObjectAvailable ) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These could be else-if's

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants