skell
Level: Legendary Smitemaster

Rank Points: 3734
Registered: 12-28-2004
IP: Logged

Okay, this might get pretty technical. Many Image Overlay commands are time sensitive. They use SDL_GetTicks() to get current time and, based on that, determine when commands should finish and next ones start.

But DROD renders in a limited number of frames per second, I think it's 60 FPS so I'll assume that's correct. So only every 16(.666...) milliseconds does something get displayed.

This means that from the point of view of the player everything that should happen between those 16 frame should happen. From the point of view of code though, there is a small chance that is not the case. Let me explain.

Suppose you have this Image Overlay script (annotated) used twice in the same room:

SetAlpha 0   // As soon as the image is loaded it is made invisible
Display 504  // Then we wait 504 ms, the 4 in tens is important
SetAlpha 255 // Make it fully visible
Display 500  // And display it for whatever amount of time, it's not important

Assuming there is no lag, after 30 frames 500 millisecond have elapsed. Okay, next frame starts, DROD is calculating things like effects and such, let's say it takes 3.x ms (meaning between 3 and 4, exclusive, milliseconds) to do it.
Then it draws things to the screen. It starts processing the first image overlay. Calls SDL_GetTicks(), it returns 503, end time for our Display command is 504, good, we keep displaying this image.
Let's process the next image overlay. Calls SDL_GetTicks(). Because we were unlucky, the time between the last call to the next call was enough for it to return 504, which means we run the next command and we make the overlay visible.
Which means that for a single frame the player saw only a single image instead of two. Not a deal breaker, but can be annoying when you want to make something pretty in your hold or achieve certain visual effect and a random chance can throw a wrench into your plans.

This could potentially be a problem for different effect if they use SDL_GetTicks() too.

--------

There are two solutions:
1. If there is an universal "Render frame now" code, we could just store the value of SDL_GetTicks() there and in other places refer to the value that was stored at that time.
2. Alternatively for this specific problem I suggest that CEffectList::DrawEffects() would make a call to SDL_GetTicks() at the start (or accept it as argument) and all CEffect will just accept the time in the Draw() command.

--------

It's also possible that it's not a situation that can happen because SDL_GetTicks() is smart enough to return the same value each time it's called until a screen was refreshed in which case I jut wasted your time.

@mrimer - thoughts?

____________________________
My website | Facebook | Twitter

---

[Last edited by skell at 10-11-2020 01:15 PM]

mrimer
Level: Legendary Smitemaster

Rank Points: 5058
Registered: 02-04-2003
IP: Logged

I appreciate the detailed write-up, and I think what you describe makes sense as a real possibility. (I'm presuming you're also actually seeing this issue somewhere in your hold development.)

I think I like the idea of unifying the timestamp used in all animations that are rendered for a given frame. I haven't thought through where exactly where to place that, but maybe something like the following:
* On a given frame, cache the value of SDL_GetTicks. Maybe at the start of the CEventHandlerWidget::Activate() loop into a member var?
* Reset this timestamp at the end of the loop.
* Provide a new method (in CEventHandlerWidget? CBitmapManager? ...?) that wraps the call to SDL_GetTicks. If a timestamp has been cached, use it, otherwise default to calling SDL_GetTicks.
* Refactor CEffect-originated classes that call SDL_GetTicks for the purposes of animation progress to call this new method instead.

This approach would allow synchronizing, while defaulting to the current behavior everywhere not invoked from within that synchronized section. (That may or may not be desirable. This approach might paper over subtle bugs in the logic or places where the refactoring was missed. Maybe omit the fallback behavior and resetting of the timestamp at the end of the frame, actually...)

____________________________
Gandalf? Yes... That's what they used to call me.
Gandalf the Grey. That was my name.
I am Gandalf the White.
And I come back to you now at the turn of the tide.

---

[Last edited by mrimer at 09-27-2020 04:32 PM]

skell
Level: Legendary Smitemaster

Rank Points: 3734
Registered: 12-28-2004
IP: Logged

mrimer wrote:
I appreciate the detailed write-up, and I think what you describe makes sense as a real possibility. (I'm presuming you're also actually seeing this issue somewhere in your hold development.)

Thankfully not but I am doing some things where this is a real possibility and I'd like to ensure it won't happen (the timings are such that it's super unlikely but, again, not impossible). I'll parse what you wrote later and see about possibly implementing the changes and will also try to make a hold that reproduces the issue (or fails, proving there is nothing to worry about).

____________________________
My website | Facebook | Twitter

skell
Level: Legendary Smitemaster

Rank Points: 3734
Registered: 12-28-2004
IP: Logged

File: SDL Ticks Problem.hold (384.4 KB)
Downloaded 43 times.
License: Public Domain

Alright, I've tried to design this situation and attached a hold which accomplishes it. Photosensitive people be warned!. Give it a minute or so for the desync to appear

It's tricky because:

1. ImageOverlay is setup in such a way, that it will only process one non-instant command per frame - eg. even if 20ms have passed since last render, if you have two Display 1 in a row (meaning display for 1 ms), only one of them will be "consumed".
2. Therefore the only way to have this occur is to have a desync caused when one Display does not finish, but another image overlay's display finishes.
3. This means it will only occur, as predicted, at the exact threshold time - the difference of 1ms between two image overlay must be enough to either finish or not finish a command.
4. Which, thanks to the blit commands being fast, means you either have to be very unlucky for this to occur, or have enormous blits happening between the sensitive overlays.

I'll fix this for 5.1.1.

The way I'd like to solve it is to store SDL_GetTicks() in BitmapManager each time UpdateScreen() is called. Then that ticks value can be referenced everywhere.

____________________________
My website | Facebook | Twitter

---

[Last edited by skell at 10-11-2020 01:17 PM]

skell
Level: Legendary Smitemaster

Rank Points: 3734
Registered: 12-28-2004
IP: Logged

And a PR.

Slightly modified the used approach - I understand now the flow of logic in Frontendlib a bit better and I did it like you suggested Mike, in CEventHandlerWidget::Activate(). I am storing it in a static variable rather than local - it's a bit less clean, I admit, in terms of isolation but much simpler to use.

____________________________
My website | Facebook | Twitter

mrimer
Level: Legendary Smitemaster

Rank Points: 5058
Registered: 02-04-2003
IP: Logged

Thanks for a sample hold that exhibits this issue. I can see it!

I merged the fix and then tried to get the issue to reproduce in the sample hold. Then I realized I was trying to solve a halting problem and I gave up.

Good work!

____________________________
Gandalf? Yes... That's what they used to call me.
Gandalf the Grey. That was my name.
I am Gandalf the White.
And I come back to you now at the turn of the tide.

skell
Level: Legendary Smitemaster

Rank Points: 3734
Registered: 12-28-2004
IP: Logged

So you're saying you halted the tests?

____________________________
My website | Facebook | Twitter

kieranmillar
Level: Smitemaster
Rank Points: 2670
Registered: 07-11-2014
IP: Logged

I tried the example hold in both the latest stable release and the latest alpha but I don't understand what I'm looking for, both seemed to behave in the same way?

skell
Level: Legendary Smitemaster

Rank Points: 3734
Registered: 12-28-2004
IP: Logged

kieranmillar wrote:
I tried the example hold in both the latest stable release and the latest alpha but I don't understand what I'm looking for, both seemed to behave in the same way?

In the top-left corner you have a few images that are super quickly changing between colors - blue, red and green I believe. They should stay that way forever, but in 5.1.0 they'll likely desync after a while, when the dice roll correctly, because of millisecond differences between things executing. For me it happened after a few seconds, but the more powerful machine you have the more likely it'll take longer for the stars to align.

In 5.1.1 they should never desync.

____________________________
My website | Facebook | Twitter

kieranmillar
Level: Smitemaster
Rank Points: 2670
Registered: 07-11-2014
IP: Logged

I left it in both versions for other a minute and couldn't see it happening, so I dunno.

Maybe someone else can try and replicate it and verify its fixed.

mrimer
Level: Legendary Smitemaster

Rank Points: 5058
Registered: 02-04-2003
IP: Logged

Yes, I can verify it's fixed.

It took longer than a minute for the issue to appear on my dev machine, but after a while, I could see multiple colors showing up concurrently in sub regions of the flashing rectangle.

____________________________
Gandalf? Yes... That's what they used to call me.
Gandalf the Grey. That was my name.
I am Gandalf the White.
And I come back to you now at the turn of the tide.