secondlife / viewer

🖥️ Second Life's official client
GNU Lesser General Public License v2.1
188 stars 42 forks source link

Material overrides are occasionally not loading. #1325

Open Dan-Linden opened 1 week ago

Dan-Linden commented 1 week ago

Environment

Second Life Release 7.1.6.8790244846 (64bit) Release Notes

You are at 13.2, 214.9, 23.1 in Rumpus Room 2050 located at simhost-01129fe6e38889eb3.aditi SLURL: secondlife://Aditi/secondlife/Rumpus%20Room%202050/13/215/23 (global coordinates 36,365.2, 14,550.9, 23.1) PMFP 2024-04-13.8669470587 Release Notes

CPU: Intel(R) Core(TM) i7-5930K CPU @ 3.50GHz (3491.91 MHz) Memory: 32610 MB OS Version: Microsoft Windows 10 64-bit (Build 19045.4291) Graphics Card Vendor: NVIDIA Corporation Graphics Card: NVIDIA GeForce GTX 1080/PCIe/SSE2

Windows Graphics Driver Version: 31.0.15.3623 OpenGL Version: 4.6.0 NVIDIA 536.23

Window size: 1645x1264 Font Size Adjustment: 96pt UI Scaling: 1 Draw distance: 128m Bandwidth: 3000kbit/s LOD factor: 1.75 Render quality: 5 Texture memory: 7304MB Disk cache: Max size 1638.4 MB (55.1% used)

J2C Decoder Version: KDU v7.10.4 Audio Driver Version: FMOD Studio 2.02.13 Dullahan: 1.14.0.202310131404 CEF: 118.4.1+g3dd6078+chromium-118.0.5993.54 Chromium: 118.0.5993.54 LibVLC Version: 3.0.16 Voice Server Version: Vivox 4.10.0000.32327.5fc3fe7c.399bd0e

Packets Lost: 0/2,294 (0.0%) April 24 2024 09:37:54

Description

Material overrides are occasionally not loading.

This issue does not reproduce with the 7.1.6.8745209917 - Maintenance-YZ Release viewer.

Reproduction steps

  1. Add a material that has all 4 textures to a box.
  2. Duplicate the box.
  3. Edit the material of the second box, and set the 4 materials to None.
  4. Relog.
  5. Verify the second box still has the 4 Material textures set to None.

Observed: The second box looks just like the first box and has the original Material. If it doesn't reproduce. Make a copy of the second box, make a tweak to the tint color and go to step "4. Relog"

1 before relog:

1  before relog

2 after relog:

2  after relog
maestrolinden commented 1 week ago

It seems to me that the issue is in loading material overrides from the object cache. Once in the broken state, if you exit the viewer, delete the viewer's objectcache folder, and visit the region again, the material overrides once again appear correctly on the object.

Dan-Linden commented 1 week ago

Another repro for this bug from Andred Darwin: 1 - Current viewer (YZ) at Taber (110,186,3102) 2 - Observe wall and small walls top colors as dark blue/gray and "Log off" 3 - Log back in using "Last location" and Featurettes viewer (7.1.6.8790244846) 4 - Observe wall and small walls top colors as dark blue/gray. 5 - Log off. 6 - Log back in with YZ, wall and small walls top are now white.

Workaround: Clear cache solves it, but its rinse and repeat

Dan-Linden commented 1 week ago

Andred Darwin did more investigation on this and found:

This PR has the cause of the issue: https://github.com/secondlife/viewer/pull/1230 I cannot repro with 7.1.6.8666822239 I can repro with 7.1.6.8727446379, and looks like the only change is the PR above.

beqjanus commented 1 week ago

Just an update. I worked with Andred to test this out. What you have here is a transition bug only, it goes away as soon as you stop switching back and forth with viewers that use an incompatible cache.

background: When the featurette viewer, which has my revised VOCache extras handling PR in, runs it will rewrite the vocache extras and it adds a new version field, so that (in theory at least) the viewer can reject/ignore/kill with fire caches that it does not recognise. The new viewers will quite happily handle the old versions, and will write out a new version. allowing for a one-way backwards compatibility check. The older viewers do not have a version number, they have no concept of backwards let alone forwards compatibility, and their error-handling is almost certainly not robust enough to deal with it. I'm not quite sure what the result of running an older viewer on a newer cache is, but it seems to make a pigs-ear of it, which renders it broken on the next restart using featurette.

I don't think it is worth investing time in trying to work out why running an older viewer breaks on a newer cache. Forcing a cache clear might be worth doing, it would prevent the import of old caches but that's hardly a major loss. The important thing is getting back all the lost disk space.

Repro/demonstration of transition-only bug.

The following test illustrates that the issue only happens when you switch between old and new, and as such it won't be a problem in the real world.

0 - clear cache so we are starting with a known position 1 - Login with the old @ Taber (110,185,3102) 2 - After all rezzed, notice the walls "dark blue/gray" (PBR Mat, with tint color) 3 - Logout and login with new... notice the walls "dark blue/gray" <-- this loads the old cache and rewrites in new format. 4 - Log back in with with NEW... <--- Should be fine, new viewer reloads new format. 5 - Log back in with OLD <-- likely to fail as cache not valid and old viewer has poor failure handling, unclear what state the extras cache is in after this. 6 - Log back in with NEW <--- this now triggers the bug.

RunitaiLinden commented 1 week ago

Pushed fix.

So... it's not just a transition issue, but there still is a transition issue. There was a bug in the extras cache reading (missing readline) that was trying to interpret the version line as a UUID, which would trigger a failure and cause the extras file to be deleted. This would cause the viewer to think it had a valid object cache with no overrides, causing all overrides to fall off of inworld objects until the next full update.

Fix was to not try to be backwards compatible (only accept caches that have a version string in the extras file), and to treat an invalid extras file as an invalid object cache (remove the object cache for the whole region, and remove it from memory in LLViewerRegion).

There was also a bug where removing overrides would not update the cache. An empty override message is a valid message indicating overrides have been removed, but the handler for that message was bugged (would cache the empty message instead of removing the cache entry). Fix was to remove the cache entry when receiving empty overrides.

As Beq says above, the error handling on older viewers is poor, so switching between viewers that expect a cache version and viewers that don't is going to reproduce this issue on viewers that don't have the fix. The only way to fix that would be to not version the extras files themselves, but I think that's probably a bad idea long term since we'll likely just end up in this situation over and over until every cache file has a version field we can work with.

igorlproductengine commented 2 days ago

Need more info. Verified on the Second Life Release 7.1.7.8883017948 (64-bit) build in the scope of https://github.com/secondlife/iqa/issues/218. According to Dan's comment to reproduce, all textures are loaded from the cache well. (Passed QA) According to the Beqjanus comment to reproduce, the cache isn't working correctly and the wall is displayed white. (Failed QA) Could @Dan-Linden please clarify if it is expected?

Dan-Linden commented 2 days ago

This passes QA because the test in the Description passed. As Runitai says above, switching between the viewer in test and an old viewer will trigger this bug because the old viewer did not have cache versioning.