View Issue Details

IDProjectCategoryView StatusLast Update
0006088The Dark ModCodingpublic26.12.2022 19:52
ReporterAluminumHaste Assigned Tostgatilov  
PrioritynormalSeveritynormalReproducibilityalways
Status resolvedResolutionfixed 
Product VersionTDM 2.10 
Target VersionTDM 2.11Fixed in VersionTDM 2.11 
Summary0006088: Quickloading from bright area to dark area, the player is fully lit for a few frames
DescriptionWhen you quickload while lit, into an area that's dark, you're still lit for a few frames and AI can see you, even though you've quickloaded into a completely dark area.

https://www.youtube.com/watch?v=goFKyExVAG0
Steps To ReproduceQuicksave in a dark area.
Run out into an AI with a torch or a lit area.
Quickload
Your lightgem is still lit for a few frames, and AI can see you, even though you're in complete darkness.

Easier to see at really low fps, set com_maxfps to 24, and the lightgem stays lit for a longer period of time.
Additional InformationYou might have to set AI visual acuity quite high, as they might not react fast enough.
TagsNo tags attached.

Relationships

related to 0003826 new shadows blink briefly when torch turns on 
related to 0003840 closeduser81 Shadow transition lags on movable light entities 

Activities

nbohr1more

nbohr1more

31.08.2022 14:51

developer   ~0015218

Probably related to the same class of problems caused by light conditions controlled via script. ( See attached trackers 3826 and 3840 )
Obsttorte

Obsttorte

09.09.2022 13:39

developer   ~0015246

This is really an odd one.
Both upon starting a new map as well when loading from a saved game the lightgem entity is created anew. All values that correspond to the lightgem value are stored for both the lightgem as well as the player class (the system is a bit redundant). I even tried initializing the values with zero on both classes with no effect.
stgatilov

stgatilov

27.11.2022 11:33

administrator   ~0015479

I think this is caused by:

1) m_LightgemImgBufferFrontend / m_LightgemImgBufferBackend image double buffer.
  It is not saved/restored, so when you load a game, the first frame we analyze lightgem image from before loading the game.
2) In RenderBackend::DrawLightgem, I see additional triple buffer lightgemPbos.
  So after the first frame of simply old image, we get another 0000014:0000003 frames from PBOs with old contents.
nbohr1more

nbohr1more

09.12.2022 20:24

developer   ~0015544

Why are we saving lightgem data when it is constantly being updated?
Cant we just remove all save \ restore functions and force the lg entity to initialize on restore?
stgatilov

stgatilov

10.12.2022 08:47

administrator   ~0015545

We don't save lightgem data: in fact, saving it would reduce latency by one frame (our of 4-5).
And I think the same problem happens on game start: there is no thing like "initialize lightgem".
Moreover, the part of the code is deep inside backend.
nbohr1more

nbohr1more

10.12.2022 14:14

developer   ~0015546

Last edited: 10.12.2022 14:18

???

In player.cpp the save and restore blocks have m_LightGemValue, m_fcolval etc all related to phases of lightgem calculation.

I tried just tinkering with removing them there but it doesn’t cure the bug. I next tried forcing values on restore but the only one that made a difference was fcolval and it also broke the lightgem after quickload.

I also tried performing a buffer free in the restore function in lightgem.cpp but that caused a crash to desktop on quickload.

I think the next things to try are:

Reset the lightgem FBO every time load operations are called

Creat a Boolean that is used the same as tdm_lg_interleave = 0
and have the restore function in player.cpp toggle it then have the lightgem function toggle it off so that at lease one empty lightgem cycle runs
 ( or use an int and have restore set it to 3 and have the lightgem decrement it )
nbohr1more

nbohr1more

10.12.2022 22:26

developer   ~0015549

Decrementing an integer to delay lg processing doesn't work.
It breaks the lightgem for some reason...

I will continue to tinker...
nbohr1more

nbohr1more

23.12.2022 04:32

developer   ~0015610

Rev 10221

New cvar "tdm_lg_reload_delay" determines how many lightgem cycles should pass after reload before lightgem begins to render again.
stgatilov

stgatilov

23.12.2022 21:12

administrator   ~0015624

Nope, it does not work.

First of all, I still see the lightgem blink full-bright, it just happens 3 seconds later.
I imagine that this can allow guards to notice the player.
Second, during these 3 seconds I can run into full-bright areas and my lightgem would be dark, guards won't notice me.
And vice versa: if the save was made in full-bright condition, then lightgem will stay full-bright for 3 seconds after loading, showing me away.
So you just inserted a huge delay, but did not fix anything.

I think here is what's wrong:
1) You made it so that lightgem is not being calculated after load.
  Instead, you should make it IGNORED during the delay but still calculated, because you want to flush PBO triple buffer in backend.
  During the delay, use old lightgem value.
2) Don't put such huge delays.
  And why did you even set different delay for capped and uncapped FPS?!
  5 frames should be enough, 2 frames should be not enough.
  If your experiments don't confirm this, then more analysis is needed, instead of just bumping the delay.
3) I'd say better save/restore m_LightgemImgBufferFrontend / m_LightgemImgBufferBackend members.
  That would technically reduce delay to 3 frames for the PBO buffering, and... just why not?
  Lightgem is a gameplay class, the tradition is to save/restore everything for them, why the exception?
nbohr1more

nbohr1more

24.12.2022 00:19

developer   ~0015625

The idea was to grant a grace period after quickload.

Ideally ( for this delay to work as advertised ) it would remain dark on quickload regardless of
where the quicksave was taken (so I guess it only works half way).
I guess we need an if condition to force lightgem dark on save but in my previous experiments
such a toggle permanently breaks lightgem processing for some reason. I will try again to be sure.

In my testing of this change, the blink when lightgem went active was significantly diminished from the original replication
so I chalked that up to the restore data becoming active but I guess you are seeing a much brighter blink than I do?

If it cures the original issue then I guess we can save\restore the bugger images.
Still seems to be a waste of resources to be keeping this dynamic data in the save file.
nbohr1more

nbohr1more

24.12.2022 00:37

developer   ~0015626

Last edited: 24.12.2022 00:42

Testing complete.

Commenting out the lightgem save\load attributes in Player.cpp does not break the lightgem.

    // savefile->WriteInt(m_LightgemValue);
    // savefile->WriteFloat(m_fColVal);
    // savefile->WriteInt(m_LightgemInterleave);


    // savefile->ReadInt(m_LightgemValue);
    // savefile->ReadFloat(m_fColVal);
    // savefile->ReadInt(m_LightgemInterleave);

When you quicksave in a bright area, quickload renders as dark.
There is a small blip when lightgem becomes active.

I will test without uncapped FPS.

Edit:

Capped FPS ( com_fixedTic 0 ) behaves the same though the blip lingers longer in this mode.
nbohr1more

nbohr1more

24.12.2022 01:53

developer   ~0015627

Last edited: 24.12.2022 01:54

I also removed the shot and shot-spot save \ restore from Lightgem.cpp

Has no apparent detrimental effect but does not cure the blip

Tried setting the lightgem, color and blend color values to 0 during the decrement:

} else {
       m_LightgemValue = 0 ;
       m_fColVal = 0;
       m_fBlendColVal = 0;
       lg_reload_delay--;

Still getting the small blip.

Lightgem still works though.
nbohr1more

nbohr1more

24.12.2022 02:57

developer   ~0015628

Rev 10222

No longer differentiate between capped and uncapped FPS
Don't save\load lightgem values in Player.cpp or Lightgem.cpp
Directly control delay frames via the tdm_lg_reload_delay
Force Lightem.Calculate to return 0.0 ( same as hidden player ) for 5 frames after the delay period has expired

Now the reload "blip" is significantly reduced.
stgatilov

stgatilov

25.12.2022 10:37

administrator   ~0015637

Still 60 frames pause for no reason, and it seems that the blink still happens.
Sorry, but I have replaced the fix in svn rev 10225.

The new fix overrides final lightgem value for 6 frames after loading, WITHOUT stopping any kind of computations.
I also applied it for starting/restating new game (override to full dark), since there was a bright blink there too.

Experimentally, 5 frames override is the bare minimum to get correct behavior.
Theoretically:
  * PBO triple buffering inside backend adds 2-3 frames of delay
  * frontend/backend double buffering probably adds 1 frame more
  * lightgem is rendered across two frames: one for top half, one for bottom half; adds 2 frames to overwrite old values if phase does not match
Also it means that our lightgem has quite a big latency, something close to 100 ms.
nbohr1more

nbohr1more

25.12.2022 12:30

developer   ~0015639

The fix works very well!

I still think there is an argument for letting players define a delay for lightgem processing after reload but
that can go into it's own bug tracker after some forum discussion.
stgatilov

stgatilov

26.12.2022 19:52

administrator   ~0015644

This delay is purely technical and it should be exposed to the user only for the case of emergency.
We know our problems, we know why we need delay and how much, so we set it to the sufficient minimum or slightly longer.

Issue History

Date Modified Username Field Change
31.08.2022 12:51 AluminumHaste New Issue
31.08.2022 13:15 AluminumHaste Steps to Reproduce Updated
31.08.2022 14:48 nbohr1more Relationship added related to 0003826
31.08.2022 14:50 nbohr1more Relationship added related to 0003840
31.08.2022 14:51 nbohr1more Note Added: 0015218
09.09.2022 13:39 Obsttorte Note Added: 0015246
27.11.2022 11:33 stgatilov Note Added: 0015479
09.12.2022 20:24 nbohr1more Note Added: 0015544
10.12.2022 07:05 nbohr1more Status new => confirmed
10.12.2022 08:47 stgatilov Note Added: 0015545
10.12.2022 14:14 nbohr1more Note Added: 0015546
10.12.2022 14:17 nbohr1more Note Edited: 0015546
10.12.2022 14:18 nbohr1more Note Edited: 0015546
10.12.2022 22:26 nbohr1more Note Added: 0015549
23.12.2022 04:32 nbohr1more Note Added: 0015610
23.12.2022 04:33 nbohr1more Status confirmed => resolved
23.12.2022 04:33 nbohr1more Resolution open => fixed
23.12.2022 04:33 nbohr1more Fixed in Version => TDM 2.11
23.12.2022 21:12 stgatilov Assigned To => stgatilov
23.12.2022 21:12 stgatilov Status resolved => confirmed
23.12.2022 21:12 stgatilov Note Added: 0015624
24.12.2022 00:19 nbohr1more Note Added: 0015625
24.12.2022 00:37 nbohr1more Note Added: 0015626
24.12.2022 00:42 nbohr1more Note Edited: 0015626
24.12.2022 01:53 nbohr1more Note Added: 0015627
24.12.2022 01:53 nbohr1more Note Edited: 0015627
24.12.2022 01:54 nbohr1more Note Edited: 0015627
24.12.2022 02:57 nbohr1more Note Added: 0015628
24.12.2022 02:58 nbohr1more Status confirmed => feedback
25.12.2022 10:37 stgatilov Note Added: 0015637
25.12.2022 12:30 nbohr1more Note Added: 0015639
25.12.2022 12:31 nbohr1more Status feedback => resolved
26.12.2022 19:52 stgatilov Note Added: 0015644