-
Notifications
You must be signed in to change notification settings - Fork 281
Prevent white pixels instead of transparency when merging sources #1295
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
base: master
Are you sure you want to change the base?
Conversation
Handle transparency when merging
| transparent_layers = [] | ||
| for layer in async_.imap(get_map_from_source, self.sources): | ||
| if layer[0] is not None: | ||
| layers.append(layer) | ||
| try: | ||
| transparent_layers.append(layer[0].image_opts.transparent) | ||
| except AttributeError: | ||
| transparent_layers.append(None) | ||
|
|
||
| if len(transparent_layers) > 0 and all(transparent_layers): | ||
| self.tile_mgr.image_opts.transparent = True |
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.
| transparent_layers = [] | |
| for layer in async_.imap(get_map_from_source, self.sources): | |
| if layer[0] is not None: | |
| layers.append(layer) | |
| try: | |
| transparent_layers.append(layer[0].image_opts.transparent) | |
| except AttributeError: | |
| transparent_layers.append(None) | |
| if len(transparent_layers) > 0 and all(transparent_layers): | |
| self.tile_mgr.image_opts.transparent = True | |
| all_transparent = True | |
| for layer in async_.imap(get_map_from_source, self.sources): | |
| if layer[0] is not None: | |
| layers.append(layer) | |
| if 'image_opts' in layer[0] and layer[0].image_opts.transparent: | |
| all_transparent = all_transparent and layer[0].image_opts.transparent | |
| else: | |
| all_transparent = False | |
| if all_transparent: | |
| self.tile_mgr.image_opts.transparent = True |
please check if this works. The code is a little clearer than using except.
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.
I remember I wrote a test for this 2 years ago. I need to check my private repository... I remember that the bug was pretty annoying and except was my last resort. Is it okay to block this for up to 2 days so that I can find the test?
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.
Yes sure. If you could add a test or two that would be really great.
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.
Thanks for your feedbacks, and thanks again @tobwen for this code snippet. It works perfectly fine as is but we plan to test your suggestion Simon. Will let you know in a few days.
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.
I think this one was it: tobwen@cbc3cca
I hope I can test it tomorrow.
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.
I gave it a try.
if 'image_opts' in layer[0] and layer[0].image_opts.transparent:
Gives me the following error :
TypeError: argument of type 'ImageSource' is not iterable
I fixed it with the following :
all_transparent = True
for layer in async_.imap(get_map_from_source, self.sources):
if layer[0] is not None:
layers.append(layer)
try:
if not layer[0].image_opts.transparent:
all_transparent = False
break
except AttributeError:
all_transparent = False
break
if all_transparent:
self.tile_mgr.image_opts.transparent = True
That works fine, I'll double check that with @damien-faivre and he should add this to this PR.
simonseyock
left a comment
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.
Thanks for the contribution!
Fixed by @tobwen in this issue 👉 #556