[BUG] Multiple Blink Buttons not synchronous

[Ushpuppy]

Make sure you're on the latest stable or beta build

• I have tested this on the latest stable or beta release

Is this a bug in companion itself or a module?

• I believe this to be a bug in companion and not a specific module

Is there an existing issue for this?

• I have searched the existing issues

Describe the bug

using the blink function with the same arguments across multiple buttons creates buttons blinking asynchronously.

Steps To Reproduce

In a fresh config
Create a simple button with a boolean based feedback
0 == 0 && blink (1000)
copy and paste the button one or more times

buttons will not blink synchronously

Expected Behavior

Expected synchronous blinking using same arguments

Environment (please complete the following information)

- Companion Version: 4.3.4
- OS: Windows 11
- Browser: Edge Version 149.0.4022.98 (Official build) (64-bit)

Additional context

No response

[thedist]

In v4.3 I'm not able to reproduce this issue, as using blink(1000) (I'm not sure why 0 == 0 && is in your example?) for me blink synchronously, even with a 32 button page full of blinking feedbacks, and this is the same with both the blink function and the old generic blinker module.

In v5 though we do have some changes to rendering which can cause button renders to appear out of sync, whether using blink, or anything else that updates buttons such as the blinker module.

[Ushpuppy]

Not sure why....I can do repeatably...AMD 7700X 32GB, 3070Ti

I see in button GUI, Emulator and screendeck interfaces

Any ideas on what to try or look at to see what is causing. I dont see in V5

[Julusian]

I don't really know what to do about this.. Our drawing has forever worked in an approach of drawing as fast as it can and outputting things immediately.
Especially with the drawing abilities improving in 5.0, it probably does result in draws in general being fractionally slower, which would make this more visible.
And this will be more visible on larger configs, as the more buttons you have changing, the more chance of other drawing work happening in the middle of what you expect to happen all at once.

The only solution I can think of for this is to completely rethink the drawing pipeline to lock it to a fixed framerate, probably also with a way to skip frames so that we can be render the page of buttons and only once all drawn output them. But not only will this add a lot of complexity, it will also add latency; rather than outputting them all immediately we would wait for the slowest one to be drawn.
While that will solve this staggered effect it will potentially make everything feel less responsive.

And this is ignoring the fact that on some systems (mostly raspberry pi) usb throughput is not great, so introduces its own rolling effect. Similarly satellite can have the same in some conditions.

[devmaiter]

▎ Hi! I'd like to look into this one — sounds like each button runs its own blink timer instead of sharing a clock. If that's the case, I'd propose syncing them to a global tick. Happy to dig in and confirm.

[Julusian]

@devmaiter I suppose I didnt say it directly, but they already are on a shared clock. I believe its just that the drawing is then async and after the synced dispatch of 'queue this for redraw', there is no attempt to keep them in sync

So my suspicion is that fixing this would require redesigning the display flow of the renders to try to synchronise them, which will be a pretty large and sprawling task.

But feel free to take a look, I could have overlooked something

[devmaiter]

I dug through the current code to understand where the desync creeps in. Confirming what you said: the clock itself is genuinely shared and epoch-aligned (VariablesBlinker.trackDependencyOnInterval — identical args map to a single __interval_* variable, ticks aligned to Date.now() % interval).

Downstream of the tick though, each stage adds independent per-button jitter, and nothing re-synchronizes:

1. Each control debounces its invalidation independently (LayeredButtonDrawer.invalidate, wait 10ms / maxWait 20ms, plus a setImmediate).
2. The GraphicsController render queue runs at most 5 renders concurrently, chaining via setImmediate; identical buttons after the first are LRU cache hits, but still pass through the queue one at a time.
3. button_drawn is emitted per button, as each render completes.
4. In the web UI each button is its own tRPC subscription — every update is PNG-encoded per subscriber and pushed as an individual message, and the browser paints whenever each one arrives.

So agreed — there's no single small bug here; a proper fix means a tick-based flush (collect invalidations for a tick, render them, emit atomically), i.e. the redesign you described.

That said, two contained mitigations short of a redesign:

• Batch button_drawn emissions per event-loop turn, so consumers repaint a batch at once (related: the TODO - could/should these be batched? in VariablesBlinker's tick emit).
• Coalesce updates client-side in the web UI, applying incoming images in a single rAF paint — that would at least sync updates landing within the same frame window.

Neither fully fixes it (render stagger can span several frames), so if you feel it's not worth the churn, that's fair — but happy to prototype either if you think one has legs.

[Julusian]

Batch button_drawn emissions per event-loop turn,

This option sounds appealing. Giving the code a quick look myself, the concurrency limits of ImageWriteQueue might be a bit pointless now. Before 5.0, that was part of the concurrency limit for the acrual rendering. but all it is guarding now is the generation of the ImageResult objects (the actual rendering is done by consumers calling drawNative on those).

Maybe even the ImageWriteQueue itself could be replaced with something better; the aim of this queue is a combination of limiting concurrency, while at the same time debouncing draws which share an id.
(Tbh looking at the other uses of ImageWriteQueue, maybe more would benefit from dropping the concurrency limits too; I suppose we mainly want the debouncing and I havent questioned the concurrency limits in a while)

In particular, I am also wondering about the ImageWriteQueue inside SurfacePluginPanel, as that will be adding some similar jitter due to the concurrency limits and lack of batching. The 'drawControls' it issues to the actual surface handlers already supports a batch of renders already, so if it could get a batch to that point then there would be more guarantee of being in sync.

Saying that, the concurrency does (in theory) give a nice side effect for the debounce of avoiding some the debounce firing before there is a renderer free for its drawing. Meaning it could be allowing the debounce more time when it wont be doing the rendering when another invalidation could come in and add to the debounce. I'm not sure what to do with this thought; this doesnt mean it shoudlnt change, just that an eye should be kept on how that impacts whether any 'stale' draws appear to make it through

Coalesce updates client-side

I am not opposed to doing something here, but for me the webui is of secondary importance, and will also be subject to the behaviour of button_drawn so it may be worth waiting to see how it behaves after messing with that.