Skip to content

Conversation

RyeMutt
Copy link
Contributor

@RyeMutt RyeMutt commented Sep 26, 2025

Description

Fixes rendering differences observed in 2025.07 from changes done for mac performance.

Related Issues

Issue Link: #4707


Checklist

Please ensure the following before requesting review:

  • [ X] I have provided a clear title and detailed description for this pull request.
  • [ X] If useful, I have included media such as screenshots and video to show off my changes.
  • [ X] The PR is linked to a relevant issue with sufficient context.
  • [ X] I have tested the changes locally and verified they work as intended.
  • [ X] All new and existing tests pass.
  • [ X] Code follows the project's style guidelines.
  • [ X] Documentation has been updated if needed.
  • [ X] Any dependent changes have been merged and published in downstream modules
  • [ X] I have reviewed the contributing guidelines.

Additional Notes

@RyeMutt RyeMutt changed the base branch from develop to release/2025.07 September 26, 2025 16:57
@RyeMutt RyeMutt marked this pull request as ready for review September 26, 2025 16:58
@aiaustin
Copy link

aiaustin commented Sep 27, 2025

I am seeing a big difference on rendering on Windows too - much darker and harsher shadow areas - when testing 2025.07 recent development builds compared to last 2025.06 release. For two comparison images below, the Midday preset was selected in both cases with same graphics settings on Windows 10 with NVidia GTX 1080 GPUU to show difference:

Current Release: Second Life Release 7.2.1.17108480561 (64bit)

7 2 1 Midday

Development Version: Second Life Release 7.2.2.17953906098 (64bit)

7 2 2 Midday

Look at avatar front, plywood object in sky and shadows under vehicle.

In case its location related this test was at secondlife://Aditi/secondlife/Rumpus%20Room%202048/142/102/23

@akleshchev akleshchev requested a review from Copilot September 29, 2025 16:14
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes rendering differences introduced in version 2025.07 by addressing texture parameter setting and screen buffer allocation issues. The changes ensure proper texture type handling and consistent buffer format usage.

  • Force GL_RGBA16F format for screen buffer allocation regardless of HDR setting
  • Modify texture parameter functions to accept explicit texture type parameters
  • Update texture parameter calls to use correct texture types instead of relying on current state

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
indra/newview/pipeline.cpp Forces GL_RGBA16F format for screen buffer allocation
indra/llrender/llrender.h Adds texture type parameters to fast texture parameter functions
indra/llrender/llrender.cpp Updates texture parameter functions to use explicit texture types

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

GLuint screenFormat = hdr ? GL_RGBA16F : GL_RGBA;

if (!mRT->screen.allocate(resX, resY, screenFormat)) return false;
if (!mRT->screen.allocate(resX, resY, GL_RGBA16F)) return false;
Copy link
Preview

Copilot AI Sep 29, 2025

Choose a reason for hiding this comment

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

The hardcoded GL_RGBA16F format ignores the computed screenFormat variable on line 859. This creates dead code and makes the HDR conditional logic meaningless. Either remove the unused variable or use it in the allocation call to maintain the intended behavior.

Suggested change
if (!mRT->screen.allocate(resX, resY, GL_RGBA16F)) return false;
if (!mRT->screen.allocate(resX, resY, screenFormat)) return false;

Copilot uses AI. Check for mistakes.

Copy link
Contributor

Choose a reason for hiding this comment

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

This line reverts to the behavior in the release/2025.06 branch, which does seem relatively safe on its own.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm concerned that the bool hdr initialization on line 812 wasn't corrected. I don't think we should force the render target format to be HDR all the time. On machines where we have forcibly downgraded them to OpenGL 3.1 Fallback Mode in order to avoid crashiness, I think we probably need to keep hdr screen format off. Possibly it should be changed to something like this?

    bool hdr = gGLManager.mGLVersion > 3.1f;

Copy link
Collaborator

Choose a reason for hiding this comment

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

@RyeMutt am I understanding this correctly?

if (tex_type == TT_CUBE_MAP || tex_type == TT_CUBE_MAP_ARRAY || tex_type == TT_TEXTURE_3D)
{
glTexParameteri(GL_TEXTURE_CUBE_MAP, GL_TEXTURE_WRAP_R, sGLAddressMode[mode]);
glTexParameteri(sGLTextureType[tex_type], GL_TEXTURE_WRAP_R, sGLAddressMode[mode]);
Copy link
Preview

Copilot AI Sep 29, 2025

Choose a reason for hiding this comment

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

Line 479 uses sGLTextureType[tex_type] but should use GL_TEXTURE_CUBE_MAP for cube maps to match the original behavior. The original code specifically used GL_TEXTURE_CUBE_MAP for the R coordinate, which may be required for proper cube map handling.

Suggested change
glTexParameteri(sGLTextureType[tex_type], GL_TEXTURE_WRAP_R, sGLAddressMode[mode]);
glTexParameteri(
tex_type == TT_CUBE_MAP ? GL_TEXTURE_CUBE_MAP : sGLTextureType[tex_type],
GL_TEXTURE_WRAP_R,
sGLAddressMode[mode]);

Copilot uses AI. Check for mistakes.

@akleshchev akleshchev linked an issue Sep 29, 2025 that may be closed by this pull request
@bennettgoble bennettgoble merged commit 79909b8 into secondlife:release/2025.07 Sep 29, 2025
13 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Sep 29, 2025
@RyeMutt RyeMutt deleted the rye/fix-lightbugs branch September 29, 2025 18:14
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2025.07 Rendering Differences
6 participants