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): canvas filter and SAM module fixes #7224

Merged
merged 7 commits into from
Oct 29, 2024

Conversation

psychedelicious
Copy link
Collaborator

Summary

  • When filtering, we used the parent entity's buffer renderer to render the filter preview. An sloppy copy-paste resulted in the filter committing the buffer when it should have been cleared when saving-as. This fixes the only cause I can imagine for this issue reported by @skunkworxdark.
  • But akshually, there's no need to use the buffer renderer at all. The canvas SAM module renders its canvas objects directly to the parent layer, without using the parent's object renderers. This better separates the logic. Updated the filter module to use this pattern instead.
  • Update the filter module with the more resilient and clearer logic from the SAM module. Namely, using a hash to determine if we've already processed the given points, and explicit methods to reset the module's ephemeral state and do a teardown once we finish filtering.
    • These two modules are very similar. It makes sense to share the same overall logic between them, but there are enough differences that they can't share a class or inherit from each other.
  • Update the filter and select object UIs to handle an edge case where you reset them, but they don't auto-process.
  • Use node "adoption" to prevent the original layer from briefly appearing while applying a filter or segment operation.
  • Update the comments in the filter module to match the documentation quality of the SAM module.

Related Issues / Discussions

this issue reported by @skunkworxdark

QA Instructions

Try out filtering and segmenting, just kinda fuzz-test it.

I don't know how to reliably repro @skunkworxdark's issue but it there should be no way it can happen now, because a filtered image is now never rendered outside the safe confines of the filter module.

Merge Plan

n/a

Checklist

  • The PR has a short but descriptive title, suitable for a changelog

We don't use the buffer rendere in this module; there's no reason to clear it.
This prevents extraneous graph cancel requests when cleaning up the abort signal after a successful run of a graph.
Let the parent module adopt the filtered/segemented image instead of destroying it and making the parent re-create it, which results in a brief flash of the parent layer's original objects before the new image is rendered.
@github-actions github-actions bot added the frontend PRs that change frontend files label Oct 29, 2024
@psychedelicious psychedelicious merged commit bc42205 into main Oct 29, 2024
14 checks passed
@psychedelicious psychedelicious deleted the psyche/fix/ui/filter-save-as branch October 29, 2024 22:19
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