Skip to content

Removed Completed event from AsyncEffectRenderer #1312

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

Conversation

Lehonti
Copy link
Contributor

@Lehonti Lehonti commented Mar 21, 2025

The callback mess was untangled and the code is now sequential and easier to follow.

Next, we might want to address the Update event and then actually create a RenderHandle class, as discussed in #1009 (comment), but that's a topic for a future pull request. I already tinkered with it, and it is less straightforward than it appears to be due to EffectData_PropertyChanged, which I think has to be moved out of LivePreviewManager, and its Start method needs to be able to be canceled externally through a CancellationToken passed as an argument.

Copy link
Member

@cameronwhite cameronwhite left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be safe since a lot of code is shifting around here, I'm going to leave reviewing this for after getting Pinta 3.0 released

// Create live preview surface.
// Start rendering.
// Listen for changes to effectConfiguration object, and restart render if needed.
// Handle selection path.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this comment went along with the code (now further down) for initializing the render bounds from the selection. It's not a particularly useful comment though

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that's fair that we want to leave it until after Pinta 3.0.

Also, thanks @cameronwhite for noticing. I removed the comment (which the commit message says is "irrelevant" but should've been "unnecessary").

@cameronwhite cameronwhite merged commit 2c44dc7 into PintaProject:master Apr 12, 2025
6 checks passed
@Lehonti Lehonti deleted the refactoring/async_renderer_no_completion_event branch April 12, 2025 16:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants