Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(ui): CA processor cancellation #6336

Merged
merged 1 commit into from
May 12, 2024

Conversation

psychedelicious
Copy link
Collaborator

Summary

When a control adapter processor config is changed, if we were already processing an image, that batch is immediately canceled. This prevents the processed image from getting stuck in a weird state if you change or reset the processor at the right (err, wrong?) moment.

  • Update internal state for control adapters to track processor batches, instead of just having a flag indicating if the image is processing. Add a slice migration to not break the user's existing app state.
  • Update preprocessor listener with more sophisticated logic to handle canceling the batch and resetting the processed image when the config changes or is reset.
  • Fixed error handling that erroneously showed "failed to queue graph" errors when an active listener instance is canceled, need to check the abort signal.

Related Issues / Discussions

Original report @JPPhoto https://discord.com/channels/1020123559063990373/1149506274971631688/1238153079950676101:

Also the global control adapter image doesn't get updated when setting processor to none

QA Instructions

  • Add a CA layer
  • Expand the advanced options
  • Play around with the processor - select different processors, reset the processor, change the settings.

The debounce is 300ms, so if you time it right, you can trigger all possible behaviours, which I will not enumerate here bc its complicated. In short, no matter how your poke it, it should be responsive and not break.

With a processor selected, it should either be processing the image or showing the processed image. With no processor, it should be showing the original image. If you change the processor in any way while it is mid-processing, it should cancel the pending processing.

Merge Plan

n/a

Checklist

  • The PR has a short but descriptive title, suitable for a changelog
  • Tests added / updated (if applicable)
  • Documentation added / updated (if applicable)

@github-actions github-actions bot added the frontend PRs that change frontend files label May 9, 2024
@psychedelicious psychedelicious enabled auto-merge (rebase) May 12, 2024 22:20
When a control adapter processor config is changed, if we were already processing an image, that batch is immediately canceled. This prevents the processed image from getting stuck in a weird state if you change or reset the processor at the right (err, wrong?) moment.

- Update internal state for control adapters to track processor batches, instead of just having a flag indicating if the image is processing. Add a slice migration to not break the user's existing app state.
- Update preprocessor listener with more sophisticated logic to handle canceling the batch and resetting the processed image when the config changes or is reset.
- Fixed error handling that erroneously showed "failed to queue graph" errors when an active listener instance is canceled, need to check the abort signal.
@psychedelicious psychedelicious force-pushed the psyche/fix/ui/ca-processor-listener branch from e2ce10b to 2f06b53 Compare May 12, 2024 22:21
@psychedelicious psychedelicious merged commit 2656f13 into main May 12, 2024
14 checks passed
@psychedelicious psychedelicious deleted the psyche/fix/ui/ca-processor-listener branch May 12, 2024 22:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
frontend PRs that change frontend files
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants