Skip to content

Comments

Fix markers ui bug#5671

Merged
WithoutPants merged 4 commits intostashapp:developfrom
skier233:markersui_bugfix
Feb 24, 2025
Merged

Fix markers ui bug#5671
WithoutPants merged 4 commits intostashapp:developfrom
skier233:markersui_bugfix

Conversation

@skier233
Copy link
Contributor

@skier233 skier233 commented Feb 20, 2025

Fixes the bug found here: #5633 (comment)

@WithoutPants
Copy link
Collaborator

@skier233 I moved loadMarkers into a separate callback function, and removed the async keywords from computeBaseHue and all its client functions. It looks to have maintained the fix, and I couldn't find a reason for having the async keyword in that function. If you can take a quick look and verify, I'll get this merged.

@WithoutPants WithoutPants added the bug Something isn't working label Feb 21, 2025
@WithoutPants WithoutPants added this to the Version 0.28.0 milestone Feb 21, 2025
@skier233
Copy link
Contributor Author

@skier233 I moved loadMarkers into a separate callback function, and removed the async keywords from computeBaseHue and all its client functions. It looks to have maintained the fix, and I couldn't find a reason for having the async keyword in that function. If you can take a quick look and verify, I'll get this merged.

Looks good. Originally the async was needed when it was using the other hashing lib and the hashing call in that lib was async but I guess that's no longer needed after that lib got swapped out.

@WithoutPants WithoutPants merged commit d915787 into stashapp:develop Feb 24, 2025
2 checks passed
@SyZeeGee
Copy link

I just had a chance to test the new dev build and although it seems better (there is less phantom markers) clicking it fast still seems to induce the bug. Figure since your mind space is still here, I'd let you know. Here is a theory though possibly specific to my use case: is it possible the signal being listened to clear markers isn't being received if the file isn't accessible ("No such file or directory" due to being on an external drive not plugged in?) I didn't have time to fully test this theory but it's possible.

Note I appreciate any work at all on this really, so thanks really. It's overall a really cool marker/video feature (for reasons past the main utility of this software. Well done

On that note, related here: @WithoutPants : when an external hard drive is not present (and the file is not playable), the way the software works currently is that it repeatedly dumps out errors (The "No such file or directory" found one, until the page or video is left.) It just doesn't seem to like the correct implementation (nor intended) here and wanted to let you know of it as well as it might be linked to the above issue. I just confirmed the CPU usage of the browser is going through the roof when it happens so it's not ideal for that reason alone. Let me know if you'd like me to open up a dedicated issue or if its known already (I didn't know what to search)

@WithoutPants
Copy link
Collaborator

On that note, related here: @WithoutPants : when an external hard drive is not present (and the file is not playable), the way the software works currently is that it repeatedly dumps out errors (The "No such file or directory" found one, until the page or video is left.) It just doesn't seem to like the correct implementation (nor intended) here and wanted to let you know of it as well as it might be linked to the above issue. I just confirmed the CPU usage of the browser is going through the roof when it happens so it's not ideal for that reason alone. Let me know if you'd like me to open up a dedicated issue or if its known already (I didn't know what to search)

Please raise a separate issue for this. I'm not aware any existing issue that covers this.

@SyZeeGee
Copy link

Thanks for opening those issues. I just posted the one about the looping file reads 👍

XGFan pushed a commit to XGFan/stash that referenced this pull request Mar 27, 2025
* Move loadMarkers to separate callback
* Remove async from findColors
---------
Co-authored-by: WithoutPants <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants