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

Make PicassoPainter's request aware of state read changes #2432

Merged
merged 1 commit into from
Nov 29, 2023

Conversation

gamepro65
Copy link
Collaborator

Based on #2423

This allows state objects the ability to update a request without requiring a recomposition + another rememberPainter call with a different key. If a state change occurs during the initial composition (ex. as a result of measured size becoming available) then we need to cancel the original request. Unfortunately Picasso will already be executing the original request; to avoid this race condition I've included a main synchronization mechanism to prevent the request from running until the current frame has completed which allows us time to cancel the initial request before processing anything unnecessarily. This only effects cases where the image does not exist in the cache already as those would be synchronously returned in time for the first frame.

@gamepro65 gamepro65 requested a review from jrodbx September 11, 2023 15:37
Copy link

@zach-klippenstein zach-klippenstein left a comment

Choose a reason for hiding this comment

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

Compose stuff LGTM.

@gamepro65 gamepro65 force-pushed the cdrury/requestState branch from 6c7b07f to 7633792 Compare November 1, 2023 14:30
Copy link
Collaborator

@jrodbx jrodbx left a comment

Choose a reason for hiding this comment

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

awesome, thanks for improving this!

val painter = picasso.rememberPainter {
it.load("http://example.com/")
}
Canvas(Modifier.fillMaxSize()) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

would it be helpful to use a named argument for the lambda here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't think so, its the only other argument and generally accepted that Canvas' lambda is DrawScope


// Only launch a new request if anything has actually changed. RequestCreator does not
// currently implement an equals method, relying here on reference equality, future improvement
// will be to implement equals which can prevent further re-requests.
Copy link
Collaborator

Choose a reason for hiding this comment

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

nice comments/explanations

@gamepro65 gamepro65 merged commit b9b27fb into master Nov 29, 2023
10 checks passed
@gamepro65 gamepro65 deleted the cdrury/requestState branch November 29, 2023 21:03
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.

3 participants