evilmartians / oklch-picker

Color Picker for LCH
https://oklch.com
Other
849 stars 65 forks source link

When having a concurrency count above 20 in Chrome First graph to update from user input breaks #141

Closed thethinur closed 7 months ago

thethinur commented 7 months ago

Like the title says, when you first load the site the first graph that changes from user input breaks. Play with Hue slider; Lightness graph wont update for rest of the session Play with Lightness slider; Chroma graph wont update... Play with Chroma; Hue Graph wont update...

Once one graph breaks the others will work just fine.

Everything else will work just fine for the session.

Google Chrome 123.0.6312.59 and 123.0.6312.86 Windows 11 22631.3296 Intel i7-13700F 16 Cores, 24 Threads Nvidia RTX 3070 running 546.01 and 551.86

Works correctly on Chrome running on a Samsung Galaxy Tab S8 Ultra

Chrome and Nvidia Driver got updated. No change.

Works fine on Firefox.

ai commented 7 months ago

Can you record a video?

Website works find on my machine.

thethinur commented 7 months ago

Yes, of course: https://github.com/evilmartians/oklch-picker/assets/7808616/ad0bfcd2-82bf-4277-9b07-b2433ac67fac

ai commented 7 months ago

Show the console. If it is empty, it is video glitch.

thethinur commented 7 months ago

image

thethinur commented 7 months ago

Only happens on the first one.

ai commented 7 months ago

This error could block the construction of the graph.

But I have no idea why you have it and I don't.

Let's wait for more reports.

thethinur commented 7 months ago

At least what is clear is it's failing when creating a new ImageData(), if I could just check variables I could at least see what might be wrong.

thethinur commented 7 months ago

image

ImageData(340 - 346 + 1, 150) -> ImageData(-5, 150) Yeah that definitely aint right.

There is wrong data in the first event post-load, might be an initialization error.

image

Yeah, first onmessage event after loading the page is wrong. Unexpected data during page-load? Could it relate to loading speed? Like polling some data that changes post-load?

thethinur commented 7 months ago

Clearer picture of the scope. image

thethinur commented 7 months ago

Ok, it's a Chromium error in general on my machine, because Microsoft Edge and Steam Overlay's Browser fails too. If I should take a stab in the dark, I'd guess it's chromium worker scheduling playing games due to my PC being way overkill.

thethinur commented 7 months ago

Ayy, I might have isolated the issue. Okay, there might be a logic error looming inside the workers. I limited concurrency and it works normally for all 20 and under, but above 20 the site might break. 21: Original Error but e.data.from is always 341 22: Site works as normal 23: An index error appears, but site continues to work as normal. 24: Original Error

ai commented 7 months ago
  1. What is your screen size and scale (100%/150%/200%)? Show window.devicePixelRatio.
  2. How many CPU cores do you have? Show navigator.hardwareConcurrency
thethinur commented 7 months ago

image

thethinur commented 7 months ago

I have 16 Cores and 24 Threads.

Firefox picks Core count for hardwareConcurrency. image Chrome picks Thread count for hardwareConcurrency. image

It's definitely scheduling playing games, but that also means in the end, there is most likely a logic error too, because this is definitely a race-condition. Something is running earlier or later and changing the outcome, at least it's consistent, a strangely deterministic race-condition.

ai commented 7 months ago

I was able to reproduce an error. Wait a little for a fix.

thethinur commented 7 months ago

That's great.

I was too unfamiliar with your tooling and JS + TS in general or I might have just tried a few things and made a pull request.

It was just a little too much work for the fun and satisfaction of debugging.

ai commented 7 months ago

Should be fixed https://github.com/evilmartians/oklch-picker/commit/8e089e831c2dcb3631d417cf8846b762c85906f9

Can you try?

thethinur commented 7 months ago

Works like a charm. Thank you!

Closing for now. I don't see anything happening when I change the concurrency to other values too.