-
Notifications
You must be signed in to change notification settings - Fork 156
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
Performance improvements for fixed resolution buffer #2507
base: main
Are you sure you want to change the base?
Conversation
…fer to speed up identical pixel transformations across layers (e.g. when subsets are present)
@dhomeier - do you understand the ruff issue here? |
@dhomeier can correct me if I'm wrong, but I think what it wants is # `value` is a bad variable name but I don't know what this thing is
for chid, value in PIXEL_CACHE.items():
if value['hash'] == current_pixel_hash:
for ipix, pix in enumerate(data.pixel_component_ids):
if (
ipix not in matching_pixel_cache and
ipix in value and
value[ipix]['bounds'] == bounds
):
matching_pixel_cache[ipix] = value[ipix] rather than (what is currently is): for chid in PIXEL_CACHE:
if PIXEL_CACHE[chid]['hash'] == current_pixel_hash:
for ipix, pix in enumerate(data.pixel_component_ids):
if (
ipix not in matching_pixel_cache and
ipix in PIXEL_CACHE[chid] and
PIXEL_CACHE[chid][ipix]['bounds'] == bounds
):
matching_pixel_cache[ipix] = PIXEL_CACHE[chid][ipix] |
I can only agree ;-) |
Co-authored-by: Derek Homeier <[email protected]>
# still use cache_id because we still want to make sure we save results from different layers | ||
# to the cache in case any layers get removed or changed. | ||
matching_pixel_cache = {} | ||
for chid, i_cache in PIXEL_CACHE.items(): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
actually I am surprised Ruff does not complain now that chid
is never used – or suggested to use PIXEL_CACHE.values()
in the first place... 😂
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh yeah, we should fix that, we should just use .values()
...
Work in progress, but can already be tested out.