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

Cache output of restrict to reduce redraw time #313

Closed
wants to merge 2 commits into from

Conversation

jwahlstrand
Copy link
Collaborator

Reduces latency a bit when zooming around large images, partially addressing #300.

Copy link
Member

@timholy timholy left a comment

Choose a reason for hiding this comment

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

This looks like a good idea. However, it might be better to just implement a proper Pyramid type (see JuliaImages/ImageBase.jl#39) and suggest that users employ that for better performance.

function copy_with_restrict!(cnvs, img::AbstractMatrix)
imgsz = size(img)
# cache of the image plus downscaled versions
ImagePyramid(img::AbstractMatrix) = Any[img]
Copy link
Member

Choose a reason for hiding this comment

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

Since this is a function, not a type, it should be lowercase.

ImageView supports n-dimensional images, so perhaps this should be generalized beyond AbstractMatrix? Or does this only happen after slice-selection? If a user is browsing a large 3d array, would the entire pyramid be recomputed each time one changes slice planes?

pyr
end

function get_image(p, i)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
function get_image(p, i)
function get_restricted_image!(p, i)

Can we use more descriptive variable names than p and i?

@jwahlstrand
Copy link
Collaborator Author

Ah, I didn't see that a pyramid type was in the works.

@timholy
Copy link
Member

timholy commented Sep 29, 2024

It's been something we could have added since restrict first debuted, so it may not land immediately. But I think it's the best way to handle this.

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